fix: reject non-zero native value in ERC20/ERC721 transfer enforcers

src/enforcers/ERC20PeriodTransferEnforcer.sol

@@ -178,8 +178,9 @@ contract ERC20PeriodTransferEnforcer is CaveatEnforcer {
     )
         private
     {
-        (address target_,, bytes calldata callData_) = _executionCallData.decodeSingle();
+        (address target_, uint256 value_, bytes calldata callData_) = _executionCallData.decodeSingle();
 
+        require(value_ == 0, "ERC20PeriodTransferEnforcer:invalid-value");
         require(callData_.length == 68, "ERC20PeriodTransferEnforcer:invalid-execution-length");
 
         (address token_, uint256 periodAmount_, uint256 periodDuration_, uint256 startDate_) = getTermsInfo(_terms);

src/enforcers/ERC20TransferAmountEnforcer.sol

@@ -82,8 +82,9 @@ contract ERC20TransferAmountEnforcer is CaveatEnforcer {
         internal
         returns (uint256 limit_, uint256 spent_)
     {
-        (address target_,, bytes calldata callData_) = _executionCallData.decodeSingle();
+        (address target_, uint256 value_, bytes calldata callData_) = _executionCallData.decodeSingle();
 
+        require(value_ == 0, "ERC20TransferAmountEnforcer:invalid-value");
         require(callData_.length == 68, "ERC20TransferAmountEnforcer:invalid-execution-length");
 
         address allowedContract_;

src/enforcers/ERC721TransferEnforcer.sol

@@ -37,7 +37,11 @@ contract ERC721TransferEnforcer is CaveatEnforcer {
         onlyDefaultExecutionMode(_mode)
     {
         (address permittedContract_, uint256 permittedTokenId_) = getTermsInfo(_terms);
-        (address target_,, bytes calldata callData_) = ExecutionLib.decodeSingle(_executionCallData);
+        (address target_, uint256 value_, bytes calldata callData_) = ExecutionLib.decodeSingle(_executionCallData);
+
+        if (value_ != 0) {
+            revert("ERC721TransferEnforcer:invalid-value");
+        }
 
         // Decode the remaining callData into NFT transfer parameters
         // The calldata should be at least 100 bytes (4 bytes for the selector + 96 bytes for the parameters)

src/enforcers/LogicalOrWrapperEnforcer.sol

@@ -47,6 +47,9 @@ import { ModeCode, Caveat } from "../utils/Types.sol";
  *  - Never assume the redeemer will select the most restrictive group.
  *  - Design caveat groups with the understanding that the redeemer will choose the path of least
  *    resistance.
+ *  - Stateful caveat enforcers keep isolated state per group (the delegation hash forwarded
+ *    to sub-enforcers is namespaced by the selected group index), so budgets/periods do not
+ *    leak across groups. To share a budget across alternative paths, use a single group.
  *
  * Use this enforcer at your own risk and ensure it aligns with your intended security model.
  */
@@ -266,11 +269,26 @@ contract LogicalOrWrapperEnforcer is CaveatEnforcer {
                     selectedGroup_.caveatArgs[i],
                     _params.mode,
                     _params.executionCallData,
-                    _params.delegationHash,
+                    _getGroupDelegationHash(_params.delegationHash, selectedGroup_.groupIndex),
                     _params.delegator,
                     _params.redeemer
                 )
             );
         }
     }
+
+    /**
+     * @notice Namespaces the delegation hash by the selected group.
+     * @dev Stateful caveat enforcers key their state by delegation hash (not by terms),
+     *      so without namespacing every group that references the same stateful enforcer
+     *      shares one state: the first-used group initialises it (e.g. pinning a period
+     *      start or filling a call counter) for all other groups, invisibly to the
+     *      delegator. With namespacing each group's state is isolated; a shared budget
+     *      can still be expressed by placing the caveats in a single group.
+     * @param _delegationHash The hash of the delegation being redeemed.
+     * @param _groupIndex The index of the caveat group selected for this redemption.
+     */
+    function _getGroupDelegationHash(bytes32 _delegationHash, uint256 _groupIndex) internal pure returns (bytes32) {
+        return keccak256(abi.encode(_delegationHash, _groupIndex));
+    }
 }

test/enforcers/ERC20PeriodTransferEnforcer.t.sol

@@ -96,6 +96,17 @@ contract ERC20PeriodTransferEnforcerTest is CaveatEnforcerBaseTest {
         );
     }
 
