-
Notifications
You must be signed in to change notification settings - Fork 5.4k
chore: bump @metamask/{keyring,seedless-onboarding}-controller
#37454
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
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
@metamask/{keyring,seedless-onboarding}-controller@metamask/{keyring,seedless-onboarding}-controller
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
✨ Files requiring CODEOWNER review ✨📜 @MetaMask/policy-reviewers (6 files, +5 -120)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. |
Builds ready [5d92049]
UI Startup Metrics (1277 ± 78 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
Builds ready [e776872]
UI Startup Metrics (1213 ± 85 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
f70e5ae to
825bbeb
Compare
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
Builds ready [0a057de]
UI Startup Metrics (1218 ± 80 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
Builds ready [acb5bf2]
UI Startup Metrics (1200 ± 106 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
cryptodev-2s
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.
LGTM!
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
Builds ready [9126053]
UI Startup Metrics (1257 ± 113 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
cryptodev-2s
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.
LGTM!
Builds ready [14cfe12]
UI Startup Metrics (1223 ± 101 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
| let payload: EncryptionResult; | ||
| if (typeof encryptedData === 'string') { | ||
| payload = JSON.parse(encryptedData); | ||
| } else { | ||
| payload = encryptedData; | ||
| } | ||
|
|
||
| return encryptor.decryptWithKey(key as EncryptionKey, payload); |
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.
Is this compatible with the encryptor passed now?
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.
There has been this broken type in KeyringController for a while now, but it was aligned with the real type coming from browser-passworder here: https://github.com/MetaMask/core/pull/7127/files#diff-cacd62f731d7f6947fe841f6ba3441c7a16f764b699801b0c212e0e6c15879bfR423 - encryptedData should never be a string when being decrypted with decryptWithKey
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.
Just making sure that the SeedlessOnboardingController doesn't expect to be able to pass a string? But I guess not since the types are valid
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.
Correct, SeedlessOnboardingController always parses the vault before passing it to decryptWithKey: https://github.com/MetaMask/core/blob/8f42d26998ec95ccbcc3dba16d6aff0490cec052/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts#L1403-L1403
cryptodev-2s
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.
LGTM!
Builds ready [3733c29]
UI Startup Metrics (1270 ± 116 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
Depends on MetaMask/core#7128
Description
Bumping
keyring-controllerandseedless-onboarding-controllerto pick up the latest changes.@metamask/browser-passworderis also being bumped to^6.0.0, and the encryptor factory has been adjusted to includekeyFromPassword,generateSaltandexportKeyas they are required byKeyringController.Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Upgrades keyring, seedless onboarding, and passworder deps and updates controller init, types, tests, and policies to the new encryptor API (removes
cacheEncryptionKey).@metamask/keyring-controllerto^25.0.0,@metamask/seedless-onboarding-controllerto^7.0.0, and@metamask/browser-passworderto^6.0.0.keyring-controller-init: removecacheEncryptionKeywhen constructingKeyringController.seedless-onboarding-controller-init: useSeedlessOnboardingController<CryptoKey | EncryptionKey>and passencryptordirectly (drop custom wrapper).encryptortype from optionalExportableKeyEncryptorto requiredEncryptorincontroller-init/types.encryptWithKey,isVaultUpdated,exportKey,generateSalt,keyFromPassword).importKeyreturning parsed key and newexportKey.cacheEncryptionKeyfrom test expectation.browser-passworderchanges (remove nested package allowances, adjust to@metamask/utils).Written by Cursor Bugbot for commit 3733c29. This will update automatically on new commits. Configure here.