chore: final review fixes (#317)

alchemyplatform · Dec 11, 2024 · baa8650 · baa8650
1 parent 0258dfa
commit baa8650
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 24 deletions.
diff --git a/src/libraries/ValidationLocatorLib.sol b/src/libraries/ValidationLocatorLib.sol
@@ -47,6 +47,9 @@ type ValidationLocator is uint168;
 // direct call validation flag.
 type ValidationLookupKey is uint168;
 
+using ValidationLocatorLib for ValidationLocator global;
+using ValidationLocatorLib for ValidationLookupKey global;
+
 library ValidationLocatorLib {
     using ValidationConfigLib for ValidationConfig;
 
@@ -183,7 +186,7 @@ library ValidationLocatorLib {
     // Only safe to call if the lookup has been asserted to be a non-direct call validation.
     function entityId(ValidationLookupKey _lookupKey) internal pure returns (uint32 result) {
         assembly ("memory-safe") {
-            result := and(shr(8, _lookupKey), 0xFFFFFFFFFFFFFFFF)
+            result := and(shr(8, _lookupKey), 0xFFFFFFFF)
         }
     }
 
@@ -306,19 +309,15 @@ library ValidationLocatorLib {
         result <<= 64;
     }
 
-    function packSignature(
-        uint32 validationEntityId,
-        bool _isGlobal,
-        bool _hasDeferredAction,
-        bytes memory signature
-    ) internal pure returns (bytes memory result) {
+    function packSignature(uint32 validationEntityId, bool _isGlobal, bytes memory signature)
+        internal
+        pure
+        returns (bytes memory result)
+    {
         uint8 options = 0;
         if (_isGlobal) {
             options |= _VALIDATION_TYPE_GLOBAL;
         }
-        if (_hasDeferredAction) {
-            options |= _HAS_DEFERRED_ACTION;
-        }
 
         return bytes.concat(abi.encodePacked(options, uint32(validationEntityId)), signature);
     }
@@ -372,6 +371,3 @@ library ValidationLocatorLib {
         return ValidationLookupKey.unwrap(a) == ValidationLookupKey.unwrap(b);
     }
 }
-
-using ValidationLocatorLib for ValidationLocator global;
-using ValidationLocatorLib for ValidationLookupKey global;
diff --git a/test/libraries/ValidationLocatorLib.t.sol b/test/libraries/ValidationLocatorLib.t.sol
@@ -57,23 +57,34 @@ contract ValidationLocatorLibTest is Test {
         assertEq(ValidationLocator.unwrap(result), ValidationLocator.unwrap(expected));
     }
 
-    function testFuzz_loadFromSignature_regular(
-        uint32 entityId,
-        bool isGlobal,
-        bool isDeferredAction,
-        bytes memory signature
-    ) public view {
-        bytes memory finalSignature =
-            ValidationLocatorLib.packSignature(entityId, isGlobal, isDeferredAction, signature);
+    function testFuzz_loadFromSignature_regular(uint32 entityId, bool isGlobal, bytes memory signature)
+        public
+        view
+    {
+        bytes memory finalSignature = ValidationLocatorLib.packSignature(entityId, isGlobal, signature);
 
         (ValidationLocator result, bytes memory remainder) = this.loadFromSignature(finalSignature);
 
-        ValidationLocator expected = ValidationLocatorLib.pack(entityId, isGlobal, isDeferredAction);
+        ValidationLocator expected = ValidationLocatorLib.pack(entityId, isGlobal, false);
 
         assertEq(ValidationLocator.unwrap(result), ValidationLocator.unwrap(expected));
         assertEq(remainder, signature);
     }
 
+    function testFuzz_packUnpackLookupKey(uint168 id) public pure {
+        ValidationLookupKey k = ValidationLookupKey.wrap(id << 8);
+
+        assertEq(uint32(id), ValidationLocatorLib.entityId(k));
+    }
+
+    function testFuzz_packUnpackDirectCallAddress(address directCallValidation) public pure {
+        ValidationLookupKey k = ValidationLookupKey.wrap(uint168(uint160(directCallValidation)) << 8);
+
+        address expected = ValidationLocatorLib.directCallAddress(k);
+
+        assertEq(expected, directCallValidation);
+    }
+
     function testFuzz_loadFromSignature_directCall(
         address directCallValidation,
         bool isGlobal,

diff --git a/test/utils/ModuleSignatureUtils.sol b/test/utils/ModuleSignatureUtils.sol
@@ -102,7 +102,7 @@ contract ModuleSignatureUtils {
             sig =
                 ValidationLocatorLib.packSignatureDirectCall(module, globalOrNot == GLOBAL_VALIDATION, false, sig);
         } else {
-            sig = ValidationLocatorLib.packSignature(entityId, globalOrNot == GLOBAL_VALIDATION, false, sig);
+            sig = ValidationLocatorLib.packSignature(entityId, globalOrNot == GLOBAL_VALIDATION, sig);
         }
 
         return sig;
@@ -140,7 +140,7 @@ contract ModuleSignatureUtils {
         if (entityId == DIRECT_CALL_VALIDATION_ENTITY_ID) {
             sig = ValidationLocatorLib.packSignatureDirectCall(module, false, false, sig);
         } else {
-            sig = ValidationLocatorLib.packSignature(entityId, false, false, sig);
+            sig = ValidationLocatorLib.packSignature(entityId, false, sig);
         }
 
         return sig;