Conversation
ccharly
force-pushed
the
cc/chore/bump-accounts-deps
branch
from
c2f9e74 to
376fd49
Compare
|
@metamaskbot publish-preview |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
|
Added Not sure if there's a better way to handle this for changelogs? |
| KnownCaipNamespace, | ||
| KnownCaipNamespace.Wallet | ||
| >; | ||
| export type NonWalletKnownCaipNamespace = |
There was a problem hiding this comment.
Do we need one for stellar?
There was a problem hiding this comment.
Well, I wasn't sure if there was some more logic needed in the package (not my domain here 😅)
So I preferred to just use an explicit enumeration instead (to avoid having other breaking change in the future). But I'll let the wallet API team to guide me on this one.
But indeed, given the name, we could re-use the Exclude and declare the missing field for the type that relies on it (but that could be seen as a breaking change though..)
There was a problem hiding this comment.
@ccharly I know you posted a comment about this, but given that Stellar is present in KnownCaipNamespace and we are removing it from this type, this does seem like a breaking change we should note in the changelog. What do you think?
There was a problem hiding this comment.
I guess you could make the argument that since chain-agnostic-permission still depends on @metamask/utils ^11.9.0, this is not a breaking change since there's no requirement that consumers must use 11.11.0 (which includes the addition of Stellar to KnownCaipNamespace. So maybe this is fine.
There was a problem hiding this comment.
Yep I know that's a bit tricky 😅 but this change should prevent us from treating new update of this enum as a breaking change in the future.
For now, Stellar is not supported yet on any clients. So we'll have to enable it once we're ready.
jiexi
left a comment
jiexi
left a comment
There was a problem hiding this comment.
packages/chain-agnostic-permission/src/scope/types.ts LGTM
|
@ccharly Given that we are switching to a |
|
Yep, that's correct @mcmire. No breaking changes with this bump. We're keeping the retro-compatibility with the previous version for now. The idea is to migrate v1 to v2 while keeping both alive for a short-period of time. Once everything gets migrated, we might remove the For now, we're just "enabling" the |
…sition-live-price-sync * origin/main: fix(transaction-pay-controller): resolve correct networkClientId for source chain in relay execute (#8492) Release/920.0.0 (#8494) chore: bump `accounts` deps (#8464) feat: adds auth to social controllers (#8485) Release/919.0.0 (#8482) feat(seedless-onboarding-controller): generic encryptor result type aligned with KeyringController TO-686 (#8411) Release/918.0.0 (#8478) chore: add periodic check for spl tokens (#8400)
9 participants
Explanation
Bumping all
accountsdeps to the latest. Mostly including the new/v2exports for the on-going work with the keyring API v2.References
N/A
Checklist
Note
Medium Risk
Touches account/keyring-adjacent packages and upgrades multiple keyring dependencies, including switching some types/imports to
@metamask/keyring-api/v2, which could cause subtle type/runtime integration issues if consumers or downstream deps assume old exports.Overview
Updates multiple controllers/services to newer keyring/account package versions (notably
@metamask/keyring-api@^23.0.1,@metamask/keyring-internal-api@^10.1.1,@metamask/keyring-snap-client@^9.0.1,@metamask/eth-snap-keyring@^21.0.1, andeth-*keyrings), with corresponding changelog and lockfile updates.Adopts new
/v2exports in key places:keyring-controllernow imports V2 keyring classes/types from@metamask/eth-hd-keyring/v2,@metamask/eth-simple-keyring/v2, and@metamask/keyring-api/v2, andmultichain-account-serviceupdatesKeyringCapabilitiestyping to come from@metamask/keyring-api/v2.Adds Stellar account-type ordering in
account-tree-controllerby includingXlmAccountType.AccountinACCOUNT_TYPE_TO_SORT_ORDER, and tightenschain-agnostic-permissiontyping by explicitly enumerating supported non-wallet CAIP namespaces rather than usingExclude.Reviewed by Cursor Bugbot for commit dc56540. Bugbot is set up for automated code reviews on this repo. Configure here.