Making things a little more clearer

gnosisguild · Jan 16, 2022 · 7ed6b0b · 7ed6b0b
1 parent 7917d0e
commit 7ed6b0b
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 19 deletions.
diff --git a/contracts/Permissions.sol b/contracts/Permissions.sol
@@ -670,25 +670,22 @@ library Permissions {
          * Note: The offset to encoded data payload is relative, (minus 32 bytes that include over buffer length and minut 4 bytes for functionSig)
          *
          * We call the byte offset encoded data payload -> "offsetPointer".
-         * At offsetPointer, there is another offset stored, the "offsetPayloadRelative"
-         * The offsetPayload points at the beggining of the encoded payload. The first 32 bytes of the payload area contain the payload's length. Depending on ParameterType:
+         * At offsetPointer, there is another offset stored, the "offsetPayload"
+         * The offsetPayload points to the beggining of the encoded payload. The first 32 bytes of the payload area contain the payload's length. Depending on ParameterType:
          * Dynamic   -> length in bytes
          * Dynamic32 -> length in bytes32
          *
          * Note: Dynamic types are: bytes, string
          * Note: Dynamic32 types are all non-nested arrays: address[] bytes32[] uint[] etc
          */
 
-        // offsetPointer         - real     offset in bytes to the location
-        // offsetPayloadRelative - relative offset in bytes (without buffer lenth and functionSig)
-        // offsetPayload         - real     offset in bytes
-
         uint256 offsetPointer = 32 + 4 + paramIndex * 32;
-        uint256 offsetPayloadRelative;
         uint256 offsetPayload;
         assembly {
-            offsetPayloadRelative := mload(add(data, offsetPointer))
-            offsetPayload := add(32, add(4, offsetPayloadRelative))
+            offsetPayload := mload(add(data, offsetPointer))
+            // the stored offset is actually relative.
+            // Must adjust to account for overral buffer lenthg and functionSig
+            offsetPayload := add(32, add(4, offsetPayload))
         }
 
         uint256 lengthPayload;

diff --git a/test/PluckParamAndDecoding.spec.ts b/test/PluckParamAndDecoding.spec.ts
@@ -86,16 +86,29 @@ describe("PluckParam - Decoding", async () => {
         OPTIONS_NONE
       );
 
-    const { data } = await testPluckParam.populateTransaction.staticDynamic(
-      "0x12345678",
-      "Hello World!"
-    );
+    const { data: dataGood } =
+      await testPluckParam.populateTransaction.staticDynamic(
+        "0x12345678",
+        "Hello World!"
+      );
+
+    const { data: dataBad } =
+      await testPluckParam.populateTransaction.staticDynamic(
+        "0x12345678",
+        "Good Morning!"
+      );
 
     await expect(
       modifier
         .connect(invoker)
-        .execTransactionFromModule(testPluckParam.address, 0, data, 0)
+        .execTransactionFromModule(testPluckParam.address, 0, dataGood, 0)
     ).to.emit(testPluckParam, "StaticDynamic");
+
+    await expect(
+      modifier
+        .connect(invoker)
+        .execTransactionFromModule(testPluckParam.address, 0, dataBad, 0)
+    ).to.be.revertedWith("ParameterNotAllowed()");
   });
 
   it("static, dynamic, dynamic32 - (address,bytes,uint32[])", async () => {
@@ -183,7 +196,7 @@ describe("PluckParam - Decoding", async () => {
       await testPluckParam.populateTransaction.staticDynamic32Dynamic(
         [123],
         ["0xabcdef12"],
-        "Good Morning!"
+        "Hello World?"
       );
 
     await expect(
@@ -233,8 +246,8 @@ describe("PluckParam - Decoding", async () => {
     const { data: dataBad } =
       await testPluckParam.populateTransaction.dynamicStaticDynamic32(
         "0x12ab45",
-        true,
-        ["0x1122", "0x3344"]
+        false,
+        ["0x1122", "0x3344", "0x5566"]
       );
 
     await expect(
@@ -283,8 +296,8 @@ describe("PluckParam - Decoding", async () => {
     const { data: dataBad } =
       await testPluckParam.populateTransaction.dynamicDynamic32Static(
         "Hello World!",
-        [1975, 2000, 2025],
-        987654321
+        [1975, 2000],
+        123456789
       );
 
     await expect(