-
-
Notifications
You must be signed in to change notification settings - Fork 24
feat: accept delegation chain as either encoded Hex or decoded Delegation[]
#140
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
f4c926d to
1ac52a5
Compare
- Adds new PermissionContext type that describes either a decoded Delegation[] or abi encoded Hex string, added to public API - Rename permissionsContext parameter in ERC7710 action to permissionContext (singular) - Accept PermissionContext in ERC-7710 actions and DelegationManager redeem paths - encodeDelegations/decodeDelegations now both accept PermissionContext
- prepDelegationHashForPasskeySign - getCaveatArrayPacketHash - deployWithSimpleFactory
1ac52a5 to
25da42d
Compare
| if (Array.isArray(delegations)) { | ||
| return delegations; | ||
| } | ||
| // decodeDelegationsCore returns DelegationStruct, so we need to map it back to Delegation |
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.
Do you think we should validate the Hex String before decoding it?
| return encodeDelegationsCore(delegationStructs); | ||
| return encodeDelegationsCore(delegationStructs); | ||
| } | ||
| return delegations; |
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.
Same question here。 should we assume that delegations is always a valid hex, or do we need to add a check?
mj-kiwi
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.
Looks good to me. Just noticed there aren’t any tests covering decodeDelegations() for both Delegation[] and hex inputs.
📝 Description
Updates various methods that accept a delegation array or permission context to accept either encoded
Hexor decodedDelegation[].🔄 What Changed?
permissionsContextparameter in ERC7710 action topermissionContext(singular)PermissionContexttype that describes either a decodedDelegation[]or abi encodedHexstring, added to public APIAdditionally removes unused internal functions
prepDelegationHashForPasskeySigngetCaveatArrayPacketHashdeployWithSimpleFactoryHint: view the changes by separate commit to see these broken out.
🚀 Why?
Depending on the specific implementation, a consumer may have the data either encoded or decoded. By accepting the data in whatever format the user has, it removes the need for the consumer to encode or decode.
By standardising the terminology it makes the overall API easier to use.
Changes
New public type
PermissionContextAdded
PermissionContext(exported from@metamask/smart-accounts-kit):This represents either a decoded delegation chain (
Delegation[]) or an ABI‑encoded hex string.🚨 Breaking
DelegatedCalltype:permissionsContextrenamed topermissionContext.Before:
After:
🚨 BREAKING
Redemptiontype:permissionsContextrenamed topermissionContextpermissionContextis also now typed asPermissionContextinstead ofHex, meaning a value ofDelegation[]may be provided.Before:
After:
🚨 Breaking:
SendTransactionWithDelegationandSendUserOperationWithDelegationSendTransactionWithDelegation: Renamed field and widened typepermissionContext: Hex→permissionsContext: PermissionContext(accepts ABI-encodedHexor decodedDelegation[])SendUserOperationWithDelegation: field inDelegatedCalltype renamed and widenedcalls: DelegatedCall,permissionsContext: Hex→permissionContext: PermissionContext(now supportsDelegation[])DelegationManager
redeemDelegationsutilities now acceptPermissionContextEncodeRedeemDelegationsParametersused in theredeemDelegationutilities (encode,execute, andsimulate)delegations: Delegation[][]is nowPermissionContext[].Before:
After:
encode/decode helper signatures now accept
PermissionContextencodeDelegations now accepts
PermissionContext:decodeDelegations now accepts
PermissionContext:📋 Checklist
Check off completed items:
🔗 Related Issues
Link to related issues:
Closes #
Related to #
📚 Additional Notes
Any additional information, concerns, or context:
Note
Medium Risk
Medium risk because it is a breaking API change that widens inputs across ERC-7710 actions and
redeemDelegations, and it changes encoding/decoding behavior that affects on-chain calldata generation.Overview
Introduces a new public
PermissionContexttype (HexorDelegation[]) and updates delegation flows to accept either encoded or decoded delegation chains.Breaking API rename: switches
permissionsContext→permissionContextin ERC-7710 delegated calls/params, and updatesRedemption.permissionContextplusDelegationManager.redeemDelegationsencode/simulate/execute to takePermissionContextand encode as needed.Updates encoding utilities so
encodeDelegations/decodeDelegationsare idempotent overPermissionContext, adjustsencodeCallsto encode delegated calls viaencodeDelegations, and removes a few unused internal helpers; e2e/unit tests and changelog are updated accordingly.Written by Cursor Bugbot for commit 25da42d. This will update automatically on new commits. Configure here.