From b817bd64914033d3584ac7ce435be85694b2ae89 Mon Sep 17 00:00:00 2001 From: hanzel98 Date: Mon, 21 Apr 2025 13:21:38 -0600 Subject: [PATCH 1/4] Refactored Token Gte Enforcers to TokenBalanceChange --- .cursorrules | 2 +- .gas-snapshot | 26 +- documents/CaveatEnforcers.md | 4 +- lcov.info | 44 ++-- script/DeployCaveatEnforcers.s.sol | 24 +- script/verification/.gas-snapshot | 82 +++--- .../verification/verify-enforcer-contracts.sh | 8 +- ...r.sol => ERC1155BalanceChangeEnforcer.sol} | 64 +++-- ...cer.sol => ERC20BalanceChangeEnforcer.sol} | 62 +++-- ...er.sol => ERC721BalanceChangeEnforcer.sol} | 62 +++-- ...er.sol => NativeBalanceChangeEnforcer.sol} | 66 +++-- ...sol => ERC1155BalanceChangeEnforcer.t.sol} | 187 ++++++++----- .../ERC20BalanceChangeEnforcer.t.sol | 246 ++++++++++++++++++ test/enforcers/ERC20BalanceGteEnforcer.t.sol | 204 --------------- ....sol => ERC721BalanceChangeEnforcer.t.sol} | 189 +++++++++----- ....sol => NativeBalanceChangeEnforcer.t.sol} | 78 ++++-- 16 files changed, 802 insertions(+), 546 deletions(-) rename src/enforcers/{ERC1155BalanceGteEnforcer.sol => ERC1155BalanceChangeEnforcer.sol} (58%) rename src/enforcers/{ERC20BalanceGteEnforcer.sol => ERC20BalanceChangeEnforcer.sol} (55%) rename src/enforcers/{ERC721BalanceGteEnforcer.sol => ERC721BalanceChangeEnforcer.sol} (58%) rename src/enforcers/{NativeBalanceGteEnforcer.sol => NativeBalanceChangeEnforcer.sol} (51%) rename test/enforcers/{ERC1155BalanceGteEnforcer.t.sol => ERC1155BalanceChangeEnforcer.t.sol} (52%) create mode 100644 test/enforcers/ERC20BalanceChangeEnforcer.t.sol delete mode 100644 test/enforcers/ERC20BalanceGteEnforcer.t.sol rename test/enforcers/{ERC721BalanceGteEnforcer.t.sol => ERC721BalanceChangeEnforcer.t.sol} (51%) rename test/enforcers/{NativeBalanceGteEnforcer.t.sol => NativeBalanceChangeEnforcer.t.sol} (60%) diff --git a/.cursorrules b/.cursorrules index 93b29a70..5293ada8 100644 --- a/.cursorrules +++ b/.cursorrules @@ -45,7 +45,7 @@ Caveat enforcers allow you to add specific conditions or restrictions to delegat - `BlockNumberEnforcer.sol` - `DeployedEnforcer.sol` - `ERC20TransferAmountEnforcer.sol` -- `ERC20BalanceGteEnforcer.sol` +- `ERC20BalanceChangeEnforcer.sol` - `NonceEnforcer.sol` - `LimitedCallsEnforcer.sol` - `IdEnforcer.sol` diff --git a/.gas-snapshot b/.gas-snapshot index 16e3f36d..622e9d85 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -69,13 +69,13 @@ DeployedEnforcerTest:test_revertIfContractIsEmpty() (gas: 57826) DeployedEnforcerTest:test_revertIfPredictedAddressDoesNotMatch() (gas: 113981) DeployedEnforcerTest:test_revertIfTermsLengthIsInvalid() (gas: 26457) DeployedEnforcerTest:test_revertsIfBytecodeDoesntExist() (gas: 113615) -ERC20BalanceGteEnforcerTest:test_allow_ifBalanceIncreases() (gas: 145796) -ERC20BalanceGteEnforcerTest:test_decodedTheTerms() (gas: 9371) -ERC20BalanceGteEnforcerTest:test_invalid_decodedTheTerms() (gas: 15278) -ERC20BalanceGteEnforcerTest:test_invalid_tokenAddress() (gas: 62209) -ERC20BalanceGteEnforcerTest:test_notAllow_expectingOverflow() (gas: 82733) -ERC20BalanceGteEnforcerTest:test_notAllow_insufficientIncrease() (gas: 107898) -ERC20BalanceGteEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 166416) +ERC20BalanceChangeEnforcerTest:test_allow_ifBalanceIncreases() (gas: 145796) +ERC20BalanceChangeEnforcerTest:test_decodedTheTerms() (gas: 9371) +ERC20BalanceChangeEnforcerTest:test_invalid_decodedTheTerms() (gas: 15278) +ERC20BalanceChangeEnforcerTest:test_invalid_tokenAddress() (gas: 62209) +ERC20BalanceChangeEnforcerTest:test_notAllow_expectingOverflow() (gas: 82733) +ERC20BalanceChangeEnforcerTest:test_notAllow_insufficientIncrease() (gas: 107898) +ERC20BalanceChangeEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 166416) ERC20TransferAmountEnforcerTest:test_methodFailsIfInvokesInvalidContract() (gas: 35068) ERC20TransferAmountEnforcerTest:test_methodFailsIfInvokesInvalidMethod() (gas: 33268) ERC20TransferAmountEnforcerTest:test_methodFailsIfInvokesInvalidTermsLength() (gas: 32374) @@ -362,12 +362,12 @@ NativeAllowanceEnforcerTest:test_decodesTheTerms() (gas: 6235) NativeAllowanceEnforcerTest:test_transferFailsIfAllowanceExceeded() (gas: 49982) NativeAllowanceEnforcerTest:test_transferSucceedsIfCalledBelowAllowance() (gas: 55151) NativeAllowanceEnforcerTest:test_transferSucceedsIfCalledBelowAllowanceMultipleCalls() (gas: 77077) -NativeBalanceGteEnforcerTest:test_allow_ifBalanceIncreases() (gas: 89672) -NativeBalanceGteEnforcerTest:test_decodedTheTerms() (gas: 9399) -NativeBalanceGteEnforcerTest:test_invalid_decodedTheTerms() (gas: 15265) -NativeBalanceGteEnforcerTest:test_notAllow_expectingOverflow() (gas: 63355) -NativeBalanceGteEnforcerTest:test_notAllow_insufficientIncrease() (gas: 66957) -NativeBalanceGteEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 103828) +NativeBalanceChangeEnforcerTest:test_allow_ifBalanceIncreases() (gas: 89672) +NativeBalanceChangeEnforcerTest:test_decodedTheTerms() (gas: 9399) +NativeBalanceChangeEnforcerTest:test_invalid_decodedTheTerms() (gas: 15265) +NativeBalanceChangeEnforcerTest:test_notAllow_expectingOverflow() (gas: 63355) +NativeBalanceChangeEnforcerTest:test_notAllow_insufficientIncrease() (gas: 66957) +NativeBalanceChangeEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 103828) NativeTokenPaymentEnforcerTest:test_allowsRedelegationAddingExtraCosts() (gas: 683336) NativeTokenPaymentEnforcerTest:test_chargePaymentFromAllowance() (gas: 1191883) NativeTokenPaymentEnforcerTest:test_decodesTheTerms() (gas: 9274) diff --git a/documents/CaveatEnforcers.md b/documents/CaveatEnforcers.md index 4e0d4403..f21cbd12 100644 --- a/documents/CaveatEnforcers.md +++ b/documents/CaveatEnforcers.md @@ -33,6 +33,6 @@ The `NativeTokenPaymentEnforcer` is a mechanism used within a delegation (D1) th This redemption may alter the state of other contracts. For example, the balance of the delegator providing the allowance will decrease, while the balance of the recipient specified by the payment delegation will increase. These state changes can impact other enforcers that rely on balance validations, depending on their order in the caveats array. -Consider a scenario where D1 includes an array of caveats: one caveat is the `NativeBalanceGteEnforcer`, which verifies that Bob’s balance has increased as a result of the execution attached to D1. The second caveat is the `NativeTokenPaymentEnforcer`, which deducts from Bob’s balance by redeeming D2. If these enforcers are not correctly ordered, they could conflict. For instance, if the `NativeTokenPaymentEnforcer` is executed before the `NativeBalanceGteEnforcer`, Bob’s balance would be reduced first, potentially causing the `NativeBalanceGteEnforcer` to fail its validation of ensuring Bob’s balance exceeds a certain threshold. +Consider a scenario where D1 includes an array of caveats: one caveat is the `NativeBalanceChangeEnforcer`, which verifies that Bob’s balance has increased as a result of the execution attached to D1. The second caveat is the `NativeTokenPaymentEnforcer`, which deducts from Bob’s balance by redeeming D2. If these enforcers are not correctly ordered, they could conflict. For instance, if the `NativeTokenPaymentEnforcer` is executed before the `NativeBalanceChangeEnforcer`, Bob’s balance would be reduced first, potentially causing the `NativeBalanceChangeEnforcer` to fail its validation of ensuring Bob’s balance exceeds a certain threshold. -Because the `NativeTokenPaymentEnforcer` modifies the state of external contracts, it is essential to carefully order enforcers in the delegation to prevent conflicts. The enforcers are designed to protect the execution process, but they do not guarantee a final state after the redemption. This means that even if the `NativeBalanceGteEnforcer` validates Bob’s balance at one point, subsequent enforcers, such as the `NativeTokenPaymentEnforcer`, may modify it later. +Because the `NativeTokenPaymentEnforcer` modifies the state of external contracts, it is essential to carefully order enforcers in the delegation to prevent conflicts. The enforcers are designed to protect the execution process, but they do not guarantee a final state after the redemption. This means that even if the `NativeBalanceChangeEnforcer` validates Bob’s balance at one point, subsequent enforcers, such as the `NativeTokenPaymentEnforcer`, may modify it later. diff --git a/lcov.info b/lcov.info index 54dfc48b..60f62431 100644 --- a/lcov.info +++ b/lcov.info @@ -797,12 +797,12 @@ BRF:6 BRH:6 end_of_record TN: -SF:src/enforcers/ERC20BalanceGteEnforcer.sol -FN:32,ERC20BalanceGteEnforcer.getHashKey -FNDA:1,ERC20BalanceGteEnforcer.getHashKey +SF:src/enforcers/ERC20BalanceChangeEnforcer.sol +FN:32,ERC20BalanceChangeEnforcer.getHashKey +FNDA:1,ERC20BalanceChangeEnforcer.getHashKey DA:33,1 -FN:43,ERC20BalanceGteEnforcer.beforeHook -FNDA:8,ERC20BalanceGteEnforcer.beforeHook +FN:43,ERC20BalanceChangeEnforcer.beforeHook +FNDA:8,ERC20BalanceChangeEnforcer.beforeHook DA:55,8 DA:56,8 DA:57,8 @@ -811,8 +811,8 @@ BRDA:57,0,1,7 DA:58,7 DA:59,7 DA:60,6 -FN:68,ERC20BalanceGteEnforcer.afterHook -FNDA:5,ERC20BalanceGteEnforcer.afterHook +FN:68,ERC20BalanceChangeEnforcer.afterHook +FNDA:5,ERC20BalanceChangeEnforcer.afterHook DA:80,5 DA:81,5 DA:82,5 @@ -820,15 +820,15 @@ DA:83,5 DA:84,5 BRDA:84,1,0,2 BRDA:84,1,1,3 -FN:93,ERC20BalanceGteEnforcer.getTermsInfo -FNDA:3,ERC20BalanceGteEnforcer.getTermsInfo +FN:93,ERC20BalanceChangeEnforcer.getTermsInfo +FNDA:3,ERC20BalanceChangeEnforcer.getTermsInfo DA:94,16 BRDA:94,2,0,2 BRDA:94,2,1,14 DA:95,14 DA:96,14 -FN:104,ERC20BalanceGteEnforcer._getHashKey -FNDA:14,ERC20BalanceGteEnforcer._getHashKey +FN:104,ERC20BalanceChangeEnforcer._getHashKey +FNDA:14,ERC20BalanceChangeEnforcer._getHashKey DA:105,14 FNF:5 FNH:5 @@ -925,12 +925,12 @@ BRF:4 BRH:4 end_of_record TN: -SF:src/enforcers/NativeBalanceGteEnforcer.sol -FN:29,NativeBalanceGteEnforcer.getHashKey -FNDA:1,NativeBalanceGteEnforcer.getHashKey +SF:src/enforcers/NativeBalanceChangeEnforcer.sol +FN:29,NativeBalanceChangeEnforcer.getHashKey +FNDA:1,NativeBalanceChangeEnforcer.getHashKey DA:30,1 -FN:41,NativeBalanceGteEnforcer.beforeHook -FNDA:7,NativeBalanceGteEnforcer.beforeHook +FN:41,NativeBalanceChangeEnforcer.beforeHook +FNDA:7,NativeBalanceChangeEnforcer.beforeHook DA:53,7 DA:54,7 DA:56,7 @@ -938,23 +938,23 @@ BRDA:56,0,0,1 BRDA:56,0,1,6 DA:57,6 DA:58,6 -FN:67,NativeBalanceGteEnforcer.afterHook -FNDA:5,NativeBalanceGteEnforcer.afterHook +FN:67,NativeBalanceChangeEnforcer.afterHook +FNDA:5,NativeBalanceChangeEnforcer.afterHook DA:79,5 DA:80,5 DA:81,5 DA:82,5 BRDA:82,1,0,1 BRDA:82,1,1,3 -FN:92,NativeBalanceGteEnforcer.getTermsInfo -FNDA:3,NativeBalanceGteEnforcer.getTermsInfo +FN:92,NativeBalanceChangeEnforcer.getTermsInfo +FNDA:3,NativeBalanceChangeEnforcer.getTermsInfo DA:93,15 BRDA:93,2,0,2 BRDA:93,2,1,13 DA:94,13 DA:95,13 -FN:103,NativeBalanceGteEnforcer._getHashKey -FNDA:13,NativeBalanceGteEnforcer._getHashKey +FN:103,NativeBalanceChangeEnforcer._getHashKey +FNDA:13,NativeBalanceChangeEnforcer._getHashKey DA:104,13 FNF:5 FNH:5 diff --git a/script/DeployCaveatEnforcers.s.sol b/script/DeployCaveatEnforcers.s.sol index 918726e0..7a3929ec 100644 --- a/script/DeployCaveatEnforcers.s.sol +++ b/script/DeployCaveatEnforcers.s.sol @@ -12,20 +12,20 @@ import { AllowedTargetsEnforcer } from "../src/enforcers/AllowedTargetsEnforcer. import { ArgsEqualityCheckEnforcer } from "../src/enforcers/ArgsEqualityCheckEnforcer.sol"; import { BlockNumberEnforcer } from "../src/enforcers/BlockNumberEnforcer.sol"; import { DeployedEnforcer } from "../src/enforcers/DeployedEnforcer.sol"; -import { ERC20BalanceGteEnforcer } from "../src/enforcers/ERC20BalanceGteEnforcer.sol"; +import { ERC20BalanceChangeEnforcer } from "../src/enforcers/ERC20BalanceChangeEnforcer.sol"; import { ERC20TransferAmountEnforcer } from "../src/enforcers/ERC20TransferAmountEnforcer.sol"; import { ERC20StreamingEnforcer } from "../src/enforcers/ERC20StreamingEnforcer.sol"; import { ERC20PeriodTransferEnforcer } from "../src/enforcers/ERC20PeriodTransferEnforcer.sol"; -import { ERC721BalanceGteEnforcer } from "../src/enforcers/ERC721BalanceGteEnforcer.sol"; +import { ERC721BalanceChangeEnforcer } from "../src/enforcers/ERC721BalanceChangeEnforcer.sol"; import { ERC721TransferEnforcer } from "../src/enforcers/ERC721TransferEnforcer.sol"; -import { ERC1155BalanceGteEnforcer } from "../src/enforcers/ERC1155BalanceGteEnforcer.sol"; +import { ERC1155BalanceChangeEnforcer } from "../src/enforcers/ERC1155BalanceChangeEnforcer.sol"; import { ExactCalldataEnforcer } from "../src/enforcers/ExactCalldataEnforcer.sol"; import { ExactExecutionBatchEnforcer } from "../src/enforcers/ExactExecutionBatchEnforcer.sol"; import { ExactExecutionEnforcer } from "../src/enforcers/ExactExecutionEnforcer.sol"; import { IdEnforcer } from "../src/enforcers/IdEnforcer.sol"; import { LimitedCallsEnforcer } from "../src/enforcers/LimitedCallsEnforcer.sol"; import { MultiTokenPeriodEnforcer } from "../src/enforcers/MultiTokenPeriodEnforcer.sol"; -import { NativeBalanceGteEnforcer } from "../src/enforcers/NativeBalanceGteEnforcer.sol"; +import { NativeBalanceChangeEnforcer } from "../src/enforcers/NativeBalanceChangeEnforcer.sol"; import { NativeTokenPaymentEnforcer } from "../src/enforcers/NativeTokenPaymentEnforcer.sol"; import { NativeTokenPeriodTransferEnforcer } from "../src/enforcers/NativeTokenPeriodTransferEnforcer.sol"; import { NativeTokenStreamingEnforcer } from "../src/enforcers/NativeTokenStreamingEnforcer.sol"; @@ -82,8 +82,8 @@ contract DeployCaveatEnforcers is Script { deployedAddress = address(new DeployedEnforcer{ salt: salt }()); console2.log("DeployedEnforcer: %s", deployedAddress); - deployedAddress = address(new ERC20BalanceGteEnforcer{ salt: salt }()); - console2.log("ERC20BalanceGteEnforcer: %s", deployedAddress); + deployedAddress = address(new ERC20BalanceChangeEnforcer{ salt: salt }()); + console2.log("ERC20BalanceChangeEnforcer: %s", deployedAddress); deployedAddress = address(new ERC20TransferAmountEnforcer{ salt: salt }()); console2.log("ERC20TransferAmountEnforcer: %s", deployedAddress); @@ -94,14 +94,14 @@ contract DeployCaveatEnforcers is Script { deployedAddress = address(new ERC20StreamingEnforcer{ salt: salt }()); console2.log("ERC20StreamingEnforcer: %s", deployedAddress); - deployedAddress = address(new ERC721BalanceGteEnforcer{ salt: salt }()); - console2.log("ERC721BalanceGteEnforcer: %s", deployedAddress); + deployedAddress = address(new ERC721BalanceChangeEnforcer{ salt: salt }()); + console2.log("ERC721BalanceChangeEnforcer: %s", deployedAddress); deployedAddress = address(new ERC721TransferEnforcer{ salt: salt }()); console2.log("ERC721TransferEnforcer: %s", deployedAddress); - deployedAddress = address(new ERC1155BalanceGteEnforcer{ salt: salt }()); - console2.log("ERC1155BalanceGteEnforcer: %s", deployedAddress); + deployedAddress = address(new ERC1155BalanceChangeEnforcer{ salt: salt }()); + console2.log("ERC1155BalanceChangeEnforcer: %s", deployedAddress); deployedAddress = address(new ExactCalldataEnforcer{ salt: salt }()); console2.log("ExactCalldataEnforcer: %s", deployedAddress); @@ -121,8 +121,8 @@ contract DeployCaveatEnforcers is Script { deployedAddress = address(new MultiTokenPeriodEnforcer{ salt: salt }()); console2.log("MultiTokenPeriodEnforcer: %s", deployedAddress); - deployedAddress = address(new NativeBalanceGteEnforcer{ salt: salt }()); - console2.log("NativeBalanceGteEnforcer: %s", deployedAddress); + deployedAddress = address(new NativeBalanceChangeEnforcer{ salt: salt }()); + console2.log("NativeBalanceChangeEnforcer: %s", deployedAddress); address argsEqualityCheckEnforcer = address(new ArgsEqualityCheckEnforcer{ salt: salt }()); console2.log("ArgsEqualityCheckEnforcer: %s", argsEqualityCheckEnforcer); diff --git a/script/verification/.gas-snapshot b/script/verification/.gas-snapshot index 2e81ade0..f77fcc5c 100644 --- a/script/verification/.gas-snapshot +++ b/script/verification/.gas-snapshot @@ -198,28 +198,28 @@ EIP7702StatelessDeleGatorTest:test_notAllow_invalidSignatureLength() (gas: 12874 EIP7702StatelessDeleGatorTest:test_notAllow_toSetLongName() (gas: 85050) EIP7702StatelessDeleGatorTest:test_notAllow_toSetLongVersion() (gas: 85047) EIP7702StatelessDeleGatorTest:test_supportsExecutionMode() (gas: 28302) -ERC1155BalanceGteEnforcerTest:test_allow_ifBalanceIncreases() (gas: 153112) -ERC1155BalanceGteEnforcerTest:test_allow_withDifferentRecipients() (gas: 159877) -ERC1155BalanceGteEnforcerTest:test_decodedTheTerms() (gas: 14742) -ERC1155BalanceGteEnforcerTest:test_differentiateDelegationHashWithRecipient() (gas: 162584) -ERC1155BalanceGteEnforcerTest:test_invalid_decodedTheTerms() (gas: 17826) -ERC1155BalanceGteEnforcerTest:test_invalid_tokenAddress() (gas: 71821) -ERC1155BalanceGteEnforcerTest:test_notAllow_expectingOverflow() (gas: 93343) -ERC1155BalanceGteEnforcerTest:test_notAllow_ifBalanceDecreases() (gas: 137130) -ERC1155BalanceGteEnforcerTest:test_notAllow_insufficientIncrease() (gas: 108208) -ERC1155BalanceGteEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 170467) -ERC1155BalanceGteEnforcerTest:test_notAllow_withPreExistingBalance() (gas: 137058) -ERC1155BalanceGteEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14435) -ERC20BalanceGteEnforcerTest:test_allow_ifBalanceIncreases() (gas: 149411) -ERC20BalanceGteEnforcerTest:test_allow_reuseDelegationWithDifferentRecipients() (gas: 151842) -ERC20BalanceGteEnforcerTest:test_decodedTheTerms() (gas: 12133) -ERC20BalanceGteEnforcerTest:test_invalid_decodedTheTerms() (gas: 18044) -ERC20BalanceGteEnforcerTest:test_invalid_tokenAddress() (gas: 64875) -ERC20BalanceGteEnforcerTest:test_notAllow_expectingOverflow() (gas: 85510) -ERC20BalanceGteEnforcerTest:test_notAllow_insufficientIncrease() (gas: 110737) -ERC20BalanceGteEnforcerTest:test_notAllow_noIncreaseToRecipient() (gas: 62566) -ERC20BalanceGteEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 170372) -ERC20BalanceGteEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14457) +ERC1155BalanceChangeEnforcerTest:test_allow_ifBalanceIncreases() (gas: 153112) +ERC1155BalanceChangeEnforcerTest:test_allow_withDifferentRecipients() (gas: 159877) +ERC1155BalanceChangeEnforcerTest:test_decodedTheTerms() (gas: 14742) +ERC1155BalanceChangeEnforcerTest:test_differentiateDelegationHashWithRecipient() (gas: 162584) +ERC1155BalanceChangeEnforcerTest:test_invalid_decodedTheTerms() (gas: 17826) +ERC1155BalanceChangeEnforcerTest:test_invalid_tokenAddress() (gas: 71821) +ERC1155BalanceChangeEnforcerTest:test_notAllow_expectingOverflow() (gas: 93343) +ERC1155BalanceChangeEnforcerTest:test_notAllow_ifBalanceDecreases() (gas: 137130) +ERC1155BalanceChangeEnforcerTest:test_notAllow_insufficientIncrease() (gas: 108208) +ERC1155BalanceChangeEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 170467) +ERC1155BalanceChangeEnforcerTest:test_notAllow_withPreExistingBalance() (gas: 137058) +ERC1155BalanceChangeEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14435) +ERC20BalanceChangeEnforcerTest:test_allow_ifBalanceIncreases() (gas: 149411) +ERC20BalanceChangeEnforcerTest:test_allow_reuseDelegationWithDifferentRecipients() (gas: 151842) +ERC20BalanceChangeEnforcerTest:test_decodedTheTerms() (gas: 12133) +ERC20BalanceChangeEnforcerTest:test_invalid_decodedTheTerms() (gas: 18044) +ERC20BalanceChangeEnforcerTest:test_invalid_tokenAddress() (gas: 64875) +ERC20BalanceChangeEnforcerTest:test_notAllow_expectingOverflow() (gas: 85510) +ERC20BalanceChangeEnforcerTest:test_notAllow_insufficientIncrease() (gas: 110737) +ERC20BalanceChangeEnforcerTest:test_notAllow_noIncreaseToRecipient() (gas: 62566) +ERC20BalanceChangeEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 170372) +ERC20BalanceChangeEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14457) ERC20PeriodTransferEnforcerTest:testInvalidContract() (gas: 30433) ERC20PeriodTransferEnforcerTest:testInvalidExecutionLength() (gas: 26549) ERC20PeriodTransferEnforcerTest:testInvalidMethod() (gas: 28747) @@ -269,18 +269,18 @@ ERC20TransferAmountEnforcerTest:test_transferFailsAboveAllowance() (gas: 1824133 ERC20TransferAmountEnforcerTest:test_transferFailsButSpentLimitIncreases() (gas: 1402646) ERC20TransferAmountEnforcerTest:test_transferFailsIfCalledAboveAllowance() (gas: 54035) ERC20TransferAmountEnforcerTest:test_transferSucceedsIfCalledBelowAllowance() (gas: 59241) -ERC721BalanceGteEnforcerTest:test_allow_ifBalanceIncreases() (gas: 189682) -ERC721BalanceGteEnforcerTest:test_allow_withDifferentRecipients() (gas: 196632) -ERC721BalanceGteEnforcerTest:test_decodedTheTerms() (gas: 12161) -ERC721BalanceGteEnforcerTest:test_differentiateDelegationHashWithRecipient() (gas: 191064) -ERC721BalanceGteEnforcerTest:test_invalid_decodedTheTerms() (gas: 17819) -ERC721BalanceGteEnforcerTest:test_invalid_tokenAddress() (gas: 60669) -ERC721BalanceGteEnforcerTest:test_notAllow_expectingOverflow() (gas: 81112) -ERC721BalanceGteEnforcerTest:test_notAllow_ifBalanceDecreases() (gas: 159441) -ERC721BalanceGteEnforcerTest:test_notAllow_insufficientIncrease() (gas: 58214) -ERC721BalanceGteEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 188962) -ERC721BalanceGteEnforcerTest:test_notAllow_withPreExistingBalance() (gas: 148820) -ERC721BalanceGteEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14457) +ERC721BalanceChangeEnforcerTest:test_allow_ifBalanceIncreases() (gas: 189682) +ERC721BalanceChangeEnforcerTest:test_allow_withDifferentRecipients() (gas: 196632) +ERC721BalanceChangeEnforcerTest:test_decodedTheTerms() (gas: 12161) +ERC721BalanceChangeEnforcerTest:test_differentiateDelegationHashWithRecipient() (gas: 191064) +ERC721BalanceChangeEnforcerTest:test_invalid_decodedTheTerms() (gas: 17819) +ERC721BalanceChangeEnforcerTest:test_invalid_tokenAddress() (gas: 60669) +ERC721BalanceChangeEnforcerTest:test_notAllow_expectingOverflow() (gas: 81112) +ERC721BalanceChangeEnforcerTest:test_notAllow_ifBalanceDecreases() (gas: 159441) +ERC721BalanceChangeEnforcerTest:test_notAllow_insufficientIncrease() (gas: 58214) +ERC721BalanceChangeEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 188962) +ERC721BalanceChangeEnforcerTest:test_notAllow_withPreExistingBalance() (gas: 148820) +ERC721BalanceChangeEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14457) ERC721TransferEnforcerTest:test_invalidTermsLength() (gas: 11392) ERC721TransferEnforcerTest:test_revertWithInvalidCallTypeMode() (gas: 14240) ERC721TransferEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14447) @@ -662,13 +662,13 @@ MultiTokenPeriodEnforcerTest:test_TransferAmountExceededErc20() (gas: 157009) MultiTokenPeriodEnforcerTest:test_TransferAmountExceededNative() (gas: 152283) MultiTokenPeriodEnforcerTest:test_TransferNotStartedErc20() (gas: 32351) MultiTokenPeriodEnforcerTest:test_TransferNotStartedNative() (gas: 28924) -NativeBalanceGteEnforcerTest:test_allow_ifBalanceIncreases() (gas: 89780) -NativeBalanceGteEnforcerTest:test_decodedTheTerms() (gas: 9363) -NativeBalanceGteEnforcerTest:test_invalid_decodedTheTerms() (gas: 15187) -NativeBalanceGteEnforcerTest:test_notAllow_expectingOverflow() (gas: 63428) -NativeBalanceGteEnforcerTest:test_notAllow_insufficientIncrease() (gas: 67070) -NativeBalanceGteEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 104252) -NativeBalanceGteEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14293) +NativeBalanceChangeEnforcerTest:test_allow_ifBalanceIncreases() (gas: 89780) +NativeBalanceChangeEnforcerTest:test_decodedTheTerms() (gas: 9363) +NativeBalanceChangeEnforcerTest:test_invalid_decodedTheTerms() (gas: 15187) +NativeBalanceChangeEnforcerTest:test_notAllow_expectingOverflow() (gas: 63428) +NativeBalanceChangeEnforcerTest:test_notAllow_insufficientIncrease() (gas: 67070) +NativeBalanceChangeEnforcerTest:test_notAllow_reenterALockedEnforcer() (gas: 104252) +NativeBalanceChangeEnforcerTest:test_revertWithInvalidExecutionMode() (gas: 14293) NativeTokenPaymentEnforcerTest:test_allowsRedelegationAddingExtraCosts() (gas: 732792) NativeTokenPaymentEnforcerTest:test_chargePaymentFromAllowance() (gas: 1217483) NativeTokenPaymentEnforcerTest:test_decodesTheTerms() (gas: 9248) diff --git a/script/verification/verify-enforcer-contracts.sh b/script/verification/verify-enforcer-contracts.sh index 1fa5e67b..23534a21 100755 --- a/script/verification/verify-enforcer-contracts.sh +++ b/script/verification/verify-enforcer-contracts.sh @@ -28,20 +28,20 @@ ENFORCERS=( "AllowedTargetsEnforcer" "BlockNumberEnforcer" "DeployedEnforcer" - "ERC20BalanceGteEnforcer" + "ERC20BalanceChangeEnforcer" "ERC20TransferAmountEnforcer" "ERC20PeriodTransferEnforcer" "ERC20StreamingEnforcer" - "ERC721BalanceGteEnforcer" + "ERC721BalanceChangeEnforcer" "ERC721TransferEnforcer" - "ERC1155BalanceGteEnforcer" + "ERC1155BalanceChangeEnforcer" "ExactCalldataBatchEnforcer" "ExactCalldataEnforcer" "ExactExecutionBatchEnforcer" "ExactExecutionEnforcer" "IdEnforcer" "LimitedCallsEnforcer" - "NativeBalanceGteEnforcer" + "NativeBalanceChangeEnforcer" "ArgsEqualityCheckEnforcer" "NativeTokenTransferAmountEnforcer" "NativeTokenStreamingEnforcer" diff --git a/src/enforcers/ERC1155BalanceGteEnforcer.sol b/src/enforcers/ERC1155BalanceChangeEnforcer.sol similarity index 58% rename from src/enforcers/ERC1155BalanceGteEnforcer.sol rename to src/enforcers/ERC1155BalanceChangeEnforcer.sol index 0bda03bf..1807126f 100644 --- a/src/enforcers/ERC1155BalanceGteEnforcer.sol +++ b/src/enforcers/ERC1155BalanceChangeEnforcer.sol @@ -7,12 +7,17 @@ import { CaveatEnforcer } from "./CaveatEnforcer.sol"; import { ModeCode } from "../utils/Types.sol"; /** - * @title ERC1155BalanceGteEnforcer + * @title ERC1155BalanceChangeEnforcer * @dev This contract enforces that the ERC1155 token balance of a recipient for a specific token ID - * has increased by at least the specified amount after the execution, measured between the `beforeHook` and `afterHook` calls. + * has changed by at least the specified amount after the execution, measured between the `beforeHook` and `afterHook` calls. + * The change can be either an increase or decrease based on the `shouldBalanceIncrease` flag. * @dev This enforcer operates only in default execution mode. + * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. Since + * enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers simultaneously. + * Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to prevent unintended + * behavior. */ -contract ERC1155BalanceGteEnforcer is CaveatEnforcer { +contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// State ////////////////////////////// mapping(bytes32 hashKey => uint256 balance) public balanceCache; @@ -47,11 +52,12 @@ contract ERC1155BalanceGteEnforcer is CaveatEnforcer { /** * @notice This function caches the recipient's ERC1155 token balance before the delegation is executed. - * @param _terms 104 bytes where: - * - first 20 bytes: address of the ERC1155 token, - * - next 20 bytes: address of the recipient, - * - next 32 bytes: token ID, - * - next 32 bytes: amount the balance should increase by. + * @param _terms 105 bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the ERC1155 token + * - next 20 bytes: address of the recipient + * - next 32 bytes: token ID + * - next 32 bytes: amount the balance should change by * @param _mode The execution mode. (Must be Default execType) * @param _delegationHash The hash of the delegation. */ @@ -68,21 +74,23 @@ contract ERC1155BalanceGteEnforcer is CaveatEnforcer { override onlyDefaultExecutionMode(_mode) { - (address token_, address recipient_, uint256 tokenId_,) = getTermsInfo(_terms); + (, address token_, address recipient_, uint256 tokenId_,) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, recipient_, tokenId_, _delegationHash); - require(!isLocked[hashKey_], "ERC1155BalanceGteEnforcer:enforcer-is-locked"); + require(!isLocked[hashKey_], "ERC1155BalanceChangeEnforcer:enforcer-is-locked"); isLocked[hashKey_] = true; uint256 balance_ = IERC1155(token_).balanceOf(recipient_, tokenId_); balanceCache[hashKey_] = balance_; } /** - * @notice This function enforces that the recipient's ERC1155 token balance has increased by at least the amount provided. - * @param _terms 104 bytes where: - * - first 20 bytes: address of the ERC1155 token, - * - next 20 bytes: address of the recipient, - * - next 32 bytes: token ID, - * - next 32 bytes: amount the balance should increase by. + * @notice This function enforces that the recipient's ERC1155 token balance has changed by at least the amount provided. + * @param _terms 105 bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the ERC1155 token + * - next 20 bytes: address of the recipient + * - next 32 bytes: token ID + * - next 32 bytes: amount the balance should change by + * @param _delegationHash The hash of the delegation. */ function afterHook( bytes calldata _terms, @@ -96,31 +104,37 @@ contract ERC1155BalanceGteEnforcer is CaveatEnforcer { public override { - (address token_, address recipient_, uint256 tokenId_, uint256 amount_) = getTermsInfo(_terms); + (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, recipient_, tokenId_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC1155(token_).balanceOf(recipient_, tokenId_); - require(balance_ >= balanceCache[hashKey_] + amount_, "ERC1155BalanceGteEnforcer:balance-not-gt"); + if (shouldBalanceIncrease_) { + require(balance_ >= balanceCache[hashKey_] + amount_, "ERC1155BalanceChangeEnforcer:insufficient-balance-increase"); + } else { + require(balance_ >= balanceCache[hashKey_] - amount_, "ERC1155BalanceChangeEnforcer:exceeded-balance-decrease"); + } } /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms Encoded data that is used during the execution hooks. + * @return shouldBalanceIncrease_ Boolean indicating if the balance should increase (true) or decrease (false). * @return token_ The address of the ERC1155 token. * @return recipient_ The address of the recipient of the token. * @return tokenId_ The ID of the ERC1155 token. - * @return amount_ The amount the balance should increase by. + * @return amount_ The amount the balance should change by. */ function getTermsInfo(bytes calldata _terms) public pure - returns (address token_, address recipient_, uint256 tokenId_, uint256 amount_) + returns (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) { - require(_terms.length == 104, "ERC1155BalanceGteEnforcer:invalid-terms-length"); - token_ = address(bytes20(_terms[:20])); - recipient_ = address(bytes20(_terms[20:40])); - tokenId_ = uint256(bytes32(_terms[40:72])); - amount_ = uint256(bytes32(_terms[72:])); + require(_terms.length == 105, "ERC1155BalanceChangeEnforcer:invalid-terms-length"); + shouldBalanceIncrease_ = _terms[0] != 0; + token_ = address(bytes20(_terms[1:21])); + recipient_ = address(bytes20(_terms[21:41])); + tokenId_ = uint256(bytes32(_terms[41:73])); + amount_ = uint256(bytes32(_terms[73:])); } ////////////////////////////// Internal Methods ////////////////////////////// diff --git a/src/enforcers/ERC20BalanceGteEnforcer.sol b/src/enforcers/ERC20BalanceChangeEnforcer.sol similarity index 55% rename from src/enforcers/ERC20BalanceGteEnforcer.sol rename to src/enforcers/ERC20BalanceChangeEnforcer.sol index f56bb171..d407791f 100644 --- a/src/enforcers/ERC20BalanceGteEnforcer.sol +++ b/src/enforcers/ERC20BalanceChangeEnforcer.sol @@ -7,15 +7,19 @@ import { CaveatEnforcer } from "./CaveatEnforcer.sol"; import { ModeCode } from "../utils/Types.sol"; /** - * @title ERC20BalanceGteEnforcer - * @dev This contract enforces that the delegator's ERC20 balance has increased by at least the specified amount + * @title ERC20BalanceChangeEnforcer + * @dev This contract enforces that the delegator's ERC20 balance has changed by at least the specified amount * after the execution has been executed, measured between the `beforeHook` and `afterHook` calls, regardless of what the execution - * is. - * @dev This contract has no enforcement of how the balance increases. It's meant to be used alongside additional enforcers to + * is. The change can be either an increase or decrease based on the `shouldBalanceIncrease` flag. + * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to * create granular permissions. * @dev This enforcer operates only in default execution mode. + * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. Since + * enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers simultaneously. + * Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to prevent unintended + * behavior. */ -contract ERC20BalanceGteEnforcer is CaveatEnforcer { +contract ERC20BalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// State ////////////////////////////// mapping(bytes32 hashKey => uint256 balance) public balanceCache; @@ -38,8 +42,11 @@ contract ERC20BalanceGteEnforcer is CaveatEnforcer { /** * @notice This function caches the delegators ERC20 balance before the delegation is executed. - * @param _terms 72 packed bytes where: the first 20 bytes is the address of the recipient, the next 20 bytes - * is the address of the token, the next 32 bytes is the amount of tokens the balance should be greater than or equal to + * @param _terms 73 packed bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the token + * - next 20 bytes: address of the recipient + * - next 32 bytes: amount the balance should change by * @param _mode The execution mode. (Must be Default execType) */ function beforeHook( @@ -55,18 +62,21 @@ contract ERC20BalanceGteEnforcer is CaveatEnforcer { override onlyDefaultExecutionMode(_mode) { - (address recipient_, address token_,) = getTermsInfo(_terms); + (, address token_, address recipient_,) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, _delegationHash); - require(!isLocked[hashKey_], "ERC20BalanceGteEnforcer:enforcer-is-locked"); + require(!isLocked[hashKey_], "ERC20BalanceChangeEnforcer:enforcer-is-locked"); isLocked[hashKey_] = true; uint256 balance_ = IERC20(token_).balanceOf(recipient_); balanceCache[hashKey_] = balance_; } /** - * @notice This function enforces that the delegators ERC20 balance has increased by at least the amount provided. - * @param _terms 72 packed bytes where: the first 20 bytes is the address of the recipient, the next 20 bytes - * is the address of the token, the next 32 bytes is the amount the balance should be greater than + * @notice This function enforces that the delegators ERC20 balance has changed by at least the amount provided. + * @param _terms 73 packed bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the token + * - next 20 bytes: address of the recipient + * - next 32 bytes: amount the balance should change by */ function afterHook( bytes calldata _terms, @@ -80,25 +90,35 @@ contract ERC20BalanceGteEnforcer is CaveatEnforcer { public override { - (address recipient_, address token_, uint256 amount_) = getTermsInfo(_terms); + (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC20(token_).balanceOf(recipient_); - require(balance_ >= balanceCache[hashKey_] + amount_, "ERC20BalanceGteEnforcer:balance-not-gt"); + if (shouldBalanceIncrease_) { + require(balance_ >= balanceCache[hashKey_] + amount_, "ERC20BalanceChangeEnforcer:insufficient-balance-increase"); + } else { + require(balance_ >= balanceCache[hashKey_] - amount_, "ERC20BalanceChangeEnforcer:exceeded-balance-decrease"); + } } /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms encoded data that is used during the execution hooks. - * @return recipient_ The address of the recipient. + * @return shouldBalanceIncrease_ Boolean indicating if the balance should increase (true) or decrease (false). * @return token_ The address of the token. - * @return amount_ The amount the balance should be greater than. + * @return recipient_ The address of the recipient. + * @return amount_ The amount the balance should change by. */ - function getTermsInfo(bytes calldata _terms) public pure returns (address recipient_, address token_, uint256 amount_) { - require(_terms.length == 72, "ERC20BalanceGteEnforcer:invalid-terms-length"); - recipient_ = address(bytes20(_terms[:20])); - token_ = address(bytes20(_terms[20:40])); - amount_ = uint256(bytes32(_terms[40:])); + function getTermsInfo(bytes calldata _terms) + public + pure + returns (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) + { + require(_terms.length == 73, "ERC20BalanceChangeEnforcer:invalid-terms-length"); + shouldBalanceIncrease_ = _terms[0] != 0; + token_ = address(bytes20(_terms[1:21])); + recipient_ = address(bytes20(_terms[21:41])); + amount_ = uint256(bytes32(_terms[41:])); } ////////////////////////////// Internal Methods ////////////////////////////// diff --git a/src/enforcers/ERC721BalanceGteEnforcer.sol b/src/enforcers/ERC721BalanceChangeEnforcer.sol similarity index 58% rename from src/enforcers/ERC721BalanceGteEnforcer.sol rename to src/enforcers/ERC721BalanceChangeEnforcer.sol index 8471e09c..3d943d48 100644 --- a/src/enforcers/ERC721BalanceGteEnforcer.sol +++ b/src/enforcers/ERC721BalanceChangeEnforcer.sol @@ -7,12 +7,17 @@ import { CaveatEnforcer } from "./CaveatEnforcer.sol"; import { ModeCode } from "../utils/Types.sol"; /** - * @title ERC721BalanceGteEnforcer - * @dev This contract enforces that the ERC721 token balance of a recipient has increased by at least the specified amount + * @title ERC721BalanceChangeEnforcer + * @dev This contract enforces that the ERC721 token balance of a recipient has changed by at least the specified amount * after the execution, measured between the `beforeHook` and `afterHook` calls, regardless of what the execution is. + * The change can be either an increase or decrease based on the `shouldBalanceIncrease` flag. * @dev This enforcer operates only in default execution mode. + * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. + * Since enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers + * simultaneously. Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to + * prevent unintended behavior. */ -contract ERC721BalanceGteEnforcer is CaveatEnforcer { +contract ERC721BalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// State ////////////////////////////// mapping(bytes32 hashKey => uint256 balance) public balanceCache; @@ -45,10 +50,11 @@ contract ERC721BalanceGteEnforcer is CaveatEnforcer { /** * @notice This function caches the delegator's ERC721 token balance before the delegation is executed. - * @param _terms 72 bytes where: - * - first 20 bytes: address of the ERC721 token, - * - next 20 bytes: address of the recipient, - * - next 32 bytes: amount the balance should increase by. + * @param _terms 73 bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the ERC721 token + * - next 20 bytes: address of the recipient + * - next 32 bytes: amount the balance should change by * @param _mode The execution mode. (Must be Default execType) * @param _delegationHash The hash of the delegation. */ @@ -65,20 +71,22 @@ contract ERC721BalanceGteEnforcer is CaveatEnforcer { override onlyDefaultExecutionMode(_mode) { - (address token_, address recipient_,) = getTermsInfo(_terms); + (, address token_, address recipient_,) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, recipient_, _delegationHash); - require(!isLocked[hashKey_], "ERC721BalanceGteEnforcer:enforcer-is-locked"); + require(!isLocked[hashKey_], "ERC721BalanceChangeEnforcer:enforcer-is-locked"); isLocked[hashKey_] = true; uint256 balance_ = IERC721(token_).balanceOf(recipient_); balanceCache[hashKey_] = balance_; } /** - * @notice This function enforces that the delegator's ERC721 token balance has increased by at least the amount provided. - * @param _terms 72 bytes where: - * - first 20 bytes: address of the ERC721 token, - * - next 20 bytes: address of the recipient, - * - next 32 bytes: amount the balance should increase by. + * @notice This function enforces that the delegator's ERC721 token balance has changed by at least the amount provided. + * @param _terms 73 bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the ERC721 token + * - next 20 bytes: address of the recipient + * - next 32 bytes: amount the balance should change by + * @param _delegationHash The hash of the delegation. */ function afterHook( bytes calldata _terms, @@ -92,25 +100,35 @@ contract ERC721BalanceGteEnforcer is CaveatEnforcer { public override { - (address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); + (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, recipient_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC721(token_).balanceOf(recipient_); - require(balance_ >= balanceCache[hashKey_] + amount_, "ERC721BalanceGteEnforcer:balance-not-gt"); + if (shouldBalanceIncrease_) { + require(balance_ >= balanceCache[hashKey_] + amount_, "ERC721BalanceChangeEnforcer:insufficient-balance-increase"); + } else { + require(balance_ >= balanceCache[hashKey_] - amount_, "ERC721BalanceChangeEnforcer:exceeded-balance-decrease"); + } } /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms Encoded data that is used during the execution hooks. + * @return shouldBalanceIncrease_ Boolean indicating if the balance should increase (true) or decrease (false). * @return token_ The address of the ERC721 token. * @return recipient_ The address of the recipient of the token. - * @return amount_ The amount the balance should increase by. + * @return amount_ The amount the balance should change by. */ - function getTermsInfo(bytes calldata _terms) public pure returns (address token_, address recipient_, uint256 amount_) { - require(_terms.length == 72, "ERC721BalanceGteEnforcer:invalid-terms-length"); - token_ = address(bytes20(_terms[:20])); - recipient_ = address(bytes20(_terms[20:40])); - amount_ = uint256(bytes32(_terms[40:])); + function getTermsInfo(bytes calldata _terms) + public + pure + returns (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) + { + require(_terms.length == 73, "ERC721BalanceChangeEnforcer:invalid-terms-length"); + shouldBalanceIncrease_ = _terms[0] != 0; + token_ = address(bytes20(_terms[1:21])); + recipient_ = address(bytes20(_terms[21:41])); + amount_ = uint256(bytes32(_terms[41:])); } ////////////////////////////// Internal Methods ////////////////////////////// diff --git a/src/enforcers/NativeBalanceGteEnforcer.sol b/src/enforcers/NativeBalanceChangeEnforcer.sol similarity index 51% rename from src/enforcers/NativeBalanceGteEnforcer.sol rename to src/enforcers/NativeBalanceChangeEnforcer.sol index cd60001c..6e77b5f3 100644 --- a/src/enforcers/NativeBalanceGteEnforcer.sol +++ b/src/enforcers/NativeBalanceChangeEnforcer.sol @@ -5,15 +5,19 @@ import { CaveatEnforcer } from "./CaveatEnforcer.sol"; import { ModeCode } from "../utils/Types.sol"; /** - * @title NativeBalanceGteEnforcer - * @dev This contract enforces that a recipient's native token balance has increased by at least the specified amount + * @title NativeBalanceChangeEnforcer + * @dev This contract enforces that a recipient's native token balance has changed by at least the specified amount * after the execution has been executed, measured between the `beforeHook` and `afterHook` calls, regardless of what the execution - * is. - * @dev This contract does not enforce how the balance increases. It is meant to be used with additional enforcers to create + * is. The change can be either an increase or decrease based on the `shouldBalanceIncrease` flag. + * @dev This contract does not enforce how the balance changes. It is meant to be used with additional enforcers to create * granular permissions. * @dev This enforcer operates only in default execution mode. + * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. + * Since enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers + * simultaneously. Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to + * prevent unintended behavior. */ -contract NativeBalanceGteEnforcer is CaveatEnforcer { +contract NativeBalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// State ////////////////////////////// mapping(bytes32 hashKey => uint256 balance) public balanceCache; @@ -35,8 +39,10 @@ contract NativeBalanceGteEnforcer is CaveatEnforcer { /** * @notice Caches the recipient's native token balance before the delegation is executed. - * @param _terms 52 packed bytes where the first 20 bytes are the recipient's address, and the next 32 bytes - * are the minimum balance increase required. + * @param _terms 53 packed bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the recipient + * - next 32 bytes: amount the balance should change by * @param _delegationHash The hash of the delegation being operated on. * @param _mode The execution mode. (Must be Default execType) */ @@ -54,17 +60,19 @@ contract NativeBalanceGteEnforcer is CaveatEnforcer { onlyDefaultExecutionMode(_mode) { bytes32 hashKey_ = _getHashKey(msg.sender, _delegationHash); - (address recipient_,) = getTermsInfo(_terms); + (, address recipient_,) = getTermsInfo(_terms); - require(!isLocked[hashKey_], "NativeBalanceGteEnforcer:enforcer-is-locked"); + require(!isLocked[hashKey_], "NativeBalanceChangeEnforcer:enforcer-is-locked"); isLocked[hashKey_] = true; balanceCache[hashKey_] = recipient_.balance; } /** - * @notice Ensures that the recipient's native token balance has increased by at least the specified amount. - * @param _terms 52 packed bytes where the first 20 bytes are the recipient's address, and the next 32 bytes - * are the minimum balance increase required. + * @notice Ensures that the recipient's native token balance has changed by at least the specified amount. + * @param _terms 53 packed bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the recipient + * - next 32 bytes: amount the balance should change by * @param _delegationHash The hash of the delegation being operated on. */ function afterHook( @@ -79,23 +87,37 @@ contract NativeBalanceGteEnforcer is CaveatEnforcer { public override { - (address recipient_, uint256 amount_) = getTermsInfo(_terms); + (bool shouldBalanceIncrease_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, _delegationHash); delete isLocked[hashKey_]; - require(recipient_.balance >= balanceCache[hashKey_] + amount_, "NativeBalanceGteEnforcer:balance-not-gt"); + if (shouldBalanceIncrease_) { + require( + recipient_.balance >= balanceCache[hashKey_] + amount_, "NativeBalanceChangeEnforcer:insufficient-balance-increase" + ); + } else { + require(recipient_.balance >= balanceCache[hashKey_] - amount_, "NativeBalanceChangeEnforcer:exceeded-balance-decrease"); + } } /** * @notice Decodes the terms used in this CaveatEnforcer. - * @param _terms 52 packed bytes where the first 20 bytes are the recipient's address, and the next 32 bytes - * specify the minimum balance increase required. - * @return recipient_ The address of the recipient who will receive the tokens. - * @return amount_ requiredIncrease_ The minimum balance increase required. + * @param _terms 53 packed bytes where: + * - first byte: boolean indicating if the balance should increase + * - next 20 bytes: address of the recipient + * - next 32 bytes: amount the balance should change by + * @return shouldBalanceIncrease_ Boolean indicating if the balance should increase (true) or decrease (false). + * @return recipient_ The address of the recipient whose balance will change. + * @return amount_ The minimum balance change required. */ - function getTermsInfo(bytes calldata _terms) public pure returns (address recipient_, uint256 amount_) { - require(_terms.length == 52, "NativeBalanceGteEnforcer:invalid-terms-length"); - recipient_ = address(bytes20(_terms[:20])); - amount_ = uint256(bytes32(_terms[20:])); + function getTermsInfo(bytes calldata _terms) + public + pure + returns (bool shouldBalanceIncrease_, address recipient_, uint256 amount_) + { + require(_terms.length == 53, "NativeBalanceChangeEnforcer:invalid-terms-length"); + shouldBalanceIncrease_ = _terms[0] != 0; + recipient_ = address(bytes20(_terms[1:21])); + amount_ = uint256(bytes32(_terms[21:])); } ////////////////////////////// Internal Methods ////////////////////////////// diff --git a/test/enforcers/ERC1155BalanceGteEnforcer.t.sol b/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol similarity index 52% rename from test/enforcers/ERC1155BalanceGteEnforcer.t.sol rename to test/enforcers/ERC1155BalanceChangeEnforcer.t.sol index f520355f..18339acf 100644 --- a/test/enforcers/ERC1155BalanceGteEnforcer.t.sol +++ b/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol @@ -6,12 +6,12 @@ import "../../src/utils/Types.sol"; import { BasicERC1155 } from "../utils/BasicERC1155.t.sol"; import { Execution } from "../../src/utils/Types.sol"; import { CaveatEnforcerBaseTest } from "./CaveatEnforcerBaseTest.t.sol"; -import { ERC1155BalanceGteEnforcer } from "../../src/enforcers/ERC1155BalanceGteEnforcer.sol"; +import { ERC1155BalanceChangeEnforcer } from "../../src/enforcers/ERC1155BalanceChangeEnforcer.sol"; import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol"; -contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { +contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { ////////////////////////////// State ////////////////////////////// - ERC1155BalanceGteEnforcer public enforcer; + ERC1155BalanceChangeEnforcer public enforcer; BasicERC1155 public token; address delegator; address delegate; @@ -22,18 +22,17 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { uint256 public tokenId = 1; ////////////////////// Set up ////////////////////// - function setUp() public override { super.setUp(); delegator = address(users.alice.deleGator); delegate = address(users.bob.deleGator); dm = address(delegationManager); - enforcer = new ERC1155BalanceGteEnforcer(); - vm.label(address(enforcer), "ERC1155 BalanceGte Enforcer"); + enforcer = new ERC1155BalanceChangeEnforcer(); + vm.label(address(enforcer), "ERC1155 Balance Change Enforcer"); token = new BasicERC1155(delegator, "ERC1155Token", "ERC1155Token", ""); vm.label(address(token), "ERC1155 Test Token"); - // Prepare the Execution data for minting + // Prepare the Execution data for minting. mintExecution = Execution({ target: address(token), value: 0, @@ -44,22 +43,25 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { ////////////////////// Basic Functionality ////////////////////// - // Validates the terms get decoded correctly + // Validates the terms get decoded correctly. + // Terms format: [bool shouldBalanceIncrease, address token, address recipient, uint256 tokenId, uint256 amount] function test_decodedTheTerms() public { - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), uint256(100)); - (address token_, address recipient_, uint256 tokenId_, uint256 amount_) = enforcer.getTermsInfo(terms_); + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); + (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = + enforcer.getTermsInfo(terms_); + assertTrue(shouldBalanceIncrease_); assertEq(token_, address(token)); assertEq(recipient_, delegator); assertEq(tokenId_, tokenId); assertEq(amount_, 100); } - // Validates that a balance has increased at least by the expected amount + // Validates that a balance has increased at least by the expected amount. function test_allow_ifBalanceIncreases() public { - // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), uint256(100)); + // Expect increase by at least 100. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); - // Increase by 100 + // Increase by 100. vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); vm.prank(delegator); @@ -67,7 +69,7 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { vm.prank(dm); enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); - // Increase by 1000 + // Increase by 1000. vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); vm.prank(delegator); @@ -78,43 +80,43 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { ////////////////////// Errors ////////////////////// - // Reverts if a balance hasn't increased by the set amount + // Reverts if a balance hasn't increased by the set amount. function test_notAllow_insufficientIncrease() public { - // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), uint256(100)); + // Expect increase by at least 100. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); - // Increase by 10, expect revert + // Increase by 10 only, expect revert. vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); vm.prank(delegator); token.mint(delegator, tokenId, 10, ""); vm.prank(dm); - vm.expectRevert(bytes("ERC1155BalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:insufficient-balance-increase")); enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // Reverts if a balance descreased in between the hooks + // Reverts if a balance decreases in between the hooks. function test_notAllow_ifBalanceDecreases() public { - // Starting with 10 tokens + // Starting with 10 tokens. vm.prank(delegator); token.mint(delegator, tokenId, 10, ""); - // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), uint256(100)); + // Expect increase by at least 100. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); - // Decrease balance by transferring tokens away + // Decrease balance by transferring tokens away. vm.prank(delegator); token.safeTransferFrom(delegator, address(1), tokenId, 10, ""); vm.prank(dm); - vm.expectRevert(bytes("ERC1155BalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:insufficient-balance-increase")); enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // Allows to check the balance of different recipients + // Allows checking the balance of different recipients. function test_allow_withDifferentRecipients() public { address[] memory recipients_ = new address[](2); recipients_[0] = delegator; @@ -122,9 +124,9 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { for (uint256 i = 0; i < recipients_.length; i++) { address currentRecipient_ = recipients_[i]; - bytes memory terms_ = abi.encodePacked(address(token), currentRecipient_, uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(true, address(token), currentRecipient_, uint256(tokenId), uint256(100)); - // Increase by 100 for each recipient + // Increase by 100 for each recipient. vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(i), address(0), delegate); vm.prank(delegator); @@ -134,34 +136,34 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { } } - // Considers any pre existing balances in the recipient + // Considers any pre-existing balances in the recipient. function test_notAllow_withPreExistingBalance() public { - // Recipient already has 50 tokens + // Recipient already has 50 tokens. vm.prank(delegator); token.mint(delegator, tokenId, 50, ""); - // Expect balance to increase by at least 100 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), uint256(100)); + // Expect balance to increase by at least 100. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); - // Increase balance by 100 + // Increase balance by 50 only. vm.prank(delegator); token.mint(delegator, tokenId, 50, ""); vm.prank(dm); - vm.expectRevert(bytes("ERC1155BalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:insufficient-balance-increase")); enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // Same delegation hash multiple recipients + // Differentiates delegation hash with different recipients. function test_differentiateDelegationHashWithRecipient() public { - bytes32 delegationHash_ = bytes32(bytes32(uint256(99999999))); + bytes32 delegationHash_ = bytes32(uint256(99999999)); address recipient2_ = address(1111111); - // Expect balance to increase by at least 100 in different recipients - bytes memory terms1_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), uint256(100)); - bytes memory terms2_ = abi.encodePacked(address(token), address(recipient2_), uint256(tokenId), uint256(100)); + // Terms for two different recipients. + bytes memory terms1_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms2_ = abi.encodePacked(true, address(token), recipient2_, uint256(tokenId), uint256(100)); vm.prank(dm); enforcer.beforeHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); @@ -169,41 +171,41 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { vm.prank(dm); enforcer.beforeHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); - // Increase balance by 100 only in recipient1 + // Increase balance by 100 only for recipient1. vm.prank(delegator); token.mint(delegator, tokenId, 100, ""); - // This one works well recipient1 increased + // Recipient1 passes. vm.prank(dm); enforcer.afterHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); - // This one fails recipient1 didn't increase + // Recipient2 did not receive tokens, so it should revert. vm.prank(dm); - vm.expectRevert(bytes("ERC1155BalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:insufficient-balance-increase")); enforcer.afterHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); - // Increase balance by 100 only in recipient2 to fix it + // Increase balance for recipient2. vm.prank(delegator); token.mint(recipient2_, tokenId, 100, ""); - // Recipient2 works well + // Recipient2 now passes. vm.prank(dm); enforcer.afterHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); } - // Reverts if the enforcer is locked + // Reverts if the enforcer is locked (i.e. if beforeHook is reentered). function test_notAllow_reenterALockedEnforcer() public { - // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), uint256(100)); + // Expect increase by at least 100. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); bytes32 delegationHash_ = bytes32(uint256(99999999)); - // Lock the enforcer vm.startPrank(dm); + // Lock the enforcer. enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); bytes32 hashKey_ = enforcer.getHashKey(address(delegationManager), address(token), address(delegator), tokenId, delegationHash_); assertTrue(enforcer.isLocked(hashKey_)); - vm.expectRevert(bytes("ERC1155BalanceGteEnforcer:enforcer-is-locked")); + vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:enforcer-is-locked")); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); vm.stopPrank(); @@ -211,44 +213,91 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { token.mint(delegator, tokenId, 1000, ""); vm.startPrank(dm); - // Unlock the enforcer + // Unlock the enforcer. enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); assertFalse(enforcer.isLocked(hashKey_)); - // Can be used again, and locks it again + // Reuse the enforcer, which locks it again. enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); assertTrue(enforcer.isLocked(hashKey_)); vm.stopPrank(); } - // Validates the terms are well-formed + ////////////////////// Decrease Tests ////////////////////// + + // Validates that a balance decrease within the allowed range passes. + // For decrease scenarios (flag = false), the final balance must be at least the cached balance minus the allowed decrease. + function test_allow_ifBalanceDoesNotDecreaseTooMuch() public { + // Mint 100 tokens to delegator. + vm.prank(delegator); + token.mint(delegator, tokenId, 100, ""); + // Confirm initial balance. + uint256 initialBalance_ = token.balanceOf(delegator, tokenId); + assertEq(initialBalance_, 100); + + // Set terms with flag = false (decrease expected), allowed decrease is 20. + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(20)); + + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); + + // Remove 10 tokens: final balance becomes 90, which is >= 100 - 20 = 80. + vm.prank(delegator); + token.safeTransferFrom(delegator, address(2), tokenId, 10, ""); + + vm.prank(dm); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); + } + + // Reverts if the balance decreases too much (i.e. final balance falls below cached balance - allowed amount). + function test_notAllow_excessiveDecrease() public { + // Mint 100 tokens to delegator. + vm.prank(delegator); + token.mint(delegator, tokenId, 100, ""); + uint256 initialBalance_ = token.balanceOf(delegator, tokenId); + assertEq(initialBalance_, 100); + + // Set terms with flag = false (decrease expected), allowed decrease is 20. + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(20)); + + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); + + // Remove 30 tokens: final balance becomes 70, which is below 100 - 20 = 80. + vm.prank(delegator); + token.safeTransferFrom(delegator, address(3), tokenId, 30, ""); + + vm.prank(dm); + vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:exceeded-balance-decrease")); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); + } + + ////////////////////// Invalid Terms Tests ////////////////////// + + // Validates that the terms are well-formed (exactly 105 bytes). function test_invalid_decodedTheTerms() public { bytes memory terms_; - // Too small - terms_ = abi.encodePacked(address(token), address(delegator), uint8(100)); - vm.expectRevert(bytes("ERC1155BalanceGteEnforcer:invalid-terms-length")); + // Too small: missing required bytes (no boolean flag, etc.). + terms_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), uint256(100)); + vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); - // Too large - terms_ = abi.encodePacked(uint256(100), uint256(100), uint256(100), uint256(100)); - vm.expectRevert(bytes("ERC1155BalanceGteEnforcer:invalid-terms-length")); + // Too large: extra bytes appended. + terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100), uint256(1)); + vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); } - // Validates that an invalid token address causes a revert + // Validates that an invalid token address causes a revert. function test_invalid_tokenAddress() public { - bytes memory terms_; - - // Invalid token address - terms_ = abi.encodePacked(address(0), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(true, address(0), address(delegator), uint256(tokenId), uint256(100)); vm.expectRevert(); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // Validates that an invalid amount causes a revert + // Reverts if an unrealistic amount triggers overflow. function test_notAllow_expectingOverflow() public { - // Expect balance to increase by max uint256, which is unrealistic - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(tokenId), type(uint256).max); + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), type(uint256).max); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -256,7 +305,7 @@ contract ERC1155BalanceGteEnforcerTest is CaveatEnforcerBaseTest { enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // should fail with invalid call type mode (try instead of default) + // Fails with an invalid execution mode (non-default). function test_revertWithInvalidExecutionMode() public { vm.prank(address(delegationManager)); vm.expectRevert("CaveatEnforcer:invalid-execution-type"); diff --git a/test/enforcers/ERC20BalanceChangeEnforcer.t.sol b/test/enforcers/ERC20BalanceChangeEnforcer.t.sol new file mode 100644 index 00000000..5d8a8e3a --- /dev/null +++ b/test/enforcers/ERC20BalanceChangeEnforcer.t.sol @@ -0,0 +1,246 @@ +// SPDX-License-Identifier: MIT AND Apache-2.0 +pragma solidity 0.8.23; + +import "forge-std/Test.sol"; +import { BasicERC20 } from "../utils/BasicERC20.t.sol"; + +import "../../src/utils/Types.sol"; +import { Execution } from "../../src/utils/Types.sol"; +import { CaveatEnforcerBaseTest } from "./CaveatEnforcerBaseTest.t.sol"; +import { ERC20BalanceChangeEnforcer } from "../../src/enforcers/ERC20BalanceChangeEnforcer.sol"; +import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol"; + +contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { + ////////////////////////////// State ////////////////////////////// + ERC20BalanceChangeEnforcer public enforcer; + BasicERC20 public token; + address delegator; + address delegate; + address recipient; + address dm; + Execution mintExecution; + bytes mintExecutionCallData; + + ////////////////////////////// Set up ////////////////////////////// + + function setUp() public override { + super.setUp(); + delegator = address(users.alice.deleGator); + delegate = address(users.bob.deleGator); + recipient = address(users.carol.deleGator); + dm = address(delegationManager); + enforcer = new ERC20BalanceChangeEnforcer(); + vm.label(address(enforcer), "ERC20 Balance Change Enforcer"); + token = new BasicERC20(delegator, "TEST", "TEST", 0); + vm.label(address(token), "ERC20 Test Token"); + mintExecution = + Execution({ target: address(token), value: 0, callData: abi.encodeWithSelector(token.mint.selector, delegator, 100) }); + mintExecutionCallData = abi.encode(mintExecution); + } + + ////////////////////////////// Basic Functionality ////////////////////////////// + + // Validates the terms get decoded correctly for an increase scenario + function test_decodedTheTerms() public { + bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); + assertTrue(shouldBalanceIncrease_); + assertEq(token_, address(token)); + assertEq(recipient_, address(recipient)); + assertEq(amount_, 100); + } + + // Validates that a balance has increased at least the expected amount + function test_allow_ifBalanceIncreases() public { + // Terms: [flag=true, token, recipient, amount=100] + bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + + // Increase by 100 + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + vm.prank(delegator); + token.mint(recipient, 100); + vm.prank(dm); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + + // Increase by 1000 + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + vm.prank(delegator); + token.mint(recipient, 1000); + vm.prank(dm); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + } + + // Validates that a delegation can be reused with different recipients (for increase) without interference + function test_allow_reuseDelegationWithDifferentRecipients() public { + // Terms for two different recipients (flag=true indicates increase expected) + bytes memory terms1_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + bytes memory terms2_ = abi.encodePacked(true, address(token), address(delegator), uint256(100)); + + // Increase for recipient + vm.prank(dm); + enforcer.beforeHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + vm.prank(delegator); + token.mint(recipient, 100); + vm.prank(dm); + enforcer.afterHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + + // Increase for delegator as recipient + vm.prank(dm); + enforcer.beforeHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + vm.prank(delegator); + token.mint(delegator, 100); + vm.prank(dm); + enforcer.afterHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + } + + // Validates that a balance decrease within the allowed range passes. + // For decreases (flag = false), the enforcer now checks that the final balance is not below the cached balance minus the + // allowed amount. + // Example: if the cached balance is 100 and the allowed decrease is 10, the final balance must be at least 90. + function test_allow_ifBalanceDoesNotDecreaseTooMuch() public { + // Set an initial balance for the recipient. + uint256 initialBalance_ = 100; + vm.prank(delegator); + token.mint(recipient, initialBalance_); + + // Terms: flag=false (decrease expected), token, recipient, allowed decrease amount = 10. + bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(10)); + + // Cache the initial balance via beforeHook. + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + + // Simulate a decrease by transferring out 5 tokens (final balance becomes 95, which is >= 100 - 10) + vm.prank(recipient); + token.transfer(delegator, 5); + + // afterHook should pass since 95 >= 90. + vm.prank(dm); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + } + + // New Test: Reverts if the balance decreases too much (i.e. final balance falls below cached balance - allowed amount) + function test_notAllow_excessiveDecrease() public { + uint256 initialBalance_ = 100; + vm.prank(delegator); + token.mint(recipient, initialBalance_); + + // Terms: flag=false (decrease expected), token, recipient, allowed maximum decrease = 10. + bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(10)); + + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + + // Simulate an excessive decrease: transfer out 20 tokens (final balance becomes 80, which is below 100 - 10). + vm.prank(recipient); + token.transfer(delegator, 20); + + vm.prank(dm); + vm.expectRevert(bytes("ERC20BalanceChangeEnforcer:exceeded-balance-decrease")); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + } + + ////////////////////////////// Errors ////////////////////////////// + + // Reverts if an increase hasn't been sufficient + function test_notAllow_insufficientIncrease() public { + // Terms: flag=true (increase expected), required increase of 100 tokens. + bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + + // Mint only 10 tokens (insufficient increase) + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + vm.prank(delegator); + token.mint(recipient, 10); + vm.prank(dm); + vm.expectRevert(bytes("ERC20BalanceChangeEnforcer:insufficient-balance-increase")); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + } + + // Reverts if the enforcer is locked (i.e. reentrant beforeHook) + function test_notAllow_reenterALockedEnforcer() public { + bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + bytes32 delegationHash_ = bytes32(uint256(99999999)); + + vm.startPrank(dm); + // First call locks the enforcer. + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, delegator, delegate); + bytes32 hashKey_ = enforcer.getHashKey(address(delegationManager), address(token), delegationHash_); + assertTrue(enforcer.isLocked(hashKey_)); + vm.expectRevert(bytes("ERC20BalanceChangeEnforcer:enforcer-is-locked")); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, delegator, delegate); + vm.stopPrank(); + + vm.startPrank(delegator); + token.mint(recipient, 1000); + vm.stopPrank(); + + vm.startPrank(dm); + // AfterHook unlocks the enforcer. + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, delegator, delegate); + assertFalse(enforcer.isLocked(hashKey_)); + // Can be used again, and the lock is reengaged. + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, delegator, delegate); + assertTrue(enforcer.isLocked(hashKey_)); + vm.stopPrank(); + } + + // Reverts if no increase happens when one is expected + function test_notAllow_noIncreaseToRecipient() public { + bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + + // Cache the initial balance. + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + + // Do not modify recipient's balance. + vm.prank(dm); + vm.expectRevert(bytes("ERC20BalanceChangeEnforcer:insufficient-balance-increase")); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + } + + // Validates that the terms are well formed (exactly 73 bytes) + function test_invalid_decodedTheTerms() public { + bytes memory terms_; + + // Too small: missing required bytes (should be 73 bytes) + terms_ = abi.encodePacked(true, address(token), address(recipient), uint8(100)); + vm.expectRevert(bytes("ERC20BalanceChangeEnforcer:invalid-terms-length")); + enforcer.getTermsInfo(terms_); + + // Too large: extra bytes beyond 73. + terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100), uint256(100)); + vm.expectRevert(bytes("ERC20BalanceChangeEnforcer:invalid-terms-length")); + enforcer.getTermsInfo(terms_); + } + + // Validates that an invalid token address (address(0)) reverts when calling beforeHook. + function test_invalid_tokenAddress() public { + bytes memory terms_ = abi.encodePacked(true, address(0), address(recipient), uint256(100)); + vm.expectRevert(); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + } + + // Reverts when the balance increase triggers an overflow. + function test_notAllow_expectingOverflow() public { + bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), type(uint256).max); + + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + vm.expectRevert(); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); + } + + // Reverts if the execution mode is invalid (not default). + function test_revertWithInvalidExecutionMode() public { + vm.prank(address(delegationManager)); + vm.expectRevert("CaveatEnforcer:invalid-execution-type"); + enforcer.beforeHook(hex"", hex"", singleTryMode, hex"", bytes32(0), address(0), address(0)); + } + + function _getEnforcer() internal view override returns (ICaveatEnforcer) { + return ICaveatEnforcer(address(enforcer)); + } +} diff --git a/test/enforcers/ERC20BalanceGteEnforcer.t.sol b/test/enforcers/ERC20BalanceGteEnforcer.t.sol deleted file mode 100644 index 17d062eb..00000000 --- a/test/enforcers/ERC20BalanceGteEnforcer.t.sol +++ /dev/null @@ -1,204 +0,0 @@ -// SPDX-License-Identifier: MIT AND Apache-2.0 -pragma solidity 0.8.23; - -import "forge-std/Test.sol"; -import { BasicERC20 } from "../utils/BasicERC20.t.sol"; - -import "../../src/utils/Types.sol"; -import { Execution } from "../../src/utils/Types.sol"; -import { CaveatEnforcerBaseTest } from "./CaveatEnforcerBaseTest.t.sol"; -import { ERC20BalanceGteEnforcer } from "../../src/enforcers/ERC20BalanceGteEnforcer.sol"; -import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol"; - -contract ERC20BalanceGteEnforcerTest is CaveatEnforcerBaseTest { - ////////////////////////////// State ////////////////////////////// - ERC20BalanceGteEnforcer public enforcer; - BasicERC20 public token; - address delegator; - address delegate; - address recipient; - address dm; - Execution mintExecution; - bytes mintExecutionCallData; - - ////////////////////// Set up ////////////////////// - - function setUp() public override { - super.setUp(); - delegator = address(users.alice.deleGator); - delegate = address(users.bob.deleGator); - recipient = address(users.carol.deleGator); - dm = address(delegationManager); - enforcer = new ERC20BalanceGteEnforcer(); - vm.label(address(enforcer), "ERC20 BalanceGte Enforcer"); - token = new BasicERC20(delegator, "TEST", "TEST", 0); - vm.label(address(token), "ERC20 Test Token"); - mintExecution = - Execution({ target: address(token), value: 0, callData: abi.encodeWithSelector(token.mint.selector, delegator, 100) }); - mintExecutionCallData = abi.encode(mintExecution); - } - - ////////////////////// Basic Functionality ////////////////////// - - // Validates the terms get decoded correctly - function test_decodedTheTerms() public { - bytes memory terms_ = abi.encodePacked(address(recipient), address(token), uint256(100)); - uint256 amount_; - address token_; - address recipient_; - (recipient_, token_, amount_) = enforcer.getTermsInfo(terms_); - assertEq(amount_, 100); - assertEq(token_, address(token)); - assertEq(recipient_, address(recipient)); - } - - // Validates that a balance has increased at least the expected amount - function test_allow_ifBalanceIncreases() public { - // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(address(recipient), address(token), uint256(100)); - - // Increase by 100 - vm.prank(dm); - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - vm.prank(delegator); - token.mint(recipient, 100); - vm.prank(dm); - enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - - // Increase by 1000 - vm.prank(dm); - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - vm.prank(delegator); - token.mint(recipient, 1000); - vm.prank(dm); - enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - } - - // Validates that the delegation can be reused with different recipients - function test_allow_reuseDelegationWithDifferentRecipients() public { - // Expect it to increase by at least 100 - bytes memory terms1_ = abi.encodePacked(address(recipient), address(token), uint256(100)); - bytes memory terms2_ = abi.encodePacked(address(delegator), address(token), uint256(100)); - - // Increase by 100, check for recipient - vm.prank(dm); - enforcer.beforeHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - vm.prank(delegator); - token.mint(recipient, 100); - vm.prank(dm); - enforcer.afterHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - - // Increase by 100, check for delegator as recipient - vm.prank(dm); - enforcer.beforeHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - vm.prank(delegator); - token.mint(delegator, 100); - vm.prank(dm); - enforcer.afterHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - } - - // ////////////////////// Errors ////////////////////// - - // Reverts if a balance hasn't increased by the set amount - function test_notAllow_insufficientIncrease() public { - // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(address(recipient), address(token), uint256(100)); - - // Increase by 10, expect revert - vm.prank(dm); - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - vm.prank(delegator); - token.mint(recipient, 10); - vm.prank(dm); - vm.expectRevert(bytes("ERC20BalanceGteEnforcer:balance-not-gt")); - enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - } - - // Reverts if a enforcer is locked - function test_notAllow_reenterALockedEnforcer() public { - // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(address(recipient), address(token), uint256(100)); - bytes32 delegationHash_ = bytes32(uint256(99999999)); - - // Increase by 100 - vm.startPrank(dm); - // Locks the enforcer - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, delegator, delegate); - bytes32 hashKey_ = enforcer.getHashKey(address(delegationManager), address(token), delegationHash_); - assertTrue(enforcer.isLocked(hashKey_)); - vm.expectRevert(bytes("ERC20BalanceGteEnforcer:enforcer-is-locked")); - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, delegator, delegate); - vm.startPrank(delegator); - token.mint(recipient, 1000); - vm.startPrank(dm); - - // Unlocks the enforcer - enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, delegator, delegate); - assertFalse(enforcer.isLocked(hashKey_)); - // Can be used again, and locks it again - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, delegator, delegate); - assertTrue(enforcer.isLocked(hashKey_)); - } - - // Reverts if the recipient didn't receive any tokens - function test_notAllow_noIncreaseToRecipient() public { - bytes memory terms_ = abi.encodePacked(address(recipient), address(token), uint256(100)); - - // BeforeHook should cache the recipient's balance - vm.prank(dm); - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - - // No tokens are minted to the recipient, so balance shouldn't increase - vm.prank(dm); - vm.expectRevert(bytes("ERC20BalanceGteEnforcer:balance-not-gt")); - enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - } - - // Validates the terms are well formed - function test_invalid_decodedTheTerms() public { - bytes memory terms_; - - // Too small - terms_ = abi.encodePacked(address(recipient), address(token), uint8(100)); - vm.expectRevert(bytes("ERC20BalanceGteEnforcer:invalid-terms-length")); - enforcer.getTermsInfo(terms_); - - // Too large - terms_ = abi.encodePacked(address(recipient), address(token), uint256(100), uint256(100)); - vm.expectRevert(bytes("ERC20BalanceGteEnforcer:invalid-terms-length")); - enforcer.getTermsInfo(terms_); - } - - // Validates the token address is a token - function test_invalid_tokenAddress() public { - bytes memory terms_; - - // Invalid token - terms_ = abi.encodePacked(address(recipient), address(0), uint256(100)); - vm.expectRevert(); - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - } - - // Validates that an invalid ID reverts - function test_notAllow_expectingOverflow() public { - // Expect balance to increase so much that the balance overflows - bytes memory terms_ = abi.encodePacked(address(recipient), address(token), type(uint256).max); - - // Increase - vm.prank(dm); - enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - vm.expectRevert(); - enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); - } - - // should fail with invalid call type mode (try instead of default) - function test_revertWithInvalidExecutionMode() public { - vm.prank(address(delegationManager)); - vm.expectRevert("CaveatEnforcer:invalid-execution-type"); - enforcer.beforeHook(hex"", hex"", singleTryMode, hex"", bytes32(0), address(0), address(0)); - } - - function _getEnforcer() internal view override returns (ICaveatEnforcer) { - return ICaveatEnforcer(address(enforcer)); - } -} diff --git a/test/enforcers/ERC721BalanceGteEnforcer.t.sol b/test/enforcers/ERC721BalanceChangeEnforcer.t.sol similarity index 51% rename from test/enforcers/ERC721BalanceGteEnforcer.t.sol rename to test/enforcers/ERC721BalanceChangeEnforcer.t.sol index 78349dc2..12ff13c5 100644 --- a/test/enforcers/ERC721BalanceGteEnforcer.t.sol +++ b/test/enforcers/ERC721BalanceChangeEnforcer.t.sol @@ -7,12 +7,12 @@ import { BasicCF721 } from "../utils/BasicCF721.t.sol"; import "../../src/utils/Types.sol"; import { Execution } from "../../src/utils/Types.sol"; import { CaveatEnforcerBaseTest } from "./CaveatEnforcerBaseTest.t.sol"; -import { ERC721BalanceGteEnforcer } from "../../src/enforcers/ERC721BalanceGteEnforcer.sol"; +import { ERC721BalanceChangeEnforcer } from "../../src/enforcers/ERC721BalanceChangeEnforcer.sol"; import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol"; -contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { +contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { ////////////////////////////// State ////////////////////////////// - ERC721BalanceGteEnforcer public enforcer; + ERC721BalanceChangeEnforcer public enforcer; BasicCF721 public token; address delegator; address delegate; @@ -27,12 +27,12 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { delegator = address(users.alice.deleGator); delegate = address(users.bob.deleGator); dm = address(delegationManager); - enforcer = new ERC721BalanceGteEnforcer(); - vm.label(address(enforcer), "ERC721 BalanceGte Enforcer"); + enforcer = new ERC721BalanceChangeEnforcer(); + vm.label(address(enforcer), "ERC721 Balance Change Enforcer"); token = new BasicCF721(delegator, "ERC721Token", "ERC721Token", ""); vm.label(address(token), "ERC721 Test Token"); - // Prepare the Execution data for minting + // Prepare the Execution data for minting. mintExecution = Execution({ target: address(token), value: 0, callData: abi.encodeWithSelector(token.mint.selector, delegator) }); mintExecutionCallData = abi.encode(mintExecution); @@ -40,19 +40,20 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { ////////////////////// Basic Functionality ////////////////////// - // Validates the terms get decoded correctly + // Validates the terms get decoded correctly (increase scenario) function test_decodedTheTerms() public { - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(1)); - (address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); + (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); + assertTrue(shouldBalanceIncrease_); assertEq(token_, address(token)); assertEq(recipient_, delegator); assertEq(amount_, 1); } - // Validates that a balance has increased at least by the expected amount + // Validates that a balance has increased by at least the expected amount function test_allow_ifBalanceIncreases() public { - // Expect it to increase by at least 1 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(1)); + // Expect increase by at least 1 + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); // Increase by 1 vm.prank(dm); @@ -62,7 +63,7 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { vm.prank(dm); enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); - // Increase by 2 + // Increase by 1 again (a second mint) vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); vm.prank(delegator); @@ -71,45 +72,42 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - ////////////////////// Errors ////////////////////// - // Reverts if a balance hasn't increased by the set amount function test_notAllow_insufficientIncrease() public { - // Expect it to increase by at least 1 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(1)); + // Expect increase by at least 1 + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); - // No increase + // No minting occurs here, so balance remains unchanged. vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); - // No minting occurs here vm.prank(dm); - vm.expectRevert(bytes("ERC721BalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:insufficient-balance-increase")); enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // Reverts if a balance decreased in between the hooks + // Reverts if the balance decreases when an increase is expected function test_notAllow_ifBalanceDecreases() public { - // Starting with 1 token + // Starting with 1 token for delegator. vm.prank(delegator); token.mint(delegator); - // Expect it to increase by at least 1 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(1)); + // Expect an increase by at least 1. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); - // Decrease balance by transferring token away - uint256 tokenIdToTransfer_ = (token.tokenId()) - 1; + // Transfer the token away, decreasing the balance. + uint256 tokenIdToTransfer_ = token.tokenId() - 1; vm.prank(delegator); token.transferFrom(delegator, address(1), tokenIdToTransfer_); vm.prank(dm); - vm.expectRevert(bytes("ERC721BalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:insufficient-balance-increase")); enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // Allows to check the balance of different recipients + // Allows checking the balance of different recipients function test_allow_withDifferentRecipients() public { address[] memory recipients_ = new address[](2); recipients_[0] = delegator; @@ -117,9 +115,9 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { for (uint256 i = 0; i < recipients_.length; i++) { address currentRecipient_ = recipients_[i]; - bytes memory terms_ = abi.encodePacked(address(token), currentRecipient_, uint256(1)); + bytes memory terms_ = abi.encodePacked(true, address(token), currentRecipient_, uint256(1)); - // Increase by 1 for each recipient + // Increase by 1 for each recipient. vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(i), address(0), delegate); vm.prank(delegator); @@ -131,30 +129,29 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { // Considers any pre-existing balances in the recipient function test_notAllow_withPreExistingBalance() public { - // Recipient already has 1 token + // Delegator already has 1 token. vm.prank(delegator); token.mint(delegator); - // Expect balance to increase by at least 1 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(1)); + // Expect balance to increase by at least 1. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); - // No increase occurs here - + // No new minting – the balance doesn't increase. vm.prank(dm); - vm.expectRevert(bytes("ERC721BalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:insufficient-balance-increase")); enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // Same delegation hash multiple recipients + // Differentiates delegation hash with different recipients function test_differentiateDelegationHashWithRecipient() public { bytes32 delegationHash_ = bytes32(uint256(99999999)); address recipient2_ = address(1111111); - // Expect balance to increase by at least 1 in different recipients - bytes memory terms1_ = abi.encodePacked(address(token), delegator, uint256(1)); - bytes memory terms2_ = abi.encodePacked(address(token), recipient2_, uint256(1)); + // Terms for two different recipients. + bytes memory terms1_ = abi.encodePacked(true, address(token), delegator, uint256(1)); + bytes memory terms2_ = abi.encodePacked(true, address(token), recipient2_, uint256(1)); vm.prank(dm); enforcer.beforeHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); @@ -162,40 +159,40 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { vm.prank(dm); enforcer.beforeHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); - // Increase balance by 1 only in recipient1 + // Increase balance by 1 only for recipient1. vm.prank(delegator); token.mint(delegator); - // This one works well recipient1 increased + // Recipient1 should pass as its balance increased. vm.prank(dm); enforcer.afterHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); - // This one fails recipient2 didn't increase + // Recipient2 did not receive a token, so it should revert. vm.prank(dm); - vm.expectRevert(bytes("ERC721BalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:insufficient-balance-increase")); enforcer.afterHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); - // Increase balance by 1 in recipient2 to fix it + // Increase balance for recipient2. vm.prank(delegator); token.mint(recipient2_); - // Recipient2 works well + // Recipient2 now passes. vm.prank(dm); enforcer.afterHook(terms2_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); } - // Reverts if the enforcer is locked + // Reverts if the enforcer is locked (i.e. if beforeHook is reentered) function test_notAllow_reenterALockedEnforcer() public { - // Expect it to increase by at least 1 - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), uint256(1)); + // Expect increase by at least 1. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); bytes32 delegationHash_ = bytes32(uint256(99999999)); - // Lock the enforcer vm.startPrank(dm); + // Lock the enforcer. enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); bytes32 hashKey_ = enforcer.getHashKey(address(delegationManager), address(token), address(delegator), delegationHash_); assertTrue(enforcer.isLocked(hashKey_)); - vm.expectRevert(bytes("ERC721BalanceGteEnforcer:enforcer-is-locked")); + vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:enforcer-is-locked")); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); vm.stopPrank(); @@ -203,44 +200,98 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { token.mint(delegator); vm.startPrank(dm); - // Unlock the enforcer + // Unlock the enforcer. enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); assertFalse(enforcer.isLocked(hashKey_)); - // Can be used again, and locks it again + // Reuse the enforcer, which locks it again. enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); assertTrue(enforcer.isLocked(hashKey_)); vm.stopPrank(); } - // Validates the terms are well-formed + // Validates that a balance decrease within the allowed range passes. + // For decrease scenarios (flag = false), the final balance must be at least the cached balance minus the allowed decrease. + // Example: if the cached balance is 2 and the allowed decrease is 1, then final balance must be >= 1. + function test_allow_ifBalanceDoesNotDecreaseTooMuch() public { + // Mint 2 tokens to delegator. + vm.prank(delegator); + token.mint(delegator); + vm.prank(delegator); + token.mint(delegator); + // Ensure initial balance is 2. + uint256 initialBalance_ = token.balanceOf(delegator); + assertEq(initialBalance_, 2); + + // Set terms with flag = false for decrease, allowed decrease is 1. + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); + + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); + + // Remove one token: final balance becomes 1 (which is 2 - 1, and thus acceptable). + uint256 tokenIdToTransfer_ = token.tokenId() - 1; + vm.prank(delegator); + token.transferFrom(delegator, address(2), tokenIdToTransfer_); + + vm.prank(dm); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); + } + + // Reverts if the balance decreases too much (i.e. final balance falls below cached balance - allowed amount) + function test_notAllow_excessiveDecrease() public { + // Mint 2 tokens to delegator. + vm.prank(delegator); + token.mint(delegator); + vm.prank(delegator); + token.mint(delegator); + // Confirm initial balance is 2. + uint256 initialBalance = token.balanceOf(delegator); + assertEq(initialBalance, 2); + + // Terms: flag = false (decrease expected), allowed decrease is 1. + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); + + vm.prank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); + + // Remove both tokens: final balance becomes 0, which is below (2 - 1) = 1. + uint256 tokenId1 = token.tokenId() - 2; + uint256 tokenId2 = token.tokenId() - 1; + vm.prank(delegator); + token.transferFrom(delegator, address(3), tokenId1); + vm.prank(delegator); + token.transferFrom(delegator, address(4), tokenId2); + + vm.prank(dm); + vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:exceeded-balance-decrease")); + enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); + } + + // Validates that the terms are well-formed (exactly 73 bytes) function test_invalid_decodedTheTerms() public { bytes memory terms_; - // Too small - terms_ = abi.encodePacked(address(token), address(delegator), uint8(1)); - vm.expectRevert(bytes("ERC721BalanceGteEnforcer:invalid-terms-length")); + // Too small: missing required bytes. + terms_ = abi.encodePacked(true, address(token), address(delegator), uint8(1)); + vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); - // Too large - terms_ = abi.encodePacked(uint256(1), uint256(1), uint256(1), uint256(1)); - vm.expectRevert(bytes("ERC721BalanceGteEnforcer:invalid-terms-length")); + // Too large: extra bytes. + terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1), uint256(1)); + vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); } - // Validates that an invalid token address causes a revert + // Validates that an invalid token address causes a revert when calling beforeHook. function test_invalid_tokenAddress() public { - bytes memory terms_; - - // Invalid token address - terms_ = abi.encodePacked(address(0), address(delegator), uint256(1)); + bytes memory terms_ = abi.encodePacked(true, address(0), address(delegator), uint256(1)); vm.expectRevert(); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // Validates that an unrealistic amount causes a revert + // Reverts if an unrealistic amount triggers overflow. function test_notAllow_expectingOverflow() public { - // Expect balance to increase by max uint256, which is unrealistic - bytes memory terms_ = abi.encodePacked(address(token), address(delegator), type(uint256).max); + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), type(uint256).max); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -248,7 +299,7 @@ contract ERC721BalanceGteEnforcerTest is CaveatEnforcerBaseTest { enforcer.afterHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } - // should fail with invalid call type mode (try instead of default) + // Fails with an invalid execution mode (non-default) function test_revertWithInvalidExecutionMode() public { vm.prank(address(delegationManager)); vm.expectRevert("CaveatEnforcer:invalid-execution-type"); diff --git a/test/enforcers/NativeBalanceGteEnforcer.t.sol b/test/enforcers/NativeBalanceChangeEnforcer.t.sol similarity index 60% rename from test/enforcers/NativeBalanceGteEnforcer.t.sol rename to test/enforcers/NativeBalanceChangeEnforcer.t.sol index 8a0bd13d..60c8e28c 100644 --- a/test/enforcers/NativeBalanceGteEnforcer.t.sol +++ b/test/enforcers/NativeBalanceChangeEnforcer.t.sol @@ -4,13 +4,13 @@ pragma solidity 0.8.23; import "../../src/utils/Types.sol"; import { Execution } from "../../src/utils/Types.sol"; import { CaveatEnforcerBaseTest } from "./CaveatEnforcerBaseTest.t.sol"; -import { NativeBalanceGteEnforcer } from "../../src/enforcers/NativeBalanceGteEnforcer.sol"; +import { NativeBalanceChangeEnforcer } from "../../src/enforcers/NativeBalanceChangeEnforcer.sol"; import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol"; import { Counter } from "../utils/Counter.t.sol"; -contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { +contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { ////////////////////////////// State ////////////////////////////// - NativeBalanceGteEnforcer public enforcer; + NativeBalanceChangeEnforcer public enforcer; address delegator; address delegate; address dm; @@ -24,8 +24,8 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { delegator = address(users.alice.deleGator); delegate = address(users.bob.deleGator); dm = address(delegationManager); - enforcer = new NativeBalanceGteEnforcer(); - vm.label(address(enforcer), "NativeBalanceGte Enforcer"); + enforcer = new NativeBalanceChangeEnforcer(); + vm.label(address(enforcer), "Native Balance Change Enforcer"); noExecution = Execution(address(0), 0, hex""); } @@ -33,11 +33,13 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { // Validates the terms get decoded correctly function test_decodedTheTerms() public { - bytes memory terms_ = abi.encodePacked(address(users.carol.deleGator), uint256(100)); + bytes memory terms_ = abi.encodePacked(true, address(users.carol.deleGator), uint256(100)); + bool shouldBalanceIncrease_; uint256 amount_; address recipient_; - (recipient_, amount_) = enforcer.getTermsInfo(terms_); - assertEq(recipient_, address(address(users.carol.deleGator))); + (shouldBalanceIncrease_, recipient_, amount_) = enforcer.getTermsInfo(terms_); + assertTrue(shouldBalanceIncrease_); + assertEq(recipient_, address(users.carol.deleGator)); assertEq(amount_, 100); } @@ -45,7 +47,7 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { function test_allow_ifBalanceIncreases() public { address recipient_ = delegator; // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(recipient_, uint256(100)); + bytes memory terms_ = abi.encodePacked(true, recipient_, uint256(100)); // Increase by 100 vm.startPrank(dm); @@ -59,19 +61,53 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { enforcer.afterHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); } - // ////////////////////// Errors ////////////////////// + // Validates that a balance has decreased at most the expected amount + function test_allow_ifBalanceDecreases() public { + address recipient_ = delegator; + vm.deal(recipient_, 1000); // Start with 1000 + // Expect it to decrease by at most 100 + bytes memory terms_ = abi.encodePacked(false, recipient_, uint256(100)); + + // Decrease by 50 + vm.startPrank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); + _decreaseBalance(delegator, 50); + enforcer.afterHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); + + // Decrease by 100 + enforcer.beforeHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); + _decreaseBalance(delegator, 100); + enforcer.afterHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); + } + + ////////////////////// Errors ////////////////////// // Reverts if a balance hasn't increased by the specified amount function test_notAllow_insufficientIncrease() public { address recipient_ = delegator; // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(recipient_, uint256(100)); + bytes memory terms_ = abi.encodePacked(true, recipient_, uint256(100)); // Increase by 10, expect revert vm.startPrank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); _increaseBalance(delegator, 10); - vm.expectRevert(bytes("NativeBalanceGteEnforcer:balance-not-gt")); + vm.expectRevert(bytes("NativeBalanceChangeEnforcer:insufficient-balance-increase")); + enforcer.afterHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); + } + + // Reverts if a balance has decreased more than the specified amount + function test_notAllow_excessiveDecrease() public { + address recipient_ = delegator; + vm.deal(recipient_, 1000); // Start with 1000 + // Expect it to decrease by at most 100 + bytes memory terms_ = abi.encodePacked(false, recipient_, uint256(100)); + + // Decrease by 150, expect revert + vm.startPrank(dm); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); + _decreaseBalance(delegator, 150); + vm.expectRevert(bytes("NativeBalanceChangeEnforcer:exceeded-balance-decrease")); enforcer.afterHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); } @@ -79,7 +115,7 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { function test_notAllow_reenterALockedEnforcer() public { address recipient_ = delegator; // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(recipient_, uint256(100)); + bytes memory terms_ = abi.encodePacked(true, recipient_, uint256(100)); bytes32 delegationHash_ = bytes32(uint256(99999999)); // Increase by 100 @@ -88,7 +124,7 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { enforcer.beforeHook(terms_, hex"", singleDefaultMode, executionCallData, delegationHash_, delegator, delegate); bytes32 hashKey_ = enforcer.getHashKey(address(delegationManager), delegationHash_); assertTrue(enforcer.isLocked(hashKey_)); - vm.expectRevert(bytes("NativeBalanceGteEnforcer:enforcer-is-locked")); + vm.expectRevert(bytes("NativeBalanceChangeEnforcer:enforcer-is-locked")); enforcer.beforeHook(terms_, hex"", singleDefaultMode, executionCallData, delegationHash_, delegator, delegate); _increaseBalance(delegator, 1000); vm.startPrank(dm); @@ -107,13 +143,13 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { bytes memory terms_; // Too small - terms_ = abi.encodePacked(recipient_, uint8(100)); - vm.expectRevert(bytes("NativeBalanceGteEnforcer:invalid-terms-length")); + terms_ = abi.encodePacked(true, recipient_, uint8(100)); + vm.expectRevert(bytes("NativeBalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); // Too large - terms_ = abi.encodePacked(uint256(100), uint256(100)); - vm.expectRevert(bytes("NativeBalanceGteEnforcer:invalid-terms-length")); + terms_ = abi.encodePacked(true, uint256(100), uint256(100)); + vm.expectRevert(bytes("NativeBalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); } @@ -122,7 +158,7 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { address recipient_ = delegator; // Expect balance to increase so much that the validation overflows - bytes memory terms_ = abi.encodePacked(recipient_, type(uint256).max); + bytes memory terms_ = abi.encodePacked(true, recipient_, type(uint256).max); vm.deal(recipient_, type(uint256).max); vm.startPrank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); @@ -141,6 +177,10 @@ contract NativeBalanceGteEnforcerTest is CaveatEnforcerBaseTest { vm.deal(_recipient, _recipient.balance + _amount); } + function _decreaseBalance(address _recipient, uint256 _amount) internal { + vm.deal(_recipient, _recipient.balance - _amount); + } + function _getEnforcer() internal view override returns (ICaveatEnforcer) { return ICaveatEnforcer(address(enforcer)); } From 18737a5c7f3b83015c02496252fa3c1132848247 Mon Sep 17 00:00:00 2001 From: hanzel98 Date: Tue, 22 Apr 2025 12:04:56 -0600 Subject: [PATCH 2/4] Improved comments, and default boolean --- .../ERC1155BalanceChangeEnforcer.sol | 40 +++++++----- src/enforcers/ERC20BalanceChangeEnforcer.sol | 38 ++++++----- src/enforcers/ERC721BalanceChangeEnforcer.sol | 46 +++++++------ src/enforcers/NativeBalanceChangeEnforcer.sol | 64 +++++++++---------- .../ERC1155BalanceChangeEnforcer.t.sol | 37 ++++++----- .../ERC20BalanceChangeEnforcer.t.sol | 42 ++++++------ .../ERC721BalanceChangeEnforcer.t.sol | 36 +++++------ .../NativeBalanceChangeEnforcer.t.sol | 24 +++---- 8 files changed, 171 insertions(+), 156 deletions(-) diff --git a/src/enforcers/ERC1155BalanceChangeEnforcer.sol b/src/enforcers/ERC1155BalanceChangeEnforcer.sol index 1807126f..7610c078 100644 --- a/src/enforcers/ERC1155BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC1155BalanceChangeEnforcer.sol @@ -8,14 +8,17 @@ import { ModeCode } from "../utils/Types.sol"; /** * @title ERC1155BalanceChangeEnforcer - * @dev This contract enforces that the ERC1155 token balance of a recipient for a specific token ID - * has changed by at least the specified amount after the execution, measured between the `beforeHook` and `afterHook` calls. - * The change can be either an increase or decrease based on the `shouldBalanceIncrease` flag. + * @dev This contract allows setting up some guardrails around balance changes. By specifying an amount and a direction + * (decrease/increase), one can enforce a maximum decrease or minimum increase in after-execution balance. + * The change can be either a decrease or increase based on the `isDecrease` flag. + * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to + * create granular permissions. * @dev This enforcer operates only in default execution mode. * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. Since * enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers simultaneously. * Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to prevent unintended - * behavior. + * behavior. Given its potential for concurrent condition fulfillment, use this enforcer at your own risk and ensure it aligns with + * your intended security model. */ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// State ////////////////////////////// @@ -53,11 +56,12 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function caches the recipient's ERC1155 token balance before the delegation is executed. * @param _terms 105 bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the ERC1155 token * - next 20 bytes: address of the recipient * - next 32 bytes: token ID - * - next 32 bytes: amount the balance should change by + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) * @param _mode The execution mode. (Must be Default execType) * @param _delegationHash The hash of the delegation. */ @@ -83,13 +87,14 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { } /** - * @notice This function enforces that the recipient's ERC1155 token balance has changed by at least the amount provided. + * @notice This function enforces that the recipient's ERC1155 token balance has changed by the expected amount. * @param _terms 105 bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the ERC1155 token * - next 20 bytes: address of the recipient * - next 32 bytes: token ID - * - next 32 bytes: amount the balance should change by + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) * @param _delegationHash The hash of the delegation. */ function afterHook( @@ -104,33 +109,34 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { public override { - (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = getTermsInfo(_terms); + (bool isDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, recipient_, tokenId_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC1155(token_).balanceOf(recipient_, tokenId_); - if (shouldBalanceIncrease_) { - require(balance_ >= balanceCache[hashKey_] + amount_, "ERC1155BalanceChangeEnforcer:insufficient-balance-increase"); - } else { + if (isDecrease_) { require(balance_ >= balanceCache[hashKey_] - amount_, "ERC1155BalanceChangeEnforcer:exceeded-balance-decrease"); + } else { + require(balance_ >= balanceCache[hashKey_] + amount_, "ERC1155BalanceChangeEnforcer:insufficient-balance-increase"); } } /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms Encoded data that is used during the execution hooks. - * @return shouldBalanceIncrease_ Boolean indicating if the balance should increase (true) or decrease (false). + * @return isDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). * @return token_ The address of the ERC1155 token. * @return recipient_ The address of the recipient of the token. * @return tokenId_ The ID of the ERC1155 token. - * @return amount_ The amount the balance should change by. + * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) */ function getTermsInfo(bytes calldata _terms) public pure - returns (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) + returns (bool isDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) { require(_terms.length == 105, "ERC1155BalanceChangeEnforcer:invalid-terms-length"); - shouldBalanceIncrease_ = _terms[0] != 0; + isDecrease_ = _terms[0] != 0; token_ = address(bytes20(_terms[1:21])); recipient_ = address(bytes20(_terms[21:41])); tokenId_ = uint256(bytes32(_terms[41:73])); diff --git a/src/enforcers/ERC20BalanceChangeEnforcer.sol b/src/enforcers/ERC20BalanceChangeEnforcer.sol index d407791f..80820a0e 100644 --- a/src/enforcers/ERC20BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC20BalanceChangeEnforcer.sol @@ -8,16 +8,17 @@ import { ModeCode } from "../utils/Types.sol"; /** * @title ERC20BalanceChangeEnforcer - * @dev This contract enforces that the delegator's ERC20 balance has changed by at least the specified amount - * after the execution has been executed, measured between the `beforeHook` and `afterHook` calls, regardless of what the execution - * is. The change can be either an increase or decrease based on the `shouldBalanceIncrease` flag. + * @dev This contract allows setting up some guardrails around balance changes. By specifying an amount and a direction + * (decrease/increase), one can enforce a maximum decrease or minimum increase in after-execution balance. + * The change can be either a decrease or increase based on the `isDecrease` flag. * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to * create granular permissions. * @dev This enforcer operates only in default execution mode. * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. Since * enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers simultaneously. * Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to prevent unintended - * behavior. + * behavior. Given its potential for concurrent condition fulfillment, use this enforcer at your own risk and ensure it aligns with + * your intended security model. */ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// State ////////////////////////////// @@ -43,10 +44,11 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function caches the delegators ERC20 balance before the delegation is executed. * @param _terms 73 packed bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the token * - next 20 bytes: address of the recipient - * - next 32 bytes: amount the balance should change by + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) * @param _mode The execution mode. (Must be Default execType) */ function beforeHook( @@ -71,12 +73,13 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { } /** - * @notice This function enforces that the delegators ERC20 balance has changed by at least the amount provided. + * @notice This function enforces that the delegators ERC20 balance has changed by the expected amount. * @param _terms 73 packed bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the token * - next 20 bytes: address of the recipient - * - next 32 bytes: amount the balance should change by + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) */ function afterHook( bytes calldata _terms, @@ -90,32 +93,33 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { public override { - (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); + (bool isDecrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC20(token_).balanceOf(recipient_); - if (shouldBalanceIncrease_) { - require(balance_ >= balanceCache[hashKey_] + amount_, "ERC20BalanceChangeEnforcer:insufficient-balance-increase"); - } else { + if (isDecrease_) { require(balance_ >= balanceCache[hashKey_] - amount_, "ERC20BalanceChangeEnforcer:exceeded-balance-decrease"); + } else { + require(balance_ >= balanceCache[hashKey_] + amount_, "ERC20BalanceChangeEnforcer:insufficient-balance-increase"); } } /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms encoded data that is used during the execution hooks. - * @return shouldBalanceIncrease_ Boolean indicating if the balance should increase (true) or decrease (false). + * @return isDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). * @return token_ The address of the token. * @return recipient_ The address of the recipient. - * @return amount_ The amount the balance should change by. + * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) */ function getTermsInfo(bytes calldata _terms) public pure - returns (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) + returns (bool isDecrease_, address token_, address recipient_, uint256 amount_) { require(_terms.length == 73, "ERC20BalanceChangeEnforcer:invalid-terms-length"); - shouldBalanceIncrease_ = _terms[0] != 0; + isDecrease_ = _terms[0] != 0; token_ = address(bytes20(_terms[1:21])); recipient_ = address(bytes20(_terms[21:41])); amount_ = uint256(bytes32(_terms[41:])); diff --git a/src/enforcers/ERC721BalanceChangeEnforcer.sol b/src/enforcers/ERC721BalanceChangeEnforcer.sol index 3d943d48..b3995f96 100644 --- a/src/enforcers/ERC721BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC721BalanceChangeEnforcer.sol @@ -8,14 +8,17 @@ import { ModeCode } from "../utils/Types.sol"; /** * @title ERC721BalanceChangeEnforcer - * @dev This contract enforces that the ERC721 token balance of a recipient has changed by at least the specified amount - * after the execution, measured between the `beforeHook` and `afterHook` calls, regardless of what the execution is. - * The change can be either an increase or decrease based on the `shouldBalanceIncrease` flag. + * @dev This contract allows setting up some guardrails around balance changes. By specifying an amount and a direction + * (decrease/increase), one can enforce a maximum decrease or minimum increase in after-execution balance. + * The change can be either a decrease or increase based on the `isDecrease` flag. + * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to + * create granular permissions. * @dev This enforcer operates only in default execution mode. - * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. - * Since enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers - * simultaneously. Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to - * prevent unintended behavior. + * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. Since + * enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers simultaneously. + * Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to prevent unintended + * behavior. Given its potential for concurrent condition fulfillment, use this enforcer at your own risk and ensure it aligns with + * your intended security model. */ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// State ////////////////////////////// @@ -51,10 +54,11 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function caches the delegator's ERC721 token balance before the delegation is executed. * @param _terms 73 bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the ERC721 token * - next 20 bytes: address of the recipient - * - next 32 bytes: amount the balance should change by + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) * @param _mode The execution mode. (Must be Default execType) * @param _delegationHash The hash of the delegation. */ @@ -80,12 +84,13 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { } /** - * @notice This function enforces that the delegator's ERC721 token balance has changed by at least the amount provided. + * @notice This function enforces that the delegator's ERC721 token balance has changed by the expected amount. * @param _terms 73 bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the ERC721 token * - next 20 bytes: address of the recipient - * - next 32 bytes: amount the balance should change by + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) * @param _delegationHash The hash of the delegation. */ function afterHook( @@ -100,32 +105,33 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { public override { - (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); + (bool isDecrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, recipient_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC721(token_).balanceOf(recipient_); - if (shouldBalanceIncrease_) { - require(balance_ >= balanceCache[hashKey_] + amount_, "ERC721BalanceChangeEnforcer:insufficient-balance-increase"); - } else { + if (isDecrease_) { require(balance_ >= balanceCache[hashKey_] - amount_, "ERC721BalanceChangeEnforcer:exceeded-balance-decrease"); + } else { + require(balance_ >= balanceCache[hashKey_] + amount_, "ERC721BalanceChangeEnforcer:insufficient-balance-increase"); } } /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms Encoded data that is used during the execution hooks. - * @return shouldBalanceIncrease_ Boolean indicating if the balance should increase (true) or decrease (false). + * @return isDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). * @return token_ The address of the ERC721 token. * @return recipient_ The address of the recipient of the token. - * @return amount_ The amount the balance should change by. + * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) */ function getTermsInfo(bytes calldata _terms) public pure - returns (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) + returns (bool isDecrease_, address token_, address recipient_, uint256 amount_) { require(_terms.length == 73, "ERC721BalanceChangeEnforcer:invalid-terms-length"); - shouldBalanceIncrease_ = _terms[0] != 0; + isDecrease_ = _terms[0] != 0; token_ = address(bytes20(_terms[1:21])); recipient_ = address(bytes20(_terms[21:41])); amount_ = uint256(bytes32(_terms[41:])); diff --git a/src/enforcers/NativeBalanceChangeEnforcer.sol b/src/enforcers/NativeBalanceChangeEnforcer.sol index 6e77b5f3..d8bb5a88 100644 --- a/src/enforcers/NativeBalanceChangeEnforcer.sol +++ b/src/enforcers/NativeBalanceChangeEnforcer.sol @@ -6,16 +6,17 @@ import { ModeCode } from "../utils/Types.sol"; /** * @title NativeBalanceChangeEnforcer - * @dev This contract enforces that a recipient's native token balance has changed by at least the specified amount - * after the execution has been executed, measured between the `beforeHook` and `afterHook` calls, regardless of what the execution - * is. The change can be either an increase or decrease based on the `shouldBalanceIncrease` flag. - * @dev This contract does not enforce how the balance changes. It is meant to be used with additional enforcers to create - * granular permissions. + * @dev This contract allows setting up some guardrails around balance changes. By specifying an amount and a direction + * (decrease/increase), one can enforce a maximum decrease or minimum increase in after-execution balance. + * The change can be either a decrease or increase based on the `isDecrease` flag. + * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to + * create granular permissions. * @dev This enforcer operates only in default execution mode. - * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. - * Since enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers - * simultaneously. Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to - * prevent unintended behavior. + * @dev Security Notice: This enforcer tracks balance changes by comparing the recipient's balance before and after execution. Since + * enforcers watching the same recipient share state, a single balance modification may satisfy multiple enforcers simultaneously. + * Users should avoid tracking the same recipient's balance on multiple enforcers in a single delegation chain to prevent unintended + * behavior. Given its potential for concurrent condition fulfillment, use this enforcer at your own risk and ensure it aligns with + * your intended security model. */ contract NativeBalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// State ////////////////////////////// @@ -38,13 +39,14 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { ////////////////////////////// Public Methods ////////////////////////////// /** - * @notice Caches the recipient's native token balance before the delegation is executed. + * @notice This function caches the delegator's native token balance before the delegation is executed. * @param _terms 53 packed bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the recipient - * - next 32 bytes: amount the balance should change by - * @param _delegationHash The hash of the delegation being operated on. + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) * @param _mode The execution mode. (Must be Default execType) + * @param _delegationHash The hash of the delegation. */ function beforeHook( bytes calldata _terms, @@ -61,19 +63,19 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { { bytes32 hashKey_ = _getHashKey(msg.sender, _delegationHash); (, address recipient_,) = getTermsInfo(_terms); - require(!isLocked[hashKey_], "NativeBalanceChangeEnforcer:enforcer-is-locked"); isLocked[hashKey_] = true; balanceCache[hashKey_] = recipient_.balance; } /** - * @notice Ensures that the recipient's native token balance has changed by at least the specified amount. + * @notice This function enforces that the delegator's native token balance has changed by the expected amount. * @param _terms 53 packed bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the recipient - * - next 32 bytes: amount the balance should change by - * @param _delegationHash The hash of the delegation being operated on. + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) + * @param _delegationHash The hash of the delegation. */ function afterHook( bytes calldata _terms, @@ -87,35 +89,33 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { public override { - (bool shouldBalanceIncrease_, address recipient_, uint256 amount_) = getTermsInfo(_terms); + (bool isDecrease_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, _delegationHash); delete isLocked[hashKey_]; - if (shouldBalanceIncrease_) { + if (isDecrease_) { + require(recipient_.balance >= balanceCache[hashKey_] - amount_, "NativeBalanceChangeEnforcer:exceeded-balance-decrease"); + } else { require( recipient_.balance >= balanceCache[hashKey_] + amount_, "NativeBalanceChangeEnforcer:insufficient-balance-increase" ); - } else { - require(recipient_.balance >= balanceCache[hashKey_] - amount_, "NativeBalanceChangeEnforcer:exceeded-balance-decrease"); } } /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms 53 packed bytes where: - * - first byte: boolean indicating if the balance should increase + * - first byte: boolean indicating if the balance should decrease * - next 20 bytes: address of the recipient - * - next 32 bytes: amount the balance should change by - * @return shouldBalanceIncrease_ Boolean indicating if the balance should increase (true) or decrease (false). + * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) + * @return isDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). * @return recipient_ The address of the recipient whose balance will change. - * @return amount_ The minimum balance change required. + * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on + * isDecrease) */ - function getTermsInfo(bytes calldata _terms) - public - pure - returns (bool shouldBalanceIncrease_, address recipient_, uint256 amount_) - { + function getTermsInfo(bytes calldata _terms) public pure returns (bool isDecrease_, address recipient_, uint256 amount_) { require(_terms.length == 53, "NativeBalanceChangeEnforcer:invalid-terms-length"); - shouldBalanceIncrease_ = _terms[0] != 0; + isDecrease_ = _terms[0] != 0; recipient_ = address(bytes20(_terms[1:21])); amount_ = uint256(bytes32(_terms[21:])); } diff --git a/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol b/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol index 18339acf..41e57896 100644 --- a/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol +++ b/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol @@ -47,9 +47,8 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Terms format: [bool shouldBalanceIncrease, address token, address recipient, uint256 tokenId, uint256 amount] function test_decodedTheTerms() public { bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); - (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = - enforcer.getTermsInfo(terms_); - assertTrue(shouldBalanceIncrease_); + (bool isDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = enforcer.getTermsInfo(terms_); + assertEq(isDecrease_, true); assertEq(token_, address(token)); assertEq(recipient_, delegator); assertEq(tokenId_, tokenId); @@ -59,7 +58,7 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates that a balance has increased at least by the expected amount. function test_allow_ifBalanceIncreases() public { // Expect increase by at least 100. - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(100)); // Increase by 100. vm.prank(dm); @@ -83,7 +82,7 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Reverts if a balance hasn't increased by the set amount. function test_notAllow_insufficientIncrease() public { // Expect increase by at least 100. - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(100)); // Increase by 10 only, expect revert. vm.prank(dm); @@ -102,7 +101,7 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { token.mint(delegator, tokenId, 10, ""); // Expect increase by at least 100. - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(100)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -124,7 +123,7 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { for (uint256 i = 0; i < recipients_.length; i++) { address currentRecipient_ = recipients_[i]; - bytes memory terms_ = abi.encodePacked(true, address(token), currentRecipient_, uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(token), currentRecipient_, uint256(tokenId), uint256(100)); // Increase by 100 for each recipient. vm.prank(dm); @@ -143,7 +142,7 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { token.mint(delegator, tokenId, 50, ""); // Expect balance to increase by at least 100. - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(100)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -162,8 +161,8 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { bytes32 delegationHash_ = bytes32(uint256(99999999)); address recipient2_ = address(1111111); // Terms for two different recipients. - bytes memory terms1_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); - bytes memory terms2_ = abi.encodePacked(true, address(token), recipient2_, uint256(tokenId), uint256(100)); + bytes memory terms1_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms2_ = abi.encodePacked(false, address(token), recipient2_, uint256(tokenId), uint256(100)); vm.prank(dm); enforcer.beforeHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); @@ -196,7 +195,7 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Reverts if the enforcer is locked (i.e. if beforeHook is reentered). function test_notAllow_reenterALockedEnforcer() public { // Expect increase by at least 100. - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(100)); bytes32 delegationHash_ = bytes32(uint256(99999999)); vm.startPrank(dm); @@ -225,7 +224,7 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { ////////////////////// Decrease Tests ////////////////////// // Validates that a balance decrease within the allowed range passes. - // For decrease scenarios (flag = false), the final balance must be at least the cached balance minus the allowed decrease. + // For decrease scenarios (flag = true), the final balance must be at least the cached balance minus the allowed decrease. function test_allow_ifBalanceDoesNotDecreaseTooMuch() public { // Mint 100 tokens to delegator. vm.prank(delegator); @@ -234,8 +233,8 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { uint256 initialBalance_ = token.balanceOf(delegator, tokenId); assertEq(initialBalance_, 100); - // Set terms with flag = false (decrease expected), allowed decrease is 20. - bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(20)); + // Set terms with flag = true (decrease expected), allowed decrease is 20. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(20)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -256,8 +255,8 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { uint256 initialBalance_ = token.balanceOf(delegator, tokenId); assertEq(initialBalance_, 100); - // Set terms with flag = false (decrease expected), allowed decrease is 20. - bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(20)); + // Set terms with flag = true (decrease expected), allowed decrease is 20. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(20)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -283,21 +282,21 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { enforcer.getTermsInfo(terms_); // Too large: extra bytes appended. - terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100), uint256(1)); + terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), uint256(100), uint256(1)); vm.expectRevert(bytes("ERC1155BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); } // Validates that an invalid token address causes a revert. function test_invalid_tokenAddress() public { - bytes memory terms_ = abi.encodePacked(true, address(0), address(delegator), uint256(tokenId), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(0), address(delegator), uint256(tokenId), uint256(100)); vm.expectRevert(); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } // Reverts if an unrealistic amount triggers overflow. function test_notAllow_expectingOverflow() public { - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), type(uint256).max); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(tokenId), type(uint256).max); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); diff --git a/test/enforcers/ERC20BalanceChangeEnforcer.t.sol b/test/enforcers/ERC20BalanceChangeEnforcer.t.sol index 5d8a8e3a..b6beeee2 100644 --- a/test/enforcers/ERC20BalanceChangeEnforcer.t.sol +++ b/test/enforcers/ERC20BalanceChangeEnforcer.t.sol @@ -42,9 +42,9 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates the terms get decoded correctly for an increase scenario function test_decodedTheTerms() public { - bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); - (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); - assertTrue(shouldBalanceIncrease_); + bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(100)); + (bool isDecrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); + assertEq(isDecrease_, false); assertEq(token_, address(token)); assertEq(recipient_, address(recipient)); assertEq(amount_, 100); @@ -52,8 +52,8 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates that a balance has increased at least the expected amount function test_allow_ifBalanceIncreases() public { - // Terms: [flag=true, token, recipient, amount=100] - bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + // Terms: [flag=false, token, recipient, amount=100] + bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(100)); // Increase by 100 vm.prank(dm); @@ -74,9 +74,9 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates that a delegation can be reused with different recipients (for increase) without interference function test_allow_reuseDelegationWithDifferentRecipients() public { - // Terms for two different recipients (flag=true indicates increase expected) - bytes memory terms1_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); - bytes memory terms2_ = abi.encodePacked(true, address(token), address(delegator), uint256(100)); + // Terms for two different recipients (flag=false indicates increase expected) + bytes memory terms1_ = abi.encodePacked(false, address(token), address(recipient), uint256(100)); + bytes memory terms2_ = abi.encodePacked(false, address(token), address(delegator), uint256(100)); // Increase for recipient vm.prank(dm); @@ -96,7 +96,7 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { } // Validates that a balance decrease within the allowed range passes. - // For decreases (flag = false), the enforcer now checks that the final balance is not below the cached balance minus the + // For decreases (flag = true), the enforcer now checks that the final balance is not below the cached balance minus the // allowed amount. // Example: if the cached balance is 100 and the allowed decrease is 10, the final balance must be at least 90. function test_allow_ifBalanceDoesNotDecreaseTooMuch() public { @@ -105,8 +105,8 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { vm.prank(delegator); token.mint(recipient, initialBalance_); - // Terms: flag=false (decrease expected), token, recipient, allowed decrease amount = 10. - bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(10)); + // Terms: flag=true (decrease expected), token, recipient, allowed decrease amount = 10. + bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(10)); // Cache the initial balance via beforeHook. vm.prank(dm); @@ -127,8 +127,8 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { vm.prank(delegator); token.mint(recipient, initialBalance_); - // Terms: flag=false (decrease expected), token, recipient, allowed maximum decrease = 10. - bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(10)); + // Terms: flag=true (decrease expected), token, recipient, allowed maximum decrease = 10. + bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(10)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); @@ -146,8 +146,8 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Reverts if an increase hasn't been sufficient function test_notAllow_insufficientIncrease() public { - // Terms: flag=true (increase expected), required increase of 100 tokens. - bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + // Terms: flag=false (increase expected), required increase of 100 tokens. + bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(100)); // Mint only 10 tokens (insufficient increase) vm.prank(dm); @@ -161,7 +161,7 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Reverts if the enforcer is locked (i.e. reentrant beforeHook) function test_notAllow_reenterALockedEnforcer() public { - bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(100)); bytes32 delegationHash_ = bytes32(uint256(99999999)); vm.startPrank(dm); @@ -189,7 +189,7 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Reverts if no increase happens when one is expected function test_notAllow_noIncreaseToRecipient() public { - bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(100)); // Cache the initial balance. vm.prank(dm); @@ -206,26 +206,26 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { bytes memory terms_; // Too small: missing required bytes (should be 73 bytes) - terms_ = abi.encodePacked(true, address(token), address(recipient), uint8(100)); + terms_ = abi.encodePacked(false, address(token), address(recipient), uint8(100)); vm.expectRevert(bytes("ERC20BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); // Too large: extra bytes beyond 73. - terms_ = abi.encodePacked(true, address(token), address(recipient), uint256(100), uint256(100)); + terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(100), uint256(100)); vm.expectRevert(bytes("ERC20BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); } // Validates that an invalid token address (address(0)) reverts when calling beforeHook. function test_invalid_tokenAddress() public { - bytes memory terms_ = abi.encodePacked(true, address(0), address(recipient), uint256(100)); + bytes memory terms_ = abi.encodePacked(false, address(0), address(recipient), uint256(100)); vm.expectRevert(); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); } // Reverts when the balance increase triggers an overflow. function test_notAllow_expectingOverflow() public { - bytes memory terms_ = abi.encodePacked(true, address(token), address(recipient), type(uint256).max); + bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), type(uint256).max); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), delegator, delegate); diff --git a/test/enforcers/ERC721BalanceChangeEnforcer.t.sol b/test/enforcers/ERC721BalanceChangeEnforcer.t.sol index 12ff13c5..09463490 100644 --- a/test/enforcers/ERC721BalanceChangeEnforcer.t.sol +++ b/test/enforcers/ERC721BalanceChangeEnforcer.t.sol @@ -43,8 +43,8 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates the terms get decoded correctly (increase scenario) function test_decodedTheTerms() public { bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); - (bool shouldBalanceIncrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); - assertTrue(shouldBalanceIncrease_); + (bool isDecrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); + assertTrue(isDecrease_); assertEq(token_, address(token)); assertEq(recipient_, delegator); assertEq(amount_, 1); @@ -53,7 +53,7 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates that a balance has increased by at least the expected amount function test_allow_ifBalanceIncreases() public { // Expect increase by at least 1 - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); // Increase by 1 vm.prank(dm); @@ -75,7 +75,7 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Reverts if a balance hasn't increased by the set amount function test_notAllow_insufficientIncrease() public { // Expect increase by at least 1 - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); // No minting occurs here, so balance remains unchanged. vm.prank(dm); @@ -92,7 +92,7 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { token.mint(delegator); // Expect an increase by at least 1. - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -115,7 +115,7 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { for (uint256 i = 0; i < recipients_.length; i++) { address currentRecipient_ = recipients_[i]; - bytes memory terms_ = abi.encodePacked(true, address(token), currentRecipient_, uint256(1)); + bytes memory terms_ = abi.encodePacked(false, address(token), currentRecipient_, uint256(1)); // Increase by 1 for each recipient. vm.prank(dm); @@ -134,7 +134,7 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { token.mint(delegator); // Expect balance to increase by at least 1. - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -150,8 +150,8 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { bytes32 delegationHash_ = bytes32(uint256(99999999)); address recipient2_ = address(1111111); // Terms for two different recipients. - bytes memory terms1_ = abi.encodePacked(true, address(token), delegator, uint256(1)); - bytes memory terms2_ = abi.encodePacked(true, address(token), recipient2_, uint256(1)); + bytes memory terms1_ = abi.encodePacked(false, address(token), delegator, uint256(1)); + bytes memory terms2_ = abi.encodePacked(false, address(token), recipient2_, uint256(1)); vm.prank(dm); enforcer.beforeHook(terms1_, hex"", singleDefaultMode, mintExecutionCallData, delegationHash_, address(0), delegate); @@ -184,7 +184,7 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Reverts if the enforcer is locked (i.e. if beforeHook is reentered) function test_notAllow_reenterALockedEnforcer() public { // Expect increase by at least 1. - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); bytes32 delegationHash_ = bytes32(uint256(99999999)); vm.startPrank(dm); @@ -222,8 +222,8 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { uint256 initialBalance_ = token.balanceOf(delegator); assertEq(initialBalance_, 2); - // Set terms with flag = false for decrease, allowed decrease is 1. - bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); + // Set terms with flag = true for decrease, allowed decrease is 1. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -248,8 +248,8 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { uint256 initialBalance = token.balanceOf(delegator); assertEq(initialBalance, 2); - // Terms: flag = false (decrease expected), allowed decrease is 1. - bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1)); + // Terms: flag = true (decrease expected), allowed decrease is 1. + bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); @@ -272,26 +272,26 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { bytes memory terms_; // Too small: missing required bytes. - terms_ = abi.encodePacked(true, address(token), address(delegator), uint8(1)); + terms_ = abi.encodePacked(false, address(token), address(delegator), uint8(1)); vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); // Too large: extra bytes. - terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1), uint256(1)); + terms_ = abi.encodePacked(false, address(token), address(delegator), uint256(1), uint256(1)); vm.expectRevert(bytes("ERC721BalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); } // Validates that an invalid token address causes a revert when calling beforeHook. function test_invalid_tokenAddress() public { - bytes memory terms_ = abi.encodePacked(true, address(0), address(delegator), uint256(1)); + bytes memory terms_ = abi.encodePacked(false, address(0), address(delegator), uint256(1)); vm.expectRevert(); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); } // Reverts if an unrealistic amount triggers overflow. function test_notAllow_expectingOverflow() public { - bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), type(uint256).max); + bytes memory terms_ = abi.encodePacked(false, address(token), address(delegator), type(uint256).max); vm.prank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, mintExecutionCallData, bytes32(0), address(0), delegate); diff --git a/test/enforcers/NativeBalanceChangeEnforcer.t.sol b/test/enforcers/NativeBalanceChangeEnforcer.t.sol index 60c8e28c..02c8722f 100644 --- a/test/enforcers/NativeBalanceChangeEnforcer.t.sol +++ b/test/enforcers/NativeBalanceChangeEnforcer.t.sol @@ -33,12 +33,12 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates the terms get decoded correctly function test_decodedTheTerms() public { - bytes memory terms_ = abi.encodePacked(true, address(users.carol.deleGator), uint256(100)); - bool shouldBalanceIncrease_; + bytes memory terms_ = abi.encodePacked(false, address(users.carol.deleGator), uint256(100)); + bool isDecrease_; uint256 amount_; address recipient_; - (shouldBalanceIncrease_, recipient_, amount_) = enforcer.getTermsInfo(terms_); - assertTrue(shouldBalanceIncrease_); + (isDecrease_, recipient_, amount_) = enforcer.getTermsInfo(terms_); + assertFalse(isDecrease_); assertEq(recipient_, address(users.carol.deleGator)); assertEq(amount_, 100); } @@ -47,7 +47,7 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { function test_allow_ifBalanceIncreases() public { address recipient_ = delegator; // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(true, recipient_, uint256(100)); + bytes memory terms_ = abi.encodePacked(false, recipient_, uint256(100)); // Increase by 100 vm.startPrank(dm); @@ -66,7 +66,7 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { address recipient_ = delegator; vm.deal(recipient_, 1000); // Start with 1000 // Expect it to decrease by at most 100 - bytes memory terms_ = abi.encodePacked(false, recipient_, uint256(100)); + bytes memory terms_ = abi.encodePacked(true, recipient_, uint256(100)); // Decrease by 50 vm.startPrank(dm); @@ -86,7 +86,7 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { function test_notAllow_insufficientIncrease() public { address recipient_ = delegator; // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(true, recipient_, uint256(100)); + bytes memory terms_ = abi.encodePacked(false, recipient_, uint256(100)); // Increase by 10, expect revert vm.startPrank(dm); @@ -101,7 +101,7 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { address recipient_ = delegator; vm.deal(recipient_, 1000); // Start with 1000 // Expect it to decrease by at most 100 - bytes memory terms_ = abi.encodePacked(false, recipient_, uint256(100)); + bytes memory terms_ = abi.encodePacked(true, recipient_, uint256(100)); // Decrease by 150, expect revert vm.startPrank(dm); @@ -115,7 +115,7 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { function test_notAllow_reenterALockedEnforcer() public { address recipient_ = delegator; // Expect it to increase by at least 100 - bytes memory terms_ = abi.encodePacked(true, recipient_, uint256(100)); + bytes memory terms_ = abi.encodePacked(false, recipient_, uint256(100)); bytes32 delegationHash_ = bytes32(uint256(99999999)); // Increase by 100 @@ -143,12 +143,12 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { bytes memory terms_; // Too small - terms_ = abi.encodePacked(true, recipient_, uint8(100)); + terms_ = abi.encodePacked(false, recipient_, uint8(100)); vm.expectRevert(bytes("NativeBalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); // Too large - terms_ = abi.encodePacked(true, uint256(100), uint256(100)); + terms_ = abi.encodePacked(false, uint256(100), uint256(100)); vm.expectRevert(bytes("NativeBalanceChangeEnforcer:invalid-terms-length")); enforcer.getTermsInfo(terms_); } @@ -158,7 +158,7 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { address recipient_ = delegator; // Expect balance to increase so much that the validation overflows - bytes memory terms_ = abi.encodePacked(true, recipient_, type(uint256).max); + bytes memory terms_ = abi.encodePacked(false, recipient_, type(uint256).max); vm.deal(recipient_, type(uint256).max); vm.startPrank(dm); enforcer.beforeHook(terms_, hex"", singleDefaultMode, executionCallData, bytes32(0), delegator, delegate); From ecfd727c44bc69f65f55150253d93812883d8668 Mon Sep 17 00:00:00 2001 From: hanzel98 Date: Tue, 22 Apr 2025 12:46:37 -0600 Subject: [PATCH 3/4] Better comments related to the isIncrease flag value --- src/enforcers/ERC1155BalanceChangeEnforcer.sol | 6 +++--- src/enforcers/ERC20BalanceChangeEnforcer.sol | 6 +++--- src/enforcers/ERC721BalanceChangeEnforcer.sol | 6 +++--- src/enforcers/NativeBalanceChangeEnforcer.sol | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/enforcers/ERC1155BalanceChangeEnforcer.sol b/src/enforcers/ERC1155BalanceChangeEnforcer.sol index 7610c078..d6281d67 100644 --- a/src/enforcers/ERC1155BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC1155BalanceChangeEnforcer.sol @@ -56,7 +56,7 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function caches the recipient's ERC1155 token balance before the delegation is executed. * @param _terms 105 bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the ERC1155 token * - next 20 bytes: address of the recipient * - next 32 bytes: token ID @@ -89,7 +89,7 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function enforces that the recipient's ERC1155 token balance has changed by the expected amount. * @param _terms 105 bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the ERC1155 token * - next 20 bytes: address of the recipient * - next 32 bytes: token ID @@ -123,7 +123,7 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms Encoded data that is used during the execution hooks. - * @return isDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). + * @return isDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). * @return token_ The address of the ERC1155 token. * @return recipient_ The address of the recipient of the token. * @return tokenId_ The ID of the ERC1155 token. diff --git a/src/enforcers/ERC20BalanceChangeEnforcer.sol b/src/enforcers/ERC20BalanceChangeEnforcer.sol index 80820a0e..bdb694e7 100644 --- a/src/enforcers/ERC20BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC20BalanceChangeEnforcer.sol @@ -44,7 +44,7 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function caches the delegators ERC20 balance before the delegation is executed. * @param _terms 73 packed bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the token * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on @@ -75,7 +75,7 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function enforces that the delegators ERC20 balance has changed by the expected amount. * @param _terms 73 packed bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the token * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on @@ -107,7 +107,7 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms encoded data that is used during the execution hooks. - * @return isDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). + * @return isDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). * @return token_ The address of the token. * @return recipient_ The address of the recipient. * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on diff --git a/src/enforcers/ERC721BalanceChangeEnforcer.sol b/src/enforcers/ERC721BalanceChangeEnforcer.sol index b3995f96..88051f91 100644 --- a/src/enforcers/ERC721BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC721BalanceChangeEnforcer.sol @@ -54,7 +54,7 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function caches the delegator's ERC721 token balance before the delegation is executed. * @param _terms 73 bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the ERC721 token * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on @@ -86,7 +86,7 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function enforces that the delegator's ERC721 token balance has changed by the expected amount. * @param _terms 73 bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the ERC721 token * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on @@ -119,7 +119,7 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms Encoded data that is used during the execution hooks. - * @return isDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). + * @return isDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). * @return token_ The address of the ERC721 token. * @return recipient_ The address of the recipient of the token. * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on diff --git a/src/enforcers/NativeBalanceChangeEnforcer.sol b/src/enforcers/NativeBalanceChangeEnforcer.sol index d8bb5a88..153cf603 100644 --- a/src/enforcers/NativeBalanceChangeEnforcer.sol +++ b/src/enforcers/NativeBalanceChangeEnforcer.sol @@ -41,7 +41,7 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function caches the delegator's native token balance before the delegation is executed. * @param _terms 53 packed bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on * isDecrease) @@ -71,7 +71,7 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { /** * @notice This function enforces that the delegator's native token balance has changed by the expected amount. * @param _terms 53 packed bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on * isDecrease) @@ -104,7 +104,7 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms 53 packed bytes where: - * - first byte: boolean indicating if the balance should decrease + * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on * isDecrease) From 06c9064e5dfcc74b0f9fbe2a9f112aac45fe77f7 Mon Sep 17 00:00:00 2001 From: hanzel98 Date: Tue, 22 Apr 2025 15:27:20 -0600 Subject: [PATCH 4/4] Renamed flag isDecrease to enforceDecrease --- .../ERC1155BalanceChangeEnforcer.sol | 18 ++++++++--------- src/enforcers/ERC20BalanceChangeEnforcer.sol | 18 ++++++++--------- src/enforcers/ERC721BalanceChangeEnforcer.sol | 18 ++++++++--------- src/enforcers/NativeBalanceChangeEnforcer.sol | 20 +++++++++---------- .../ERC1155BalanceChangeEnforcer.t.sol | 5 +++-- .../ERC20BalanceChangeEnforcer.t.sol | 4 ++-- .../ERC721BalanceChangeEnforcer.t.sol | 4 ++-- .../NativeBalanceChangeEnforcer.t.sol | 6 +++--- 8 files changed, 47 insertions(+), 46 deletions(-) diff --git a/src/enforcers/ERC1155BalanceChangeEnforcer.sol b/src/enforcers/ERC1155BalanceChangeEnforcer.sol index d6281d67..cc60c037 100644 --- a/src/enforcers/ERC1155BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC1155BalanceChangeEnforcer.sol @@ -10,7 +10,7 @@ import { ModeCode } from "../utils/Types.sol"; * @title ERC1155BalanceChangeEnforcer * @dev This contract allows setting up some guardrails around balance changes. By specifying an amount and a direction * (decrease/increase), one can enforce a maximum decrease or minimum increase in after-execution balance. - * The change can be either a decrease or increase based on the `isDecrease` flag. + * The change can be either a decrease or increase based on the `enforceDecrease` flag. * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to * create granular permissions. * @dev This enforcer operates only in default execution mode. @@ -61,7 +61,7 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { * - next 20 bytes: address of the recipient * - next 32 bytes: token ID * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) * @param _mode The execution mode. (Must be Default execType) * @param _delegationHash The hash of the delegation. */ @@ -94,7 +94,7 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { * - next 20 bytes: address of the recipient * - next 32 bytes: token ID * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) * @param _delegationHash The hash of the delegation. */ function afterHook( @@ -109,11 +109,11 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { public override { - (bool isDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = getTermsInfo(_terms); + (bool enforceDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, recipient_, tokenId_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC1155(token_).balanceOf(recipient_, tokenId_); - if (isDecrease_) { + if (enforceDecrease_) { require(balance_ >= balanceCache[hashKey_] - amount_, "ERC1155BalanceChangeEnforcer:exceeded-balance-decrease"); } else { require(balance_ >= balanceCache[hashKey_] + amount_, "ERC1155BalanceChangeEnforcer:insufficient-balance-increase"); @@ -123,20 +123,20 @@ contract ERC1155BalanceChangeEnforcer is CaveatEnforcer { /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms Encoded data that is used during the execution hooks. - * @return isDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). + * @return enforceDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). * @return token_ The address of the ERC1155 token. * @return recipient_ The address of the recipient of the token. * @return tokenId_ The ID of the ERC1155 token. * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) */ function getTermsInfo(bytes calldata _terms) public pure - returns (bool isDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) + returns (bool enforceDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) { require(_terms.length == 105, "ERC1155BalanceChangeEnforcer:invalid-terms-length"); - isDecrease_ = _terms[0] != 0; + enforceDecrease_ = _terms[0] != 0; token_ = address(bytes20(_terms[1:21])); recipient_ = address(bytes20(_terms[21:41])); tokenId_ = uint256(bytes32(_terms[41:73])); diff --git a/src/enforcers/ERC20BalanceChangeEnforcer.sol b/src/enforcers/ERC20BalanceChangeEnforcer.sol index bdb694e7..b94f26d9 100644 --- a/src/enforcers/ERC20BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC20BalanceChangeEnforcer.sol @@ -10,7 +10,7 @@ import { ModeCode } from "../utils/Types.sol"; * @title ERC20BalanceChangeEnforcer * @dev This contract allows setting up some guardrails around balance changes. By specifying an amount and a direction * (decrease/increase), one can enforce a maximum decrease or minimum increase in after-execution balance. - * The change can be either a decrease or increase based on the `isDecrease` flag. + * The change can be either a decrease or increase based on the `enforceDecrease` flag. * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to * create granular permissions. * @dev This enforcer operates only in default execution mode. @@ -48,7 +48,7 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { * - next 20 bytes: address of the token * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) * @param _mode The execution mode. (Must be Default execType) */ function beforeHook( @@ -79,7 +79,7 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { * - next 20 bytes: address of the token * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) */ function afterHook( bytes calldata _terms, @@ -93,11 +93,11 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { public override { - (bool isDecrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); + (bool enforceDecrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC20(token_).balanceOf(recipient_); - if (isDecrease_) { + if (enforceDecrease_) { require(balance_ >= balanceCache[hashKey_] - amount_, "ERC20BalanceChangeEnforcer:exceeded-balance-decrease"); } else { require(balance_ >= balanceCache[hashKey_] + amount_, "ERC20BalanceChangeEnforcer:insufficient-balance-increase"); @@ -107,19 +107,19 @@ contract ERC20BalanceChangeEnforcer is CaveatEnforcer { /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms encoded data that is used during the execution hooks. - * @return isDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). + * @return enforceDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). * @return token_ The address of the token. * @return recipient_ The address of the recipient. * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) */ function getTermsInfo(bytes calldata _terms) public pure - returns (bool isDecrease_, address token_, address recipient_, uint256 amount_) + returns (bool enforceDecrease_, address token_, address recipient_, uint256 amount_) { require(_terms.length == 73, "ERC20BalanceChangeEnforcer:invalid-terms-length"); - isDecrease_ = _terms[0] != 0; + enforceDecrease_ = _terms[0] != 0; token_ = address(bytes20(_terms[1:21])); recipient_ = address(bytes20(_terms[21:41])); amount_ = uint256(bytes32(_terms[41:])); diff --git a/src/enforcers/ERC721BalanceChangeEnforcer.sol b/src/enforcers/ERC721BalanceChangeEnforcer.sol index 88051f91..20ca5cc7 100644 --- a/src/enforcers/ERC721BalanceChangeEnforcer.sol +++ b/src/enforcers/ERC721BalanceChangeEnforcer.sol @@ -10,7 +10,7 @@ import { ModeCode } from "../utils/Types.sol"; * @title ERC721BalanceChangeEnforcer * @dev This contract allows setting up some guardrails around balance changes. By specifying an amount and a direction * (decrease/increase), one can enforce a maximum decrease or minimum increase in after-execution balance. - * The change can be either a decrease or increase based on the `isDecrease` flag. + * The change can be either a decrease or increase based on the `enforceDecrease` flag. * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to * create granular permissions. * @dev This enforcer operates only in default execution mode. @@ -58,7 +58,7 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { * - next 20 bytes: address of the ERC721 token * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) * @param _mode The execution mode. (Must be Default execType) * @param _delegationHash The hash of the delegation. */ @@ -90,7 +90,7 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { * - next 20 bytes: address of the ERC721 token * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) * @param _delegationHash The hash of the delegation. */ function afterHook( @@ -105,11 +105,11 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { public override { - (bool isDecrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); + (bool enforceDecrease_, address token_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, token_, recipient_, _delegationHash); delete isLocked[hashKey_]; uint256 balance_ = IERC721(token_).balanceOf(recipient_); - if (isDecrease_) { + if (enforceDecrease_) { require(balance_ >= balanceCache[hashKey_] - amount_, "ERC721BalanceChangeEnforcer:exceeded-balance-decrease"); } else { require(balance_ >= balanceCache[hashKey_] + amount_, "ERC721BalanceChangeEnforcer:insufficient-balance-increase"); @@ -119,19 +119,19 @@ contract ERC721BalanceChangeEnforcer is CaveatEnforcer { /** * @notice Decodes the terms used in this CaveatEnforcer. * @param _terms Encoded data that is used during the execution hooks. - * @return isDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). + * @return enforceDecrease_ Boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00). * @return token_ The address of the ERC721 token. * @return recipient_ The address of the recipient of the token. * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) */ function getTermsInfo(bytes calldata _terms) public pure - returns (bool isDecrease_, address token_, address recipient_, uint256 amount_) + returns (bool enforceDecrease_, address token_, address recipient_, uint256 amount_) { require(_terms.length == 73, "ERC721BalanceChangeEnforcer:invalid-terms-length"); - isDecrease_ = _terms[0] != 0; + enforceDecrease_ = _terms[0] != 0; token_ = address(bytes20(_terms[1:21])); recipient_ = address(bytes20(_terms[21:41])); amount_ = uint256(bytes32(_terms[41:])); diff --git a/src/enforcers/NativeBalanceChangeEnforcer.sol b/src/enforcers/NativeBalanceChangeEnforcer.sol index 153cf603..4ee08532 100644 --- a/src/enforcers/NativeBalanceChangeEnforcer.sol +++ b/src/enforcers/NativeBalanceChangeEnforcer.sol @@ -8,7 +8,7 @@ import { ModeCode } from "../utils/Types.sol"; * @title NativeBalanceChangeEnforcer * @dev This contract allows setting up some guardrails around balance changes. By specifying an amount and a direction * (decrease/increase), one can enforce a maximum decrease or minimum increase in after-execution balance. - * The change can be either a decrease or increase based on the `isDecrease` flag. + * The change can be either a decrease or increase based on the `enforceDecrease` flag. * @dev This contract has no enforcement of how the balance changes. It's meant to be used alongside additional enforcers to * create granular permissions. * @dev This enforcer operates only in default execution mode. @@ -44,7 +44,7 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) * @param _mode The execution mode. (Must be Default execType) * @param _delegationHash The hash of the delegation. */ @@ -74,7 +74,7 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) * @param _delegationHash The hash of the delegation. */ function afterHook( @@ -89,10 +89,10 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { public override { - (bool isDecrease_, address recipient_, uint256 amount_) = getTermsInfo(_terms); + (bool enforceDecrease_, address recipient_, uint256 amount_) = getTermsInfo(_terms); bytes32 hashKey_ = _getHashKey(msg.sender, _delegationHash); delete isLocked[hashKey_]; - if (isDecrease_) { + if (enforceDecrease_) { require(recipient_.balance >= balanceCache[hashKey_] - amount_, "NativeBalanceChangeEnforcer:exceeded-balance-decrease"); } else { require( @@ -107,15 +107,15 @@ contract NativeBalanceChangeEnforcer is CaveatEnforcer { * - first byte: boolean indicating if the balance should decrease (true | 0x01) or increase (false | 0x00) * - next 20 bytes: address of the recipient * - next 32 bytes: balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) - * @return isDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). + * enforceDecrease) + * @return enforceDecrease_ Boolean indicating if the balance should decrease (true) or increase (false). * @return recipient_ The address of the recipient whose balance will change. * @return amount_ Balance change guardrail amount (i.e., minimum increase OR maximum decrease, depending on - * isDecrease) + * enforceDecrease) */ - function getTermsInfo(bytes calldata _terms) public pure returns (bool isDecrease_, address recipient_, uint256 amount_) { + function getTermsInfo(bytes calldata _terms) public pure returns (bool enforceDecrease_, address recipient_, uint256 amount_) { require(_terms.length == 53, "NativeBalanceChangeEnforcer:invalid-terms-length"); - isDecrease_ = _terms[0] != 0; + enforceDecrease_ = _terms[0] != 0; recipient_ = address(bytes20(_terms[1:21])); amount_ = uint256(bytes32(_terms[21:])); } diff --git a/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol b/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol index 41e57896..3227e3b0 100644 --- a/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol +++ b/test/enforcers/ERC1155BalanceChangeEnforcer.t.sol @@ -47,8 +47,9 @@ contract ERC1155BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Terms format: [bool shouldBalanceIncrease, address token, address recipient, uint256 tokenId, uint256 amount] function test_decodedTheTerms() public { bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(tokenId), uint256(100)); - (bool isDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = enforcer.getTermsInfo(terms_); - assertEq(isDecrease_, true); + (bool enforceDecrease_, address token_, address recipient_, uint256 tokenId_, uint256 amount_) = + enforcer.getTermsInfo(terms_); + assertEq(enforceDecrease_, true); assertEq(token_, address(token)); assertEq(recipient_, delegator); assertEq(tokenId_, tokenId); diff --git a/test/enforcers/ERC20BalanceChangeEnforcer.t.sol b/test/enforcers/ERC20BalanceChangeEnforcer.t.sol index b6beeee2..161b7b3b 100644 --- a/test/enforcers/ERC20BalanceChangeEnforcer.t.sol +++ b/test/enforcers/ERC20BalanceChangeEnforcer.t.sol @@ -43,8 +43,8 @@ contract ERC20BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates the terms get decoded correctly for an increase scenario function test_decodedTheTerms() public { bytes memory terms_ = abi.encodePacked(false, address(token), address(recipient), uint256(100)); - (bool isDecrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); - assertEq(isDecrease_, false); + (bool enforceDecrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); + assertEq(enforceDecrease_, false); assertEq(token_, address(token)); assertEq(recipient_, address(recipient)); assertEq(amount_, 100); diff --git a/test/enforcers/ERC721BalanceChangeEnforcer.t.sol b/test/enforcers/ERC721BalanceChangeEnforcer.t.sol index 09463490..43fc94fc 100644 --- a/test/enforcers/ERC721BalanceChangeEnforcer.t.sol +++ b/test/enforcers/ERC721BalanceChangeEnforcer.t.sol @@ -43,8 +43,8 @@ contract ERC721BalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates the terms get decoded correctly (increase scenario) function test_decodedTheTerms() public { bytes memory terms_ = abi.encodePacked(true, address(token), address(delegator), uint256(1)); - (bool isDecrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); - assertTrue(isDecrease_); + (bool enforceDecrease_, address token_, address recipient_, uint256 amount_) = enforcer.getTermsInfo(terms_); + assertTrue(enforceDecrease_); assertEq(token_, address(token)); assertEq(recipient_, delegator); assertEq(amount_, 1); diff --git a/test/enforcers/NativeBalanceChangeEnforcer.t.sol b/test/enforcers/NativeBalanceChangeEnforcer.t.sol index 02c8722f..9b5760f8 100644 --- a/test/enforcers/NativeBalanceChangeEnforcer.t.sol +++ b/test/enforcers/NativeBalanceChangeEnforcer.t.sol @@ -34,11 +34,11 @@ contract NativeBalanceChangeEnforcerTest is CaveatEnforcerBaseTest { // Validates the terms get decoded correctly function test_decodedTheTerms() public { bytes memory terms_ = abi.encodePacked(false, address(users.carol.deleGator), uint256(100)); - bool isDecrease_; + bool enforceDecrease_; uint256 amount_; address recipient_; - (isDecrease_, recipient_, amount_) = enforcer.getTermsInfo(terms_); - assertFalse(isDecrease_); + (enforceDecrease_, recipient_, amount_) = enforcer.getTermsInfo(terms_); + assertFalse(enforceDecrease_); assertEq(recipient_, address(users.carol.deleGator)); assertEq(amount_, 100); }