-
Notifications
You must be signed in to change notification settings - Fork 5.5k
fix: Checksum EVM addresses in the address list #38539
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
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
✨ Files requiring CODEOWNER review ✨🔑 @MetaMask/accounts-engineers (2 files, +168 -13)
|
ui/components/multichain-accounts/multichain-address-row/multichain-address-row.tsx
Outdated
Show resolved
Hide resolved
Builds ready [fc89638]
UI Startup Metrics (1255 ± 116 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [45938f5]
UI Startup Metrics (1220 ± 101 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
45938f5 to
221ffd6
Compare
Builds ready [221ffd6]
UI Startup Metrics (1268 ± 107 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
ccharly
left a comment
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.
Code LGTM (not manually tested)!
The only nit comment I would have would be to not use the as Record<string, unknown> and maybe using GROUP_MOCK_ID instead. However it seems the test are "bleeding" and this causes problem between all tests. That's ok, but I feel like something is wrong with the test setup!
Description
The EVM address on the address list page was not checksummed while it was on on the home page. This is important for users so that their addresses match what they see in the ecosystem.
Call the following formatting function.
Changelog
CHANGELOG entry: Fixed bug where the EVM addresses were not checksummed
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/MUL-1325
Manual testing steps
Screenshots/Recordings
Before
After
Screen.Recording.2025-12-03.at.11.51.13.PM.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Checksums EVM addresses for display, copy, and search in the multichain address list while preserving non‑EVM formats, with accompanying tests.
normalizeSafeAddressto provide a normalizedaddressforMultichainAddressRowand for copy actions.normalizedAddressin addition to network name.normalizedAddressper item and render list using the normalized values.sortByPriorityNetworkswith a generic type.Written by Cursor Bugbot for commit 221ffd6. This will update automatically on new commits. Configure here.