+    /// @notice Reverts if a token transfer execution carries non-zero native value.
+    function testRevertOnNonZeroValue() public {
+        bytes memory terms_ = abi.encodePacked(address(basicERC20), periodAmount, periodDuration, startDate);
+        bytes memory callData_ = _encodeERC20Transfer(bob, 10 ether);
+        bytes memory execCallData_ = _encodeSingleExecution(address(basicERC20), 1, callData_);
+        vm.expectRevert("ERC20PeriodTransferEnforcer:invalid-value");
+        erc20PeriodTransferEnforcer.beforeHook(
+            terms_, "", singleDefaultMode, execCallData_, dummyDelegationHash, address(0), redeemer
+        );
+    }
+
     /// @notice Reverts if the target contract in execution data does not match the token in terms_.
     function testInvalidContract() public {
         bytes memory terms_ = abi.encodePacked(address(basicERC20), periodAmount, periodDuration, startDate);

test/enforcers/ERC20TransferAmountEnforcer.t.sol

@@ -200,6 +200,35 @@ contract ERC20TransferAmountEnforcerTest is CaveatEnforcerBaseTest {
         assertEq(erc20TransferAmountEnforcer.spentMap(address(delegationManager), delegationHash_), 0);
     }
 
+    // should FAIL with non-zero native value on a token transfer
+    function test_revertOnNonZeroValue() public {
+        uint256 spendingLimit_ = 1 ether;
+        Execution memory execution_ = Execution({
+            target: address(basicERC20),
+            value: 1,
+            callData: abi.encodeWithSelector(IERC20.transfer.selector, address(users.bob.deleGator), spendingLimit_)
+        });
+        bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData);
+        bytes memory inputTerms_ = abi.encodePacked(address(basicERC20), spendingLimit_);
+        Caveat[] memory caveats_ = new Caveat[](1);
+        caveats_[0] = Caveat({ args: hex"", enforcer: address(erc20TransferAmountEnforcer), terms: inputTerms_ });
+        Delegation memory delegation_ = Delegation({
+            delegate: address(users.bob.deleGator),
+            delegator: address(users.alice.deleGator),
+            authority: ROOT_AUTHORITY,
+            caveats: caveats_,
+            salt: 0,
+            signature: hex""
+        });
+        bytes32 delegationHash_ = EncoderLib._getDelegationHash(delegation_);
+        vm.prank(address(delegationManager));
+        vm.expectRevert("ERC20TransferAmountEnforcer:invalid-value");
+        erc20TransferAmountEnforcer.beforeHook(
+            inputTerms_, hex"", singleDefaultMode, executionCallData_, delegationHash_, address(0), address(0)
+        );
+        assertEq(erc20TransferAmountEnforcer.spentMap(address(delegationManager), delegationHash_), 0);
+    }
+
     // should FAIL to INVOKE invalid method
     function test_methodFailsIfInvokesInvalidMethod() public {
         uint256 spendingLimit_ = 1 ether;

test/enforcers/ERC721TransferEnforcer.t.sol

@@ -129,6 +129,28 @@ contract ERC721TransferEnforcerTest is CaveatEnforcerBaseTest {
         );
     }
 
