feat: improve chomp upgrade process#9387
Draft
Jwhiles wants to merge 1 commit into
Draft
Conversation
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PR: GET-first address association for the Money Account upgrade flow
Repos: MetaMask/core → metamask-mobile (adoption to follow)
Packages:
@metamask/chomp-api-service(breaking → 4.0.0),@metamask/money-account-upgrade-controller(breaking → 3.0.0)Explanation
The
associate-addressstep of the Money Account upgrade flow currently signs aCHOMP Authenticationmessage and POSTs it to/v1/auth/addresson every run,relying on the response to discover that the address was already associated. That
means a keyring signing operation happens even when there is nothing to do.
This PR makes the step check first and sign only when needed:
@metamask/chomp-api-service: addsgetAssociatedAddresses()(
GET /v1/auth/address), which returns the authenticated profile's activeaddress associations. Results are parsed into canonical form (lowercased
addresses,
statusguaranteed'active') and are never served from cache,since the response is scoped to the authenticated profile and consumers use it
to decide whether to sign.
@metamask/money-account-upgrade-controller: theassociate-addressstepnow calls the new action first and reports
already-done— without touchingthe keyring — when the address is already associated. The lookup is an
optimization: if it fails, the step falls through to the previous
sign-and-submit behaviour.
While verifying the endpoint's semantics against the CHOMP API source, I found
that the existing 409 handling was wrong. CHOMP returns
201 +
status: 'active'when the address is already associated with theauthenticated profile; a 409 means the address is associated with a
different profile (or, rarely, that two same-profile requests raced on the
initial create). The old code treated 409 as a success case and tried to parse
its error body as an association result, which would have failed with a
confusing validation error. Now:
associateAddressthrows anHttpErroron any non-OK response (breaking).race resolves to
already-done; a genuine cross-profile conflict fails thestep with a clear error.
The controller change is breaking because
MoneyAccountUpgradeControllerMessengerconsumers must now grant
ChompApiService:getAssociatedAddressesand pair itwith
@metamask/chomp-api-service>= 4.0.0.References
Checklist