fix to field in redeem delegations guide#2158
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| calls: [ | ||
| { | ||
| to: delegateSmartAccount.address, | ||
| to: delegatorSmartAccount.address, |
There was a problem hiding this comment.
I'm not actually sure if we should document delegatorSmartAccount. It assumes the delegatorSmartAccount will be present always which is not the case, if the delegations are stored, or if delegations are used in different environment to redeem like server, it won't stand true. Hence it was using delegateSmartAccount.address. Maybe we should change the documentation for redeemDelegation rather than quickstart. WDYT?
There was a problem hiding this comment.
If we keep delegateSmartAccount here, what about the issue stated in #2082?
Using the delegate's address causes the
UserOperationto fail, often with no clear error unless debugged deeply. The delegation signature isn't validated on the delegate’s account, which leads to invalid operations.
There was a problem hiding this comment.
It should work fine, when redeemDelegations is called on Smart Account, it just works as a proxy for delegation manager. So all the calls are passed and processed by Delegation manager.
There was a problem hiding this comment.
Ok, I've switched to updating the Redeem delegation guide instead, lmk if this is correct.
to field in delegation quickstartto field in redeem delegations guide
There was a problem hiding this comment.
Bug: Delegation Redemption Targets Incorrect Contract
The smart account example for delegation redemption incorrectly targets the delegate's smart account (delegateSmartAccount.address) instead of the DelegationManager contract. The redeemDelegationCalldata is encoded for DelegationManager.redeemDelegations(). Sending this calldata to the delegate's account causes the UserOperation to fail, as the delegate's account does not implement the redeemDelegations function or validate delegation signatures. This logical error occurs in two places and contradicts the EOA example, which correctly targets the DelegationManager.
delegation-toolkit/how-to/redeem-delegation.md#L68-L69
metamask-docs/delegation-toolkit/how-to/redeem-delegation.md
Lines 68 to 69 in 876f11e
delegation-toolkit/how-to/redeem-delegation.md#L258-L259
metamask-docs/delegation-toolkit/how-to/redeem-delegation.md
Lines 258 to 259 in 876f11e
Was this report helpful? Give feedback by reacting with 👍 or 👎
Description
Fix
tofield in the Redeem delegations guide to make it consistent with the quickstart.Issue(s) fixed
Fixes #2082
Checklist
Complete this checklist before merging your PR: