safe-global / safe-react-apps

A repository for applications for Safe Web Interface
MIT License
99 stars 76 forks source link

[Walletconnect] Safe web wallet won't return transactions hash #712

Closed dev-johnny-gh closed 8 months ago

dev-johnny-gh commented 1 year ago

Bug description

I use Walletconnect v2 to connect my dapp and gnosis web wallet.

After I call a contract function and send a transaction, I can't get the transaction hash from the transaction response. Actually, the promise is stuck and will never return.

The below code is part of my dapp code which is related to Walletconnect and the contract call. It can work when I use the im token wallet with Walletconnect, but it fails on the gnosis web wallet. Considering my code works on the im token, I suspect that there is something wrong with the gnosis web wallet. Can anyone help to find out what's wrong?

const clientProvider = await WalletConnectEthProvider.init({
  projectId: PROJECT_ID,
  chains: evmChains,
  rpcMap: rpcMapByEVMChain,
  showQrModal: true,
});

const web3Provider = new Web3Provider(clientProvider, 'any');
const signer = web3Provider.getSigner();
const bridge = ethers_contracts.Bridge__factory.connect(tokenBridgeAddress, signer);

const { hash } = await bridge.wrapAndTransferETH(recipientChain, recipientAddress, feeParsed, createNonce(), {
  ...overrides,
  value: transferAmountParsed,
});

Also I check the wallet connect web socket and see these messages:

{"id":"1687248751469507840","jsonrpc":"2.0","method":"irn_publish","params":{"topic":"0e0c449f377eab6365d4655d9b3aa23e8f569860aaba822d97c0b438200727cb","message":"AAT/Q5Rl9Fn6Xdn5aLIQwJVN957+2TWLgDgWM+jK62u1otJJMue6gyscgm/n5xik3Y3lJKU+3AOVNgVhqo2RjkL4TTUhaaNL9xSvnzqcDokDr3OE0NVBg/OMKz9Qfj+q3X6hjZsutOirenaP0QLgpzHYJG4iJuVvFVY+30XqyYJNmaf/gVqRE/KjwHgJlelwlef/rkVSMwKV2N6YK3Bc77xrdtps0D3z6p/RlLOwPxY3wY7hM8RFWuUlcXqR5NiHI/ilGa0E/r67QwW1Knt1vRfOPubqMDw0JYccSxoJ663CxhLJwif0SJyEnV/LM+3v45eZESw3aVJRXwblBK4ExB4xDOMOyTkPXH4icPwoaU3bGWfCcVnc9343sSjv+pZg1B9Qymw6Z5yuq90sBHKksRoSiJ73rVIYN2+J5hKTH0NQquKMaxt9i3m6XAbAByxL3DkIQK1AZHmonMmH1hs9od7KJ9YgrugSMqpH2WFM61//Y0x12iKtdEp7XrGroqOZyz2hV+eoiFQTHkUwr16JX7n/flWdhNIGTkA7+CqPJV3ZrnwWs690e3pxsZg3AQbnWadqI8nGMor440BoUtl/JVbyGqf3KFr0/J6ePff8OR3r3QgycEpB2YE1R5xGW7mv6eET2rqDGbTHTY7BE6cmfHiS/byY/QmZk0/08oveDnCL2VveH9bRB+Cx02UcVTbm9V7Cgp+H4Rk2KsC76dZvm0zXHv+OIYBc1TpLdwQOK7HZmBPiahMqv9RddRsRrrric0NmaAHr4ADlt1yba6cH5JkOYoOtc6/40bj96A==","ttl":300,"prompt":true,"tag":1108}}

{"id":1687248751469507840,"jsonrpc":"2.0","result":true}

{"id":431935688311040,"jsonrpc":"2.0","method":"irn_subscription","params":{"id":"d590d8be142397ee7c7d036e00e5d32e9a663b99f519a8d00d8aa40b6dcdeb2a","data":{"topic":"0e0c449f377eab6365d4655d9b3aa23e8f569860aaba822d97c0b438200727cb","message":"AHTEaDEv9PwQbyYwVSk6Q+sWN8msvl03JY1+IuAnW07DZm9KrX+Rac/UwOqRWSSBz7aqADUllu3vgbYjWejFWN83TbpbXM7EQw6wZ7T8IPnutWp1KwNEGXZz0jReD3W7LDj99RBqyMDbI2rq+J5JsVEmoivB5YTuPVCrZPdeAECzIkCbemgZ/FRpG8evAROtRg0=","publishedAt":1687248782464,"tag":1109}}}

{"id":431935688311040,"jsonrpc":"2.0","result":true}

Environment

Expected result

the hash will be returned

Obtained result

the promise is stuck and will never return

katspaugh commented 1 year ago

There's no transaction hash because the underlying transaction needs to be wrapped into a Safe transaction and signed by owners. Only when it's fully signed do we have an actual hash. It happens asynchronously in most cases, so we cannot return a hash right away.

