MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.95k stars 4.89k forks source link

[Bug]: Metamask extension marks transaction permanent pending #26563

Closed VladChernenko closed 1 month ago

VladChernenko commented 1 month ago

Describe the bug

GM, @Gudahtt and community ๐Ÿ‘‹

Just checked issues #25693 and this one(#25655) in attempts to solve same issue

Intro

We have a sharded EVM in our project Klyntar and definitely wants Metamask compatibility๐ŸฆŠ๐Ÿ˜ƒ We've rewritten the whole JSON-RPC(required methods only) from a scratch and add JS version of EVM.

Seems everything good - balances resolved correctly, chainID and other stuff API works great.

But when I try to send transaction, the status is allways pending, but in reality it's successful and already executed in VM.

As a proof(and mb it will be helpful) - I'll send the screen

image

Here you can see that the status = 1 and tx body & receipt already in VM and state.

What is strange here?

The strange here is that mobile Metamask version is fully compatible with our core - again proof please ๐Ÿ‘‡

image

So, I have issues only with browser extensions

Troubleshooting

So, weaponizing the BurpSuite I started to monitor network connections and notify that:

1) Mobile version sends only obvious queries like eth_estimateGas, eth_blockNumber, eth_sendRawTransaction and so on 2) Browser extensions sends additional eth_call queries to VM via RPC

E.g:


DEBUG: Accept body =>  {
  id: 190,
  jsonrpc: '2.0',
  method: 'eth_call',
  params: [
    {
      to: '0x069bdf66961ce2d38ebe48dd2e095f2c8015ac82',
      data: '0x01ffc9a7d9b67a2600000000000000000000000000000000000000000000000000000000'
    },
    '0x319'
  ]
}
DEBUG: Accept body =>  {
  id: 192,
  jsonrpc: '2.0',
  method: 'eth_call',
  params: [
    {
      to: '0x069bdf66961ce2d38ebe48dd2e095f2c8015ac82',
      data: '0x95d89b41'
    },
    '0x319'
  ]
}
DEBUG: Accept body =>  {
  id: 193,
  jsonrpc: '2.0',
  method: 'eth_call',
  params: [
    {
      to: '0x069bdf66961ce2d38ebe48dd2e095f2c8015ac82',
      data: '0x313ce567'
    },
    '0x319'
  ]
}

My node answer with 0x0 for such queries, because it's simple address => address tx, not a contract call.

But, problem 2

When I've tried to test public network (Sepolia), Metamask sent only obvious queries to Infura URL.

Again, as I proof, I tracked the whole workflow - from building tx to green status of tx finalization:

image

image

image

image

image

Finally, I received the full interaction session with Sepolia URL and there were no eth_call messages at all ๐Ÿ‘€

image

Expected behavior

Final

I've expected that the tx workflow should looks smth like this:

1) Build transaction 2) Send it 3) Metamask monitor for tx receipt 4) Once receipt received - check the status to green(in UI) and show to user

But, somewhy Metamask gives the permanent Pending status for normal tx ๐Ÿ‘€

So, what to do with it? This issue is only for local networks? Or what? Should I downgrade Metamask version? Or wait for fix? Thank you in advance ๐Ÿ˜Š

Screenshots/Recordings

Included in previous parts of template

Steps to reproduce

Extension

  1. Add custom network to Metamask, let it be local RPC node (maybe Metamask triggers only on http://localhost URLs)
  2. Try to send transaction
  3. Everything ok until process of receiving receipt. In network monitoring tools (e.g. BurpSuite, Wireshark) you may find that receipt resolved correctly, Metamask receive it, but still no status update from yellow(Pending) to green(Success)
  4. Then Metamask tries to call eth_call multiple times, resend transaction and fails

Mobile version

  1. Add custom network
  2. Try send default transaction
  3. Everything should be ok, no eth_call calls and you'll get green status immediately(ofc, dependent on you project speed)

Error messages or log output

Included in previous parts of report

Detection stage

In production (default)

Version

11.16.16

Build type

None

Browser

Chrome, Firefox

Operating system

Windows

Hardware wallet

No response

Additional context

No response

Severity

No response

VladChernenko commented 1 month ago

UPD: Bug solution proposal or how to spend tons of hours because of a single byte ๐Ÿฅฒ

Decided to ping all participants of discussion (@Gudahtt๐ŸฆŠ, @jpuri and @gauthierpetetin) to add correct labels and help the community. Because seems such problem is vaild not only for Metamask, but for other wallets too.

Intro

I decided to continue research and check compatibility of our core with other wallets extensions. For test I used:

  1. OKX Wallet
  2. Trust Wallet
  3. Coinbase Wallet
  4. Metamask (extension + mobile version)

The next strange thing I bumped with was the fact that:

  1. OKX Wallet works perfect โœ…
  2. Coinbase Wallet had the same issue as Metamask - permanent pending state โŒ›

This helps understand that RPC & EVM works correctly, so problems on the vendors' side ๐Ÿค”

Solution

So, after a while, after traffic monitoring I decided just compare (AGAIN) receipt structure, because noticed that extensions tries to query receipt again and again, but seems failed to parse it correctly.

Devil is in the details ๐Ÿ˜ˆ

So, EVM (package @ethereumjs/vm) v6.4.2 produce the receipt like this:

image

Notice that the field status is integer here.

Then, I decided to check the docs and noticed that JSON-RPC examples shows that status field is hex-encoded string.

Sources:

  1. https://docs.infura.io/api/networks/ethereum/json-rpc-methods/eth_gettransactionreceipt
  2. https://www.quicknode.com/docs/ethereum/eth_getTransactionReceipt
  3. https://docs.metamask.io/wallet/reference/eth_gettransactionreceipt/

And images:

Quicknode:

image

Infura:

image

Your own docs:

image

On the other hand, Alchemy example shows that status is integer:

image

Test after fix

After I changed the type of status manually (from 1 to 0x1) - it started to works correctly โœ…โœ… โœ…

image

Note that here, the status in receipt (on the left) is hex string


Advices

  1. Confusing fact that Metamask mobile accepted receipt with integer value, but extension - not. Fix it pls, add hex+integer support or at least some one (but same) option
  2. Other wallet extension(except OKX, they support both status format) have the same bug
  3. Add it to your docs or inform the other devs to save their time ๐Ÿ˜Š

Thank you, Metamask Team ๐ŸฆŠ

gauthierpetetin commented 1 month ago

Ignore messages from @ahmar7