Skip to content

Conversation

vinistevam
Copy link
Contributor

@vinistevam vinistevam commented Jul 29, 2025

Explanation

The issue happens when users tries to send a token but provider returns an error when trying to identify if the address is a contract or not. It's an edge case identified on BNB.

This PR improves transaction type detection logic when interacting with an address that may or may not be a contract. Previously, if an error occurred while trying to determine whether an address was a contract (via eth_getCode), the logic would default to returning simpleSend, which could lead to incorrect transaction classification.

Key Changes

  • Improved error handling in readAddressAsContract:
    This helper now returns an error field alongside contractCode and isContractAddress, enabling downstream logic to make better-informed decisions.

  • Enhanced determineTransactionType logic:
    If eth_getCode fails, we no longer assume a simpleSend. Instead, we:
    Fallback to checking the presence of transaction data and attempt decoding via the 4-byte method selector.

References

Fixes MetaMask/metamask-mobile#16574

Changelog

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@vinistevam vinistevam changed the title fix: prevent wrong transaction type assignment when read contract fails fix: improve transaction type detection when contract code lookup fails Jul 29, 2025
@vinistevam
Copy link
Contributor Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/account-tree-controller": "0.7.0-preview-3f5f144a",
  "@metamask-previews/accounts-controller": "32.0.1-preview-3f5f144a",
  "@metamask-previews/address-book-controller": "6.1.1-preview-3f5f144a",
  "@metamask-previews/announcement-controller": "7.0.3-preview-3f5f144a",
  "@metamask-previews/app-metadata-controller": "1.0.0-preview-3f5f144a",
  "@metamask-previews/approval-controller": "7.1.3-preview-3f5f144a",
  "@metamask-previews/assets-controllers": "73.0.1-preview-3f5f144a",
  "@metamask-previews/base-controller": "8.0.1-preview-3f5f144a",
  "@metamask-previews/bridge-controller": "37.0.0-preview-3f5f144a",
  "@metamask-previews/bridge-status-controller": "37.0.0-preview-3f5f144a",
  "@metamask-previews/build-utils": "3.0.3-preview-3f5f144a",
  "@metamask-previews/chain-agnostic-permission": "1.0.0-preview-3f5f144a",
  "@metamask-previews/composable-controller": "11.0.0-preview-3f5f144a",
  "@metamask-previews/controller-utils": "11.11.0-preview-3f5f144a",
  "@metamask-previews/delegation-controller": "0.6.0-preview-3f5f144a",
  "@metamask-previews/earn-controller": "4.0.0-preview-3f5f144a",
  "@metamask-previews/eip1193-permission-middleware": "1.0.0-preview-3f5f144a",
  "@metamask-previews/ens-controller": "17.0.1-preview-3f5f144a",
  "@metamask-previews/error-reporting-service": "2.0.0-preview-3f5f144a",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-3f5f144a",
  "@metamask-previews/foundryup": "1.0.0-preview-3f5f144a",
  "@metamask-previews/gas-fee-controller": "24.0.0-preview-3f5f144a",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-3f5f144a",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-3f5f144a",
  "@metamask-previews/keyring-controller": "22.1.0-preview-3f5f144a",
  "@metamask-previews/logging-controller": "6.0.4-preview-3f5f144a",
  "@metamask-previews/message-manager": "12.0.2-preview-3f5f144a",
  "@metamask-previews/messenger": "0.0.0-preview-3f5f144a",
  "@metamask-previews/multichain-account-service": "0.3.0-preview-3f5f144a",
  "@metamask-previews/multichain-api-middleware": "1.0.0-preview-3f5f144a",
  "@metamask-previews/multichain-network-controller": "0.11.0-preview-3f5f144a",
  "@metamask-previews/multichain-transactions-controller": "4.0.0-preview-3f5f144a",
  "@metamask-previews/name-controller": "8.0.3-preview-3f5f144a",
  "@metamask-previews/network-controller": "24.0.1-preview-3f5f144a",
  "@metamask-previews/notification-services-controller": "15.0.0-preview-3f5f144a",
  "@metamask-previews/permission-controller": "11.0.6-preview-3f5f144a",
  "@metamask-previews/permission-log-controller": "4.0.0-preview-3f5f144a",
  "@metamask-previews/phishing-controller": "13.1.0-preview-3f5f144a",
  "@metamask-previews/polling-controller": "14.0.0-preview-3f5f144a",
  "@metamask-previews/preferences-controller": "18.4.1-preview-3f5f144a",
  "@metamask-previews/profile-sync-controller": "22.0.0-preview-3f5f144a",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-3f5f144a",
  "@metamask-previews/remote-feature-flag-controller": "1.6.0-preview-3f5f144a",
  "@metamask-previews/sample-controllers": "1.0.0-preview-3f5f144a",
  "@metamask-previews/seedless-onboarding-controller": "2.5.0-preview-3f5f144a",
  "@metamask-previews/selected-network-controller": "23.0.0-preview-3f5f144a",
  "@metamask-previews/signature-controller": "32.0.0-preview-3f5f144a",
  "@metamask-previews/token-search-discovery-controller": "3.3.0-preview-3f5f144a",
  "@metamask-previews/transaction-controller": "59.0.0-preview-3f5f144a",
  "@metamask-previews/user-operation-controller": "38.0.0-preview-3f5f144a"
}

@vinistevam vinistevam marked this pull request as ready for review July 29, 2025 10:43
@vinistevam vinistevam requested review from a team as code owners July 29, 2025 10:43
cursor[bot]

This comment was marked as outdated.

OGPoyraz
OGPoyraz previously approved these changes Aug 1, 2025
}

if (!isContractAddress) {
if (!isContractAddress && !hasReadAddressAsContractFailed) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Simple Send Misclassified as Contract Interaction

When eth_getCode fails, the updated logic bypasses the simpleSend classification. This causes plain native asset transfers (those with no transaction data) to be incorrectly categorized as contractInteraction. This regresses behavior for simple sends, especially on providers where getCode intermittently errors (e.g., BNB), affecting UI and downstream logic.

Fix in Cursor Fix in Web

@vinistevam vinistevam closed this Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Incorrect recipient and token when sending BEP-20 on BNB chain
2 participants