dev-johnny-gh commented 1 year ago

@katspaugh thanks for reply. i got what you mean, but i only have one signer, so i think we don't need to wait, at least gnosis can return the safe transaction hash or something i can identify this transaction later.

otherwise, how can i know the transaction is executed and get the transaction data from it?

dev-johnny-gh commented 1 year ago

@katspaugh another idea, can we keep the promise pending, until the transaction is signed by all signers, then return the transaction hash to the promise.

katspaugh commented 1 year ago

Yeah, I think the latter is doable, and it would resolve almost immediately in a 1/1 situation. 👍

katspaugh commented 1 year ago

@dasanra @mmv08 wdyt?

mmv08 commented 1 year ago
  1. We return the safe transaction hash, which can be used to retrieve the transaction status
  2. I have looked into a similar issue before and found out that Walletconnect does some shenanigans with the hash in the response and doesn't return it if it can't retrieve the receipt or something of that nature. IMO it's a bug in the walleconnect in the first place.
mmv08 commented 1 year ago

It was not walletconnect but ethers.js:

    async sendTransaction(transaction: Deferrable<TransactionRequest>): Promise<TransactionResponse> {
        // This cannot be mined any earlier than any recent block
        const blockNumber = await this.provider._getInternalBlockNumber(100 + 2 * this.provider.pollingInterval);

        // Send the transaction
        const hash = await this.sendUncheckedTransaction(transaction);

        try {
            // Unfortunately, JSON-RPC only provides and opaque transaction hash
            // for a response, and we need the actual transaction, so we poll
            // for it; it should show up very quickly
            return await poll(async () => {
                const tx = await this.provider.getTransaction(hash);
                if (tx === null) { return undefined; }
                return this.provider._wrapTransaction(tx, hash, blockNumber);
            }, { oncePoll: this.provider });
        } catch (error) {
            (<any>error).transactionHash = hash;
            throw error;
        }
    }

They poll for the transaction receipt after sending, and since it's not an ethereum tx hash, it never resolves.

dev-johnny-gh commented 1 year ago

@mmv08 So, if I understand you correctly, I think the contract calls will use sendTransaction function above, and this function will wait until the this.provider.getTransaction(hash) can get the transaction. Otherwise, it will keep polling, right?

But gnosis returns the safe transaction hash, which can't retrieve an actual transaction on the chain, so my code is stuck.

So, how can I resolve this issue? I guess I can't use a workaround to prevent the ethers from using the sendTransaction function on contract calls.

Can you return the transaction hash instead of the safe transaction hash?

katspaugh commented 1 year ago

Modifying the WalletConnect safe app to wait for all signatures to be given before resolving the promise seems like a reasonable solution. Mimicking what we do for EIP1271 signatures.

mmv08 commented 1 year ago

Can you return the transaction hash instead of the safe transaction hash?

We decided against this in the past because it is not guaranteed that the transaction will be executed and the ethereum transaction hash will be available. Also, with the safe transaction hash, you can use our backend to fetch transaction status and track the signing process. You're looking at this problem from a 1/1 safe user perspective, but there are much more sophisticated setups with 3+ geographically distributed signers.

Ideally, we'd like https://eips.ethereum.org/EIPS/eip-5792 to gain more adoption to fix this problem, as it's evident that the current Ethereum JSON-RPC API doesn't fit multisig wallet use cases.

katspaugh commented 1 year ago

What do you suggest in the meanwhile though, @mmv08? Dapps can issue transactions but they cannot detect it they went through or not.

mmv08 commented 1 year ago

Theoretically, we should support ether's .wait method: https://github.com/safe-global/safe-react-apps/issues/377 I'd suggest checking why this stopped working

katspaugh commented 1 year ago

~The 1inch app is experiencing the same issue and it started a month or two ago. Might be some frontend change on our side?~

https://github.com/safe-global/safe-wallet-web/issues/2115

mmv08 commented 1 year ago

The 1inch is experiencing the same issue and it started a month or two ago. Might be some frontend change on our side?

safe-global/safe-wallet-web#2115

they seem like different issues to me. In this one, the promise never resolves on the dapp side, and in the linked one, the wallet UI freezes

dev-johnny-gh commented 1 year ago

Theoretically, we should support ether's .wait method: #377 I'd suggest checking why this stopped working

I got feedback, our user said that he could use the gnosis with walletconnect before. So I switch back to the previous version of my dapp. but seems it's broken too. Just a heads up.

dev-johnny-gh commented 1 year ago

any update? @mmv08

mmv08 commented 1 year ago

I don't work on apps, so there are no updates from my side

katspaugh commented 1 year ago

@clemoon @dasanra could you prioritize this?

liliya-soroka commented 8 months ago

cc @katspaugh , will we overtake it and fix for the walletconnect native integration ?

katspaugh commented 8 months ago

Our native WalletConnect integration returns a tx hash for 1/X immediate executions.