Merged
Conversation
Contributor
|
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. |
Contributor
✨ Files requiring CODEOWNER review ✨🔑 @MetaMask/accounts-engineers (1 files, +23 -4)
|
ui/pages/permissions-connect/connection-trust-signal-gate/trust-signal-modal.test.tsx
Outdated
Show resolved
Hide resolved
| state === TrustSignalDisplayState.Unknown || | ||
| state === TrustSignalDisplayState.Verified | ||
| ) { | ||
| return <>{children}</>; |
There was a problem hiding this comment.
Dismissed state persists across origin changes
Medium Severity
The dismissed state in ConnectionTrustSignalGate is never reset when origin changes. If a user dismisses a warning/block modal for one malicious origin, and the component stays mounted while processing the next queued connection request with a different malicious origin, dismissed remains true and the trust signal modal is silently bypassed. The gate needs to reset dismissed back to false when origin changes.
Contributor
Builds ready [595f725]
⚡ Performance Benchmarks (1356 ± 113 ms)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
ui/pages/permissions-connect/connection-trust-signal-gate/trust-signal-modal.test.tsx
Outdated
Show resolved
Hide resolved
Contributor
Builds ready [bf2f303]
⚡ Performance Benchmarks (1357 ± 105 ms)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
ui/pages/permissions-connect/connection-trust-signal-gate/connection-trust-signal-gate.test.tsx
Outdated
Show resolved
Hide resolved
Contributor
Builds ready [c332ed9]
⚡ Performance Benchmarks (1388 ± 99 ms)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
...es/multichain-accounts/multichain-accounts-connect-page/multichain-accounts-connect-page.tsx
Show resolved
Hide resolved
Contributor
Builds ready [c66eef3]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
Builds ready [e019ea0]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
Builds ready [58e473d]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
Author
|
hardcoded values still need to removed before merging |
7 tasks
Contributor
|
Seeing a weird effect with the popup lingering after connecting on this PR (haven't check if on Screen.Recording.2026-03-05.at.2.49.48.PM.movScreen.Recording.2026-03-05.at.2.49.32.PM.movScreen.Recording.2026-03-05.at.2.56.07.PM.movEDIT: I am seeing the same thing on |
adonesky1
previously approved these changes
Mar 5, 2026
Contributor
adonesky1
left a comment
There was a problem hiding this comment.
LGTM! Lets just remove those hardcoded values and ship it!
Contributor
Builds ready [f4aca55]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
adonesky1
approved these changes
Mar 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds trust signal modals that gate the wallet connection flow based on dApp trustworthiness from the
dapp-scanningAPI. It shows a block modal when flagged, which requires confirmation in order to advance to the connect screen. Also changes existing connect page confirmation buttons colors and icons based on the trust state.Changelog
CHANGELOG entry: Added confirmation modals for
blocktrust signals on the connect page.Related issues
Fixes WAPI-1071
Manual testing steps
Verified
https://app.uniswap.orgBlock
https://revoke.cashTip
JIRA ticket contains the design links.
Screenshots/Recordings
Before
After
trust-signals-extension.mov
Pre-merge author checklist
Warning
Before merging, Remove temporary
TEST_TRUST_OVERRIDEShostname map fromuseOriginTrustSignals.tsPre-merge reviewer checklist
Note
Medium Risk
Touches the dapp connection/permissions flow by conditionally blocking UI behind a modal and changing the primary connect CTA styling, which could impact users’ ability to connect if trust-signal state is miscomputed.
Overview
Adds a new
ConnectionTrustSignalGatewrapper aroundPermissionsConnectthat shows a blockingTrustSignalModalwhenuseOriginTrustSignalsreports a Malicious origin, requiring an explicit “Connect Anyway” action to proceed.Updates
MultichainAccountsConnectPageto consume the hook’sstatedirectly and reflect trust status in the header (green verified icon, red danger icon for malicious) and to render the Connect button in danger style with a warning icon when malicious. New i18n strings and unit tests are included for the gate and modal.Written by Cursor Bugbot for commit f4aca55. This will update automatically on new commits. Configure here.