Skip to content

Conversation

@OGPoyraz
Copy link
Member

@OGPoyraz OGPoyraz commented Oct 28, 2025

Description

This PR aims to integrate extra validations and alert when sending to burn address in EVM transfer transactions.
The list of the additions:

  1. Extend isNative check to check asset address when creating transaction in send flow
  2. Implement new "burnAddress" alert in transfer confirmations - this will also be raised if nested transactions have burn address too.
  3. Filter contacts that has burn address to prevent proceeding to confirmations

Changelog

CHANGELOG entry: Implement extra validations and alert when sending burn address in EVM transfer transactions

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/6124

Manual testing steps

  • You shouldn't be able to proceed to confirmation when you type 0x0000000000000000000000000000000000000000 as recipient
  • Only possibility is to get a confirmation with burn address alert is from test-dapp
  1. Go into the section where you can type recipient
  2. Type 0x0000000000000000000000000000000000000000 to "To" and type something in "Amount" 0x123
  3. When you create transaction, you should be able to see confirmation

Screenshots/Recordings

Before

After

Screenshot 2025-10-28 at 11 46 23 Screenshot 2025-10-28 at 11 46 17

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Adds burn-address detection across send/confirm flows (alerting, contact filtering, nested recipient support) and updates native-token checks for transaction typing.

  • Confirmations:
    • Burn-address alert: New useBurnAddressAlert, AlertKeys.BurnAddress, RowAlertKey.BurnAddress, i18n strings, and metrics mapping; integrated into useConfirmationAlerts and rendered in from-to-row via AlertRow.
    • Recipient detection: Refactor useTransferRecipient and add useNestedTransactionTransferRecipients to extract recipients from nested transactions.
    • UI: InfoRow now supports optional label and renders labelChildren without a label; added labelContainerWithoutLabel style.
  • Send flow:
    • Contacts: useContacts filters out EVM burn addresses using exported LOWER_CASED_BURN_ADDRESSES.
    • Native token check: isNativeToken now matches chain-specific native token addresses (getNativeTokenAddress); used in prepareEVMTransaction/submitEvmTransaction for typing.
  • Validation/Utils:
    • Export LOWER_CASED_BURN_ADDRESSES from send-address-validations for reuse.
  • Tests: Added/updated unit tests for burn-address alert, confirmation alerts aggregation, contacts filtering, recipient hooks (incl. nested), and generic.isNativeToken behavior.

Written by Cursor Bugbot for commit 2d0f1aa. This will update automatically on new commits. Configure here.

@OGPoyraz OGPoyraz requested a review from a team as a code owner October 28, 2025 12:01
@metamaskbot metamaskbot added the team-confirmations Push issues to confirmations team label Oct 28, 2025
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

>
{labelChildren}
</View>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

The 2 label children created now can be merged by adding condition.

Copy link
Member Author

Choose a reason for hiding this comment

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

{(hasLabel || labelChildren) && (
  <View style={{
    ...styles.labelContainer,
    ...(!hasLabel && styles.labelContainerWithoutLabel),
  }}>
    {hasLabel && (
      <>
        <Text variant={TextVariant.BodyMDMedium} color={variant}>
          {label}
        </Text>
        {labelChildren}
        {!labelChildren && tooltip && (
          <Tooltip
            content={tooltip}
            onPress={onTooltipPress}
            title={tooltipTitle ?? label}
          />
        )}
      </>
    )}
    {!hasLabel && labelChildren}
  </View>
)}

Wouldn't it look more complicated if we merge conditions here?

import { useTransferRecipient } from '../../../../hooks/transactions/useTransferRecipient';
import { RowAlertKey } from '../../../UI/info-row/alert-row/constants';
import InfoSection from '../../../UI/info-row/info-section';
import AlertRow from '../../../UI/info-row/alert-row';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: sorting imports

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not going to address sortings anymore as lint rules of the repo doesn't dictate it.

maxCharLength={MAX_CHAR_LENGTH}
/>
{/* Intentional empty label to trigger the alert row without a label */}
<AlertRow alertField={RowAlertKey.BurnAddress} label={''}>
Copy link
Contributor

Choose a reason for hiding this comment

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

We can make label an optional parameter in AlertRow

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

contact.address.startsWith('0x') && contact.address.length === 42
contact.address.startsWith('0x') &&
contact.address.length === 42 &&
!LOWER_CASED_BURN_ADDRESSES.includes(contact.address.toLowerCase())
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are showing alert of burn address do we need to filter it out here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's still needed - if contact has burn address, it's still bypassing regardless of the validations. This is an extra guard to prevent error prone. We can also remove it once they remove possibility of adding burn address as contact.

bschorchit
bschorchit previously approved these changes Oct 28, 2025
Copy link

@bschorchit bschorchit left a comment

Choose a reason for hiding this comment

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

Approving the copy added

cursor[bot]

This comment was marked as outdated.

@OGPoyraz OGPoyraz requested a review from jpuri October 29, 2025 08:32
@sonarqubecloud
Copy link

@OGPoyraz OGPoyraz added this pull request to the merge queue Oct 29, 2025
Merged via the queue into main with commit aa91983 Oct 29, 2025
87 checks passed
@OGPoyraz OGPoyraz deleted the ogp/6124 branch October 29, 2025 11:16
@github-actions github-actions bot locked and limited conversation to collaborators Oct 29, 2025
@metamaskbot metamaskbot added the release-7.59.0 Issue or pull request that will be included in release 7.59.0 label Oct 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.59.0 Issue or pull request that will be included in release 7.59.0 size-L team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants