Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Zero Value Enforcers #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions src/enforcers/NoCalldataEnforcer.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// SPDX-License-Identifier: MIT AND Apache-2.0
pragma solidity 0.8.23;

import { ExecutionLib } from "@erc7579/lib/ExecutionLib.sol";
import { ModeLib } from "@erc7579/lib/ModeLib.sol";

import { CaveatEnforcer } from "./CaveatEnforcer.sol";
import { Execution, ModeCode } from "../utils/Types.sol";
import { CALLTYPE_SINGLE, CALLTYPE_BATCH } from "../utils/Constants.sol";

/**
* @title NoCalldataEnforcer
Copy link
Contributor

@hanzel98 hanzel98 Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some places inside our contracts where calldata is lowercase but in others it is callData

* @dev This contract enforces that the execution has no calldata.
* @dev This caveat enforcer only works when the execution is in single mode.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @dev This caveat enforcer only works when the execution is in single mode.
* @dev This caveat enforcer only works in single and batch modes.

*/
contract NoCalldataEnforcer is CaveatEnforcer {
using ExecutionLib for bytes;

////////////////////////////// Public Methods //////////////////////////////

/**
* @notice Allows the delegator to restrict the calldata that is executed
* @dev This function enforces that the execution has no calldata.
* @param _mode The execution mode for the execution.
* @param _executionCallData The execution the delegate is trying try to execute.
*/
function beforeHook(
bytes calldata,
bytes calldata,
ModeCode _mode,
bytes calldata _executionCallData,
bytes32,
address,
address
)
public
pure
override
{
if (ModeLib.getCallType(_mode) == CALLTYPE_SINGLE) {
(,, bytes calldata callData_) = _executionCallData.decodeSingle();
require(callData_.length == 0, "NoCalldataEnforcer:calldata-not-allowed");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit concerned about the errors being exactly the same, it might help to differentiate them

} else if (ModeLib.getCallType(_mode) == CALLTYPE_BATCH) {
(Execution[] calldata executions_) = _executionCallData.decodeBatch();
for (uint256 i = 0; i < executions_.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (uint256 i = 0; i < executions_.length; i++) {
for (uint256 i = 0; i < executions_.length; ++i) {

require(executions_[i].callData.length == 0, "NoCalldataEnforcer:calldata-not-allowed");
}
} else {
revert("NoCalldataEnforcer:invalid-calltype");
}
}
}
52 changes: 52 additions & 0 deletions src/enforcers/NoValueEnforcer.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// SPDX-License-Identifier: MIT AND Apache-2.0
pragma solidity 0.8.23;

import { ExecutionLib } from "@erc7579/lib/ExecutionLib.sol";
import { ModeLib } from "@erc7579/lib/ModeLib.sol";

import { CaveatEnforcer } from "./CaveatEnforcer.sol";
import { Execution, ModeCode } from "../utils/Types.sol";
import { CALLTYPE_SINGLE, CALLTYPE_BATCH } from "../utils/Constants.sol";

/**
* @title NoValueEnforcer
* @dev This contract enforces that the execution has no value.
* @dev This caveat enforcer only works when the execution is in single mode.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @dev This caveat enforcer only works when the execution is in single mode.
* @dev This caveat enforcer only works in single and batch modes

*/
contract NoValueEnforcer is CaveatEnforcer {
using ExecutionLib for bytes;

////////////////////////////// Public Methods //////////////////////////////

/**
* @notice Allows the delegator to restrict the value that is executed
* @dev This function enforces that the execution has no value.
* @param _mode The execution mode for the execution.
* @param _executionCallData The execution the delegate is trying try to execute.
*/
function beforeHook(
bytes calldata,
bytes calldata,
ModeCode _mode,
bytes calldata _executionCallData,
bytes32,
address,
address
)
public
pure
override
{
if (ModeLib.getCallType(_mode) == CALLTYPE_SINGLE) {
(, uint256 value_,) = _executionCallData.decodeSingle();
require(value_ == 0, "NoValueEnforcer:value-not-allowed");
} else if (ModeLib.getCallType(_mode) == CALLTYPE_BATCH) {
(Execution[] calldata executions_) = _executionCallData.decodeBatch();
for (uint256 i = 0; i < executions_.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (uint256 i = 0; i < executions_.length; i++) {
for (uint256 i = 0; i < executions_.length; ++i) {

require(executions_[i].value == 0, "NoValueEnforcer:value-not-allowed");
}
} else {
revert("NoValueEnforcer:invalid-calltype");
}
}
}
2 changes: 1 addition & 1 deletion test/enforcers/AllowedCalldataEnforcer.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract AllowedCalldataEnforcerTest is CaveatEnforcerBaseTest {
function setUp() public override {
super.setUp();
allowedCalldataEnforcer = new AllowedCalldataEnforcer();
vm.label(address(allowedCalldataEnforcer), "Equal Parameters Enforcer");
vm.label(address(allowedCalldataEnforcer), "Allowed Calldata Enforcer");
basicCF20 = new BasicERC20(address(users.alice.deleGator), "TestToken1", "TestToken1", 100 ether);
basicCF721 = new BasicCF721(address(users.alice.deleGator), "TestNFT", "TestNFT", "");
}
Expand Down
118 changes: 118 additions & 0 deletions test/enforcers/NoCalldataEnforcer.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// SPDX-License-Identifier: MIT AND Apache-2.0
pragma solidity 0.8.23;

import "forge-std/Test.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { BytesLib } from "@bytes-utils/BytesLib.sol";
import { ModeLib } from "@erc7579/lib/ModeLib.sol";
import { ExecutionLib } from "@erc7579/lib/ExecutionLib.sol";

import { Counter } from "../utils/Counter.t.sol";
import { Execution, Caveat, Delegation, ModeCode } from "../../src/utils/Types.sol";
import { CaveatEnforcerBaseTest } from "./CaveatEnforcerBaseTest.t.sol";
import { NoCalldataEnforcer } from "../../src/enforcers/NoCalldataEnforcer.sol";
import { IDelegationManager } from "../../src/interfaces/IDelegationManager.sol";
import { EncoderLib } from "../../src/libraries/EncoderLib.sol";
import { BasicERC20, IERC20 } from "../utils/BasicERC20.t.sol";
import { BasicCF721 } from "../utils/BasicCF721.t.sol";
import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol";

contract DummyContract {
function stringFn(uint256[] calldata _str) public { }
function arrayFn(string calldata _str) public { }
}

contract NoCalldataEnforcerTest is CaveatEnforcerBaseTest {
using ModeLib for ModeCode;

////////////////////////////// State //////////////////////////////
NoCalldataEnforcer public noCalldataEnforcer;
DummyContract public c;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe DummyContract public dummy; instead


ModeCode public singleMode = ModeLib.encodeSimpleSingle();
ModeCode public batchMode = ModeLib.encodeSimpleBatch();

////////////////////// Set up //////////////////////

function setUp() public override {
super.setUp();
noCalldataEnforcer = new NoCalldataEnforcer();
vm.label(address(noCalldataEnforcer), "No Calldata Enforcer");
c = new DummyContract();
}

////////////////////// Valid cases //////////////////////

// should allow an execution in single mode with no calldata
function test_singleMethodNoCalldataIsAllowed() public {
// Create the execution that would be executed
Execution memory execution_ = Execution({ target: address(c), value: 0, callData: hex"" });
bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData);

vm.prank(address(delegationManager));
noCalldataEnforcer.beforeHook(hex"", hex"", singleMode, executionCallData_, keccak256(""), address(0), address(0));
}

// should allow an execution in batch mode with no calldata
function test_batchMethodNoCalldataIsAllowed() public {
// Create the execution that would be executed
Execution memory execution_ = Execution({ target: address(c), value: 0, callData: hex"" });
Execution[] memory executions_ = new Execution[](2);
executions_[0] = execution_;
executions_[1] = execution_;
bytes memory executionCallData_ = ExecutionLib.encodeBatch(executions_);

vm.prank(address(delegationManager));
noCalldataEnforcer.beforeHook(hex"", hex"", batchMode, executionCallData_, keccak256(""), address(0), address(0));
}

////////////////////// Invalid cases //////////////////////

// should not allow an execution in single mode with calldata
function test_singleMethodCalldataIsNotAllowed() public {
// Create the execution that would be executed
Execution memory execution_ = Execution({
target: address(c),
value: 0,
callData: abi.encodeWithSelector(DummyContract.stringFn.selector, uint256(1))
});
bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData);

vm.prank(address(delegationManager));
vm.expectRevert("NoCalldataEnforcer:calldata-not-allowed");
noCalldataEnforcer.beforeHook(hex"", hex"", singleMode, executionCallData_, keccak256(""), address(0), address(0));
}

// should not allow an execution in batch mode with calldata
function test_batchMethodCalldataIsNotAllowed() public {
// Create the execution that would be executed
Execution memory execution_ = Execution({
target: address(c),
value: 0,
callData: abi.encodeWithSelector(DummyContract.stringFn.selector, uint256(1))
});
Execution[] memory executions_ = new Execution[](2);
executions_[0] = execution_;
executions_[1] = execution_;
bytes memory executionCallData_ = ExecutionLib.encodeBatch(executions_);

vm.prank(address(delegationManager));
vm.expectRevert("NoCalldataEnforcer:calldata-not-allowed");
noCalldataEnforcer.beforeHook(hex"", hex"", batchMode, executionCallData_, keccak256(""), address(0), address(0));

// Make a subset of the executions have no calldata
execution_ = Execution({ target: address(c), value: 0, callData: hex"" });
executions_[0] = execution_;
executionCallData_ = ExecutionLib.encodeBatch(executions_);

vm.prank(address(delegationManager));
vm.expectRevert("NoCalldataEnforcer:calldata-not-allowed");
noCalldataEnforcer.beforeHook(hex"", hex"", batchMode, executionCallData_, keccak256(""), address(0), address(0));
}

////////////////////// Integration //////////////////////

function _getEnforcer() internal view override returns (ICaveatEnforcer) {
return ICaveatEnforcer(address(noCalldataEnforcer));
}
}
110 changes: 110 additions & 0 deletions test/enforcers/NoValueEnforcer.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// SPDX-License-Identifier: MIT AND Apache-2.0
pragma solidity 0.8.23;

import "forge-std/Test.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { BytesLib } from "@bytes-utils/BytesLib.sol";
import { ModeLib } from "@erc7579/lib/ModeLib.sol";
import { ExecutionLib } from "@erc7579/lib/ExecutionLib.sol";

import { Counter } from "../utils/Counter.t.sol";
import { Execution, Caveat, Delegation, ModeCode } from "../../src/utils/Types.sol";
import { CaveatEnforcerBaseTest } from "./CaveatEnforcerBaseTest.t.sol";
import { NoValueEnforcer } from "../../src/enforcers/NoValueEnforcer.sol";
import { IDelegationManager } from "../../src/interfaces/IDelegationManager.sol";
import { EncoderLib } from "../../src/libraries/EncoderLib.sol";
import { BasicERC20, IERC20 } from "../utils/BasicERC20.t.sol";
import { BasicCF721 } from "../utils/BasicCF721.t.sol";
import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol";

contract DummyContract {
function stringFn(uint256[] calldata _str) public { }
function arrayFn(string calldata _str) public { }
}

contract NoValueEnforcerTest is CaveatEnforcerBaseTest {
using ModeLib for ModeCode;

////////////////////////////// State //////////////////////////////
NoValueEnforcer public noValueEnforcer;
DummyContract public c;

ModeCode public singleMode = ModeLib.encodeSimpleSingle();
ModeCode public batchMode = ModeLib.encodeSimpleBatch();

////////////////////// Set up //////////////////////

function setUp() public override {
super.setUp();
noValueEnforcer = new NoValueEnforcer();
vm.label(address(noValueEnforcer), "No Value Enforcer");
c = new DummyContract();
}

////////////////////// Valid cases //////////////////////

// should allow an execution in single mode with no calldata
function test_singleMethodNoCalldataIsAllowed() public {
Comment on lines +46 to +47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename tests, these are value tests not calldata

// Create the execution that would be executed
Execution memory execution_ = Execution({ target: address(c), value: 0, callData: hex"" });
bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData);

vm.prank(address(delegationManager));
noValueEnforcer.beforeHook(hex"", hex"", singleMode, executionCallData_, keccak256(""), address(0), address(0));
}

// should allow an execution in batch mode with no calldata
function test_batchMethodNoCalldataIsAllowed() public {
// Create the execution that would be executed
Execution memory execution_ = Execution({ target: address(c), value: 0, callData: hex"" });
Execution[] memory executions_ = new Execution[](2);
executions_[0] = execution_;
executions_[1] = execution_;
bytes memory executionCallData_ = ExecutionLib.encodeBatch(executions_);

vm.prank(address(delegationManager));
noValueEnforcer.beforeHook(hex"", hex"", batchMode, executionCallData_, keccak256(""), address(0), address(0));
}

////////////////////// Invalid cases //////////////////////

// should not allow an execution in single mode with calldata
function test_singleMethodCalldataIsNotAllowed() public {
// Create the execution that would be executed
Execution memory execution_ = Execution({ target: address(c), value: 1, callData: hex"" });
bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData);

vm.prank(address(delegationManager));
vm.expectRevert("NoValueEnforcer:value-not-allowed");
noValueEnforcer.beforeHook(hex"", hex"", singleMode, executionCallData_, keccak256(""), address(0), address(0));
}

// should not allow an execution in batch mode with calldata
function test_batchMethodCalldataIsNotAllowed() public {
// Create the execution that would be executed
Execution memory execution_ = Execution({ target: address(c), value: 1, callData: hex"" });
Execution[] memory executions_ = new Execution[](2);
executions_[0] = execution_;
executions_[1] = execution_;
bytes memory executionCallData_ = ExecutionLib.encodeBatch(executions_);

vm.prank(address(delegationManager));
vm.expectRevert("NoValueEnforcer:value-not-allowed");
noValueEnforcer.beforeHook(hex"", hex"", batchMode, executionCallData_, keccak256(""), address(0), address(0));

// Make a subset of the executions have no calldata
execution_ = Execution({ target: address(c), value: 0, callData: hex"" });
executions_[0] = execution_;
executionCallData_ = ExecutionLib.encodeBatch(executions_);

vm.prank(address(delegationManager));
vm.expectRevert("NoValueEnforcer:value-not-allowed");
noValueEnforcer.beforeHook(hex"", hex"", batchMode, executionCallData_, keccak256(""), address(0), address(0));
}

////////////////////// Integration //////////////////////

function _getEnforcer() internal view override returns (ICaveatEnforcer) {
return ICaveatEnforcer(address(noValueEnforcer));
}
}
Loading