Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(usdt-approval): 🐛 fix wallet connect with safe causing same nonce to appear twice #2543

Merged
merged 4 commits into from
May 25, 2023

Conversation

tukantje
Copy link
Contributor

Summary

Fixes #2532

Safe has an issue with wallet connect where when we send two transactions back to back, they repeat the nonce, causing transaction to fail.

This change introduces a fix by;

  • Check if we are dealing with a safe wallet in wallet connect
  • If so, poll for safe transaction execution every second
  • Proceed only when this happens

To Test

  1. Open CoW Swap
  2. Connect to the Safe using WC option
  3. Use Anxo's USDT https://goerli.etherscan.io/address/0x7b77F953e703E80CD97F6911385c0b1ceabC96Bc
  4. Approve or trade to have some left over approvals (for example 5 USDT)
  5. Enter an amount higher than the approval amount (for example 10 USDT)
  6. Press on the Approve button and run TX
  7. Second tx should not fail.

Background

safe-global/safe-wallet-monorepo#2034
https://cowservices.slack.com/archives/C03DVQREAH0/p1684929286045829

@tukantje tukantje requested review from a team May 24, 2023 16:12
@tukantje tukantje self-assigned this May 24, 2023
@vercel
Copy link

vercel bot commented May 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback

🌃 Cosmos ↗︎

Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soft approved, i haven't tested if it works. Just checked the code

src/common/hooks/useZeroApprove.ts Outdated Show resolved Hide resolved
src/common/hooks/useZeroApprove.ts Show resolved Hide resolved
src/common/hooks/useZeroApprove.ts Show resolved Hide resolved
@anxolin anxolin added the RELEASE Included in the release that is being closed label May 24, 2023
Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @tukantje , great job!


// For Wallet Connect based Safe Wallet connections, wait for transaction to be executed.
if (txReceipt && safeApiKit && isSafeWallet && isWalletConnect) {
await waitForSafeTransactionExecution({ safeApiKit, txHash: txReceipt.hash })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, looks nicer!

@anxolin anxolin merged commit 7f5445e into release/1.38.0 May 25, 2023
@github-actions github-actions bot locked and limited conversation to collaborators May 25, 2023
@alfetopito alfetopito deleted the fix/safe-with-wallet-connect-usdt-approval branch May 25, 2023 14:11
Comment on lines +43 to +44
const isWalletConnect = useIsActiveWallet(walletConnectConnection)
const isSafeWallet = useIsSafeWallet()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I have a hook for that

export function useIsSafeViaWc(): boolean {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice - will iterate on it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
RELEASE Included in the release that is being closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants