search

safe-global / safe-wallet-web

Safe{Wallet} – smart account wallet
https://app.safe.global
GNU General Public License v3.0
358 stars 429 forks source link

WalletConnect+dapp/Nested safe: the same new tx pops up twice #3225

Open liliya-soroka opened 9 months ago

liliya-soroka commented 9 months ago

Bug description

WalletConnect+dapp: new tx appears again after closing if execution was cancelled in the owner wallet

Environment

Steps to reproduce

First case ( any dapp connected via WC)

  1. Connect to CowSwap using wallet connect v2 in the 1/N safe
  2. start swap
  3. go to the safe wallet and click "execute" for the Swap tx
  4. reject swap in the Metamask
  5. go to the safe UI and close swap transaction Current result: New swap transaction appears again without any action from the user

The second case with nested safes connected via WC (6.11.2024)

  1. Open parent and child safe in different browsers
  2. connect parent safe as a signer via WC
  3. trigger tx in the child safe and start the sign process as a parent safe
  4. review the triggered tx in the parent safe ( confirmation screen)
  5. click "Open nested tx link"
  6. a new tab with a nested tx in the child safe is opened - >wait for some time Current result: Create tx is triggered ( same second tx as in the case above according to the comments in https://www.notion.so/safe-global/Decode-nested-approveHash-transactions-1228180fe573800ebc6acd3f8e54035c?pvs=4

Expected result

make sure that the tx is closed and is not opened again without the user's action

katspaugh commented 9 months ago

I've seen this too, it even appears sometimes after refreshing the page.

schmanu commented 9 months ago

I could not reproduce this with the Steps you provided. But I also saw this before but mainly on scam dapps where I suspect they just retriggered the tx. We cannot stop the dapp from retrying the tx on failure for instance.

katspaugh commented 9 months ago

I think it's not the dapp, it's somehow stored in WalletConnect and re-appears automatically on page reload.

liliya-soroka commented 9 months ago

@schmanu , make sure that you follow steps one to one

https://github.com/safe-global/safe-wallet-web/assets/338622/f0a6eca7-32fa-4fce-96d0-163ef3d3390a

usame-algan commented 8 months ago

I can also reproduce this on app.safe.global but not on localhost. Also tried to run it locally with build and static-serve without success.

I can somewhat "force" the issue by pressing the submit button in Cowswap multiple times. Effectively, this causes multiple eth_sendTransaction requests which we handle on our side by opening the tx-flow for the first request and await a promise that only resolves/rejects once the tx-flow closes. Then the second request is handled by opening the tx-flow again etc.

To me this seems like expected behaviour which we should not prevent but obviously if the button was only pressed once there should only be one request but this doesn't seem to be the case on prod.

~Clearing the indexedDB on app.safe.global solves the issue for me so I agree with @katspaugh that this has to do with some (stale) WC state.~

usame-algan commented 8 months ago

After some more investigation I think this issue happens only if WC is really slow. I am not sure yet what leads to that but I think we should try to find simple steps to reproduce this performance issue with WC and that way we will also be able to solve this issue with transactions popping up more than once.

I am able to resolve the performance issue (and the issue with multiple txs) when I clear the site data but I am not sure what specific steps are required to break it again. @liliya-soroka could you try clearing your site data and try to find steps to reproduce for this?

Image

katspaugh commented 8 months ago

For me it happens consistently, you just need to refresh the page with an open tx flow with a WC tx.

liliya-soroka commented 1 week ago

The issue still exist and additional case with nested safe is added : 6.11.2024