refactor: decompose rule decoders, and refactor permission decoding#8730
Conversation
…rom makePermissionDecoder.
…, just return the config.
…ions from return object
jeffsmale90
force-pushed
the
refactor/decompose-rule-decoders
branch
from
71e997b to
4d784ae
Compare
jeffsmale90
force-pushed
the
refactor/decompose-rule-decoders
branch
from
4d784ae to
98adc20
Compare
jeffsmale90
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f94844b. Configure here.
mj-kiwi
left a comment
mj-kiwi
left a comment
There was a problem hiding this comment.
Only minor comment tweaks, otherwise LGTM.
| * @param args - The arguments to this function. | ||
| * @param args.contractAddresses - Checksummed enforcer addresses for the chain. | ||
| * @param args.caveats - Checksummed caveats from the delegation. | ||
| * @returns A `{ rule }` result containing the redeemer addresses when a |
There was a problem hiding this comment.
I think the return type should be Rule | null
| * @param args.contractAddresses - Checksummed enforcer addresses for the chain. | ||
| * @param args.caveats - Checksummed caveats from the delegation. | ||
| * @param args.requiredEnforcers - Required enforcer counts for the permission. | ||
| * @returns A `{ rule }` result containing the payee address when an |
There was a problem hiding this comment.
I think the return type should be Rule | null
| * @param args.contractAddresses - Checksummed enforcer addresses for the chain. | ||
| * @param args.caveats - Checksummed caveats from the delegation. | ||
| * @param args.requiredEnforcers - Required enforcer counts for the permission. | ||
| * @returns A `{ rule }` result containing the payee addresses when an |
There was a problem hiding this comment.
I think the return type should be Rule | null
There was a problem hiding this comment.
I think the address and signer fields that don't exist — the actual output uses from and to. Also omits origin and rules fields.
There was a problem hiding this comment.
👍 I removed that entire section of the comment as I think it's unnecessary, given we document the return type
| * - All of its required enforcers are present in the provided list; and | ||
| * - No provided enforcer falls outside the union of the rule's required and | ||
| * - No provided enforcer falls outside the union of the decoder's required and | ||
| * optional enforcers (currently only `TimestampEnforcer` is allowed extra). |
There was a problem hiding this comment.
RedeemerEnforcer, AllowedCalldataEnforcer, and AllowedTargetsEnforcer are also optional.
There was a problem hiding this comment.
I'm removing this function in it's entirety as it's no longer used.
- Update comments on rule decoders to specify correct Rule | null return type - Remove unused findDecoderWithMatchingCaveatAddresses, and rewrite existing tests to cover plural findDecodersWithMatchingCaveatAddresses
mj-kiwi
left a comment
mj-kiwi
left a comment
There was a problem hiding this comment.
LGTM. Just wondering if we should note this breaking change in the changelog.
2 participants
Explanation
Refactor of permission decoding logic in order to:
decodersrather thanrules- rule is a highly overloaded term)no functional change, therefore no changelog required
References
Checklist
Note
Medium Risk
Refactors the core permission identification/decoding path (rule matching, validation, and reconstructed output), so subtle mismatches in enforcer selection or rule extraction could change which permission type is detected or what
rules/expiryare returned.Overview
Refactors permission decoding to use permission decoders (terminology change from “rules”) and removes the old
decodePermission/rulesimplementation, includingfindRuleWithMatchingCaveatAddresses.Decoding is now driven by
createPermissionDecodersForContracts+findDecodersWithMatchingCaveatAddressesand disambiguation viaselectUniqueDecoderAndDecodedPermission, with rule extraction split into composableRuleDecoders (newexpiry,redeemer, and native/ERC20payeedecoders).Adds
EXECUTION_PERMISSION_EXPIRY_RULE_TYPEand updates tests to reflect the new decoder APIs and to assert the new rule decoding behavior (including emitting anexpiryrule and hoisting its timestamp to top-levelexpiry).Reviewed by Cursor Bugbot for commit f097d31. Bugbot is set up for automated code reviews on this repo. Configure here.