+    /// @notice Tests that a transfer reverts when the execution carries non-zero native value.
+    function test_revertOnNonZeroValue() public {
+        Execution memory execution_ = Execution({
+            target: address(token),
+            value: 1,
+            callData: abi.encodeWithSelector(IERC721.transferFrom.selector, address(this), address(0xBEEF), TOKEN_ID)
+        });
+        bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData);
+
+        vm.prank(address(delegationManager));
+        vm.expectRevert("ERC721TransferEnforcer:invalid-value");
+        erc721TransferEnforcer.beforeHook(
+            abi.encodePacked(address(token), TOKEN_ID),
+            hex"",
+            singleDefaultMode,
+            executionCallData_,
+            keccak256(""),
+            address(0),
+            address(0)
+        );
+    }
+
     /// @notice Tests that a transfer reverts when using an unauthorized function selector.
     function test_unauthorizedSelector_wrongMethod() public {
         bytes4 dummySelector_ = bytes4(keccak256("foo(address,address,uint256)"));

test/enforcers/LogicalOrWrapperEnforcer.t.sol

@@ -14,6 +14,7 @@ import { LogicalOrWrapperEnforcer } from "../../src/enforcers/LogicalOrWrapperEn
 import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol";
 import { AllowedMethodsEnforcer } from "../../src/enforcers/AllowedMethodsEnforcer.sol";
 import { AllowedTargetsEnforcer } from "../../src/enforcers/AllowedTargetsEnforcer.sol";
+import { LimitedCallsEnforcer } from "../../src/enforcers/LimitedCallsEnforcer.sol";
 import { NativeTokenTransferAmountEnforcer } from "../../src/enforcers/NativeTokenTransferAmountEnforcer.sol";
 import { TimestampEnforcer } from "../../src/enforcers/TimestampEnforcer.sol";
 import { ArgsEqualityCheckEnforcer } from "../../src/enforcers/ArgsEqualityCheckEnforcer.sol";
@@ -139,6 +140,12 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
 
     ////////////////////// Helper Functions //////////////////////
 
+    /// @dev The namespaced delegation hash the wrapper forwards to sub-enforcers for a
+    /// given group — mirrors LogicalOrWrapperEnforcer._getGroupDelegationHash
+    function _groupDelegationHash(bytes32 delegationHash_, uint256 groupIndex_) internal pure returns (bytes32) {
+        return keccak256(abi.encode(delegationHash_, groupIndex_));
+    }
+
     function _createCaveatGroup(
         address[] memory _enforcers,
         bytes[] memory _terms
@@ -306,6 +313,54 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         );
     }
 
