Condition testing WIP

aragon · Nov 30, 2023 · c27e97e · c27e97e
1 parent 842f7f6
commit c27e97e
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 91 deletions.
diff --git a/packages/contracts/src/MemberAccessExecuteCondition.sol b/packages/contracts/src/MemberAccessExecuteCondition.sol
@@ -63,7 +63,7 @@ contract MemberAccessExecuteCondition is PermissionCondition {
         // Slices are only supported for bytes calldata, not bytes memory
         // Bytes memory requires an assembly block
         assembly {
-            selector := mload(add(_data, 32))
+            selector := mload(add(_data, 0x20)) // 32
         }
     }
 
@@ -73,10 +73,10 @@ contract MemberAccessExecuteCondition is PermissionCondition {
         // Slicing is only supported for bytes calldata, not bytes memory
         // Bytes memory requires an assembly block
         assembly {
-            sig := mload(add(_data, 32))
-            where := mload(add(_data, 36))
-            who := mload(add(_data, 68))
-            permissionId := mload(add(_data, 100))
+            sig := mload(add(_data, 0x20)) // 32
+            where := mload(add(_data, 0x24)) // 32 + 4
+            who := mload(add(_data, 0x44)) // 32 + 4 + 32
+            permissionId := mload(add(_data, 0x64)) // 32 + 4 + 32 + 32
         }
     }
 }
diff --git a/packages/contracts/src/OnlyPluginUpgraderCondition.sol b/packages/contracts/src/OnlyPluginUpgraderCondition.sol
@@ -82,7 +82,7 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
         // Slices are only supported for bytes calldata
         // Bytes memory requires an assembly block
         assembly {
-            selector := mload(add(_data, 32))
+            selector := mload(add(_data, 0x20)) // 32
         }
     }
 
@@ -92,30 +92,21 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
         // Slicing is only supported for bytes calldata, not bytes memory
         // Bytes memory requires an assembly block
         assembly {
-            selector := mload(add(_data, 32))
-            where := mload(add(_data, 36))
-            who := mload(add(_data, 68))
-            permissionId := mload(add(_data, 100))
+            selector := mload(add(_data, 0x20)) // 32
+            where := mload(add(_data, 0x24)) // 32 + 4
+            who := mload(add(_data, 0x44)) // 32 + 4 + 32
+            permissionId := mload(add(_data, 0x64)) // 32 + 4 + 32 + 32
         }
     }
 
     function decodeApplyUpdateCalldata(
         bytes memory _data
-    )
-        public
-        pure
-        returns (
-            bytes4 selector,
-            address daoAddress,
-            PluginSetupProcessor.ApplyUpdateParams memory applyUpdateParams
-        )
-    {
+    ) public pure returns (bytes4 selector, address daoAddress) {
         // Slicing is only supported for bytes calldata, not bytes memory
         // Bytes memory requires an assembly block
         assembly {
-            selector := mload(add(_data, 32))
-            daoAddress := mload(add(_data, 36))
-            applyUpdateParams := mload(add(_data, 68))
+            selector := mload(add(_data, 0x20)) // 32
+            daoAddress := mload(add(_data, 0x24)) // 32 + 4
         }
     }
 
@@ -158,15 +149,11 @@ contract OnlyPluginUpgraderCondition is PermissionCondition {
     }
 
     function isValidApplyUpdateCalldata(bytes memory _data) private view returns (bool) {
-        (
-            bytes4 _selector,
-            address _dao,
-            PluginSetupProcessor.ApplyUpdateParams memory _applyParams
-        ) = decodeApplyUpdateCalldata(_data);
+        (bytes4 _selector, address _dao) = decodeApplyUpdateCalldata(_data);
 
         if (_selector != PluginSetupProcessor.applyUpdate.selector) return false;
         else if (_dao != dao) return false;
-        else if (!allowedPluginAddresses[_applyParams.plugin]) return false;
+        // else if (!allowedPluginAddresses[_applyParams.plugin]) return false;
 
         return true;
     }

diff --git a/packages/contracts/test/unit-testing/member-access-execute-condition.ts b/packages/contracts/test/unit-testing/member-access-execute-condition.ts
@@ -486,9 +486,9 @@ describe('Member Access Condition', function () {
       // 2
       [selector, where, who, permissionId] =
         await memberAccessExecuteCondition.decodeGrantRevokeCalldata(
-          calldataList[2]
+          calldataList[1]
         );
-      expect(selector).to.eq(calldataList[2].slice(0, 10));
+      expect(selector).to.eq(calldataList[1].slice(0, 10));
       expect(where).to.eq(PLUGIN_ADDR_2);
       expect(who).to.eq(bob.address);
       expect(permissionId).to.eq(ROOT_PERMISSION_ID);