search

MetaMask / metamask-mobile

Mobile web browser providing access to websites that use the Ethereum blockchain
https://metamask.io
Other
2.16k stars 1.12k forks source link

[Bug]: Submitting a repeated nonce from a different device causes app crash #11167

Open plasmacorral opened 2 months ago

plasmacorral commented 2 months ago

Describe the bug

Initially observed today in feature QA on PR 10821, but then confirmed to be reproducible on production v7.30.0 build 1410.

When testing two devices side by side, submitting a transaction too soon on the second device historically produced a nonce error. However, while testing PR 10821, I noticed that submitting a transaction with the same nonce on device B causes the mobile client to crash on device B. Only after re-watching my recording do I see the error before the app closes.

This issue occurs on both iOS and Android, depending on which device submits the transaction first.

Expected behavior

Nonce error without app closure/crash

Screenshots/Recordings

Recording on v7.30.0 build 1410 (android on left, iOS on right) The following error precedes the app closure/crash:

Recording on v7.24.1 build 1346.
Note the first attempt the error is shown "Transaction error already known" and then on retry "Transaction error replacement transaction underpriced". These errors are notably less helpful to the user, but there is no app closure/crash.

Steps to reproduce

  1. Have two devices running side by side with the same SRP
  2. Connect to the test dapp
  3. Initiate a send tx with the test dapp on both devices
  4. confirm the send tx on device A, then immediately confirm a send on device B
  5. Note that Device B will see MetaMask close or an app crash message on iOS

Could accomplish the same by enabling custom nonce in settings>advanced and then subtracting 1 from the nonce before attempting to confirm a new tx.

Error messages or log output

Transaction error Nonce too low: next nonce 105, tx nonce 104

Detection stage

Version

7.30.0

Build type

None

Device

Pixel 5a and iPhone Xs

Operating system

iOS, Android

Additional context

This is an edge case, but we should handle it more gracefully than an app closure/crash.

Severity

No response

Gudahtt commented 1 month ago

Recategorized as Sev2 because it does have a negative user impact (Sev3 is for negligible negative user impact)