Connecting to Safes via WalletConnect

[cag]

Right now, connecting directly to a Safe via WalletConnect doesn't really work: https://github.com/gnosis/contract-proxy-kit#support-for-connection-to-a-gnosis-safe

This means that Safes will actually have their own proxy Safe, and attempt to execTransaction an execTransaction. The outer execTransaction will use the Safe's own gas estimation routines to supply the safeTxGas parameter. This causes large transactions to fail because the inner execTransaction will swallow reversions and return a success/fail flag if enough gas gets sent such that the 1/64 gas remaining from a call is enough to finish running execTransaction, meaning Omen market's cannot be created via the Safe for example: https://etherscan.io/tx/0xbf5722dd28d8c53357eca924eb6a2dfc7d230595c99e2f9985d90a7553f06fc2

[cag]

Should either fix the WalletConnected Safe detection and make sure the gs_multi_send call works, or remove the WalletConnect Safe functionality and recommend people go for the Safe Apps option.

[cag]

Problem with fixing it now is there are probably people out there who have Safe Safe proxies who would suddenly appear to lose their stuff after a dapp updates to fixed logic.

OTOH refusing to fix this means Safe WalletConnect users will never be able to create markets or add liquidity for example, or any other number of expensive actions that fulfill the criteria in this issue.

WDYT @germartinez @rmeissner

[rmeissner]

The WalletConnect Safe App will not work with the CPK because of multiple issue (this one for example: safe-global/safe-react-apps#56). Once these are solved it is trivial to implement support for gs_multi_send in the Safe App.

Right now, connecting directly to a Safe via WalletConnect doesn't really work

The legacy app should still work with this, right?

[cag]

I remember a report from Martin on iOS that the legacy app didn't use the direct route...

[cag]

Okay, so the app in question was Omen, and today for some reason I was able to WalletConnect to Omen via my Safe finally, which means I was able to finally dig into what's going on, and... it's kinda complicated.

Right now, the CPK does a detection routine that makes an assumption that the provider passed into the Ethereum library (web3 or ethers) is just an instance of the @walletconnect/web3-provider. With that assumption, it's just a matter of inspecting that object located at web3.currentProvider or signer.provider.provider.

However, that assumption is wrong when interacting with web3-react, a popular solution for obtaining a connection to an Ethereum library in frontends, and one which Omen uses.

What happens with web3-react is the provider is, I think, a web3-provider-engine that composes multiple subproviders, one of which is the @walletconnect/web3-subprovider. The subproviders are enumerated by the list property _providers. These providers look reasonably like the regular ones, so I figure I can go deeper into the providers to see if there's a subprovider that matches the interface of @walletconnect/web3-provider.

But then it gets weird, as there is one that looks like the right provider, but instead of the wc property containing the WalletConnect instance that contains the peerMeta info that confirms that we're connected to a Safe, there's a _walletConnector property and an asynchronous method that returns that property called getWalletConnector.

I think getWalletConnector is probably the preferred way of accessing that property. Kinda hard to do an actual test of this detection stuff though and whether it actually causes the desired behavior to occur.

TL;DR: Gotta do detection via something like this:

async function checkConnectedToSafe(provider: any): Promise<boolean> {
  if (provider == null) return false

  const wc = await provider.getWalletConnector?.() ||
    await provider.connection?.getWalletConnector?.() ||
    provider.wc ||
    provider.connection?.wc

  if (wc?.peerMeta?.name?.startsWith?.('Gnosis Safe')) {
    return true
  }

  if (provider._providers) {
    return (await Promise.all(provider._providers.map(checkConnectedToSafe))).includes(true)
  }

  return false
}