Make operation available to CustomConditionChecker.

gnosisguild · Nov 29, 2023 · a19c0eb · a19c0eb
1 parent d4b6539
commit a19c0eb
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -111,15 +111,15 @@ 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(
         testContract.address,
         0,
         (await testContract.populateTransaction.oneParamStatic(a))
           .data as string,
-        0
+        operation
       );
   }