From a19c0ebda97f7d645335f2c386818546641f832b Mon Sep 17 00:00:00 2001 From: cristovaoth Date: Wed, 29 Nov 2023 13:08:56 +0100 Subject: [PATCH] Make operation available to CustomConditionChecker. --- packages/evm/contracts/PermissionChecker.sol | 15 ++++++++- .../adapters/AvatarIsOwnerOfERC721.sol | 1 + packages/evm/contracts/adapters/Types.sol | 1 + .../evm/contracts/test/TestCustomChecker.sol | 5 +++ packages/evm/test/operators/22Custom.spec.ts | 33 +++++++++++++++++++ packages/evm/test/operators/setup.ts | 4 +-- 6 files changed, 56 insertions(+), 3 deletions(-) diff --git a/packages/evm/contracts/PermissionChecker.sol b/packages/evm/contracts/PermissionChecker.sol index 79ec616f8..756d78c51 100644 --- a/packages/evm/contracts/PermissionChecker.sol +++ b/packages/evm/contracts/PermissionChecker.sol @@ -161,7 +161,12 @@ abstract contract PermissionChecker is Core, Periphery { role, key, data, - Context({to: to, value: value, consumptions: consumptions}) + Context({ + to: to, + value: value, + operation: operation, + consumptions: consumptions + }) ); } else if (role.targets[to].clearance == Clearance.Target) { return ( @@ -304,6 +309,7 @@ abstract contract PermissionChecker is Core, Periphery { Context({ to: context.to, value: context.value, + operation: context.operation, consumptions: result.consumptions }) ); @@ -340,6 +346,7 @@ abstract contract PermissionChecker is Core, Periphery { Context({ to: context.to, value: context.value, + operation: context.operation, consumptions: result.consumptions }) ); @@ -375,6 +382,7 @@ abstract contract PermissionChecker is Core, Periphery { Context({ to: context.to, value: context.value, + operation: context.operation, consumptions: result.consumptions }) ); @@ -433,6 +441,7 @@ abstract contract PermissionChecker is Core, Periphery { Context({ to: context.to, value: context.value, + operation: context.operation, consumptions: result.consumptions }) ); @@ -465,6 +474,7 @@ abstract contract PermissionChecker is Core, Periphery { Context({ to: context.to, value: context.value, + operation: context.operation, consumptions: result.consumptions }) ); @@ -509,6 +519,7 @@ abstract contract PermissionChecker is Core, Periphery { Context({ to: context.to, value: context.value, + operation: context.operation, consumptions: result.consumptions }) ); @@ -626,6 +637,7 @@ abstract contract PermissionChecker is Core, Periphery { context.to, context.value, data, + context.operation, payload.location, payload.size, extra @@ -704,6 +716,7 @@ abstract contract PermissionChecker is Core, Periphery { address to; uint256 value; Consumption[] consumptions; + Enum.Operation operation; } struct Result { diff --git a/packages/evm/contracts/adapters/AvatarIsOwnerOfERC721.sol b/packages/evm/contracts/adapters/AvatarIsOwnerOfERC721.sol index 99a1f60f5..fb34fcc0c 100644 --- a/packages/evm/contracts/adapters/AvatarIsOwnerOfERC721.sol +++ b/packages/evm/contracts/adapters/AvatarIsOwnerOfERC721.sol @@ -15,6 +15,7 @@ contract AvatarIsOwnerOfERC721 is ICustomCondition { address to, uint256 /* value */, bytes calldata data, + Enum.Operation /* operation */, uint256 location, uint256 size, bytes12 /* extra */ diff --git a/packages/evm/contracts/adapters/Types.sol b/packages/evm/contracts/adapters/Types.sol index f27b5648c..7e39b7765 100644 --- a/packages/evm/contracts/adapters/Types.sol +++ b/packages/evm/contracts/adapters/Types.sol @@ -30,6 +30,7 @@ interface ICustomCondition { address to, uint256 value, bytes calldata data, + Enum.Operation operation, uint256 location, uint256 size, bytes12 extra diff --git a/packages/evm/contracts/test/TestCustomChecker.sol b/packages/evm/contracts/test/TestCustomChecker.sol index 8f5de1639..73e92507b 100644 --- a/packages/evm/contracts/test/TestCustomChecker.sol +++ b/packages/evm/contracts/test/TestCustomChecker.sol @@ -8,12 +8,17 @@ contract TestCustomChecker is ICustomCondition { address, uint256, bytes calldata data, + Enum.Operation operation, uint256 location, uint256 size, bytes12 extra ) public pure returns (bool success, bytes32 reason) { uint256 param = uint256(bytes32(data[location:location + size])); + if (operation != Enum.Operation.Call) { + return (false, bytes32(0)); + } + if (param > 100) { return (true, 0); } else { diff --git a/packages/evm/test/operators/22Custom.spec.ts b/packages/evm/test/operators/22Custom.spec.ts index 99a85e20a..b1112db66 100644 --- a/packages/evm/test/operators/22Custom.spec.ts +++ b/packages/evm/test/operators/22Custom.spec.ts @@ -5,6 +5,7 @@ import { expect } from "chai"; import { setupOneParamStatic } from "./setup"; import { BYTES32_ZERO, + ExecutionOptions, Operator, ParameterType, PermissionCheckerStatus, @@ -75,6 +76,38 @@ describe("Operator - Custom", async () => { `0xaabbccddeeff1122334455660000000000000000000000000000000000000000` ); }); + it("evaluates operator Custom - result is check fail due to operation", async () => { + const { roles, customChecker, scopeFunction, invoke } = await loadFixture( + setup + ); + + const extra = "aabbccddeeff112233445566"; + await scopeFunction( + [ + { + parent: 0, + paramType: ParameterType.Calldata, + operator: Operator.Matches, + compValue: "0x", + }, + { + parent: 0, + paramType: ParameterType.Static, + operator: Operator.Custom, + compValue: `${customChecker.address}${extra}`, + }, + ], + ExecutionOptions.Both + ); + + await expect(invoke(101, 1)) + .to.be.revertedWithCustomError(roles, "ConditionViolation") + .withArgs( + PermissionCheckerStatus.CustomConditionViolation, + `0x0000000000000000000000000000000000000000000000000000000000000000` + ); + }); + it.skip("adapter does not implement ICustomChecker", async () => { const { roles, scopeFunction, invoke } = await loadFixture(setup); diff --git a/packages/evm/test/operators/setup.ts b/packages/evm/test/operators/setup.ts index a5e857032..d7ba5f2eb 100644 --- a/packages/evm/test/operators/setup.ts +++ b/packages/evm/test/operators/setup.ts @@ -111,7 +111,7 @@ export async function setupOneParamStatic() { const { owner, invoker, roles, testContract, scopeFunction } = await baseSetup("oneParamStatic"); - async function invoke(a: BigNumberish) { + async function invoke(a: BigNumberish, operation: 0 | 1 = 0) { return roles .connect(invoker) .execTransactionFromModule( @@ -119,7 +119,7 @@ export async function setupOneParamStatic() { 0, (await testContract.populateTransaction.oneParamStatic(a)) .data as string, - 0 + operation ); }