MetaMask / metamask-extension

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

[Bug]: Transactions - Sending ETH to a Contract (without any calldata) displays the name of `Token Id Rebase` #27120

Closed seaona closed 1 month ago

seaona commented 1 month ago

Describe the bug

When I'm sending ETH to a contract, I see Contract Interactionin the last confirmation screen. However, when I confirm that transaction, I then see Token Id Rebase into the Activity list.

Expected behavior

I should see Contract Interaction, like it's currently happening in prod

Screenshots/Recordings

https://github.com/user-attachments/assets/9095ae44-fcf9-496f-a057-d9f8cc7abccb

Steps to reproduce

  1. Select Sepolia
  2. Start a Send tx
  3. Add this recipient 0x0Aa40a73021569D4614614cDE1A5ae2B4D96f77a
  4. Send
  5. See how the transaction name is Token Id Rebase

In the case it's needed for debugging, the contract is just this;

// SPDX-License-Identifier: MIT
pragma solidity ^0.6.0;

contract SendMe {
    receive() external payable { }
}

Error messages or log output

No response

Detection stage

During release testing

Version

12.3.0

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

bergeron commented 1 month ago

I do see Contract Interaction if I disable this setting:

Screenshot 2024-09-17 at 10 57 32 AM

With the setting on I got _get All Approved Token Shares For

I'm not immediately familiar with how tx decoding is implemented.

bergeron commented 1 month ago

At one level what happens is that the function signature is 0x, so we hit this:

https://www.4byte.directory/api/v1/signatures/?hex_signature=0x

And take the oldest entry by date. But I'm not sure what it should do instead.

https://github.com/user-attachments/assets/2f6df00b-19c3-4fa4-8d16-168a3f6f68a6

bergeron commented 1 month ago

The difference I see between 12.2 and 12.3 is PR 25759

app/scripts/lib/transaction/metrics.ts calls getMethodData which uses 4byte to identify the contract method. This additionally persists the result in state.metamask.knownMethodData:

  "knownMethodData": {
    "0x": {
      "name": "_get All Approved Token Shares For",
      "params": [
        {
          "type": "address"
        }
      ]
    }
  }

Which is later used when rendering the tx on activity tab.

I don't know what the proper fix should be. Looking to @jpuri or @matthewwalsh0

metamaskbot commented 1 month ago

Missing release label release-12.3.0 on issue. Adding release label release-12.3.0 on issue, as issue is linked to PR #27363 which has this release label.