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

NFT bridge causing unnecessary Approval transactions #3019

Open
MidnightLightning opened this issue Jun 1, 2023 · 0 comments
Open

NFT bridge causing unnecessary Approval transactions #3019

MidnightLightning opened this issue Jun 1, 2023 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@MidnightLightning
Copy link

MidnightLightning commented Jun 1, 2023

Description and context

When using the transferFromEth function to transfer an NFT from Ethereum mainnet, the Wormhole SDK script always has the user send an approve() call, even if the Wormhole bridge contract already has access to that token.

https://github.com/wormhole-foundation/wormhole/blob/main/sdk/js/src/nft_bridge/transfer.ts#L44-L46

This is a wasteful behavior, since if the bridge contract already has access (through some other approval process, or the user started a bridge process and got interrupted partway through), triggering another on-chain approve() call is just wasted gas.

Steps to reproduce

  1. Trigger a transferFromEth call for a specific token
  2. In the wallet, submit the initial approve() action.
  3. While the approve action is pending, close the Dapp tab, or navigate away from the app.
  4. After the approve action succeeds, come back to the Dapp and attempt to continue the transfer.

Experienced behavior

There's no way to "continue" the transfer. If transferFromEth is called again, the user is requested to submit an additional (useless) approve() action.

Expected behavior

The transferFromEth call skips the approve() call and goes on to the bridge.transferNFT call.

Solution recommendation

Knowing that the token is an ERC721-compliant token, the transferFromEth function should check token.getApproved(tokenID) and see if it's equal to nftBridgeAddress, and token.isApprovedForAll(signer.address, nftBridgeAddress) and see if that's true. If either of those checks pass, skip the step of the user needing to call token.approve again.

@MidnightLightning MidnightLightning added the bug Something isn't working label Jun 1, 2023
@aadam-10 aadam-10 added this to the Maintenance - SDK milestone Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants
@MidnightLightning @aadam-10 @kev1n-peters and others