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

[Bug]: When attempting to send an ERC-721 token, users encounter an issue where the "Next" button is disabled, preventing the transaction from proceeding. Furthermore, this issue persists into subsequent attempts to send ERC-20 tokens, with the "Next" button remaining disabled on the Confirmation screen #12317

Open
sleepytanya opened this issue Nov 17, 2024 · 16 comments · May be fixed by #13019
Assignees
Labels
regression-RC-7.34.0 Regression bug that was found in release candidate (RC) for release 7.34.0 regression-RC-7.36.0 Regression bug that was found in release candidate (RC) for release 7.36.0 Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing team-bridge team-confirmations Push issues to confirmations team type-bug Something isn't working

Comments

@sleepytanya
Copy link
Contributor

sleepytanya commented Nov 17, 2024

Describe the bug

When attempting to send an ERC-721 token, users encounter an issue where the "Next" button is disabled, preventing the transaction from proceeding. Furthermore, this issue persists into subsequent attempts to send ERC-20 tokens, with the "Next" button remaining disabled on the Confirmation screen and 'Fiat conversions are not available at this moment' message shown.
Additionally on the assets picker while on NFT tab only ERC 20 tokens displayed.

Seems like it first appeared in 7.43.0, also present in 7.36.0

Related: #12972

Expected behavior

Screenshots/Recordings

Sending ERC 721 works as expected in 7.33.2:

7_33_2.mov

Bug in 7.34.0:

7_34_0.mov

Bug in 7.36.0:

7_36_0.mov

Steps to reproduce

  1. Have an account with ERC 721 tokens
  2. Switch to NFTs tab
  3. Start Send transaction for the ERC 721 token
  4. Observe that 'Next' button is disabled
  5. Switch to the Tokens tab
  6. Start Send transaction for any token
  7. Observe 'Fiat conversions are not available at this moment' message and disabled 'Next' button

Error messages or log output

No response

Detection stage

In production (default)

Version

7.34.0 (1462)

Build type

None

Device

iPhone 15

Operating system

iOS

Additional context

No response

Severity

No response

@sleepytanya sleepytanya added type-bug Something isn't working team-assets regression-RC-7.34.0 Regression bug that was found in release candidate (RC) for release 7.34.0 labels Nov 17, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Nov 17, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Nov 17, 2024
@sleepytanya sleepytanya added the team-confirmations Push issues to confirmations team label Nov 19, 2024
@digiwand digiwand added team-bridge and removed team-confirmations Push issues to confirmations team labels Nov 19, 2024
@sleepytanya sleepytanya added the regression-RC-7.36.0 Regression bug that was found in release candidate (RC) for release 7.36.0 label Nov 22, 2024
@sleepytanya
Copy link
Contributor Author

Same behavior on Android:

nftAndroid.mov

@Unik0rnMaggie
Copy link
Contributor

Sending NFT works, but message with 'Fiat conversions are not available at this moment' is displayed when attempting to send any token and MetaMask app closes

  • Device: Pixel 6 Pro
  • OS: Android 14
  • MM version: 7.36.0

I obtained the build from here

Sending.NFT.mov

@sleepytanya
Copy link
Contributor Author

Present in RC 7.36.0 (1503):

2.mov
Screenshot 2024-11-25 at 20 30 16

@Unik0rnMaggie
Copy link
Contributor

In RC v7.36.0 (1501) from here

Before sending the ERC721 token:

  • 3 NFTs imported on Sepolia network
  • 1 NFT imported on Ethereum network
Sending.ERC721.token.mov

After sending the ERC721 token:

  • MetaMask app closes
  • Upon re-log in, the 3 NFTs on Sepolia network disappeared
  • the Transaction List is not showing any activity
  • the NFT on Ethereum network is still showing and message appears when trying to send it again:
    "You don't own this collectible".
After.sending.ERC721.token.mov

@sleepytanya sleepytanya added the regression-RC-7.37.0 Regression bug that was found in release candidate (RC) for release 7.37.0 label Nov 29, 2024
@sleepytanya
Copy link
Contributor Author

Disabled 'Next' button bug is present in 7.37.0 (1506):

RPReplay_Final1732861827.MP4

@Unik0rnMaggie
Copy link
Contributor

Also present in 7.37.0 (1508) - Android

Send.ERC721.-.disabled.next.button.mov

@sethkfman sethkfman removed the regression-RC-7.37.0 Regression bug that was found in release candidate (RC) for release 7.37.0 label Dec 6, 2024
@sleepytanya
Copy link
Contributor Author

Present in the latest main, Bitrise build.