+    /// @notice Tests that stateful enforcers keep per-group isolated state when the same
+    /// stateful enforcer appears in multiple groups: each group's allowance is independent.
+    function test_statefulEnforcerStateIsIsolatedPerGroup() public {
+        LimitedCallsEnforcer limitedCallsEnforcer_ = new LimitedCallsEnforcer();
+
+        // Two groups, both referencing the same stateful enforcer with limit = 1 call each
+        LogicalOrWrapperEnforcer.CaveatGroup[] memory groups_ = new LogicalOrWrapperEnforcer.CaveatGroup[](2);
+        for (uint256 g_ = 0; g_ < 2; g_++) {
+            address[] memory enforcers_ = new address[](1);
+            enforcers_[0] = address(limitedCallsEnforcer_);
+            bytes[] memory terms_ = new bytes[](1);
+            terms_[0] = abi.encode(uint256(1));
+            groups_[g_] = _createCaveatGroup(enforcers_, terms_);
+        }
+
+        Execution memory execution_ = Execution({ target: address(aliceDeleGatorCounter), value: 0, callData: hex"" });
+        bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData);
+        bytes32 delegationHash_ = keccak256("some-delegation");
+
+        bytes[] memory caveatArgs_ = new bytes[](1);
+        caveatArgs_[0] = hex"";
+
+        // Group 0: first call consumes its allowance
+        vm.prank(address(delegationManager));
+        logicalOrWrapperEnforcer.beforeHook(
+            abi.encode(groups_), abi.encode(_createSelectedGroup(0, caveatArgs_)), singleDefaultMode, executionCallData_, delegationHash_, address(0), address(0)
+        );
+        // Group 0: second call exceeds its own limit
+        vm.prank(address(delegationManager));
+        vm.expectRevert("LimitedCallsEnforcer:limit-exceeded");
+        logicalOrWrapperEnforcer.beforeHook(
+            abi.encode(groups_), abi.encode(_createSelectedGroup(0, caveatArgs_)), singleDefaultMode, executionCallData_, delegationHash_, address(0), address(0)
+        );
+
+        // Group 1: independent allowance — without per-group namespacing this would revert
+        // because the shared counter was already exhausted by group 0
+        vm.prank(address(delegationManager));
+        logicalOrWrapperEnforcer.beforeHook(
+            abi.encode(groups_), abi.encode(_createSelectedGroup(1, caveatArgs_)), singleDefaultMode, executionCallData_, delegationHash_, address(0), address(0)
+        );
+        // Group 1: second call also exceeds its own limit
+        vm.prank(address(delegationManager));
+        vm.expectRevert("LimitedCallsEnforcer:limit-exceeded");
+        logicalOrWrapperEnforcer.beforeHook(
+            abi.encode(groups_), abi.encode(_createSelectedGroup(1, caveatArgs_)), singleDefaultMode, executionCallData_, delegationHash_, address(0), address(0)
+        );
+    }
+
     /// @notice Tests that the ArgsEqualityCheckEnforcer works correctly when terms match args through the LogicalOrWrapperEnforcer
     function test_argsEqualityCheckEnforcerSuccess() public {
         // Create a group with a single caveat (args equality check)
@@ -684,7 +739,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         bytes32 hashKey_ = erc20BalanceChangeEnforcer.getHashKey(
             address(logicalOrWrapperEnforcer), // LogicalOrWrapperEnforcer is the caller
             address(mockToken),
-            keccak256("")
+            _groupDelegationHash(keccak256(""), 0)
         );
         assertTrue(erc20BalanceChangeEnforcer.isLocked(hashKey_), "Balance cache should be locked during execution_");
         // Verify the cached balance is stored
@@ -711,7 +766,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         bytes32 afterHashKey_ = erc20BalanceChangeEnforcer.getHashKey(
             address(logicalOrWrapperEnforcer), // LogicalOrWrapperEnforcer is the caller
             address(mockToken),
-            keccak256("")
+            _groupDelegationHash(keccak256(""), 0)
         );
         assertFalse(erc20BalanceChangeEnforcer.isLocked(afterHashKey_), "Balance cache should be unlocked after afterHook");
         // Note: balanceCache is not cleared, but isLocked is false, so it can be reused
@@ -812,7 +867,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         // Validate state: spentMap should track the transfer amount
         uint256 spentAmount_ = erc20TransferAmountEnforcer.spentMap(
             address(logicalOrWrapperEnforcer), // LogicalOrWrapperEnforcer is the delegationManager
-            keccak256("")
+            _groupDelegationHash(keccak256(""), 0)
         );
         assertEq(spentAmount_, 50 ether, "SpentMap should track 50 ether spent");
 
@@ -859,7 +914,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         );
 
         // Validate state after first transfer
-        uint256 spentAfterFirst_ = erc20TransferAmountEnforcer.spentMap(address(logicalOrWrapperEnforcer), keccak256(""));
+        uint256 spentAfterFirst_ = erc20TransferAmountEnforcer.spentMap(address(logicalOrWrapperEnforcer), _groupDelegationHash(keccak256(""), 0));
         assertEq(spentAfterFirst_, 50 ether, "SpentMap should show 50 ether spent after first transfer");
 
         // Simulate the transfer (would be done by the actual execution_ in prod)
@@ -1415,7 +1470,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
 
         // Validate state after first call
         assertEq(
-            erc20TransferAmountEnforcer.spentMap(address(logicalOrWrapperEnforcer), delegationHash_),
+            erc20TransferAmountEnforcer.spentMap(address(logicalOrWrapperEnforcer), _groupDelegationHash(delegationHash_, 0)),
             30 ether,
             "Should have 30 ether spent after first call"
         );
