Conversation
Jwhiles
had a problem deploying
to
default-branch
GitHub Actions
Failure
|
Warning MetaMask internal reviewing guidelines:
|
| const MAX_UINT256 = | ||
| '115792089237316195423570985008687907853269984665640564039457584007913129639935'; | ||
|
|
||
| // Sourced from https://github.com/MetaMask/snap-cash-account-poc/blob/70709e15ddc56288dd9eefa45b425a756f25d2fb/packages/snap/src/api/config.ts#L39-L40 |
There was a problem hiding this comment.
not sure if this the correct address here - I sourced this from the POC - but it might be that this value should also come from the chomp service details endpoint
Jwhiles
force-pushed
the
musd-445
branch
3 times, most recently
from
33e853a to
16087d2
Compare
| to: delegateAddress, | ||
| caveats: [ | ||
| { type: 'redeemer', redeemers: [vedaVaultAdapterAddress] }, | ||
| { type: 'valueLte', maxValue: 0n }, |
There was a problem hiding this comment.
So if you are using scope then valueLte will automatically be added to the delegation. No need to specify it here.
But maybe a better way would be to not use @metamask/smart-accounts-kit which is a bit bigger package but use @metamask/delegation-core to construct the delegation. Its should still be simple enough but will have much less dependencies then SAK.
There was a problem hiding this comment.
I think here is an example of using delegation core: https://github.com/MetaMask/metamask-extension/pull/41809/changes
There was a problem hiding this comment.
Ow and to get addresses you can use this package: https://github.com/MetaMask/smart-accounts-kit/tree/main/packages/delegation-deployments
| delegatorImplAddress: Hex; | ||
| erc20TransferAmountEnforcer: Hex; | ||
| musdTokenAddress: Hex; | ||
| redeemerEnforcer: Hex; |
There was a problem hiding this comment.
Why are we defining the enforcer addresses here and overriding them? I think we should use the ones provided from the package?
| readonly #steps: Step[] = [ | ||
| associateAddressStep, | ||
| eip7702AuthorizationStep, | ||
| buildDelegationStep, |
There was a problem hiding this comment.
we actually have to build 2 delegations. Both are pretty much the same except the token address is different. One is for deposits (mUSD) and second is for withdrawals (vmUSD => the boringVaultAddress)
| ); | ||
| } | ||
|
|
||
| return 'completed'; |
There was a problem hiding this comment.
we need to also store the delegation then into user authenticated storate + send it to chomp API and intent (delegation hash and some metadata). Not sure if that is means as part of another step.
Jwhiles
changed the title
Jwhiles
force-pushed
the
musd-445
branch
3 times, most recently
from
55a57a7 to
c4c6220
Compare
|
@metamaskbot publish-preview |
|
@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. |
|
@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. |
Adds a third step to the upgrade sequence that builds, signs, and submits the auto-deposit delegation that authorises CHOMP's delegate to move mUSD into the Veda vault on the user's behalf. The step: - Looks up existing delegations via AuthenticatedUserStorageService:listDelegations and skips when one matches the configured (delegator, delegate, chain, token). - Builds a per-call 32-byte salt and constructs the delegation with redeemer + valueLte + erc20TransferAmount caveats. - Signs as EIP-712 V4 typed data via KeyringController:signTypedMessage. - Submits to ChompApiService:verifyDelegation; throws on rejection. The `InitConfig` passed to `init()` carries the delegator-impl and caveat-enforcer addresses; the messenger gains the three new allowed actions.
…yments Replaces the @metamask/smart-accounts-kit dependency with the lower-level @metamask/delegation-core (caveat-term encoders) and @metamask/delegation-deployments (Delegation Framework contract registry). - `init()` now takes only a chainId; the EIP-7702 delegator-impl and caveat-enforcer addresses are resolved from `DELEGATOR_CONTRACTS['1.3.0'][chainId]` rather than being passed in. `InitConfig` is no longer exported. - The build-delegation step builds the three caveats directly with delegation-core's `createERC20TransferAmountTerms` / `createValueLteTerms` / `createRedeemerTerms`, and constructs the EIP-712 typed-data message inline (13 lines). - Drops a duplicate `valueLteEnforcer` caveat that the smart-accounts-kit `erc20TransferAmount` scope helper was inadvertently appending on top of the explicit one we passed in. Net dependency size: ~3 MB → ~650 kB. No behaviour change beyond the duplicate-caveat fix.
Hands off delegation signing (and DelegationManager address resolution) to @metamask/delegation-controller, which the wallet client already wires up globally with a `getDelegationEnvironment` callback. - Adds `@metamask/delegation-controller` as a dependency. - Swaps `KeyringController:signTypedMessage` for `DelegationController:signDelegation` in the messenger allowlist. - Drops `delegationManager` from `UpgradeConfig` / `StepContext`; this controller no longer needs to know the DelegationManager address — DelegationController resolves it. - Removes the inlined `SIGNABLE_DELEGATION_TYPED_DATA` and salt hex→bigint conversion from build-delegations (~25 lines). The build-delegation step still resolves enforcer + EIP-7702 impl addresses from `@metamask/delegation-deployments` directly, since those are statically typed and DelegationController only exposes them via a string-keyed bag.
The build-delegation step now signs two delegations per upgrade — one authorising transfers of mUSD (deposit-side) and one authorising transfers of the Veda boring vault share token vmUSD (withdrawal-side). Both delegations share delegator, delegate, and redeemer (the Veda vault adapter); only the ERC20TransferAmount caveat's token differs. The "already-done" check runs per-token, so re-running the upgrade after a partial failure only re-signs the missing delegation. Signing is sequential, deposit before withdrawal, so the user sees one prompt at a time. The withdrawal-side token is the Veda boring vault contract address. This is hardcoded per chain in the controller (mainnet only) until the CHOMP service-details API exposes it; misconfigured chains throw at init() time.
After CHOMP verifies a delegation, the build-delegation step now also calls AuthenticatedUserStorageService:createDelegation so the signed delegation is stored against the user's profile. Without this the listDelegations matcher on the next run would never find a stored record and we'd re-sign on every upgrade attempt. Order is verify-then-store: if storage fails after CHOMP verification, nothing is persisted and the next run rebuilds from scratch with a fresh salt. The inverse (store-then-verify) would risk persisting a delegation CHOMP later rejects. Metadata records the per-token symbol (mUSD / vmUSD), the cash-deposit / cash-withdrawal intent type, MAX_UINT256 as the allowance, and a delegationHash derived from @metamask/delegation-core's hashDelegation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
Explanation
In this PR we
This has been tested in the mobile client - and it currently reaches the final step of the upgrade process where we
POST https://chomp.dev-api.cx.metamask.io/v1/intent- this currently returns a 500 error after a long delay. We're still investigating the cause of this, as there may be changes required to chomp.References
Checklist
Note
Medium Risk
Moderate risk due to breaking API/messenger changes in
@metamask/money-account-upgrade-controllerand new delegation/intent creation flows that affect account-upgrade sequencing and external service interactions.Overview
Completes the Money Account upgrade sequence by adding two new steps: build + persist auto-deposit/withdraw delegations (signed via
@metamask/delegation-controller, verified by CHOMP, stored in@metamask/authenticated-user-storage) and register CHOMP intents for those delegations (skipping already-active intents and re-registering revoked ones).Introduces breaking changes to
MoneyAccountUpgradeController:init()now accepts{ chainId, boringVaultAddress }, resolves Delegation Framework contract/enforcer addresses from@metamask/delegation-deployments, expands required messenger allowed-actions, and removes the exportedInitConfigtype.Updates
ChompApiServiceto default to a retry policy that does not retry most 4xx responses (except429), while still retrying5xxand network errors; adds targeted tests for this behavior and forpolicyOptionsoverrides. Also makes the EIP-7702 authorization step treat CHOMP409conflicts asalready-donefor retry-safe operation.Reviewed by Cursor Bugbot for commit d0eb979. Bugbot is set up for automated code reviews on this repo. Configure here.