Currently, a user's interaction with ERC-721 tokens is limited to merely opening the NFT tab. Subsequently, this action renders the option to send any token on Ethereum unavailable:

nft.mov

@sleepytanya
Copy link
Contributor Author

Present in the RC 7.38.0(1521):

1.mp4

@smilingkylan smilingkylan added the Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing label Jan 6, 2025
@gambinish gambinish self-assigned this Jan 9, 2025
@Unik0rnMaggie
Copy link
Contributor

In v7.38.0 (1523) the Next button is enabled, but unable to send ERC721 due to the following errors:

  • intrinsic gas too low: gas 21000, minimum needed 21596 for Ethereum Mainnet
  • intrinsic gas exceeds gas limit for Linea
  • Internal JSON-RPC error for other popular networks

When trying to raise the gas limit and the gas fee, the transactions are failing

Sending.NFT.error.mov

@sleepytanya
Copy link
Contributor Author

sleepytanya commented Jan 10, 2025

iPhone 15, 1523 build - 'Next' is disabled for ERC 721 send:

Image
ScreenRecording_01-10-2025.18-14-32_1.MP4

@sleepytanya
Copy link
Contributor Author

Cancel screen for ERC721 send:

Image

@Unik0rnMaggie
Copy link
Contributor

In v7.38.0 (1524) the Next button is enabled, but unable to send ERC721 due to the following errors:

  • intrinsic gas too low: gas 21000, minimum needed 21596 for Ethereum Mainnet
  • intrinsic gas exceeds gas limit for Linea
    - Internal JSON-RPC error for other popular networks
Send.NFT.error.mov

@Unik0rnMaggie
Copy link
Contributor

Unik0rnMaggie commented Jan 15, 2025

For iOS, v7.38.0 (1524):

  • "Next button" disabled for Ethereum Mainnet
  • "intrinsic gas exceeds gas limit" error for Linea
  • "Internal JSON-RPC" error for other popular networks

"Fiat conversions are not available" message and balance undefined - "Next" button disabled when attempting to send ERC20 token

Send.NFT.iOS.mov
Fiat.conversions.are.not.available.mov

@gambinish gambinish linked a pull request Jan 15, 2025 that will close this issue
7 tasks
@gambinish gambinish added the team-confirmations Push issues to confirmations team label Jan 16, 2025
@gambinish
Copy link
Contributor

gambinish commented Jan 16, 2025

I’ve investigated this issue but haven’t made significant progress yet. It appears to involve two distinct problems:

  1. ERC-721 tokens cannot be sent in the send flow.
  2. ERC-20 tokens cannot be sent in the send flow.

I agree with the Sev1 classification; however, I’m not sure this should block the 7.38 release as it is not technically a regression from the previous release, and has been present in production since at least version 7.34.

I have a very naive fix for the first issue (which I don’t believe is suitable for merging) but haven’t made substantial progress on the second. I don't have a ton of context regarding the send flow, which has been a challenge.

From what I’ve observed, the root cause seems to be that estimatedTotalGas is undefined in several places, which causes the send button to remain disabled. estimatedTotalGas is usually a state variable in the component, which is calculated here

Was there a recent change in how gas is calculated for sends in the TransactionsController? Could this be related to the estimateGasLimit function?

I noticed that we are now passing a networkClientId and I wonder if we should be passing a chainId instead.

@gambinish gambinish removed their assignment Jan 16, 2025
@gambinish gambinish removed team-assets release-blocker This bug is blocking the next release labels Jan 16, 2025
@Unik0rnMaggie
Copy link
Contributor

On v7.39.0 (1529) iOS:

  • The "Next button" is disabled for Ethereum Mainnet and Linea
  • The MM app crashes when sending an NFT on another popular network. Despite this, the Tx is still successful:
iOS.v7.39.0.Send.NFT.mov

@Unik0rnMaggie
Copy link
Contributor

On v7.39.0 (1529) Android:

  • There is an error for Ethereum Mainnet described in this bug
    Despite this error, the Tx successful.

  • The MM app crashes when sending an NFT on Linea or another popular network. Despite this error, the Tx successful.

Android.v7.39.0.Send.NFT.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-RC-7.34.0 Regression bug that was found in release candidate (RC) for release 7.34.0 regression-RC-7.36.0 Regression bug that was found in release candidate (RC) for release 7.36.0 Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing team-bridge team-confirmations Push issues to confirmations team type-bug Something isn't working
Projects
Status: To be fixed
Status: To be fixed
Development

Successfully merging a pull request may close this issue.

7 participants