From 7bc3e65d29bab25492a2ee1c136e481b76de8b3c Mon Sep 17 00:00:00 2001 From: MJ Kiwi Date: Mon, 4 May 2026 15:37:14 +1200 Subject: [PATCH 1/9] feat: enhance permission rules with payee enforcers - Added support for payee enforcers in makePermissionRule, allowing for extraction of allowedCalldata and allowedTargets addresses. - Updated native token rules (allowance, periodic, stream) to include payee enforcers. - Introduced new PayeeRule type for execution permission rules. - Modified utility functions to handle new payee enforcer logic. - Updated tests to validate the inclusion of new enforcers in permission rules. --- ...et-supported-execution-permissions.test.ts | 4 +- .../gator-permissions-controller/CHANGELOG.md | 3 + .../gator-permissions-controller/package.json | 4 +- .../src/constants.ts | 8 + .../rules/erc20TokenAllowance.ts | 16 +- .../rules/erc20TokenPeriodic.ts | 16 +- .../rules/erc20TokenRevocation.ts | 14 +- .../rules/erc20TokenStream.ts | 16 +- .../rules/makePermissionRule.test.ts | 565 +++++++++++++++++- .../rules/makePermissionRule.ts | 163 ++++- .../rules/nativeTokenAllowance.ts | 16 +- .../rules/nativeTokenPeriodic.ts | 16 +- .../rules/nativeTokenStream.ts | 16 +- .../src/decodePermission/types.ts | 2 + .../src/decodePermission/utils.test.ts | 89 ++- .../src/decodePermission/utils.ts | 12 + .../gator-permissions-controller/src/index.ts | 2 + .../src/payeeRule.ts | 13 + yarn.lock | 26 +- 19 files changed, 955 insertions(+), 46 deletions(-) create mode 100644 packages/gator-permissions-controller/src/payeeRule.ts diff --git a/packages/eth-json-rpc-middleware/src/methods/wallet-get-supported-execution-permissions.test.ts b/packages/eth-json-rpc-middleware/src/methods/wallet-get-supported-execution-permissions.test.ts index 2fcec4d658..2ff8e60f79 100644 --- a/packages/eth-json-rpc-middleware/src/methods/wallet-get-supported-execution-permissions.test.ts +++ b/packages/eth-json-rpc-middleware/src/methods/wallet-get-supported-execution-permissions.test.ts @@ -10,7 +10,7 @@ import { createWalletGetSupportedExecutionPermissionsHandler } from './wallet-ge const RESULT_MOCK: GetSupportedExecutionPermissionsResult = { 'native-token-allowance': { chainIds: ['0x123', '0x345'] as Hex[], - ruleTypes: ['expiry', 'redeemer'], + ruleTypes: ['expiry', 'redeemer', 'payee'], }, 'erc20-token-allowance': { chainIds: ['0x123'] as Hex[], @@ -18,7 +18,7 @@ const RESULT_MOCK: GetSupportedExecutionPermissionsResult = { }, 'erc721-token-allowance': { chainIds: ['0x123'] as Hex[], - ruleTypes: ['expiry', 'redeemer'], + ruleTypes: ['expiry', 'redeemer', 'payee'], }, }; diff --git a/packages/gator-permissions-controller/CHANGELOG.md b/packages/gator-permissions-controller/CHANGELOG.md index 3795fe6e8e..d81f31f44a 100644 --- a/packages/gator-permissions-controller/CHANGELOG.md +++ b/packages/gator-permissions-controller/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Support payee-restricting caveats when decoding execution permissions + - Permission decoding now recognizes `AllowedTargetsEnforcer`, `AllowedCalldataEnforcer`, and `LogicalOrWrapperEnforcer` as optional caveats for payee extraction on execution permission types and extracts a `payee` rule containing the allowlisted addresses. + - Export new `EXECUTION_PERMISSION_PAYEE_RULE_TYPE` constant and `PayeeRule` type. - Support `RedeemerEnforcer` caveat when decoding execution permissions ([#8537](https://github.com/MetaMask/core/pull/8537)) - Permission decoding now recognizes the `RedeemerEnforcer` as an optional caveat on all execution permission types and extracts a `redeemer` rule containing the allowlisted addresses. - `DecodedPermission` type now includes an optional `rules` property for rules recovered from caveats. diff --git a/packages/gator-permissions-controller/package.json b/packages/gator-permissions-controller/package.json index a02743d2ac..30ced8cc83 100644 --- a/packages/gator-permissions-controller/package.json +++ b/packages/gator-permissions-controller/package.json @@ -56,8 +56,8 @@ "@metamask/7715-permission-types": "^0.6.0", "@metamask/abi-utils": "^2.0.3", "@metamask/base-controller": "^9.1.0", - "@metamask/delegation-core": "^1.1.0", - "@metamask/delegation-deployments": "^0.12.0", + "@metamask/delegation-core": "link:../../../smart-accounts-kit/packages/delegation-core", + "@metamask/delegation-deployments": "link:../../../smart-accounts-kit/packages/delegation-deployments", "@metamask/messenger": "^1.2.0", "@metamask/network-controller": "^30.1.0", "@metamask/snaps-controllers": "^19.0.0", diff --git a/packages/gator-permissions-controller/src/constants.ts b/packages/gator-permissions-controller/src/constants.ts index feda806752..87d7a6d3b7 100644 --- a/packages/gator-permissions-controller/src/constants.ts +++ b/packages/gator-permissions-controller/src/constants.ts @@ -10,3 +10,11 @@ export const DELEGATION_FRAMEWORK_VERSION = '1.3.0'; * supported execution permission type. */ export const EXECUTION_PERMISSION_REDEEMER_RULE_TYPE = 'redeemer' as const; + +/** + * `Rule.type` / `wallet_getSupportedExecutionPermissions` `ruleTypes` entry for + * payee allowlists (AllowedCalldataEnforcer / AllowedTargetsEnforcer, optionally + * wrapped in LogicalOrWrapperEnforcer). Hosts should advertise this for every + * supported execution permission type that supports payee restrictions. + */ +export const EXECUTION_PERMISSION_PAYEE_RULE_TYPE = 'payee' as const; diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenAllowance.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenAllowance.ts index 47d41de17f..e55eedb340 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenAllowance.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenAllowance.ts @@ -33,12 +33,26 @@ export function makeErc20TokenAllowanceRule( erc20PeriodicEnforcer, valueLteEnforcer, nonceEnforcer, + allowedCalldataEnforcer, + allowedTargetsEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-allowance', - optionalEnforcers: [timestampEnforcer, redeemerEnforcer], + optionalEnforcers: [ + timestampEnforcer, + redeemerEnforcer, + allowedCalldataEnforcer, + logicalOrWrapperEnforcer, + ], redeemerEnforcer, + payeeEnforcers: { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedCalldataEnforcer, + logicalOrWrapperEnforcer, + }, timestampEnforcer, requiredEnforcers: { [erc20PeriodicEnforcer]: 1, diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts index 7ccd7430e1..3fdaa34398 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts @@ -29,12 +29,26 @@ export function makeErc20TokenPeriodicRule( erc20PeriodicEnforcer, valueLteEnforcer, nonceEnforcer, + allowedCalldataEnforcer, + allowedTargetsEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-periodic', - optionalEnforcers: [timestampEnforcer, redeemerEnforcer], + optionalEnforcers: [ + timestampEnforcer, + redeemerEnforcer, + allowedCalldataEnforcer, + logicalOrWrapperEnforcer, + ], redeemerEnforcer, + payeeEnforcers: { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedCalldataEnforcer, + logicalOrWrapperEnforcer, + }, timestampEnforcer, requiredEnforcers: { [erc20PeriodicEnforcer]: 1, diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts index 8dde558038..cdcfe0e35f 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts @@ -24,14 +24,26 @@ export function makeErc20TokenRevocationRule( const { timestampEnforcer, allowedCalldataEnforcer, + allowedTargetsEnforcer, valueLteEnforcer, nonceEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-revocation', - optionalEnforcers: [timestampEnforcer, redeemerEnforcer], + optionalEnforcers: [ + timestampEnforcer, + redeemerEnforcer, + logicalOrWrapperEnforcer, + ], redeemerEnforcer, + payeeEnforcers: { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedCalldataEnforcer, + logicalOrWrapperEnforcer, + }, timestampEnforcer, requiredEnforcers: { [allowedCalldataEnforcer]: 2, diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts index ee08c3852f..86eaeec7d4 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts @@ -28,12 +28,26 @@ export function makeErc20TokenStreamRule( erc20StreamingEnforcer, valueLteEnforcer, nonceEnforcer, + allowedCalldataEnforcer, + allowedTargetsEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-stream', - optionalEnforcers: [timestampEnforcer, redeemerEnforcer], + optionalEnforcers: [ + timestampEnforcer, + redeemerEnforcer, + allowedCalldataEnforcer, + logicalOrWrapperEnforcer, + ], redeemerEnforcer, + payeeEnforcers: { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedCalldataEnforcer, + logicalOrWrapperEnforcer, + }, timestampEnforcer, requiredEnforcers: { [erc20StreamingEnforcer]: 1, diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts index 37a7906840..45ff503200 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts @@ -1,4 +1,9 @@ -import { createTimestampTerms } from '@metamask/delegation-core'; +import { + createAllowedCalldataTerms, + createAllowedTargetsTerms, + createLogicalOrWrapperTerms, + createTimestampTerms, +} from '@metamask/delegation-core'; import { CHAIN_ID, DELEGATOR_CONTRACTS, @@ -13,6 +18,23 @@ describe('makePermissionRule', () => { const timestampEnforcer = contracts.TimestampEnforcer; const requiredEnforcer = contracts.NonceEnforcer; const redeemerEnforcer = contracts.RedeemerEnforcer; + const allowedCalldataEnforcer = contracts.AllowedCalldataEnforcer; + const allowedTargetsEnforcer = contracts.AllowedTargetsEnforcer; + const logicalOrWrapperEnforcer = contracts.LogicalOrWrapperEnforcer; + + const payeeEnforcersNative = { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedTargetsEnforcer, + logicalOrWrapperEnforcer, + }; + + const payeeEnforcersErc20 = { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedCalldataEnforcer, + logicalOrWrapperEnforcer, + }; it('calls optional validate callback when provided and decoding succeeds', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); @@ -21,6 +43,7 @@ describe('makePermissionRule', () => { permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, optionalEnforcers: [], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -60,6 +83,7 @@ describe('makePermissionRule', () => { permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, optionalEnforcers: [], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -95,6 +119,7 @@ describe('makePermissionRule', () => { permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, optionalEnforcers: [], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -131,6 +156,7 @@ describe('makePermissionRule', () => { permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, optionalEnforcers: [], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -169,6 +195,7 @@ describe('makePermissionRule', () => { permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, optionalEnforcers: [], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -208,6 +235,7 @@ describe('makePermissionRule', () => { permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, optionalEnforcers: [], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -239,6 +267,7 @@ describe('makePermissionRule', () => { permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, optionalEnforcers: [], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -276,4 +305,538 @@ describe('makePermissionRule', () => { }, ]); }); + + it('includes payee rule when AllowedTargetsEnforcer caveat is present (native)', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress = '0x2222222222222222222222222222222222222222' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [allowedTargetsEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ targets: [payeeAddress] }), + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toStrictEqual([ + { + type: 'payee', + data: { + addresses: [getChecksumAddress(payeeAddress)], + }, + }, + ]); + }); + + it('includes payee rule when AllowedCalldataEnforcer caveat is present (erc20)', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress = '0x3333333333333333333333333333333333333333' as Hex; + const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}`; + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [allowedCalldataEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: paddedAddress, + }), + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toStrictEqual([ + { + type: 'payee', + data: { + addresses: [getChecksumAddress(payeeAddress)], + }, + }, + ]); + }); + + it('includes payee rule with multiple addresses via LogicalOrWrapperEnforcer (native)', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; + const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ + targets: [payeeAddress1], + }), + args: '0x00', + }, + ], + [ + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ + targets: [payeeAddress2], + }), + args: '0x00', + }, + ], + ], + }), + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toStrictEqual([ + { + type: 'payee', + data: { + addresses: [ + getChecksumAddress(payeeAddress1), + getChecksumAddress(payeeAddress2), + ], + }, + }, + ]); + }); + + it('includes both redeemer and payee rules when both caveats present', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const redeemerAddr = '1111111111111111111111111111111111111111' as const; + const payeeAddress = '0x2222222222222222222222222222222222222222' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [redeemerEnforcer, allowedTargetsEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: redeemerEnforcer, + terms: `0x${redeemerAddr}` as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ targets: [payeeAddress] }), + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toHaveLength(2); + expect(result.rules).toStrictEqual([ + { + type: 'redeemer', + data: { + addresses: [getChecksumAddress(`0x${redeemerAddr}` as Hex)], + }, + }, + { + type: 'payee', + data: { + addresses: [getChecksumAddress(payeeAddress)], + }, + }, + ]); + }); + + it('includes payee rule with multiple addresses via LogicalOrWrapperEnforcer (erc20)', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; + const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; + const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; + const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded1, + }), + args: '0x00', + }, + ], + [ + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded2, + }), + args: '0x00', + }, + ], + ], + }), + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toStrictEqual([ + { + type: 'payee', + data: { + addresses: [ + getChecksumAddress(payeeAddress1), + getChecksumAddress(payeeAddress2), + ], + }, + }, + ]); + }); + + it('does not include payee rule when no payee caveat is present', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toBeUndefined(); + }); + + it('returns true from caveatAddressesMatch when enforcers match rule', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [timestampEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + expect( + rule.caveatAddressesMatch([requiredEnforcer, timestampEnforcer]), + ).toBe(true); + expect(rule.caveatAddressesMatch([requiredEnforcer])).toBe(true); + expect(rule.caveatAddressesMatch([])).toBe(false); + }); + + it('ignores unrecognised inner enforcers in LogicalOrWrapperEnforcer', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress = '0x4444444444444444444444444444444444444444' as Hex; + const unknownEnforcer = '0x9999999999999999999999999999999999999999' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ + targets: [payeeAddress], + }), + args: '0x00', + }, + ], + [ + { + enforcer: unknownEnforcer, + terms: + '0x0000000000000000000000000000000000000000000000000000000000000001', + args: '0x00', + }, + ], + ], + }), + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toStrictEqual([ + { + type: 'payee', + data: { + addresses: [getChecksumAddress(payeeAddress)], + }, + }, + ]); + }); + + it('returns null payee when LogicalOrWrapper inner caveats are all unrecognised', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const unknownEnforcer = '0x9999999999999999999999999999999999999999' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: unknownEnforcer, + terms: + '0x0000000000000000000000000000000000000000000000000000000000000001', + args: '0x00', + }, + ], + ], + }), + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toBeUndefined(); + }); + + it('returns null payee when singlePayeeEnforcer is unrecognised', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const unknownEnforcer = '0x8888888888888888888888888888888888888888' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: unknownEnforcer, + logicalOrWrapperEnforcer, + }, + optionalEnforcers: [unknownEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: unknownEnforcer, + terms: + '0x0000000000000000000000000000000000000000000000000000000000000001' as Hex, + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toBeUndefined(); + }); + + it('handles multiple singlePayeeEnforcer caveats gracefully (e.g. revocation)', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + + const rule = makePermissionRule({ + permissionType: 'erc20-token-revocation', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [], + requiredEnforcers: { + [allowedCalldataEnforcer]: 2, + [requiredEnforcer]: 1, + }, + validateAndDecodeData, + }); + + const caveats = [ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedCalldataEnforcer, + terms: + '0x0000000000000000000000000000000000000000000000000000000000000000095ea7b3' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedCalldataEnforcer, + terms: + '0x00000000000000000000000000000000000000000000000000000000000000240000000000000000000000000000000000000000000000000000000000000000' as Hex, + args: '0x' as Hex, + }, + ]; + + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toBeUndefined(); + }); }); diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts index a2cefb1f21..a9ec2477eb 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts @@ -1,10 +1,18 @@ import type { Rule } from '@metamask/7715-permission-types'; import type { Caveat } from '@metamask/delegation-core'; -import { decodeRedeemerTerms } from '@metamask/delegation-core'; +import { + decodeAllowedCalldataTerms, + decodeAllowedTargetsTerms, + decodeLogicalOrWrapperTerms, + decodeRedeemerTerms, +} from '@metamask/delegation-core'; import { getChecksumAddress, isStrictHexString } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; -import { EXECUTION_PERMISSION_REDEEMER_RULE_TYPE } from '../../constants'; +import { + EXECUTION_PERMISSION_PAYEE_RULE_TYPE, + EXECUTION_PERMISSION_REDEEMER_RULE_TYPE, +} from '../../constants'; import type { ChecksumCaveat, DecodedPermission, @@ -26,6 +34,11 @@ import { * @param args - The arguments to this function. * @param args.optionalEnforcers - Enforcer addresses that may appear in addition to required. * @param args.redeemerEnforcer - Address of the RedeemerEnforcer used to extract redeemer rules. + * @param args.payeeEnforcers - Addresses of enforcers used to extract payee rules. + * @param args.payeeEnforcers.allowedCalldataEnforcer - AllowedCalldataEnforcer address (ERC20 payee). + * @param args.payeeEnforcers.allowedTargetsEnforcer - AllowedTargetsEnforcer address (native payee). + * @param args.payeeEnforcers.singlePayeeEnforcer - The specific enforcer used for single-payee in this permission type. + * @param args.payeeEnforcers.logicalOrWrapperEnforcer - The LogicalOrWrapperEnforcer for multi-payee. * @param args.timestampEnforcer - Address of the TimestampEnforcer used to extract expiry. * @param args.permissionType - The permission type identifier. * @param args.requiredEnforcers - Map of required enforcer address to required count. @@ -35,6 +48,7 @@ import { export function makePermissionRule({ optionalEnforcers, redeemerEnforcer, + payeeEnforcers, timestampEnforcer, permissionType, requiredEnforcers, @@ -42,6 +56,12 @@ export function makePermissionRule({ }: { optionalEnforcers: Hex[]; redeemerEnforcer: Hex; + payeeEnforcers: { + allowedCalldataEnforcer: Hex; + allowedTargetsEnforcer: Hex; + singlePayeeEnforcer: Hex; + logicalOrWrapperEnforcer: Hex; + }; timestampEnforcer: Hex; permissionType: PermissionType; requiredEnforcers: Record; @@ -106,22 +126,143 @@ export function makePermissionRule({ throwIfNotFound: false, }); - let rules: Rule[] | undefined; + const rules: Rule[] = []; if (redeemerTerms) { - rules = [ - { - type: EXECUTION_PERMISSION_REDEEMER_RULE_TYPE, - data: { - addresses: decodeRedeemerTerms(redeemerTerms).redeemers, - }, + rules.push({ + type: EXECUTION_PERMISSION_REDEEMER_RULE_TYPE, + data: { + addresses: decodeRedeemerTerms(redeemerTerms).redeemers, }, - ]; + }); + } + + const payeeAddresses = extractPayeeAddresses( + checksumCaveats, + payeeEnforcers, + ); + if (payeeAddresses) { + rules.push({ + type: EXECUTION_PERMISSION_PAYEE_RULE_TYPE, + data: { addresses: payeeAddresses }, + }); } - return { isValid: true, expiry, data, rules }; + return { + isValid: true, + expiry, + data, + rules: rules.length > 0 ? rules : undefined, + }; } catch (caughtError) { return { isValid: false, error: caughtError as Error }; } }, }; } + +/** + * Extracts a payee address from a single-payee enforcer caveat. + * + * @param terms - Hex-encoded caveat terms. + * @param enforcerAddress - The enforcer address to determine decoding strategy. + * @param payeeEnforcerAddresses - Known payee enforcer addresses for comparison. + * @param payeeEnforcerAddresses.allowedCalldataEnforcer - AllowedCalldataEnforcer address. + * @param payeeEnforcerAddresses.allowedTargetsEnforcer - AllowedTargetsEnforcer address. + * @returns The checksummed payee address, or null if the enforcer is unrecognised. + */ +function extractPayeeAddressFromCaveat( + terms: Hex, + enforcerAddress: Hex, + payeeEnforcerAddresses: { + allowedCalldataEnforcer: Hex; + allowedTargetsEnforcer: Hex; + }, +): Hex | null { + const checksumEnforcer = getChecksumAddress(enforcerAddress); + + if (checksumEnforcer === payeeEnforcerAddresses.allowedCalldataEnforcer) { + const decoded = decodeAllowedCalldataTerms(terms); + const address = `0x${decoded.value.slice(-40)}`; + return getChecksumAddress(address); + } + + if (checksumEnforcer === payeeEnforcerAddresses.allowedTargetsEnforcer) { + const decoded = decodeAllowedTargetsTerms(terms); + return getChecksumAddress(decoded.targets[0]); + } + + return null; +} + +/** + * Extracts payee addresses from caveats, handling both single-payee + * (direct enforcer) and multi-payee (LogicalOrWrapperEnforcer) cases. + * + * @param caveats - Checksummed caveats from the delegation. + * @param enforcers - Payee enforcer addresses. + * @param enforcers.allowedCalldataEnforcer - AllowedCalldataEnforcer address. + * @param enforcers.allowedTargetsEnforcer - AllowedTargetsEnforcer address. + * @param enforcers.singlePayeeEnforcer - The specific enforcer for single-payee in this permission type. + * @param enforcers.logicalOrWrapperEnforcer - The LogicalOrWrapperEnforcer address. + * @returns Array of checksummed payee addresses, or null if no payee caveat is found. + */ +function extractPayeeAddresses( + caveats: ChecksumCaveat[], + enforcers: { + allowedCalldataEnforcer: Hex; + allowedTargetsEnforcer: Hex; + singlePayeeEnforcer: Hex; + logicalOrWrapperEnforcer: Hex; + }, +): Hex[] | null { + const knownEnforcers = { + allowedCalldataEnforcer: enforcers.allowedCalldataEnforcer, + allowedTargetsEnforcer: enforcers.allowedTargetsEnforcer, + }; + + const logicalOrTerms = getTermsByEnforcer({ + caveats, + enforcer: enforcers.logicalOrWrapperEnforcer, + throwIfNotFound: false, + }); + + if (logicalOrTerms) { + const decoded = decodeLogicalOrWrapperTerms(logicalOrTerms); + const addresses: Hex[] = []; + for (const group of decoded.caveatGroups) { + for (const innerCaveat of group) { + const address = extractPayeeAddressFromCaveat( + innerCaveat.terms, + innerCaveat.enforcer, + knownEnforcers, + ); + if (address) { + addresses.push(address); + } + } + } + return addresses.length > 0 ? addresses : null; + } + + try { + const singlePayeeTerms = getTermsByEnforcer({ + caveats, + enforcer: enforcers.singlePayeeEnforcer, + throwIfNotFound: false, + }); + + if (singlePayeeTerms) { + const address = extractPayeeAddressFromCaveat( + singlePayeeTerms, + enforcers.singlePayeeEnforcer, + knownEnforcers, + ); + return address ? [address] : null; + } + } catch { + // Multiple caveats for the single-payee enforcer (e.g. allowedCalldataEnforcer + // used as required in erc20-token-revocation). Not a payee caveat. + } + + return null; +} diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts index 4ead98866c..8aae60bcb1 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts @@ -33,12 +33,26 @@ export function makeNativeTokenAllowanceRule( nativeTokenPeriodicEnforcer, exactCalldataEnforcer, nonceEnforcer, + allowedCalldataEnforcer, + allowedTargetsEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'native-token-allowance', - optionalEnforcers: [timestampEnforcer, redeemerEnforcer], + optionalEnforcers: [ + timestampEnforcer, + redeemerEnforcer, + allowedTargetsEnforcer, + logicalOrWrapperEnforcer, + ], redeemerEnforcer, + payeeEnforcers: { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedTargetsEnforcer, + logicalOrWrapperEnforcer, + }, timestampEnforcer, requiredEnforcers: { [nativeTokenPeriodicEnforcer]: 1, diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts index a86a020219..6400ef812a 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts @@ -28,12 +28,26 @@ export function makeNativeTokenPeriodicRule( nativeTokenPeriodicEnforcer, exactCalldataEnforcer, nonceEnforcer, + allowedCalldataEnforcer, + allowedTargetsEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'native-token-periodic', - optionalEnforcers: [timestampEnforcer, redeemerEnforcer], + optionalEnforcers: [ + timestampEnforcer, + redeemerEnforcer, + allowedTargetsEnforcer, + logicalOrWrapperEnforcer, + ], redeemerEnforcer, + payeeEnforcers: { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedTargetsEnforcer, + logicalOrWrapperEnforcer, + }, timestampEnforcer, requiredEnforcers: { [nativeTokenPeriodicEnforcer]: 1, diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts index 4c19a036dc..c0b88e4bcf 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts @@ -23,12 +23,26 @@ export function makeNativeTokenStreamRule( nativeTokenStreamingEnforcer, exactCalldataEnforcer, nonceEnforcer, + allowedCalldataEnforcer, + allowedTargetsEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'native-token-stream', - optionalEnforcers: [timestampEnforcer, redeemerEnforcer], + optionalEnforcers: [ + timestampEnforcer, + redeemerEnforcer, + allowedTargetsEnforcer, + logicalOrWrapperEnforcer, + ], redeemerEnforcer, + payeeEnforcers: { + allowedCalldataEnforcer, + allowedTargetsEnforcer, + singlePayeeEnforcer: allowedTargetsEnforcer, + logicalOrWrapperEnforcer, + }, timestampEnforcer, requiredEnforcers: { [nativeTokenStreamingEnforcer]: 1, diff --git a/packages/gator-permissions-controller/src/decodePermission/types.ts b/packages/gator-permissions-controller/src/decodePermission/types.ts index 08ae71abd6..9f24d8bd7a 100644 --- a/packages/gator-permissions-controller/src/decodePermission/types.ts +++ b/packages/gator-permissions-controller/src/decodePermission/types.ts @@ -99,7 +99,9 @@ export type ChecksumEnforcersByChainId = { timestampEnforcer: Hex; nonceEnforcer: Hex; allowedCalldataEnforcer: Hex; + allowedTargetsEnforcer: Hex; redeemerEnforcer: Hex; + logicalOrWrapperEnforcer: Hex; }; /** Caveat with checksummed enforcer address; used by rule decode functions. */ diff --git a/packages/gator-permissions-controller/src/decodePermission/utils.test.ts b/packages/gator-permissions-controller/src/decodePermission/utils.test.ts index 7cf7f43d54..ee7f9ec765 100644 --- a/packages/gator-permissions-controller/src/decodePermission/utils.test.ts +++ b/packages/gator-permissions-controller/src/decodePermission/utils.test.ts @@ -22,7 +22,9 @@ const buildContracts = (): DeployedContractsByName => ({ ValueLteEnforcer: '0x7777777777777777777777777777777777777777', NonceEnforcer: '0x8888888888888888888888888888888888888888', AllowedCalldataEnforcer: '0x9999999999999999999999999999999999999999', + AllowedTargetsEnforcer: '0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', RedeemerEnforcer: '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + LogicalOrWrapperEnforcer: '0xcccccccccccccccccccccccccccccccccccccccc', }); describe('getChecksumEnforcersByChainId', () => { @@ -52,7 +54,13 @@ describe('getChecksumEnforcersByChainId', () => { allowedCalldataEnforcer: getChecksumAddress( contracts.AllowedCalldataEnforcer, ), + allowedTargetsEnforcer: getChecksumAddress( + contracts.AllowedTargetsEnforcer, + ), redeemerEnforcer: getChecksumAddress(contracts.RedeemerEnforcer), + logicalOrWrapperEnforcer: getChecksumAddress( + contracts.LogicalOrWrapperEnforcer, + ), }); }); @@ -78,7 +86,9 @@ describe('createPermissionRulesForChainId', () => { timestampEnforcer, nonceEnforcer, allowedCalldataEnforcer, + allowedTargetsEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, } = getChecksumEnforcersByChainId(contracts); // erc20-token-stream @@ -101,13 +111,23 @@ describe('createPermissionRulesForChainId', () => { expect(byType['native-token-stream'].permissionType).toBe( 'native-token-stream', ); - expect(byType['native-token-stream'].optionalEnforcers.size).toBe(2); + expect(byType['native-token-stream'].optionalEnforcers.size).toBe(4); expect( byType['native-token-stream'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); expect( byType['native-token-stream'].optionalEnforcers.has(redeemerEnforcer), ).toBe(true); + expect( + byType['native-token-stream'].optionalEnforcers.has( + allowedTargetsEnforcer, + ), + ).toBe(true); + expect( + byType['native-token-stream'].optionalEnforcers.has( + logicalOrWrapperEnforcer, + ), + ).toBe(true); expect(byType['native-token-stream'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['native-token-stream'].requiredEnforcers.entries()), @@ -124,13 +144,23 @@ describe('createPermissionRulesForChainId', () => { expect(byType['native-token-periodic'].permissionType).toBe( 'native-token-periodic', ); - expect(byType['native-token-periodic'].optionalEnforcers.size).toBe(2); + expect(byType['native-token-periodic'].optionalEnforcers.size).toBe(4); expect( byType['native-token-periodic'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); expect( byType['native-token-periodic'].optionalEnforcers.has(redeemerEnforcer), ).toBe(true); + expect( + byType['native-token-periodic'].optionalEnforcers.has( + allowedTargetsEnforcer, + ), + ).toBe(true); + expect( + byType['native-token-periodic'].optionalEnforcers.has( + logicalOrWrapperEnforcer, + ), + ).toBe(true); expect(byType['native-token-periodic'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['native-token-periodic'].requiredEnforcers.entries()), @@ -147,13 +177,23 @@ describe('createPermissionRulesForChainId', () => { expect(byType['erc20-token-stream'].permissionType).toBe( 'erc20-token-stream', ); - expect(byType['erc20-token-stream'].optionalEnforcers.size).toBe(2); + expect(byType['erc20-token-stream'].optionalEnforcers.size).toBe(4); expect( byType['erc20-token-stream'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); expect( byType['erc20-token-stream'].optionalEnforcers.has(redeemerEnforcer), ).toBe(true); + expect( + byType['erc20-token-stream'].optionalEnforcers.has( + allowedCalldataEnforcer, + ), + ).toBe(true); + expect( + byType['erc20-token-stream'].optionalEnforcers.has( + logicalOrWrapperEnforcer, + ), + ).toBe(true); expect(byType['erc20-token-stream'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['erc20-token-stream'].requiredEnforcers.entries()), @@ -170,13 +210,23 @@ describe('createPermissionRulesForChainId', () => { expect(byType['erc20-token-periodic'].permissionType).toBe( 'erc20-token-periodic', ); - expect(byType['erc20-token-periodic'].optionalEnforcers.size).toBe(2); + expect(byType['erc20-token-periodic'].optionalEnforcers.size).toBe(4); expect( byType['erc20-token-periodic'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); expect( byType['erc20-token-periodic'].optionalEnforcers.has(redeemerEnforcer), ).toBe(true); + expect( + byType['erc20-token-periodic'].optionalEnforcers.has( + allowedCalldataEnforcer, + ), + ).toBe(true); + expect( + byType['erc20-token-periodic'].optionalEnforcers.has( + logicalOrWrapperEnforcer, + ), + ).toBe(true); expect(byType['erc20-token-periodic'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['erc20-token-periodic'].requiredEnforcers.entries()), @@ -193,13 +243,23 @@ describe('createPermissionRulesForChainId', () => { expect(byType['native-token-allowance'].permissionType).toBe( 'native-token-allowance', ); - expect(byType['native-token-allowance'].optionalEnforcers.size).toBe(2); + expect(byType['native-token-allowance'].optionalEnforcers.size).toBe(4); expect( byType['native-token-allowance'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); expect( byType['native-token-allowance'].optionalEnforcers.has(redeemerEnforcer), ).toBe(true); + expect( + byType['native-token-allowance'].optionalEnforcers.has( + allowedTargetsEnforcer, + ), + ).toBe(true); + expect( + byType['native-token-allowance'].optionalEnforcers.has( + logicalOrWrapperEnforcer, + ), + ).toBe(true); expect(byType['native-token-allowance'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['native-token-allowance'].requiredEnforcers.entries()), @@ -216,13 +276,23 @@ describe('createPermissionRulesForChainId', () => { expect(byType['erc20-token-allowance'].permissionType).toBe( 'erc20-token-allowance', ); - expect(byType['erc20-token-allowance'].optionalEnforcers.size).toBe(2); + expect(byType['erc20-token-allowance'].optionalEnforcers.size).toBe(4); expect( byType['erc20-token-allowance'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); expect( byType['erc20-token-allowance'].optionalEnforcers.has(redeemerEnforcer), ).toBe(true); + expect( + byType['erc20-token-allowance'].optionalEnforcers.has( + allowedCalldataEnforcer, + ), + ).toBe(true); + expect( + byType['erc20-token-allowance'].optionalEnforcers.has( + logicalOrWrapperEnforcer, + ), + ).toBe(true); expect(byType['erc20-token-allowance'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['erc20-token-allowance'].requiredEnforcers.entries()), @@ -239,13 +309,18 @@ describe('createPermissionRulesForChainId', () => { expect(byType['erc20-token-revocation'].permissionType).toBe( 'erc20-token-revocation', ); - expect(byType['erc20-token-revocation'].optionalEnforcers.size).toBe(2); + expect(byType['erc20-token-revocation'].optionalEnforcers.size).toBe(3); expect( byType['erc20-token-revocation'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); expect( byType['erc20-token-revocation'].optionalEnforcers.has(redeemerEnforcer), ).toBe(true); + expect( + byType['erc20-token-revocation'].optionalEnforcers.has( + logicalOrWrapperEnforcer, + ), + ).toBe(true); expect(byType['erc20-token-revocation'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['erc20-token-revocation'].requiredEnforcers.entries()), diff --git a/packages/gator-permissions-controller/src/decodePermission/utils.ts b/packages/gator-permissions-controller/src/decodePermission/utils.ts index 603dcd6772..9b132444b9 100644 --- a/packages/gator-permissions-controller/src/decodePermission/utils.ts +++ b/packages/gator-permissions-controller/src/decodePermission/utils.ts @@ -20,7 +20,9 @@ const ENFORCER_CONTRACT_NAMES = { ValueLteEnforcer: 'ValueLteEnforcer', NonceEnforcer: 'NonceEnforcer', AllowedCalldataEnforcer: 'AllowedCalldataEnforcer', + AllowedTargetsEnforcer: 'AllowedTargetsEnforcer', RedeemerEnforcer: 'RedeemerEnforcer', + LogicalOrWrapperEnforcer: 'LogicalOrWrapperEnforcer', }; /** @@ -109,10 +111,18 @@ export const getChecksumEnforcersByChainId = ( ENFORCER_CONTRACT_NAMES.AllowedCalldataEnforcer, ); + const allowedTargetsEnforcer = getChecksumContractAddress( + ENFORCER_CONTRACT_NAMES.AllowedTargetsEnforcer, + ); + const redeemerEnforcer = getChecksumContractAddress( ENFORCER_CONTRACT_NAMES.RedeemerEnforcer, ); + const logicalOrWrapperEnforcer = getChecksumContractAddress( + ENFORCER_CONTRACT_NAMES.LogicalOrWrapperEnforcer, + ); + return { erc20StreamingEnforcer, erc20PeriodicEnforcer, @@ -123,7 +133,9 @@ export const getChecksumEnforcersByChainId = ( timestampEnforcer, nonceEnforcer, allowedCalldataEnforcer, + allowedTargetsEnforcer, redeemerEnforcer, + logicalOrWrapperEnforcer, }; }; diff --git a/packages/gator-permissions-controller/src/index.ts b/packages/gator-permissions-controller/src/index.ts index 125248cb72..0661ccf966 100644 --- a/packages/gator-permissions-controller/src/index.ts +++ b/packages/gator-permissions-controller/src/index.ts @@ -1,6 +1,7 @@ export { default as GatorPermissionsController } from './GatorPermissionsController'; export { DELEGATION_FRAMEWORK_VERSION, + EXECUTION_PERMISSION_PAYEE_RULE_TYPE, EXECUTION_PERMISSION_REDEEMER_RULE_TYPE, } from './constants'; export type { @@ -37,6 +38,7 @@ export type { SupportedPermissionType, } from './types'; +export type { PayeeRule } from './payeeRule'; export type { RedeemerRule } from './redeemerRule'; export type { NativeTokenStreamPermission, diff --git a/packages/gator-permissions-controller/src/payeeRule.ts b/packages/gator-permissions-controller/src/payeeRule.ts new file mode 100644 index 0000000000..0bf8c12e5e --- /dev/null +++ b/packages/gator-permissions-controller/src/payeeRule.ts @@ -0,0 +1,13 @@ +import type { Hex } from '@metamask/utils'; + +/** + * Execution permission rule restricting which addresses may receive payments + * (on-chain AllowedCalldataEnforcer / AllowedTargetsEnforcer caveat, optionally + * wrapped in a LogicalOrWrapperEnforcer for multiple payees). + */ +export type PayeeRule = { + type: 'payee'; + data: { + addresses: Hex[]; + }; +}; diff --git a/yarn.lock b/yarn.lock index 075331c838..f455d4f59e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3478,23 +3478,17 @@ __metadata: languageName: unknown linkType: soft -"@metamask/delegation-core@npm:^1.1.0": - version: 1.1.0 - resolution: "@metamask/delegation-core@npm:1.1.0" - dependencies: - "@metamask/abi-utils": "npm:^3.0.0" - "@metamask/utils": "npm:^11.4.0" - "@noble/hashes": "npm:^1.8.0" - checksum: 10/672f9e2e2b4e8c312f2cd2ff166bbc508fbdb6e141fe92e678abc9993b9ccbdd17db711477a9b97b6ce3919fa6d51d759c16f6c6fda3f89cb95e303b8aa76f7d +"@metamask/delegation-core@link:../../../smart-accounts-kit/packages/delegation-core::locator=%40metamask%2Fgator-permissions-controller%40workspace%3Apackages%2Fgator-permissions-controller": + version: 0.0.0-use.local + resolution: "@metamask/delegation-core@link:../../../smart-accounts-kit/packages/delegation-core::locator=%40metamask%2Fgator-permissions-controller%40workspace%3Apackages%2Fgator-permissions-controller" languageName: node - linkType: hard + linkType: soft -"@metamask/delegation-deployments@npm:^0.12.0": - version: 0.12.0 - resolution: "@metamask/delegation-deployments@npm:0.12.0" - checksum: 10/fd3b373efc1857cc867b44b4ca33db0cf8487c1109d6f2ed7e3ce10e6a65d4165b7fcc034cab92d919d6f0833e3749a055ff862adc8d7a348cdd3a0f593f6aa6 +"@metamask/delegation-deployments@link:../../../smart-accounts-kit/packages/delegation-deployments::locator=%40metamask%2Fgator-permissions-controller%40workspace%3Apackages%2Fgator-permissions-controller": + version: 0.0.0-use.local + resolution: "@metamask/delegation-deployments@link:../../../smart-accounts-kit/packages/delegation-deployments::locator=%40metamask%2Fgator-permissions-controller%40workspace%3Apackages%2Fgator-permissions-controller" languageName: node - linkType: hard + linkType: soft "@metamask/earn-controller@workspace:packages/earn-controller": version: 0.0.0-use.local @@ -4108,8 +4102,8 @@ __metadata: "@metamask/abi-utils": "npm:^2.0.3" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/base-controller": "npm:^9.1.0" - "@metamask/delegation-core": "npm:^1.1.0" - "@metamask/delegation-deployments": "npm:^0.12.0" + "@metamask/delegation-core": "link:../../../smart-accounts-kit/packages/delegation-core" + "@metamask/delegation-deployments": "link:../../../smart-accounts-kit/packages/delegation-deployments" "@metamask/messenger": "npm:^1.2.0" "@metamask/network-controller": "npm:^30.1.0" "@metamask/snaps-controllers": "npm:^19.0.0" From e0665d989e22e7db5451157e78bb07f7bd2a603e Mon Sep 17 00:00:00 2001 From: MJ Kiwi Date: Tue, 5 May 2026 10:20:29 +1200 Subject: [PATCH 2/9] fix: specify type for address in extractPayeeAddressFromCaveat function --- .../src/decodePermission/rules/makePermissionRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts index a9ec2477eb..6c68fc7216 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts @@ -182,7 +182,7 @@ function extractPayeeAddressFromCaveat( if (checksumEnforcer === payeeEnforcerAddresses.allowedCalldataEnforcer) { const decoded = decodeAllowedCalldataTerms(terms); - const address = `0x${decoded.value.slice(-40)}`; + const address:Hex = `0x${decoded.value.slice(-40)}`; return getChecksumAddress(address); } From 7ce3f1448b5e7e1d5144a432092226f14f67b7dc Mon Sep 17 00:00:00 2001 From: MJ Kiwi Date: Tue, 5 May 2026 19:18:37 +1200 Subject: [PATCH 3/9] feat: update delegation-core and delegation-deployments dependencies to latest versions --- .../gator-permissions-controller/package.json | 4 +-- yarn.lock | 26 ++++++++++++------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/gator-permissions-controller/package.json b/packages/gator-permissions-controller/package.json index 30ced8cc83..2e848fb207 100644 --- a/packages/gator-permissions-controller/package.json +++ b/packages/gator-permissions-controller/package.json @@ -56,8 +56,8 @@ "@metamask/7715-permission-types": "^0.6.0", "@metamask/abi-utils": "^2.0.3", "@metamask/base-controller": "^9.1.0", - "@metamask/delegation-core": "link:../../../smart-accounts-kit/packages/delegation-core", - "@metamask/delegation-deployments": "link:../../../smart-accounts-kit/packages/delegation-deployments", + "@metamask/delegation-core": "^2.0.0", + "@metamask/delegation-deployments": "^1.3.0", "@metamask/messenger": "^1.2.0", "@metamask/network-controller": "^30.1.0", "@metamask/snaps-controllers": "^19.0.0", diff --git a/yarn.lock b/yarn.lock index f455d4f59e..e3500b36ea 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3478,17 +3478,23 @@ __metadata: languageName: unknown linkType: soft -"@metamask/delegation-core@link:../../../smart-accounts-kit/packages/delegation-core::locator=%40metamask%2Fgator-permissions-controller%40workspace%3Apackages%2Fgator-permissions-controller": - version: 0.0.0-use.local - resolution: "@metamask/delegation-core@link:../../../smart-accounts-kit/packages/delegation-core::locator=%40metamask%2Fgator-permissions-controller%40workspace%3Apackages%2Fgator-permissions-controller" +"@metamask/delegation-core@npm:^2.0.0": + version: 2.0.0 + resolution: "@metamask/delegation-core@npm:2.0.0" + dependencies: + "@metamask/abi-utils": "npm:^3.0.0" + "@metamask/utils": "npm:^11.4.0" + "@noble/hashes": "npm:^1.8.0" + checksum: 10/b473160e4cb4a6d463c6015de6e90d057034d2e8f2905068e1f44f93c8247618c5d84a155e86dfaa125dacb040951643517b9a76961bf8d215c194dc4d1cc0ad languageName: node - linkType: soft + linkType: hard -"@metamask/delegation-deployments@link:../../../smart-accounts-kit/packages/delegation-deployments::locator=%40metamask%2Fgator-permissions-controller%40workspace%3Apackages%2Fgator-permissions-controller": - version: 0.0.0-use.local - resolution: "@metamask/delegation-deployments@link:../../../smart-accounts-kit/packages/delegation-deployments::locator=%40metamask%2Fgator-permissions-controller%40workspace%3Apackages%2Fgator-permissions-controller" +"@metamask/delegation-deployments@npm:^1.3.0": + version: 1.3.0 + resolution: "@metamask/delegation-deployments@npm:1.3.0" + checksum: 10/58f4aafb5f0e3cbc543811cbc0100efab4ed67b9c9794b83192962153e4edbe12fd6ab6fa7be689503309862a65eb7fde771f632893d38ab54f8171aa682b34f languageName: node - linkType: soft + linkType: hard "@metamask/earn-controller@workspace:packages/earn-controller": version: 0.0.0-use.local @@ -4102,8 +4108,8 @@ __metadata: "@metamask/abi-utils": "npm:^2.0.3" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/base-controller": "npm:^9.1.0" - "@metamask/delegation-core": "link:../../../smart-accounts-kit/packages/delegation-core" - "@metamask/delegation-deployments": "link:../../../smart-accounts-kit/packages/delegation-deployments" + "@metamask/delegation-core": "npm:^2.0.0" + "@metamask/delegation-deployments": "npm:^1.3.0" "@metamask/messenger": "npm:^1.2.0" "@metamask/network-controller": "npm:^30.1.0" "@metamask/snaps-controllers": "npm:^19.0.0" From 3cc2d7b7fff2df4b7e4a90cc9c5a3991782579a4 Mon Sep 17 00:00:00 2001 From: MJ Kiwi Date: Tue, 5 May 2026 19:35:03 +1200 Subject: [PATCH 4/9] fix: format type declaration for address in extractPayeeAddressFromCaveat function --- .../src/decodePermission/rules/makePermissionRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts index 6c68fc7216..bfa07835e9 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts @@ -182,7 +182,7 @@ function extractPayeeAddressFromCaveat( if (checksumEnforcer === payeeEnforcerAddresses.allowedCalldataEnforcer) { const decoded = decodeAllowedCalldataTerms(terms); - const address:Hex = `0x${decoded.value.slice(-40)}`; + const address: Hex = `0x${decoded.value.slice(-40)}`; return getChecksumAddress(address); } From 48941ca904323a65f03e41b8777182e8205cbbc3 Mon Sep 17 00:00:00 2001 From: MJ Kiwi Date: Tue, 5 May 2026 20:20:17 +1200 Subject: [PATCH 5/9] feat: enhance payee rule extraction and validation for execution permissions --- .../gator-permissions-controller/CHANGELOG.md | 4 +- .../rules/makePermissionRule.test.ts | 538 ++++++++++++++++-- .../rules/makePermissionRule.ts | 220 ++++--- .../rules/nativeTokenAllowance.ts | 1 - .../rules/nativeTokenPeriodic.ts | 1 - .../rules/nativeTokenStream.ts | 1 - .../src/decodePermission/utils.test.ts | 21 +- 7 files changed, 627 insertions(+), 159 deletions(-) diff --git a/packages/gator-permissions-controller/CHANGELOG.md b/packages/gator-permissions-controller/CHANGELOG.md index d81f31f44a..65ac8c8cf4 100644 --- a/packages/gator-permissions-controller/CHANGELOG.md +++ b/packages/gator-permissions-controller/CHANGELOG.md @@ -9,9 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Support payee-restricting caveats when decoding execution permissions - - Permission decoding now recognizes `AllowedTargetsEnforcer`, `AllowedCalldataEnforcer`, and `LogicalOrWrapperEnforcer` as optional caveats for payee extraction on execution permission types and extracts a `payee` rule containing the allowlisted addresses. - - Export new `EXECUTION_PERMISSION_PAYEE_RULE_TYPE` constant and `PayeeRule` type. +- Add `payee` rule to execution permission decoding for all known permission types ([#8668](https://github.com/MetaMask/core/pull/8668)) - Support `RedeemerEnforcer` caveat when decoding execution permissions ([#8537](https://github.com/MetaMask/core/pull/8537)) - Permission decoding now recognizes the `RedeemerEnforcer` as an optional caveat on all execution permission types and extracts a `redeemer` rule containing the allowlisted addresses. - `DecodedPermission` type now includes an optional `rules` property for rules recovered from caveats. diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts index 45ff503200..ef3c379d7b 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts @@ -1,3 +1,4 @@ +import { encodeSingle } from '@metamask/abi-utils'; import { createAllowedCalldataTerms, createAllowedTargetsTerms, @@ -8,7 +9,7 @@ import { CHAIN_ID, DELEGATOR_CONTRACTS, } from '@metamask/delegation-deployments'; -import { getChecksumAddress } from '@metamask/utils'; +import { bytesToHex, getChecksumAddress } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; import { makePermissionRule } from './makePermissionRule'; @@ -21,6 +22,7 @@ describe('makePermissionRule', () => { const allowedCalldataEnforcer = contracts.AllowedCalldataEnforcer; const allowedTargetsEnforcer = contracts.AllowedTargetsEnforcer; const logicalOrWrapperEnforcer = contracts.LogicalOrWrapperEnforcer; + const EMPTY_ARGS = new Uint8Array(); const payeeEnforcersNative = { allowedCalldataEnforcer, @@ -396,7 +398,7 @@ describe('makePermissionRule', () => { ]); }); - it('includes payee rule with multiple addresses via LogicalOrWrapperEnforcer (native)', () => { + it('includes payee rule with multiple addresses via AllowedTargetsEnforcer (native)', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; @@ -406,7 +408,7 @@ describe('makePermissionRule', () => { timestampEnforcer, redeemerEnforcer, payeeEnforcers: payeeEnforcersNative, - optionalEnforcers: [logicalOrWrapperEnforcer], + optionalEnforcers: [allowedTargetsEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, }); @@ -418,28 +420,9 @@ describe('makePermissionRule', () => { args: '0x' as Hex, }, { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedTargetsEnforcer, - terms: createAllowedTargetsTerms({ - targets: [payeeAddress1], - }), - args: '0x00', - }, - ], - [ - { - enforcer: allowedTargetsEnforcer, - terms: createAllowedTargetsTerms({ - targets: [payeeAddress2], - }), - args: '0x00', - }, - ], - ], + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ + targets: [payeeAddress1, payeeAddress2], }), args: '0x' as Hex, }, @@ -554,7 +537,7 @@ describe('makePermissionRule', () => { startIndex: 4, value: padded1, }), - args: '0x00', + args: EMPTY_ARGS, }, ], [ @@ -564,7 +547,7 @@ describe('makePermissionRule', () => { startIndex: 4, value: padded2, }), - args: '0x00', + args: EMPTY_ARGS, }, ], ], @@ -592,6 +575,48 @@ describe('makePermissionRule', () => { ]); }); + it('rejects LogicalOrWrapperEnforcer for native payee caveats', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress = '0x4444444444444444444444444444444444444444' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ + targets: [payeeAddress], + }), + args: EMPTY_ARGS, + }, + ], + ], + }), + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + it('does not include payee rule when no payee caveat is present', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); @@ -642,16 +667,17 @@ describe('makePermissionRule', () => { expect(rule.caveatAddressesMatch([])).toBe(false); }); - it('ignores unrecognised inner enforcers in LogicalOrWrapperEnforcer', () => { + it('rejects unrecognised inner enforcers in LogicalOrWrapperEnforcer', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); const payeeAddress = '0x4444444444444444444444444444444444444444' as Hex; + const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}`; const unknownEnforcer = '0x9999999999999999999999999999999999999999' as Hex; const rule = makePermissionRule({ - permissionType: 'native-token-stream', + permissionType: 'erc20-token-stream', timestampEnforcer, redeemerEnforcer, - payeeEnforcers: payeeEnforcersNative, + payeeEnforcers: payeeEnforcersErc20, optionalEnforcers: [logicalOrWrapperEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -669,11 +695,12 @@ describe('makePermissionRule', () => { caveatGroups: [ [ { - enforcer: allowedTargetsEnforcer, - terms: createAllowedTargetsTerms({ - targets: [payeeAddress], + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: paddedAddress, }), - args: '0x00', + args: EMPTY_ARGS, }, ], [ @@ -681,7 +708,7 @@ describe('makePermissionRule', () => { enforcer: unknownEnforcer, terms: '0x0000000000000000000000000000000000000000000000000000000000000001', - args: '0x00', + args: EMPTY_ARGS, }, ], ], @@ -692,29 +719,18 @@ describe('makePermissionRule', () => { const result = rule.validateAndDecodePermission(caveats); - expect(result.isValid).toBe(true); - if (!result.isValid) { - throw new Error('Expected valid result'); - } - expect(result.rules).toStrictEqual([ - { - type: 'payee', - data: { - addresses: [getChecksumAddress(payeeAddress)], - }, - }, - ]); + expect(result.isValid).toBe(false); }); - it('returns null payee when LogicalOrWrapper inner caveats are all unrecognised', () => { + it('rejects LogicalOrWrapper when inner caveats are all unrecognised', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); const unknownEnforcer = '0x9999999999999999999999999999999999999999' as Hex; const rule = makePermissionRule({ - permissionType: 'native-token-stream', + permissionType: 'erc20-token-stream', timestampEnforcer, redeemerEnforcer, - payeeEnforcers: payeeEnforcersNative, + payeeEnforcers: payeeEnforcersErc20, optionalEnforcers: [logicalOrWrapperEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -735,7 +751,7 @@ describe('makePermissionRule', () => { enforcer: unknownEnforcer, terms: '0x0000000000000000000000000000000000000000000000000000000000000001', - args: '0x00', + args: EMPTY_ARGS, }, ], ], @@ -746,14 +762,10 @@ describe('makePermissionRule', () => { const result = rule.validateAndDecodePermission(caveats); - expect(result.isValid).toBe(true); - if (!result.isValid) { - throw new Error('Expected valid result'); - } - expect(result.rules).toBeUndefined(); + expect(result.isValid).toBe(false); }); - it('returns null payee when singlePayeeEnforcer is unrecognised', () => { + it('rejects when singlePayeeEnforcer is unrecognised', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); const unknownEnforcer = '0x8888888888888888888888888888888888888888' as Hex; @@ -788,11 +800,413 @@ describe('makePermissionRule', () => { const result = rule.validateAndDecodePermission(caveats); - expect(result.isValid).toBe(true); - if (!result.isValid) { - throw new Error('Expected valid result'); - } - expect(result.rules).toBeUndefined(); + expect(result.isValid).toBe(false); + }); + + it('rejects LogicalOrWrapperEnforcer when a group has more than one inner caveat', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; + const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; + const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; + const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded1, + }), + args: EMPTY_ARGS, + }, + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded2, + }), + args: EMPTY_ARGS, + }, + ], + ], + }), + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects LogicalOrWrapperEnforcer with no caveat groups', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: bytesToHex(encodeSingle('((address,bytes,bytes)[])[]', [])), + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects LogicalOrWrapperEnforcer with non-empty args', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress = '0x4444444444444444444444444444444444444444' as Hex; + const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}`; + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: paddedAddress, + }), + args: EMPTY_ARGS, + }, + ], + ], + }), + args: '0x00' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects when both LogicalOrWrapperEnforcer and single-payee caveats are present', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; + const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; + const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; + const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [allowedCalldataEnforcer, logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded1, + }), + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded2, + }), + args: EMPTY_ARGS, + }, + ], + ], + }), + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects an ERC20 payee caveat with the wrong calldata start index', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress = '0x3333333333333333333333333333333333333333' as Hex; + const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}`; + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [allowedCalldataEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 36, + value: paddedAddress, + }), + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects an ERC20 payee caveat when the calldata value is not one address', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [allowedCalldataEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: '0x1234', + }), + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects a payee caveat with non-zero args', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress = '0x2222222222222222222222222222222222222222' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [allowedTargetsEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ targets: [payeeAddress] }), + args: '0x01' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects a native payee caveat with no targets', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [allowedTargetsEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedTargetsEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects multiple LogicalOrWrapperEnforcer caveats', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; + const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; + const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; + const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; + + const rule = makePermissionRule({ + permissionType: 'erc20-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [logicalOrWrapperEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded1, + }), + args: EMPTY_ARGS, + }, + ], + ], + }), + args: '0x' as Hex, + }, + { + enforcer: logicalOrWrapperEnforcer, + terms: createLogicalOrWrapperTerms({ + caveatGroups: [ + [ + { + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded2, + }), + args: EMPTY_ARGS, + }, + ], + ], + }), + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); + }); + + it('rejects multiple single-payee caveats', () => { + const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress1 = '0x2222222222222222222222222222222222222222' as Hex; + const payeeAddress2 = '0x3333333333333333333333333333333333333333' as Hex; + + const rule = makePermissionRule({ + permissionType: 'native-token-stream', + timestampEnforcer, + redeemerEnforcer, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [allowedTargetsEnforcer], + requiredEnforcers: { [requiredEnforcer]: 1 }, + validateAndDecodeData, + }); + + const result = rule.validateAndDecodePermission([ + { + enforcer: requiredEnforcer, + terms: '0x' as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ targets: [payeeAddress1] }), + args: '0x' as Hex, + }, + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ targets: [payeeAddress2] }), + args: '0x' as Hex, + }, + ]); + + expect(result.isValid).toBe(false); }); it('handles multiple singlePayeeEnforcer caveats gracefully (e.g. revocation)', () => { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts index bfa07835e9..c90d438c13 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts @@ -24,9 +24,22 @@ import { buildEnforcerCountsAndSet, enforcersMatchRule, extractExpiryFromCaveatTerms, + getByteLength, getTermsByEnforcer, } from '../utils'; +const ERC20_TRANSFER_PAYEE_START_INDEX = 4; +const ERC20_PAYEE_VALUE_BYTE_LENGTH = 32; +const PAYEE_CAVEAT_ARGS = '0x' as const; +const LOGICAL_OR_WRAPPER_CAVEAT_ARGS = '0x' as const; + +type PayeeEnforcerAddresses = { + allowedCalldataEnforcer: Hex; + allowedTargetsEnforcer: Hex; + singlePayeeEnforcer: Hex; + logicalOrWrapperEnforcer: Hex; +}; + /** * Creates a single permission rule with the given type, enforcer sets, and * decode/validate callbacks. @@ -35,10 +48,6 @@ import { * @param args.optionalEnforcers - Enforcer addresses that may appear in addition to required. * @param args.redeemerEnforcer - Address of the RedeemerEnforcer used to extract redeemer rules. * @param args.payeeEnforcers - Addresses of enforcers used to extract payee rules. - * @param args.payeeEnforcers.allowedCalldataEnforcer - AllowedCalldataEnforcer address (ERC20 payee). - * @param args.payeeEnforcers.allowedTargetsEnforcer - AllowedTargetsEnforcer address (native payee). - * @param args.payeeEnforcers.singlePayeeEnforcer - The specific enforcer used for single-payee in this permission type. - * @param args.payeeEnforcers.logicalOrWrapperEnforcer - The LogicalOrWrapperEnforcer for multi-payee. * @param args.timestampEnforcer - Address of the TimestampEnforcer used to extract expiry. * @param args.permissionType - The permission type identifier. * @param args.requiredEnforcers - Map of required enforcer address to required count. @@ -56,12 +65,7 @@ export function makePermissionRule({ }: { optionalEnforcers: Hex[]; redeemerEnforcer: Hex; - payeeEnforcers: { - allowedCalldataEnforcer: Hex; - allowedTargetsEnforcer: Hex; - singlePayeeEnforcer: Hex; - logicalOrWrapperEnforcer: Hex; - }; + payeeEnforcers: PayeeEnforcerAddresses; timestampEnforcer: Hex; permissionType: PermissionType; requiredEnforcers: Record; @@ -136,9 +140,10 @@ export function makePermissionRule({ }); } - const payeeAddresses = extractPayeeAddresses( + const payeeAddresses = tryExtractPayeeAddresses( checksumCaveats, payeeEnforcers, + requiredEnforcersMap, ); if (payeeAddresses) { rules.push({ @@ -161,41 +166,91 @@ export function makePermissionRule({ } /** - * Extracts a payee address from a single-payee enforcer caveat. + * Attempts to extract payee addresses from a payee enforcer caveat. * - * @param terms - Hex-encoded caveat terms. - * @param enforcerAddress - The enforcer address to determine decoding strategy. + * @param caveat - The payee caveat to decode. * @param payeeEnforcerAddresses - Known payee enforcer addresses for comparison. * @param payeeEnforcerAddresses.allowedCalldataEnforcer - AllowedCalldataEnforcer address. * @param payeeEnforcerAddresses.allowedTargetsEnforcer - AllowedTargetsEnforcer address. - * @returns The checksummed payee address, or null if the enforcer is unrecognised. + * @returns The checksummed payee addresses, or null if the enforcer is unrecognised. */ -function extractPayeeAddressFromCaveat( - terms: Hex, - enforcerAddress: Hex, +function tryExtractPayeeAddressesFromCaveat( + caveat: Caveat, payeeEnforcerAddresses: { allowedCalldataEnforcer: Hex; allowedTargetsEnforcer: Hex; }, -): Hex | null { - const checksumEnforcer = getChecksumAddress(enforcerAddress); +): Hex[] | null { + const checksumEnforcer = getChecksumAddress(caveat.enforcer); if (checksumEnforcer === payeeEnforcerAddresses.allowedCalldataEnforcer) { - const decoded = decodeAllowedCalldataTerms(terms); + assertPayeeCaveatArgs(caveat.args); + + const decoded = decodeAllowedCalldataTerms(caveat.terms); + if (decoded.startIndex !== ERC20_TRANSFER_PAYEE_START_INDEX) { + throw new Error( + `Invalid AllowedCalldataEnforcer payee terms: startIndex must be ${ERC20_TRANSFER_PAYEE_START_INDEX}`, + ); + } + + if (getByteLength(decoded.value) !== ERC20_PAYEE_VALUE_BYTE_LENGTH) { + throw new Error( + `Invalid AllowedCalldataEnforcer payee terms: value must be ${ERC20_PAYEE_VALUE_BYTE_LENGTH} bytes`, + ); + } + const address: Hex = `0x${decoded.value.slice(-40)}`; - return getChecksumAddress(address); + return [getChecksumAddress(address)]; } if (checksumEnforcer === payeeEnforcerAddresses.allowedTargetsEnforcer) { - const decoded = decodeAllowedTargetsTerms(terms); - return getChecksumAddress(decoded.targets[0]); + assertPayeeCaveatArgs(caveat.args); + + const decoded = decodeAllowedTargetsTerms(caveat.terms); + return decoded.targets.map((target) => getChecksumAddress(target)); } return null; } /** - * Extracts payee addresses from caveats, handling both single-payee + * Asserts that payee caveat args match the expected empty value. + * + * @param args - The caveat args to validate. + */ +function assertPayeeCaveatArgs(args: Hex): void { + if (args !== PAYEE_CAVEAT_ARGS) { + throw new Error(`Invalid payee caveat args: must be ${PAYEE_CAVEAT_ARGS}`); + } +} + +/** + * Extracts payee addresses from the specified payee caveat. + * + * @param caveat - The caveat to decode. + * @param enforcers - Payee enforcer addresses. + * @returns The checksummed payee addresses. + */ +function extractPayeeAddressesFromExpectedCaveat( + caveat: Caveat, + enforcers: PayeeEnforcerAddresses, +): Hex[] { + if (getChecksumAddress(caveat.enforcer) !== enforcers.singlePayeeEnforcer) { + throw new Error( + 'Invalid payee caveat: must use the configured single-payee enforcer', + ); + } + + const addresses = tryExtractPayeeAddressesFromCaveat(caveat, enforcers); + if (!addresses) { + throw new Error('Invalid payee caveat: unable to decode payee address'); + } + + return addresses; +} + +/** + * Attempts to extract payee addresses from caveats, handling both single-payee * (direct enforcer) and multi-payee (LogicalOrWrapperEnforcer) cases. * * @param caveats - Checksummed caveats from the delegation. @@ -204,64 +259,83 @@ function extractPayeeAddressFromCaveat( * @param enforcers.allowedTargetsEnforcer - AllowedTargetsEnforcer address. * @param enforcers.singlePayeeEnforcer - The specific enforcer for single-payee in this permission type. * @param enforcers.logicalOrWrapperEnforcer - The LogicalOrWrapperEnforcer address. + * @param requiredEnforcers - Required enforcer counts for the permission rule. * @returns Array of checksummed payee addresses, or null if no payee caveat is found. */ -function extractPayeeAddresses( +function tryExtractPayeeAddresses( caveats: ChecksumCaveat[], - enforcers: { - allowedCalldataEnforcer: Hex; - allowedTargetsEnforcer: Hex; - singlePayeeEnforcer: Hex; - logicalOrWrapperEnforcer: Hex; - }, + enforcers: PayeeEnforcerAddresses, + requiredEnforcers: Map, ): Hex[] | null { - const knownEnforcers = { - allowedCalldataEnforcer: enforcers.allowedCalldataEnforcer, - allowedTargetsEnforcer: enforcers.allowedTargetsEnforcer, - }; + const logicalOrCaveats = caveats.filter( + (caveat) => caveat.enforcer === enforcers.logicalOrWrapperEnforcer, + ); - const logicalOrTerms = getTermsByEnforcer({ - caveats, - enforcer: enforcers.logicalOrWrapperEnforcer, - throwIfNotFound: false, - }); - - if (logicalOrTerms) { - const decoded = decodeLogicalOrWrapperTerms(logicalOrTerms); - const addresses: Hex[] = []; - for (const group of decoded.caveatGroups) { - for (const innerCaveat of group) { - const address = extractPayeeAddressFromCaveat( - innerCaveat.terms, - innerCaveat.enforcer, - knownEnforcers, - ); - if (address) { - addresses.push(address); - } - } - } - return addresses.length > 0 ? addresses : null; + if (logicalOrCaveats.length > 1) { + throw new Error('Invalid caveats'); } - try { - const singlePayeeTerms = getTermsByEnforcer({ - caveats, - enforcer: enforcers.singlePayeeEnforcer, - throwIfNotFound: false, - }); + const requiredSinglePayeeCount = + requiredEnforcers.get(enforcers.singlePayeeEnforcer) ?? 0; + const singlePayeeCaveats = caveats.filter( + (caveat) => caveat.enforcer === enforcers.singlePayeeEnforcer, + ); + + if (singlePayeeCaveats.length > requiredSinglePayeeCount + 1) { + throw new Error('Invalid caveats'); + } + + const singlePayeeCaveat = + singlePayeeCaveats[requiredSinglePayeeCount] ?? null; - if (singlePayeeTerms) { - const address = extractPayeeAddressFromCaveat( - singlePayeeTerms, - enforcers.singlePayeeEnforcer, - knownEnforcers, + if (logicalOrCaveats.length === 1 && singlePayeeCaveat) { + throw new Error( + 'Invalid payee caveats: use either LogicalOrWrapperEnforcer or a single-payee caveat', + ); + } + + if (logicalOrCaveats.length === 1) { + if (enforcers.singlePayeeEnforcer !== enforcers.allowedCalldataEnforcer) { + throw new Error( + 'Invalid LogicalOrWrapperEnforcer payee caveat: only ERC20 payees may use LogicalOrWrapperEnforcer', ); - return address ? [address] : null; } - } catch { - // Multiple caveats for the single-payee enforcer (e.g. allowedCalldataEnforcer - // used as required in erc20-token-revocation). Not a payee caveat. + + const [logicalOrCaveat] = logicalOrCaveats; + if (logicalOrCaveat.args !== LOGICAL_OR_WRAPPER_CAVEAT_ARGS) { + throw new Error( + `Invalid LogicalOrWrapperEnforcer payee args: must be ${LOGICAL_OR_WRAPPER_CAVEAT_ARGS}`, + ); + } + + const decoded = decodeLogicalOrWrapperTerms(logicalOrCaveat.terms); + if (decoded.caveatGroups.length === 0) { + throw new Error( + 'Invalid LogicalOrWrapperEnforcer payee terms: must contain at least one caveat group', + ); + } + + return decoded.caveatGroups.flatMap((group) => { + if (group.length !== 1) { + throw new Error( + 'Invalid LogicalOrWrapperEnforcer payee terms: each caveat group must contain exactly one caveat', + ); + } + + const addresses = extractPayeeAddressesFromExpectedCaveat( + group[0], + enforcers, + ); + + return addresses; + }); + } + + if (singlePayeeCaveat) { + return extractPayeeAddressesFromExpectedCaveat( + singlePayeeCaveat, + enforcers, + ); } return null; diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts index 8aae60bcb1..61f1012f5c 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts @@ -44,7 +44,6 @@ export function makeNativeTokenAllowanceRule( timestampEnforcer, redeemerEnforcer, allowedTargetsEnforcer, - logicalOrWrapperEnforcer, ], redeemerEnforcer, payeeEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts index 6400ef812a..2826d896a7 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts @@ -39,7 +39,6 @@ export function makeNativeTokenPeriodicRule( timestampEnforcer, redeemerEnforcer, allowedTargetsEnforcer, - logicalOrWrapperEnforcer, ], redeemerEnforcer, payeeEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts index c0b88e4bcf..871b0a3ea2 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts @@ -34,7 +34,6 @@ export function makeNativeTokenStreamRule( timestampEnforcer, redeemerEnforcer, allowedTargetsEnforcer, - logicalOrWrapperEnforcer, ], redeemerEnforcer, payeeEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/utils.test.ts b/packages/gator-permissions-controller/src/decodePermission/utils.test.ts index ee7f9ec765..38e7efde2b 100644 --- a/packages/gator-permissions-controller/src/decodePermission/utils.test.ts +++ b/packages/gator-permissions-controller/src/decodePermission/utils.test.ts @@ -111,7 +111,7 @@ describe('createPermissionRulesForChainId', () => { expect(byType['native-token-stream'].permissionType).toBe( 'native-token-stream', ); - expect(byType['native-token-stream'].optionalEnforcers.size).toBe(4); + expect(byType['native-token-stream'].optionalEnforcers.size).toBe(3); expect( byType['native-token-stream'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); @@ -123,11 +123,6 @@ describe('createPermissionRulesForChainId', () => { allowedTargetsEnforcer, ), ).toBe(true); - expect( - byType['native-token-stream'].optionalEnforcers.has( - logicalOrWrapperEnforcer, - ), - ).toBe(true); expect(byType['native-token-stream'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['native-token-stream'].requiredEnforcers.entries()), @@ -144,7 +139,7 @@ describe('createPermissionRulesForChainId', () => { expect(byType['native-token-periodic'].permissionType).toBe( 'native-token-periodic', ); - expect(byType['native-token-periodic'].optionalEnforcers.size).toBe(4); + expect(byType['native-token-periodic'].optionalEnforcers.size).toBe(3); expect( byType['native-token-periodic'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); @@ -156,11 +151,6 @@ describe('createPermissionRulesForChainId', () => { allowedTargetsEnforcer, ), ).toBe(true); - expect( - byType['native-token-periodic'].optionalEnforcers.has( - logicalOrWrapperEnforcer, - ), - ).toBe(true); expect(byType['native-token-periodic'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['native-token-periodic'].requiredEnforcers.entries()), @@ -243,7 +233,7 @@ describe('createPermissionRulesForChainId', () => { expect(byType['native-token-allowance'].permissionType).toBe( 'native-token-allowance', ); - expect(byType['native-token-allowance'].optionalEnforcers.size).toBe(4); + expect(byType['native-token-allowance'].optionalEnforcers.size).toBe(3); expect( byType['native-token-allowance'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); @@ -255,11 +245,6 @@ describe('createPermissionRulesForChainId', () => { allowedTargetsEnforcer, ), ).toBe(true); - expect( - byType['native-token-allowance'].optionalEnforcers.has( - logicalOrWrapperEnforcer, - ), - ).toBe(true); expect(byType['native-token-allowance'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['native-token-allowance'].requiredEnforcers.entries()), From 775801392fba57fb8f15d54da1a488ed665963c3 Mon Sep 17 00:00:00 2001 From: jeffsmale90 <6363749+jeffsmale90@users.noreply.github.com> Date: Wed, 6 May 2026 10:57:27 +1200 Subject: [PATCH 6/9] chore: removes `LogicalOrWrapperEnforcer` decoding for `payee` rule (#8709) ## Explanation Updates permission decoding logic, disallowing multiple `payee` addresses for `erc20` token type permissions. The `LogicalOrEnforcer` implementation requires the `caveatGroupIndex` to be passed as an argument at redemption time. This means that the enforcer is not EIP-7710 compliant - the redeemer must be able to decode, and understand the `permissionContext`. This PR removes `LogicalOrEnforcer` decoding logic, and allows only a single `payee` address to be specified via the `AllowedCalldataEnforcer`. ## References https://github.com/MetaMask/snap-7715-permissions/pull/313 ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them --- > [!NOTE] > **Medium Risk** > Changes permission decoding semantics for `payee` rules (removing multi-payee support and tightening caveat validation), which could affect clients relying on previously accepted caveat shapes. Risk is mitigated by updated unit tests covering new acceptance/rejection cases. > > **Overview** > Updates `gator-permissions-controller` permission decoding to **remove `LogicalOrWrapperEnforcer` support** when extracting `payee` rules, effectively limiting ERC-20 payee decoding to a single address via `AllowedCalldataEnforcer` and simplifying payee rule documentation. > > `makePermissionRule` now enforces stricter payee caveat rules (e.g., rejects multiple single-payee caveats and disallows configuring the single-payee enforcer as required) and **temporarily suppresses emitting `payee` rules for `erc20-token-revocation`**. Tests are rewritten/added to reflect the new decoding behavior and error messages, and `LogicalOrWrapperEnforcer` is removed from enforcer/type plumbing. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b1369e55d43f4c955845171ee35f95df4a5efb3e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot). --- eslint-suppressions.json | 2 +- .../src/constants.ts | 6 +- .../rules/erc20TokenAllowance.ts | 3 - .../rules/erc20TokenPeriodic.ts | 3 - .../rules/erc20TokenRevocation.test.ts | 60 +- .../rules/erc20TokenRevocation.ts | 3 - .../rules/erc20TokenStream.ts | 3 - .../rules/makePermissionRule.test.ts | 614 ++++-------------- .../rules/makePermissionRule.ts | 152 +---- .../rules/nativeTokenAllowance.ts | 2 - .../rules/nativeTokenPeriodic.ts | 2 - .../rules/nativeTokenStream.ts | 2 - .../src/decodePermission/types.ts | 1 - .../src/decodePermission/utils.test.ts | 33 +- .../src/decodePermission/utils.ts | 6 - .../src/payeeRule.ts | 3 +- 16 files changed, 217 insertions(+), 678 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index a6cfd180a5..d939f9223b 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -2348,4 +2348,4 @@ "count": 10 } } -} +} \ No newline at end of file diff --git a/packages/gator-permissions-controller/src/constants.ts b/packages/gator-permissions-controller/src/constants.ts index 87d7a6d3b7..bc75549418 100644 --- a/packages/gator-permissions-controller/src/constants.ts +++ b/packages/gator-permissions-controller/src/constants.ts @@ -13,8 +13,8 @@ export const EXECUTION_PERMISSION_REDEEMER_RULE_TYPE = 'redeemer' as const; /** * `Rule.type` / `wallet_getSupportedExecutionPermissions` `ruleTypes` entry for - * payee allowlists (AllowedCalldataEnforcer / AllowedTargetsEnforcer, optionally - * wrapped in LogicalOrWrapperEnforcer). Hosts should advertise this for every - * supported execution permission type that supports payee restrictions. + * payee allowlists (AllowedCalldataEnforcer / AllowedTargetsEnforcer). Hosts + * should advertise this for every supported execution permission type that supports + * payee restrictions. */ export const EXECUTION_PERMISSION_PAYEE_RULE_TYPE = 'payee' as const; diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenAllowance.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenAllowance.ts index e55eedb340..01c5e02ea9 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenAllowance.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenAllowance.ts @@ -36,7 +36,6 @@ export function makeErc20TokenAllowanceRule( allowedCalldataEnforcer, allowedTargetsEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-allowance', @@ -44,14 +43,12 @@ export function makeErc20TokenAllowanceRule( timestampEnforcer, redeemerEnforcer, allowedCalldataEnforcer, - logicalOrWrapperEnforcer, ], redeemerEnforcer, payeeEnforcers: { allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedCalldataEnforcer, - logicalOrWrapperEnforcer, }, timestampEnforcer, requiredEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts index 3fdaa34398..5b84141063 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts @@ -32,7 +32,6 @@ export function makeErc20TokenPeriodicRule( allowedCalldataEnforcer, allowedTargetsEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-periodic', @@ -40,14 +39,12 @@ export function makeErc20TokenPeriodicRule( timestampEnforcer, redeemerEnforcer, allowedCalldataEnforcer, - logicalOrWrapperEnforcer, ], redeemerEnforcer, payeeEnforcers: { allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedCalldataEnforcer, - logicalOrWrapperEnforcer, }, timestampEnforcer, requiredEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.test.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.test.ts index b27c8b5af4..32d2a37cd2 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.test.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.test.ts @@ -3,14 +3,19 @@ import { CHAIN_ID, DELEGATOR_CONTRACTS, } from '@metamask/delegation-deployments'; +import { getChecksumAddress } from '@metamask/utils'; import { createPermissionRulesForContracts } from '.'; describe('erc20-token-revocation rule', () => { const chainId = CHAIN_ID.sepolia; const contracts = DELEGATOR_CONTRACTS['1.3.0'][chainId]; - const { TimestampEnforcer, AllowedCalldataEnforcer, ValueLteEnforcer } = - contracts; + const { + TimestampEnforcer, + AllowedCalldataEnforcer, + ValueLteEnforcer, + RedeemerEnforcer, + } = contracts; const permissionRules = createPermissionRulesForContracts(contracts); const rule = permissionRules.find( (candidate) => candidate.permissionType === 'erc20-token-revocation', @@ -217,5 +222,56 @@ describe('erc20-token-revocation rule', () => { expect(result.expiry).toBe(1720000); expect(result.data).toStrictEqual({}); + expect(result.rules).toBeUndefined(); + }); + + it('includes redeemer rule but not payee when RedeemerEnforcer caveat is present', () => { + const packedAddr = '1111111111111111111111111111111111111111' as const; + const approveSelectorTerms = + '0x0000000000000000000000000000000000000000000000000000000000000000095ea7b3' as const; + const zeroAmountTerms = + '0x00000000000000000000000000000000000000000000000000000000000000240000000000000000000000000000000000000000000000000000000000000000' as const; + const zeroValueLteTerms = + '0x0000000000000000000000000000000000000000000000000000000000000000' as const; + const caveats = [ + expiryCaveat, + { + enforcer: AllowedCalldataEnforcer, + terms: approveSelectorTerms, + args: '0x' as const, + }, + { + enforcer: AllowedCalldataEnforcer, + terms: zeroAmountTerms, + args: '0x' as const, + }, + { + enforcer: ValueLteEnforcer, + terms: zeroValueLteTerms, + args: '0x' as const, + }, + { + enforcer: RedeemerEnforcer, + terms: `0x${packedAddr}` as const, + args: '0x' as const, + }, + ]; + const result = rule.validateAndDecodePermission(caveats); + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toStrictEqual([ + { + type: 'redeemer', + data: { + addresses: [ + getChecksumAddress( + '0x1111111111111111111111111111111111111111' as const, + ), + ], + }, + }, + ]); }); }); diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts index cdcfe0e35f..4ccb086c1c 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts @@ -28,21 +28,18 @@ export function makeErc20TokenRevocationRule( valueLteEnforcer, nonceEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-revocation', optionalEnforcers: [ timestampEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, ], redeemerEnforcer, payeeEnforcers: { allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedCalldataEnforcer, - logicalOrWrapperEnforcer, }, timestampEnforcer, requiredEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts index 86eaeec7d4..c87c782173 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts @@ -31,7 +31,6 @@ export function makeErc20TokenStreamRule( allowedCalldataEnforcer, allowedTargetsEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-stream', @@ -39,14 +38,12 @@ export function makeErc20TokenStreamRule( timestampEnforcer, redeemerEnforcer, allowedCalldataEnforcer, - logicalOrWrapperEnforcer, ], redeemerEnforcer, payeeEnforcers: { allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedCalldataEnforcer, - logicalOrWrapperEnforcer, }, timestampEnforcer, requiredEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts index ef3c379d7b..2232688780 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts @@ -1,15 +1,13 @@ -import { encodeSingle } from '@metamask/abi-utils'; import { createAllowedCalldataTerms, createAllowedTargetsTerms, - createLogicalOrWrapperTerms, createTimestampTerms, } from '@metamask/delegation-core'; import { CHAIN_ID, DELEGATOR_CONTRACTS, } from '@metamask/delegation-deployments'; -import { bytesToHex, getChecksumAddress } from '@metamask/utils'; +import { getChecksumAddress } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; import { makePermissionRule } from './makePermissionRule'; @@ -21,21 +19,17 @@ describe('makePermissionRule', () => { const redeemerEnforcer = contracts.RedeemerEnforcer; const allowedCalldataEnforcer = contracts.AllowedCalldataEnforcer; const allowedTargetsEnforcer = contracts.AllowedTargetsEnforcer; - const logicalOrWrapperEnforcer = contracts.LogicalOrWrapperEnforcer; - const EMPTY_ARGS = new Uint8Array(); const payeeEnforcersNative = { allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedTargetsEnforcer, - logicalOrWrapperEnforcer, }; const payeeEnforcersErc20 = { allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedCalldataEnforcer, - logicalOrWrapperEnforcer, }; it('calls optional validate callback when provided and decoding succeeds', () => { @@ -398,16 +392,15 @@ describe('makePermissionRule', () => { ]); }); - it('includes payee rule with multiple addresses via AllowedTargetsEnforcer (native)', () => { + it('does not include payee rule when only AllowedTargetsEnforcer caveat is present (erc20)', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; - const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; + const payeeAddress = '0x2222222222222222222222222222222222222222' as Hex; const rule = makePermissionRule({ - permissionType: 'native-token-stream', + permissionType: 'erc20-token-stream', timestampEnforcer, redeemerEnforcer, - payeeEnforcers: payeeEnforcersNative, + payeeEnforcers: payeeEnforcersErc20, optionalEnforcers: [allowedTargetsEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, @@ -421,9 +414,7 @@ describe('makePermissionRule', () => { }, { enforcer: allowedTargetsEnforcer, - terms: createAllowedTargetsTerms({ - targets: [payeeAddress1, payeeAddress2], - }), + terms: createAllowedTargetsTerms({ targets: [payeeAddress] }), args: '0x' as Hex, }, ]; @@ -434,88 +425,70 @@ describe('makePermissionRule', () => { if (!result.isValid) { throw new Error('Expected valid result'); } - expect(result.rules).toStrictEqual([ - { - type: 'payee', - data: { - addresses: [ - getChecksumAddress(payeeAddress1), - getChecksumAddress(payeeAddress2), - ], - }, - }, - ]); + expect(result.rules).toBeUndefined(); }); - it('includes both redeemer and payee rules when both caveats present', () => { + it('rejects multiple AllowedCalldataEnforcer caveats for erc20 payee decoding', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); - const redeemerAddr = '1111111111111111111111111111111111111111' as const; - const payeeAddress = '0x2222222222222222222222222222222222222222' as Hex; + const payeeAddress1 = '0x2222222222222222222222222222222222222222' as Hex; + const payeeAddress2 = '0x3333333333333333333333333333333333333333' as Hex; + const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; + const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; const rule = makePermissionRule({ - permissionType: 'native-token-stream', + permissionType: 'erc20-token-stream', timestampEnforcer, redeemerEnforcer, - payeeEnforcers: payeeEnforcersNative, - optionalEnforcers: [redeemerEnforcer, allowedTargetsEnforcer], + payeeEnforcers: payeeEnforcersErc20, + optionalEnforcers: [allowedCalldataEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, }); - const caveats = [ + const result = rule.validateAndDecodePermission([ { enforcer: requiredEnforcer, terms: '0x' as Hex, args: '0x' as Hex, }, { - enforcer: redeemerEnforcer, - terms: `0x${redeemerAddr}` as Hex, + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded1, + }), args: '0x' as Hex, }, { - enforcer: allowedTargetsEnforcer, - terms: createAllowedTargetsTerms({ targets: [payeeAddress] }), + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: padded2, + }), args: '0x' as Hex, }, - ]; - - const result = rule.validateAndDecodePermission(caveats); + ]); - expect(result.isValid).toBe(true); - if (!result.isValid) { - throw new Error('Expected valid result'); + expect(result.isValid).toBe(false); + if (result.isValid) { + throw new Error('Expected invalid result'); } - expect(result.rules).toHaveLength(2); - expect(result.rules).toStrictEqual([ - { - type: 'redeemer', - data: { - addresses: [getChecksumAddress(`0x${redeemerAddr}` as Hex)], - }, - }, - { - type: 'payee', - data: { - addresses: [getChecksumAddress(payeeAddress)], - }, - }, - ]); + expect(result.error.message).toBe( + 'Invalid payee caveats: multiple singlePayeeEnforcer caveats', + ); }); - it('includes payee rule with multiple addresses via LogicalOrWrapperEnforcer (erc20)', () => { + it('includes payee rule with multiple addresses via AllowedTargetsEnforcer (native)', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; - const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; - const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; const rule = makePermissionRule({ - permissionType: 'erc20-token-stream', + permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [logicalOrWrapperEnforcer], + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [allowedTargetsEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, }); @@ -527,30 +500,9 @@ describe('makePermissionRule', () => { args: '0x' as Hex, }, { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: padded1, - }), - args: EMPTY_ARGS, - }, - ], - [ - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: padded2, - }), - args: EMPTY_ARGS, - }, - ], - ], + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ + targets: [payeeAddress1, payeeAddress2], }), args: '0x' as Hex, }, @@ -575,57 +527,57 @@ describe('makePermissionRule', () => { ]); }); - it('rejects LogicalOrWrapperEnforcer for native payee caveats', () => { + it('does not include payee rule when only AllowedCalldataEnforcer caveat is present (native)', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress = '0x4444444444444444444444444444444444444444' as Hex; + const payeeAddress = '0x3333333333333333333333333333333333333333' as Hex; + const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}`; const rule = makePermissionRule({ permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, payeeEnforcers: payeeEnforcersNative, - optionalEnforcers: [logicalOrWrapperEnforcer], + optionalEnforcers: [allowedCalldataEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, }); - const result = rule.validateAndDecodePermission([ + const caveats = [ { enforcer: requiredEnforcer, terms: '0x' as Hex, args: '0x' as Hex, }, { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedTargetsEnforcer, - terms: createAllowedTargetsTerms({ - targets: [payeeAddress], - }), - args: EMPTY_ARGS, - }, - ], - ], + enforcer: allowedCalldataEnforcer, + terms: createAllowedCalldataTerms({ + startIndex: 4, + value: paddedAddress, }), args: '0x' as Hex, }, - ]); + ]; - expect(result.isValid).toBe(false); + const result = rule.validateAndDecodePermission(caveats); + + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toBeUndefined(); }); - it('does not include payee rule when no payee caveat is present', () => { + it('includes both redeemer and payee rules when both caveats present', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); + const redeemerAddr = '1111111111111111111111111111111111111111' as const; + const payeeAddress = '0x2222222222222222222222222222222222222222' as Hex; const rule = makePermissionRule({ permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, payeeEnforcers: payeeEnforcersNative, - optionalEnforcers: [], + optionalEnforcers: [redeemerEnforcer, allowedTargetsEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, }); @@ -636,6 +588,16 @@ describe('makePermissionRule', () => { terms: '0x' as Hex, args: '0x' as Hex, }, + { + enforcer: redeemerEnforcer, + terms: `0x${redeemerAddr}` as Hex, + args: '0x' as Hex, + }, + { + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ targets: [payeeAddress] }), + args: '0x' as Hex, + }, ]; const result = rule.validateAndDecodePermission(caveats); @@ -644,10 +606,24 @@ describe('makePermissionRule', () => { if (!result.isValid) { throw new Error('Expected valid result'); } - expect(result.rules).toBeUndefined(); + expect(result.rules).toHaveLength(2); + expect(result.rules).toStrictEqual([ + { + type: 'redeemer', + data: { + addresses: [getChecksumAddress(`0x${redeemerAddr}` as Hex)], + }, + }, + { + type: 'payee', + data: { + addresses: [getChecksumAddress(payeeAddress)], + }, + }, + ]); }); - it('returns true from caveatAddressesMatch when enforcers match rule', () => { + it('does not include payee rule when no payee caveat is present', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); const rule = makePermissionRule({ @@ -655,30 +631,7 @@ describe('makePermissionRule', () => { timestampEnforcer, redeemerEnforcer, payeeEnforcers: payeeEnforcersNative, - optionalEnforcers: [timestampEnforcer], - requiredEnforcers: { [requiredEnforcer]: 1 }, - validateAndDecodeData, - }); - - expect( - rule.caveatAddressesMatch([requiredEnforcer, timestampEnforcer]), - ).toBe(true); - expect(rule.caveatAddressesMatch([requiredEnforcer])).toBe(true); - expect(rule.caveatAddressesMatch([])).toBe(false); - }); - - it('rejects unrecognised inner enforcers in LogicalOrWrapperEnforcer', () => { - const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress = '0x4444444444444444444444444444444444444444' as Hex; - const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}`; - const unknownEnforcer = '0x9999999999999999999999999999999999999999' as Hex; - - const rule = makePermissionRule({ - permissionType: 'erc20-token-stream', - timestampEnforcer, - redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [logicalOrWrapperEnforcer], + optionalEnforcers: [], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, }); @@ -689,80 +642,35 @@ describe('makePermissionRule', () => { terms: '0x' as Hex, args: '0x' as Hex, }, - { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: paddedAddress, - }), - args: EMPTY_ARGS, - }, - ], - [ - { - enforcer: unknownEnforcer, - terms: - '0x0000000000000000000000000000000000000000000000000000000000000001', - args: EMPTY_ARGS, - }, - ], - ], - }), - args: '0x' as Hex, - }, ]; const result = rule.validateAndDecodePermission(caveats); - expect(result.isValid).toBe(false); + expect(result.isValid).toBe(true); + if (!result.isValid) { + throw new Error('Expected valid result'); + } + expect(result.rules).toBeUndefined(); }); - it('rejects LogicalOrWrapper when inner caveats are all unrecognised', () => { + it('returns true from caveatAddressesMatch when enforcers match rule', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); - const unknownEnforcer = '0x9999999999999999999999999999999999999999' as Hex; const rule = makePermissionRule({ - permissionType: 'erc20-token-stream', + permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [logicalOrWrapperEnforcer], + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [timestampEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, }); - const caveats = [ - { - enforcer: requiredEnforcer, - terms: '0x' as Hex, - args: '0x' as Hex, - }, - { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: unknownEnforcer, - terms: - '0x0000000000000000000000000000000000000000000000000000000000000001', - args: EMPTY_ARGS, - }, - ], - ], - }), - args: '0x' as Hex, - }, - ]; - - const result = rule.validateAndDecodePermission(caveats); - - expect(result.isValid).toBe(false); + expect( + rule.caveatAddressesMatch([requiredEnforcer, timestampEnforcer]), + ).toBe(true); + expect(rule.caveatAddressesMatch([requiredEnforcer])).toBe(true); + expect(rule.caveatAddressesMatch([])).toBe(false); }); it('rejects when singlePayeeEnforcer is unrecognised', () => { @@ -777,7 +685,6 @@ describe('makePermissionRule', () => { allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: unknownEnforcer, - logicalOrWrapperEnforcer, }, optionalEnforcers: [unknownEnforcer], requiredEnforcers: { [requiredEnforcer]: 1 }, @@ -803,147 +710,20 @@ describe('makePermissionRule', () => { expect(result.isValid).toBe(false); }); - it('rejects LogicalOrWrapperEnforcer when a group has more than one inner caveat', () => { - const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; - const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; - const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; - const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; - - const rule = makePermissionRule({ - permissionType: 'erc20-token-stream', - timestampEnforcer, - redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [logicalOrWrapperEnforcer], - requiredEnforcers: { [requiredEnforcer]: 1 }, - validateAndDecodeData, - }); - - const result = rule.validateAndDecodePermission([ - { - enforcer: requiredEnforcer, - terms: '0x' as Hex, - args: '0x' as Hex, - }, - { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: padded1, - }), - args: EMPTY_ARGS, - }, - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: padded2, - }), - args: EMPTY_ARGS, - }, - ], - ], - }), - args: '0x' as Hex, - }, - ]); - - expect(result.isValid).toBe(false); - }); - - it('rejects LogicalOrWrapperEnforcer with no caveat groups', () => { + it('rejects when singlePayeeEnforcer is configured as a required enforcer', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); + const payeeAddress = '0x2222222222222222222222222222222222222222' as Hex; const rule = makePermissionRule({ - permissionType: 'erc20-token-stream', - timestampEnforcer, - redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [logicalOrWrapperEnforcer], - requiredEnforcers: { [requiredEnforcer]: 1 }, - validateAndDecodeData, - }); - - const result = rule.validateAndDecodePermission([ - { - enforcer: requiredEnforcer, - terms: '0x' as Hex, - args: '0x' as Hex, - }, - { - enforcer: logicalOrWrapperEnforcer, - terms: bytesToHex(encodeSingle('((address,bytes,bytes)[])[]', [])), - args: '0x' as Hex, - }, - ]); - - expect(result.isValid).toBe(false); - }); - - it('rejects LogicalOrWrapperEnforcer with non-empty args', () => { - const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress = '0x4444444444444444444444444444444444444444' as Hex; - const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}`; - - const rule = makePermissionRule({ - permissionType: 'erc20-token-stream', + permissionType: 'native-token-stream', timestampEnforcer, redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [logicalOrWrapperEnforcer], - requiredEnforcers: { [requiredEnforcer]: 1 }, - validateAndDecodeData, - }); - - const result = rule.validateAndDecodePermission([ - { - enforcer: requiredEnforcer, - terms: '0x' as Hex, - args: '0x' as Hex, - }, - { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: paddedAddress, - }), - args: EMPTY_ARGS, - }, - ], - ], - }), - args: '0x00' as Hex, + payeeEnforcers: payeeEnforcersNative, + optionalEnforcers: [], + requiredEnforcers: { + [requiredEnforcer]: 1, + [allowedTargetsEnforcer]: 1, }, - ]); - - expect(result.isValid).toBe(false); - }); - - it('rejects when both LogicalOrWrapperEnforcer and single-payee caveats are present', () => { - const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; - const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; - const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; - const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; - - const rule = makePermissionRule({ - permissionType: 'erc20-token-stream', - timestampEnforcer, - redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [allowedCalldataEnforcer, logicalOrWrapperEnforcer], - requiredEnforcers: { [requiredEnforcer]: 1 }, validateAndDecodeData, }); @@ -954,40 +734,25 @@ describe('makePermissionRule', () => { args: '0x' as Hex, }, { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: padded1, - }), - args: '0x' as Hex, - }, - { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: padded2, - }), - args: EMPTY_ARGS, - }, - ], - ], - }), + enforcer: allowedTargetsEnforcer, + terms: createAllowedTargetsTerms({ targets: [payeeAddress] }), args: '0x' as Hex, }, ]); expect(result.isValid).toBe(false); + if (result.isValid) { + throw new Error('Expected invalid result'); + } + expect(result.error.message).toBe( + 'Invalid payee caveats: singlePayeeEnforcer may not be a required caveat', + ); }); it('rejects an ERC20 payee caveat with the wrong calldata start index', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress = '0x3333333333333333333333333333333333333333' as Hex; - const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}`; + const payeeAddress = '0x3333333333333333333333333333333333333333' as const; + const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}` as const; const rule = makePermissionRule({ permissionType: 'erc20-token-stream', @@ -1050,36 +815,6 @@ describe('makePermissionRule', () => { expect(result.isValid).toBe(false); }); - it('rejects a payee caveat with non-zero args', () => { - const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress = '0x2222222222222222222222222222222222222222' as Hex; - - const rule = makePermissionRule({ - permissionType: 'native-token-stream', - timestampEnforcer, - redeemerEnforcer, - payeeEnforcers: payeeEnforcersNative, - optionalEnforcers: [allowedTargetsEnforcer], - requiredEnforcers: { [requiredEnforcer]: 1 }, - validateAndDecodeData, - }); - - const result = rule.validateAndDecodePermission([ - { - enforcer: requiredEnforcer, - terms: '0x' as Hex, - args: '0x' as Hex, - }, - { - enforcer: allowedTargetsEnforcer, - terms: createAllowedTargetsTerms({ targets: [payeeAddress] }), - args: '0x01' as Hex, - }, - ]); - - expect(result.isValid).toBe(false); - }); - it('rejects a native payee caveat with no targets', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); @@ -1109,70 +844,6 @@ describe('makePermissionRule', () => { expect(result.isValid).toBe(false); }); - it('rejects multiple LogicalOrWrapperEnforcer caveats', () => { - const validateAndDecodeData = jest.fn().mockReturnValue({}); - const payeeAddress1 = '0x4444444444444444444444444444444444444444' as Hex; - const payeeAddress2 = '0x5555555555555555555555555555555555555555' as Hex; - const padded1 = `0x${payeeAddress1.slice(2).padStart(64, '0')}`; - const padded2 = `0x${payeeAddress2.slice(2).padStart(64, '0')}`; - - const rule = makePermissionRule({ - permissionType: 'erc20-token-stream', - timestampEnforcer, - redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [logicalOrWrapperEnforcer], - requiredEnforcers: { [requiredEnforcer]: 1 }, - validateAndDecodeData, - }); - - const result = rule.validateAndDecodePermission([ - { - enforcer: requiredEnforcer, - terms: '0x' as Hex, - args: '0x' as Hex, - }, - { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: padded1, - }), - args: EMPTY_ARGS, - }, - ], - ], - }), - args: '0x' as Hex, - }, - { - enforcer: logicalOrWrapperEnforcer, - terms: createLogicalOrWrapperTerms({ - caveatGroups: [ - [ - { - enforcer: allowedCalldataEnforcer, - terms: createAllowedCalldataTerms({ - startIndex: 4, - value: padded2, - }), - args: EMPTY_ARGS, - }, - ], - ], - }), - args: '0x' as Hex, - }, - ]); - - expect(result.isValid).toBe(false); - }); - it('rejects multiple single-payee caveats', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); const payeeAddress1 = '0x2222222222222222222222222222222222222222' as Hex; @@ -1208,49 +879,4 @@ describe('makePermissionRule', () => { expect(result.isValid).toBe(false); }); - - it('handles multiple singlePayeeEnforcer caveats gracefully (e.g. revocation)', () => { - const validateAndDecodeData = jest.fn().mockReturnValue({}); - - const rule = makePermissionRule({ - permissionType: 'erc20-token-revocation', - timestampEnforcer, - redeemerEnforcer, - payeeEnforcers: payeeEnforcersErc20, - optionalEnforcers: [], - requiredEnforcers: { - [allowedCalldataEnforcer]: 2, - [requiredEnforcer]: 1, - }, - validateAndDecodeData, - }); - - const caveats = [ - { - enforcer: requiredEnforcer, - terms: '0x' as Hex, - args: '0x' as Hex, - }, - { - enforcer: allowedCalldataEnforcer, - terms: - '0x0000000000000000000000000000000000000000000000000000000000000000095ea7b3' as Hex, - args: '0x' as Hex, - }, - { - enforcer: allowedCalldataEnforcer, - terms: - '0x00000000000000000000000000000000000000000000000000000000000000240000000000000000000000000000000000000000000000000000000000000000' as Hex, - args: '0x' as Hex, - }, - ]; - - const result = rule.validateAndDecodePermission(caveats); - - expect(result.isValid).toBe(true); - if (!result.isValid) { - throw new Error('Expected valid result'); - } - expect(result.rules).toBeUndefined(); - }); }); diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts index c90d438c13..131309c424 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts @@ -3,7 +3,6 @@ import type { Caveat } from '@metamask/delegation-core'; import { decodeAllowedCalldataTerms, decodeAllowedTargetsTerms, - decodeLogicalOrWrapperTerms, decodeRedeemerTerms, } from '@metamask/delegation-core'; import { getChecksumAddress, isStrictHexString } from '@metamask/utils'; @@ -30,14 +29,11 @@ import { const ERC20_TRANSFER_PAYEE_START_INDEX = 4; const ERC20_PAYEE_VALUE_BYTE_LENGTH = 32; -const PAYEE_CAVEAT_ARGS = '0x' as const; -const LOGICAL_OR_WRAPPER_CAVEAT_ARGS = '0x' as const; type PayeeEnforcerAddresses = { allowedCalldataEnforcer: Hex; allowedTargetsEnforcer: Hex; singlePayeeEnforcer: Hex; - logicalOrWrapperEnforcer: Hex; }; /** @@ -140,16 +136,21 @@ export function makePermissionRule({ }); } - const payeeAddresses = tryExtractPayeeAddresses( - checksumCaveats, - payeeEnforcers, - requiredEnforcersMap, - ); - if (payeeAddresses) { - rules.push({ - type: EXECUTION_PERMISSION_PAYEE_RULE_TYPE, - data: { addresses: payeeAddresses }, - }); + // todo: this is a temporary fix to exclude payee rules from erc20-token-revocation + // a nicer solution may be to pass an array of permissionRule decoders to the makePermissionRule + // function. + if (permissionType !== "erc20-token-revocation") { + const payeeAddresses = tryExtractPayeeAddresses( + checksumCaveats, + payeeEnforcers, + requiredEnforcersMap, + ); + if (payeeAddresses) { + rules.push({ + type: EXECUTION_PERMISSION_PAYEE_RULE_TYPE, + data: { addresses: payeeAddresses }, + }); + } } return { @@ -174,28 +175,26 @@ export function makePermissionRule({ * @param payeeEnforcerAddresses.allowedTargetsEnforcer - AllowedTargetsEnforcer address. * @returns The checksummed payee addresses, or null if the enforcer is unrecognised. */ -function tryExtractPayeeAddressesFromCaveat( +function extractPayeeAddressesFromCaveat( caveat: Caveat, payeeEnforcerAddresses: { allowedCalldataEnforcer: Hex; allowedTargetsEnforcer: Hex; }, -): Hex[] | null { +): Hex[] { const checksumEnforcer = getChecksumAddress(caveat.enforcer); if (checksumEnforcer === payeeEnforcerAddresses.allowedCalldataEnforcer) { - assertPayeeCaveatArgs(caveat.args); - const decoded = decodeAllowedCalldataTerms(caveat.terms); if (decoded.startIndex !== ERC20_TRANSFER_PAYEE_START_INDEX) { throw new Error( - `Invalid AllowedCalldataEnforcer payee terms: startIndex must be ${ERC20_TRANSFER_PAYEE_START_INDEX}`, + `Invalid payee caveat: AllowedCalldataEnforcer startIndex must be ${ERC20_TRANSFER_PAYEE_START_INDEX}`, ); } if (getByteLength(decoded.value) !== ERC20_PAYEE_VALUE_BYTE_LENGTH) { throw new Error( - `Invalid AllowedCalldataEnforcer payee terms: value must be ${ERC20_PAYEE_VALUE_BYTE_LENGTH} bytes`, + `Invalid payee caveat: AllowedCalldataEnforcer value must be ${ERC20_PAYEE_VALUE_BYTE_LENGTH} bytes long`, ); } @@ -204,61 +203,22 @@ function tryExtractPayeeAddressesFromCaveat( } if (checksumEnforcer === payeeEnforcerAddresses.allowedTargetsEnforcer) { - assertPayeeCaveatArgs(caveat.args); - const decoded = decodeAllowedTargetsTerms(caveat.terms); - return decoded.targets.map((target) => getChecksumAddress(target)); - } - - return null; -} - -/** - * Asserts that payee caveat args match the expected empty value. - * - * @param args - The caveat args to validate. - */ -function assertPayeeCaveatArgs(args: Hex): void { - if (args !== PAYEE_CAVEAT_ARGS) { - throw new Error(`Invalid payee caveat args: must be ${PAYEE_CAVEAT_ARGS}`); - } -} - -/** - * Extracts payee addresses from the specified payee caveat. - * - * @param caveat - The caveat to decode. - * @param enforcers - Payee enforcer addresses. - * @returns The checksummed payee addresses. - */ -function extractPayeeAddressesFromExpectedCaveat( - caveat: Caveat, - enforcers: PayeeEnforcerAddresses, -): Hex[] { - if (getChecksumAddress(caveat.enforcer) !== enforcers.singlePayeeEnforcer) { - throw new Error( - 'Invalid payee caveat: must use the configured single-payee enforcer', - ); + return decoded.targets.map(getChecksumAddress); } - const addresses = tryExtractPayeeAddressesFromCaveat(caveat, enforcers); - if (!addresses) { - throw new Error('Invalid payee caveat: unable to decode payee address'); - } - - return addresses; + throw new Error('Invalid payee caveat: unrecognised enforcer'); } /** * Attempts to extract payee addresses from caveats, handling both single-payee - * (direct enforcer) and multi-payee (LogicalOrWrapperEnforcer) cases. + * (direct enforcer) and multi-payee (RedeemerEnforcer). * * @param caveats - Checksummed caveats from the delegation. * @param enforcers - Payee enforcer addresses. * @param enforcers.allowedCalldataEnforcer - AllowedCalldataEnforcer address. * @param enforcers.allowedTargetsEnforcer - AllowedTargetsEnforcer address. * @param enforcers.singlePayeeEnforcer - The specific enforcer for single-payee in this permission type. - * @param enforcers.logicalOrWrapperEnforcer - The LogicalOrWrapperEnforcer address. * @param requiredEnforcers - Required enforcer counts for the permission rule. * @returns Array of checksummed payee addresses, or null if no payee caveat is found. */ @@ -267,75 +227,23 @@ function tryExtractPayeeAddresses( enforcers: PayeeEnforcerAddresses, requiredEnforcers: Map, ): Hex[] | null { - const logicalOrCaveats = caveats.filter( - (caveat) => caveat.enforcer === enforcers.logicalOrWrapperEnforcer, - ); - - if (logicalOrCaveats.length > 1) { - throw new Error('Invalid caveats'); + if (requiredEnforcers.has(enforcers.singlePayeeEnforcer)) { + throw new Error('Invalid payee caveats: singlePayeeEnforcer may not be a required caveat'); } - const requiredSinglePayeeCount = - requiredEnforcers.get(enforcers.singlePayeeEnforcer) ?? 0; const singlePayeeCaveats = caveats.filter( (caveat) => caveat.enforcer === enforcers.singlePayeeEnforcer, ); - if (singlePayeeCaveats.length > requiredSinglePayeeCount + 1) { - throw new Error('Invalid caveats'); - } - - const singlePayeeCaveat = - singlePayeeCaveats[requiredSinglePayeeCount] ?? null; - - if (logicalOrCaveats.length === 1 && singlePayeeCaveat) { - throw new Error( - 'Invalid payee caveats: use either LogicalOrWrapperEnforcer or a single-payee caveat', - ); + // this should not be possible, unless the singlePayeeCaveat is also included for a different rule, for the permission itself + if (singlePayeeCaveats.length > 1) { + throw new Error('Invalid payee caveats: multiple singlePayeeEnforcer caveats'); } - if (logicalOrCaveats.length === 1) { - if (enforcers.singlePayeeEnforcer !== enforcers.allowedCalldataEnforcer) { - throw new Error( - 'Invalid LogicalOrWrapperEnforcer payee caveat: only ERC20 payees may use LogicalOrWrapperEnforcer', - ); - } - - const [logicalOrCaveat] = logicalOrCaveats; - if (logicalOrCaveat.args !== LOGICAL_OR_WRAPPER_CAVEAT_ARGS) { - throw new Error( - `Invalid LogicalOrWrapperEnforcer payee args: must be ${LOGICAL_OR_WRAPPER_CAVEAT_ARGS}`, - ); - } - - const decoded = decodeLogicalOrWrapperTerms(logicalOrCaveat.terms); - if (decoded.caveatGroups.length === 0) { - throw new Error( - 'Invalid LogicalOrWrapperEnforcer payee terms: must contain at least one caveat group', - ); - } - - return decoded.caveatGroups.flatMap((group) => { - if (group.length !== 1) { - throw new Error( - 'Invalid LogicalOrWrapperEnforcer payee terms: each caveat group must contain exactly one caveat', - ); - } - - const addresses = extractPayeeAddressesFromExpectedCaveat( - group[0], - enforcers, - ); - - return addresses; - }); - } + const singlePayeeCaveat = singlePayeeCaveats[0] ?? null; - if (singlePayeeCaveat) { - return extractPayeeAddressesFromExpectedCaveat( - singlePayeeCaveat, - enforcers, - ); + if (singlePayeeCaveat) { + return extractPayeeAddressesFromCaveat(singlePayeeCaveat, enforcers); } return null; diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts index 61f1012f5c..9f5910cc1b 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenAllowance.ts @@ -36,7 +36,6 @@ export function makeNativeTokenAllowanceRule( allowedCalldataEnforcer, allowedTargetsEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'native-token-allowance', @@ -50,7 +49,6 @@ export function makeNativeTokenAllowanceRule( allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedTargetsEnforcer, - logicalOrWrapperEnforcer, }, timestampEnforcer, requiredEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts index 2826d896a7..cb1a0c3d29 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts @@ -31,7 +31,6 @@ export function makeNativeTokenPeriodicRule( allowedCalldataEnforcer, allowedTargetsEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'native-token-periodic', @@ -45,7 +44,6 @@ export function makeNativeTokenPeriodicRule( allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedTargetsEnforcer, - logicalOrWrapperEnforcer, }, timestampEnforcer, requiredEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts index 871b0a3ea2..57454b3929 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts @@ -26,7 +26,6 @@ export function makeNativeTokenStreamRule( allowedCalldataEnforcer, allowedTargetsEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, } = enforcers; return makePermissionRule({ permissionType: 'native-token-stream', @@ -40,7 +39,6 @@ export function makeNativeTokenStreamRule( allowedCalldataEnforcer, allowedTargetsEnforcer, singlePayeeEnforcer: allowedTargetsEnforcer, - logicalOrWrapperEnforcer, }, timestampEnforcer, requiredEnforcers: { diff --git a/packages/gator-permissions-controller/src/decodePermission/types.ts b/packages/gator-permissions-controller/src/decodePermission/types.ts index 9f24d8bd7a..e27d4a8033 100644 --- a/packages/gator-permissions-controller/src/decodePermission/types.ts +++ b/packages/gator-permissions-controller/src/decodePermission/types.ts @@ -101,7 +101,6 @@ export type ChecksumEnforcersByChainId = { allowedCalldataEnforcer: Hex; allowedTargetsEnforcer: Hex; redeemerEnforcer: Hex; - logicalOrWrapperEnforcer: Hex; }; /** Caveat with checksummed enforcer address; used by rule decode functions. */ diff --git a/packages/gator-permissions-controller/src/decodePermission/utils.test.ts b/packages/gator-permissions-controller/src/decodePermission/utils.test.ts index 38e7efde2b..2cc7902c1c 100644 --- a/packages/gator-permissions-controller/src/decodePermission/utils.test.ts +++ b/packages/gator-permissions-controller/src/decodePermission/utils.test.ts @@ -24,7 +24,6 @@ const buildContracts = (): DeployedContractsByName => ({ AllowedCalldataEnforcer: '0x9999999999999999999999999999999999999999', AllowedTargetsEnforcer: '0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', RedeemerEnforcer: '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', - LogicalOrWrapperEnforcer: '0xcccccccccccccccccccccccccccccccccccccccc', }); describe('getChecksumEnforcersByChainId', () => { @@ -58,9 +57,6 @@ describe('getChecksumEnforcersByChainId', () => { contracts.AllowedTargetsEnforcer, ), redeemerEnforcer: getChecksumAddress(contracts.RedeemerEnforcer), - logicalOrWrapperEnforcer: getChecksumAddress( - contracts.LogicalOrWrapperEnforcer, - ), }); }); @@ -88,7 +84,6 @@ describe('createPermissionRulesForChainId', () => { allowedCalldataEnforcer, allowedTargetsEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, } = getChecksumEnforcersByChainId(contracts); // erc20-token-stream @@ -167,7 +162,7 @@ describe('createPermissionRulesForChainId', () => { expect(byType['erc20-token-stream'].permissionType).toBe( 'erc20-token-stream', ); - expect(byType['erc20-token-stream'].optionalEnforcers.size).toBe(4); + expect(byType['erc20-token-stream'].optionalEnforcers.size).toBe(3); expect( byType['erc20-token-stream'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); @@ -179,11 +174,6 @@ describe('createPermissionRulesForChainId', () => { allowedCalldataEnforcer, ), ).toBe(true); - expect( - byType['erc20-token-stream'].optionalEnforcers.has( - logicalOrWrapperEnforcer, - ), - ).toBe(true); expect(byType['erc20-token-stream'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['erc20-token-stream'].requiredEnforcers.entries()), @@ -200,7 +190,7 @@ describe('createPermissionRulesForChainId', () => { expect(byType['erc20-token-periodic'].permissionType).toBe( 'erc20-token-periodic', ); - expect(byType['erc20-token-periodic'].optionalEnforcers.size).toBe(4); + expect(byType['erc20-token-periodic'].optionalEnforcers.size).toBe(3); expect( byType['erc20-token-periodic'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); @@ -212,11 +202,6 @@ describe('createPermissionRulesForChainId', () => { allowedCalldataEnforcer, ), ).toBe(true); - expect( - byType['erc20-token-periodic'].optionalEnforcers.has( - logicalOrWrapperEnforcer, - ), - ).toBe(true); expect(byType['erc20-token-periodic'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['erc20-token-periodic'].requiredEnforcers.entries()), @@ -261,7 +246,7 @@ describe('createPermissionRulesForChainId', () => { expect(byType['erc20-token-allowance'].permissionType).toBe( 'erc20-token-allowance', ); - expect(byType['erc20-token-allowance'].optionalEnforcers.size).toBe(4); + expect(byType['erc20-token-allowance'].optionalEnforcers.size).toBe(3); expect( byType['erc20-token-allowance'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); @@ -273,11 +258,6 @@ describe('createPermissionRulesForChainId', () => { allowedCalldataEnforcer, ), ).toBe(true); - expect( - byType['erc20-token-allowance'].optionalEnforcers.has( - logicalOrWrapperEnforcer, - ), - ).toBe(true); expect(byType['erc20-token-allowance'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['erc20-token-allowance'].requiredEnforcers.entries()), @@ -294,18 +274,13 @@ describe('createPermissionRulesForChainId', () => { expect(byType['erc20-token-revocation'].permissionType).toBe( 'erc20-token-revocation', ); - expect(byType['erc20-token-revocation'].optionalEnforcers.size).toBe(3); + expect(byType['erc20-token-revocation'].optionalEnforcers.size).toBe(2); expect( byType['erc20-token-revocation'].optionalEnforcers.has(timestampEnforcer), ).toBe(true); expect( byType['erc20-token-revocation'].optionalEnforcers.has(redeemerEnforcer), ).toBe(true); - expect( - byType['erc20-token-revocation'].optionalEnforcers.has( - logicalOrWrapperEnforcer, - ), - ).toBe(true); expect(byType['erc20-token-revocation'].requiredEnforcers.size).toBe(3); expect( Array.from(byType['erc20-token-revocation'].requiredEnforcers.entries()), diff --git a/packages/gator-permissions-controller/src/decodePermission/utils.ts b/packages/gator-permissions-controller/src/decodePermission/utils.ts index 9b132444b9..4177159c64 100644 --- a/packages/gator-permissions-controller/src/decodePermission/utils.ts +++ b/packages/gator-permissions-controller/src/decodePermission/utils.ts @@ -22,7 +22,6 @@ const ENFORCER_CONTRACT_NAMES = { AllowedCalldataEnforcer: 'AllowedCalldataEnforcer', AllowedTargetsEnforcer: 'AllowedTargetsEnforcer', RedeemerEnforcer: 'RedeemerEnforcer', - LogicalOrWrapperEnforcer: 'LogicalOrWrapperEnforcer', }; /** @@ -119,10 +118,6 @@ export const getChecksumEnforcersByChainId = ( ENFORCER_CONTRACT_NAMES.RedeemerEnforcer, ); - const logicalOrWrapperEnforcer = getChecksumContractAddress( - ENFORCER_CONTRACT_NAMES.LogicalOrWrapperEnforcer, - ); - return { erc20StreamingEnforcer, erc20PeriodicEnforcer, @@ -135,7 +130,6 @@ export const getChecksumEnforcersByChainId = ( allowedCalldataEnforcer, allowedTargetsEnforcer, redeemerEnforcer, - logicalOrWrapperEnforcer, }; }; diff --git a/packages/gator-permissions-controller/src/payeeRule.ts b/packages/gator-permissions-controller/src/payeeRule.ts index 0bf8c12e5e..4731c16c35 100644 --- a/packages/gator-permissions-controller/src/payeeRule.ts +++ b/packages/gator-permissions-controller/src/payeeRule.ts @@ -2,8 +2,7 @@ import type { Hex } from '@metamask/utils'; /** * Execution permission rule restricting which addresses may receive payments - * (on-chain AllowedCalldataEnforcer / AllowedTargetsEnforcer caveat, optionally - * wrapped in a LogicalOrWrapperEnforcer for multiple payees). + * (on-chain AllowedCalldataEnforcer / AllowedTargetsEnforcer caveat). */ export type PayeeRule = { type: 'payee'; From 5a71fe9b246ce6b7d45c188c3f3288b1ff4712b5 Mon Sep 17 00:00:00 2001 From: MJ Kiwi Date: Wed, 6 May 2026 11:06:02 +1200 Subject: [PATCH 7/9] fix: correct formatting and improve error messages in payee rule handling --- eslint-suppressions.json | 2 +- .../gator-permissions-controller/src/constants.ts | 2 +- .../src/decodePermission/rules/makePermissionRule.ts | 12 ++++++++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index d939f9223b..a6cfd180a5 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -2348,4 +2348,4 @@ "count": 10 } } -} \ No newline at end of file +} diff --git a/packages/gator-permissions-controller/src/constants.ts b/packages/gator-permissions-controller/src/constants.ts index bc75549418..7a4ad01c57 100644 --- a/packages/gator-permissions-controller/src/constants.ts +++ b/packages/gator-permissions-controller/src/constants.ts @@ -13,7 +13,7 @@ export const EXECUTION_PERMISSION_REDEEMER_RULE_TYPE = 'redeemer' as const; /** * `Rule.type` / `wallet_getSupportedExecutionPermissions` `ruleTypes` entry for - * payee allowlists (AllowedCalldataEnforcer / AllowedTargetsEnforcer). Hosts + * payee allowlists (AllowedCalldataEnforcer / AllowedTargetsEnforcer). Hosts * should advertise this for every supported execution permission type that supports * payee restrictions. */ diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts index 131309c424..b3966908fa 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.ts @@ -139,7 +139,7 @@ export function makePermissionRule({ // todo: this is a temporary fix to exclude payee rules from erc20-token-revocation // a nicer solution may be to pass an array of permissionRule decoders to the makePermissionRule // function. - if (permissionType !== "erc20-token-revocation") { + if (permissionType !== 'erc20-token-revocation') { const payeeAddresses = tryExtractPayeeAddresses( checksumCaveats, payeeEnforcers, @@ -228,7 +228,9 @@ function tryExtractPayeeAddresses( requiredEnforcers: Map, ): Hex[] | null { if (requiredEnforcers.has(enforcers.singlePayeeEnforcer)) { - throw new Error('Invalid payee caveats: singlePayeeEnforcer may not be a required caveat'); + throw new Error( + 'Invalid payee caveats: singlePayeeEnforcer may not be a required caveat', + ); } const singlePayeeCaveats = caveats.filter( @@ -237,12 +239,14 @@ function tryExtractPayeeAddresses( // this should not be possible, unless the singlePayeeCaveat is also included for a different rule, for the permission itself if (singlePayeeCaveats.length > 1) { - throw new Error('Invalid payee caveats: multiple singlePayeeEnforcer caveats'); + throw new Error( + 'Invalid payee caveats: multiple singlePayeeEnforcer caveats', + ); } const singlePayeeCaveat = singlePayeeCaveats[0] ?? null; - if (singlePayeeCaveat) { + if (singlePayeeCaveat) { return extractPayeeAddressesFromCaveat(singlePayeeCaveat, enforcers); } From c9540d592d0a7d4cb4f2beee46a69a6ccbc0bb44 Mon Sep 17 00:00:00 2001 From: MJ Kiwi Date: Wed, 6 May 2026 11:13:03 +1200 Subject: [PATCH 8/9] fix: streamline optional enforcers array formatting and improve readability in permission rule --- .../src/decodePermission/rules/erc20TokenRevocation.ts | 5 +---- .../src/decodePermission/rules/makePermissionRule.test.ts | 3 ++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts index 4ccb086c1c..a127829d8e 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenRevocation.ts @@ -31,10 +31,7 @@ export function makeErc20TokenRevocationRule( } = enforcers; return makePermissionRule({ permissionType: 'erc20-token-revocation', - optionalEnforcers: [ - timestampEnforcer, - redeemerEnforcer, - ], + optionalEnforcers: [timestampEnforcer, redeemerEnforcer], redeemerEnforcer, payeeEnforcers: { allowedCalldataEnforcer, diff --git a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts index 2232688780..fef9a97124 100644 --- a/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts +++ b/packages/gator-permissions-controller/src/decodePermission/rules/makePermissionRule.test.ts @@ -752,7 +752,8 @@ describe('makePermissionRule', () => { it('rejects an ERC20 payee caveat with the wrong calldata start index', () => { const validateAndDecodeData = jest.fn().mockReturnValue({}); const payeeAddress = '0x3333333333333333333333333333333333333333' as const; - const paddedAddress = `0x${payeeAddress.slice(2).padStart(64, '0')}` as const; + const paddedAddress = + `0x${payeeAddress.slice(2).padStart(64, '0')}` as const; const rule = makePermissionRule({ permissionType: 'erc20-token-stream', From a5c332eb0d8fa010ff114718ba755170275fbab7 Mon Sep 17 00:00:00 2001 From: MJ Kiwi Date: Wed, 6 May 2026 17:33:46 +1200 Subject: [PATCH 9/9] fix: remove 'payee' rule type from execution permissions mock data --- .../wallet-get-supported-execution-permissions.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eth-json-rpc-middleware/src/methods/wallet-get-supported-execution-permissions.test.ts b/packages/eth-json-rpc-middleware/src/methods/wallet-get-supported-execution-permissions.test.ts index 2ff8e60f79..2fcec4d658 100644 --- a/packages/eth-json-rpc-middleware/src/methods/wallet-get-supported-execution-permissions.test.ts +++ b/packages/eth-json-rpc-middleware/src/methods/wallet-get-supported-execution-permissions.test.ts @@ -10,7 +10,7 @@ import { createWalletGetSupportedExecutionPermissionsHandler } from './wallet-ge const RESULT_MOCK: GetSupportedExecutionPermissionsResult = { 'native-token-allowance': { chainIds: ['0x123', '0x345'] as Hex[], - ruleTypes: ['expiry', 'redeemer', 'payee'], + ruleTypes: ['expiry', 'redeemer'], }, 'erc20-token-allowance': { chainIds: ['0x123'] as Hex[], @@ -18,7 +18,7 @@ const RESULT_MOCK: GetSupportedExecutionPermissionsResult = { }, 'erc721-token-allowance': { chainIds: ['0x123'] as Hex[], - ruleTypes: ['expiry', 'redeemer', 'payee'], + ruleTypes: ['expiry', 'redeemer'], }, };