-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Implement extra validations and alert when sending burn address in EVM transfer transactions #21772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| > | ||
| {labelChildren} | ||
| </View> | ||
| )} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: sorting imports
There was a problem hiding this comment.
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={''}> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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()) |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this 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
|



Description
This PR aims to integrate extra validations and alert when sending to burn address in EVM transfer transactions.
The list of the additions:
isNativecheck to check asset address when creating transaction in send flowChangelog
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
0x0000000000000000000000000000000000000000as recipient0x0000000000000000000000000000000000000000to "To" and type something in "Amount" 0x123Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Adds burn-address detection across send/confirm flows (alerting, contact filtering, nested recipient support) and updates native-token checks for transaction typing.
useBurnAddressAlert,AlertKeys.BurnAddress,RowAlertKey.BurnAddress, i18n strings, and metrics mapping; integrated intouseConfirmationAlertsand rendered infrom-to-rowviaAlertRow.useTransferRecipientand adduseNestedTransactionTransferRecipientsto extract recipients from nested transactions.InfoRownow supports optionallabeland renderslabelChildrenwithout a label; addedlabelContainerWithoutLabelstyle.useContactsfilters out EVM burn addresses using exportedLOWER_CASED_BURN_ADDRESSES.isNativeTokennow matches chain-specific native token addresses (getNativeTokenAddress); used inprepareEVMTransaction/submitEvmTransactionfor typing.LOWER_CASED_BURN_ADDRESSESfromsend-address-validationsfor reuse.generic.isNativeTokenbehavior.Written by Cursor Bugbot for commit 2d0f1aa. This will update automatically on new commits. Configure here.