-
-
Notifications
You must be signed in to change notification settings - Fork 231
fix(bridge-status-controller): use SnapKeyring.submitRequest to dispatch Snap keyring requests #5924
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
base: main
Are you sure you want to change the base?
Conversation
…tch Snap keyring requests
8512029
to
bd4ede5
Compare
Array [ | ||
"SnapController:handleRequest", | ||
Object { | ||
"handler": "onKeyringRequest", | ||
"origin": "metamask", | ||
"request": Object { | ||
"id": "test-uuid-1234", | ||
"jsonrpc": "2.0", | ||
"method": "keyring_submitRequest", | ||
"params": Object { | ||
"account": undefined, | ||
"id": "test-uuid-1234", | ||
"request": Object { | ||
"method": "signAndSendTransaction", | ||
"params": Object { | ||
"account": Object { | ||
"address": "0x123...", | ||
}, | ||
"scope": "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp", | ||
"transaction": "AQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACAAQAHDXLY8oVRIwA8ZdRSGjM5RIZJW8Wv+Twyw3NqU4Hov+OHoHp/dmeDvstKbICW3ezeGR69t3/PTAvdXgZVdJFJXaxkoKXUTWfEAyQyCCG9nwVoDsd10OFdnM9ldSi+9SLqHpqWVDV+zzkmftkF//DpbXxqeH8obNXHFR7pUlxG9uNVOn64oNsFdeUvD139j1M51iRmUY839Y25ET4jDRscT081oGb+rLnywLjLSrIQx6MkqNBhCFbxqY1YmoGZVORW/QMGRm/lIRcy/+ytunLDm+e8jOW7xfcSayxDmzpAAAAAjJclj04kifG7PRApFI4NgwtaE5na/xCEBI572Nvp+FkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAbd9uHXZaGT2cvhRs7reawctIXtX1s3kTqM9YV+/wCpBHnVW/IxwG7udMVuzmgVB/2xst6j9I5RArHNola8E4+0P/on9df2SnTAmx8pWHneSwmrNt/J3VFLMhqns4zl6JmXkZ+niuxMhAGrmKBaBo94uMv2Sl+Xh3i+VOO0m5BdNZ1ElenbwQylHQY+VW1ydG1MaUEeNpG+EVgswzPMwPoLBgAFAsBcFQAGAAkDQA0DAAAAAAAHBgABAhMICQAHBgADABYICQEBCAIAAwwCAAAAUEYVOwAAAAAJAQMBEQoUCQADBAETCgsKFw0ODxARAwQACRQj5RfLl3rjrSoBAAAAQ2QAAVBGFTsAAAAAyYZnBwAAAABkAAAJAwMAAAEJDAkAAAIBBBMVCQjGASBMKQwnooTbKNxdBwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAUHTKomh4KXvNgA0ovYKS5F8GIOBgAAAAAAAAAAAAAAAAAQgAAAAAAAAAAAAAAAAAAAAAAAEIF7RFOAwAAAAAAAAAAAAAAaAIAAAAAAAC4CwAAAAAAAOAA2mcAAAAAAAAAAAAAAAAAAAAApapuIXG0FuHSfsU8qME9s/kaic0AAwGCsZdSuxV5eCm+Ria4LEQPgTg4bg65gNrTAefEzpAfPQgCABIMAgAAAAAAAAAAAAAACAIABQwCAAAAsIOFAAAAAAADWk6DVOZO8lMFQg2r0dgfltD6tRL/B1hH3u00UzZdgqkAAxEqIPdq2eRt/F6mHNmFe7iwZpdrtGmHNJMFlK7c6Bc6k6kjBezr6u/tAgvu3OGsJSwSElmcOHZ21imqH/rhJ2KgqDJdBPFH4SYIM1kBAAA=", | ||
}, | ||
}, | ||
"scope": "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp", | ||
}, | ||
}, | ||
"snapId": "test-snap", | ||
}, | ||
], |
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 no longer uses the SnapController
to dispatch those request.
"@metamask/gas-fee-controller": "npm:^23.0.0" | ||
"@metamask/keyring-api": "npm:^18.0.0" | ||
"@metamask/multichain-transactions-controller": "npm:^2.0.0" | ||
"@metamask/network-controller": "npm:^23.5.1" | ||
"@metamask/polling-controller": "npm:^13.0.0" | ||
"@metamask/snaps-controllers": "npm:^12.3.1" |
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.
Removed this dependency now that we rely solely on the SnapKeyring
to dispatch keyring requests.
Explanation
I missed this when introducing the
KeyringRequest.origin
feature.The main problem is that this controller is dispatching "raw Snap request", thus, preventing any type-checking when doing keyring API updades.
By using
SnapKeyring
this should prevent this kind of problems in the future and it aligns well on how other controllers interact with the Snap keyring requests!References
N/A
Changelog
Checklist