@@ -1437,7 +1492,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
 
         // Validate final state
         assertEq(
-            erc20TransferAmountEnforcer.spentMap(address(logicalOrWrapperEnforcer), delegationHash_),
+            erc20TransferAmountEnforcer.spentMap(address(logicalOrWrapperEnforcer), _groupDelegationHash(delegationHash_, 0)),
             80 ether,
             "Should have 80 ether spent after second call"
         );
@@ -1489,7 +1544,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         // Validate state: streaming allowance should be initialized and track spent amount
         (,,,, uint256 spent_) = erc20StreamingEnforcer.streamingAllowances(
             address(logicalOrWrapperEnforcer), // LogicalOrWrapperEnforcer is the delegationManager
-            keccak256("")
+            _groupDelegationHash(keccak256(""), 0)
         );
         assertEq(spent_, 5 ether, "Spent amount should be 5 ether");
     }
@@ -1584,7 +1639,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         // Validate state: periodic allowance should be initialized and track transferred amount
         (,,, uint256 lastTransferPeriod_, uint256 transferredInCurrentPeriod_) = erc20PeriodTransferEnforcer.periodicAllowances(
             address(logicalOrWrapperEnforcer), // LogicalOrWrapperEnforcer is the delegationManager
-            keccak256("")
+            _groupDelegationHash(keccak256(""), 0)
         );
         assertEq(lastTransferPeriod_, 1, "Last transfer period should be 1 (current period)");
         assertEq(transferredInCurrentPeriod_, 50 ether, "Transferred in current period should be 50 ether");
@@ -1826,7 +1881,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
 
         // Validate state: Check spent amount
         assertEq(
-            erc20TransferAmountEnforcer.spentMap(address(logicalOrWrapperEnforcer), keccak256("")),
+            erc20TransferAmountEnforcer.spentMap(address(logicalOrWrapperEnforcer), _groupDelegationHash(keccak256(""), 0)),
             75 ether,
             "ERC20 transfer enforcer should track 75 ether spent"
         );
@@ -2274,7 +2329,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         // Validate enforcer state (LogicalOrWrapperEnforcer acts as delegationManager for wrapped enforcer)
         bytes32 delegationHash_ = EncoderLib._getDelegationHash(delegation);
         (,,, uint256 lastTransferPeriod, uint256 transferredInCurrentPeriod) =
-            erc20PeriodTransferEnforcer.periodicAllowances(address(logicalOrWrapperEnforcer), delegationHash_);
+            erc20PeriodTransferEnforcer.periodicAllowances(address(logicalOrWrapperEnforcer), _groupDelegationHash(delegationHash_, 0));
         assertEq(lastTransferPeriod, 1, "Should be in first period");
         assertEq(transferredInCurrentPeriod, 30 ether, "Should track 30 ether transferred");
     }
@@ -2301,7 +2356,8 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
 
         // Validate enforcer state (LogicalOrWrapperEnforcer acts as delegationManager for wrapped enforcer)
         bytes32 delegationHash_ = EncoderLib._getDelegationHash(delegation);
-        (,,,, uint256 spent) = erc20StreamingEnforcer.streamingAllowances(address(logicalOrWrapperEnforcer), delegationHash_);
+        (,,,, uint256 spent) =
+            erc20StreamingEnforcer.streamingAllowances(address(logicalOrWrapperEnforcer), _groupDelegationHash(delegationHash_, 0));
         assertEq(spent, 5 ether, "Should track 5 ether spent");
     }
 
@@ -2330,7 +2386,7 @@ contract LogicalOrWrapperEnforcerTest is CaveatEnforcerBaseTest {
         bytes memory terms_ = abi.encodePacked(address(mockToken), uint256(50 ether), uint256(1 days), block.timestamp);
         bytes memory args = abi.encode(uint256(0));
         (uint256 available,,) =
-            multiTokenPeriodEnforcer.getAvailableAmount(delegationHash_, address(logicalOrWrapperEnforcer), terms_, args);
+            multiTokenPeriodEnforcer.getAvailableAmount(_groupDelegationHash(delegationHash_, 0), address(logicalOrWrapperEnforcer), terms_, args);
         assertEq(available, 30 ether, "Should have 30 ether remaining (50 - 20)");
     }