feat: migrate containerId to fixed size type

ritual-net · Dec 6, 2023 · ecb491c · ecb491c
1 parent 2d04a7f
commit ecb491c
Show file tree

Hide file tree

Showing 8 changed files with 24 additions and 20 deletions.
diff --git a/src/Coordinator.sol b/src/Coordinator.sol
@@ -46,9 +46,10 @@ contract Coordinator is Manager {
         /// @dev uint24 is too small at ~16.7M (<30M mainnet gas limit), but uint32 is more than enough (~4.2B wei)
         uint32 maxGasLimit;
         /// @notice Container identifier used by off-chain Infernet nodes to determine which container is used to fulfill a subscription
+        /// @dev Represented as fixed size hash of stringified list of containers
         /// @dev Can be used to specify a linear DAG of containers by seperating container names with a "," delimiter ("A,B,C")
-        /// @dev Better represented by a string[] type but constrained to string to keep struct and functions simple
-        string containerId;
+        /// @dev Better represented by a string[] type but constrained to hash(string) to keep struct and functions simple
+        bytes32 containerId;
         /// @notice Optional container input parameters
         /// @dev If left empty, off-chain Infernet nodes call public view fn: `BaseConsumer(owner).getContainerInputs()`
         bytes inputs;
@@ -368,7 +369,7 @@ contract Coordinator is Manager {
             maxGasLimit: maxGasLimit,
             frequency: frequency,
             period: period,
-            containerId: containerId,
+            containerId: keccak256(abi.encode(containerId)),
             inputs: inputs
         });
 

diff --git a/src/EIP712Coordinator.sol b/src/EIP712Coordinator.sol
@@ -27,21 +27,21 @@ contract EIP712Coordinator is EIP712, Coordinator {
     uint256 public constant DELEGATEE_OVERHEAD_CACHED_WEI = 600 wei;
 
     /// @notice Gas overhead in wei to create a new subscription via delegatee signature
-    /// @dev Make note that this does not account for gas costs of dynamic inputs (containerId, inputs), just base overhead
+    /// @dev Make note that this does not account for gas costs of dynamic `inputs`, just base overhead
     /// @dev Can fit within uint24, see comment for `DELEGATEE_OVERHEAD_CACHED_WEI` for details
-    uint256 public constant DELEGATEE_OVERHEAD_CREATE_WEI = 91_200 wei;
+    uint256 public constant DELEGATEE_OVERHEAD_CREATE_WEI = 130_200 wei;
 
     /// @notice EIP-712 struct(Subscription) typeHash
     bytes32 private constant EIP712_SUBSCRIPTION_TYPEHASH = keccak256(
-        "Subscription(address owner,uint32 activeAt,uint32 period,uint32 frequency,uint16 redundancy,uint48 maxGasPrice,uint32 maxGasLimit,string containerId,bytes inputs)"
+        "Subscription(address owner,uint32 activeAt,uint32 period,uint32 frequency,uint16 redundancy,uint48 maxGasPrice,uint32 maxGasLimit,bytes32 containerId,bytes inputs)"
     );
 
     /// @notice EIP-712 struct(DelegateSubscription) typeHash
     /// @dev struct(DelegateSubscription) == { uint32 nonce, uint32 expiry, Subscription sub }
     /// @dev The `nonce` represents the nonce of the subscribing contract (sub-owner); prevents signature replay
     /// @dev The `expiry` is when the delegated subscription signature expires and can no longer be used
     bytes32 private constant EIP712_DELEGATE_SUBSCRIPTION_TYPEHASH = keccak256(
-        "DelegateSubscription(uint32 nonce,uint32 expiry,Subscription sub)Subscription(address owner,uint32 activeAt,uint32 period,uint32 frequency,uint16 redundancy,uint48 maxGasPrice,uint32 maxGasLimit,string containerId,bytes inputs)"
+        "DelegateSubscription(uint32 nonce,uint32 expiry,Subscription sub)Subscription(address owner,uint32 activeAt,uint32 period,uint32 frequency,uint16 redundancy,uint48 maxGasPrice,uint32 maxGasLimit,bytes32 containerId,bytes inputs)"
     );
 
     /*//////////////////////////////////////////////////////////////
@@ -139,8 +139,8 @@ contract EIP712Coordinator is EIP712, Coordinator {
                             sub.redundancy,
                             sub.maxGasPrice,
                             sub.maxGasLimit,
+                            sub.containerId,
                             // Hash dynamic values
-                            keccak256(bytes(sub.containerId)),
                             keccak256(sub.inputs)
                         )
                     )

diff --git a/test/Coordinator.t.sol b/test/Coordinator.t.sol
@@ -31,6 +31,9 @@ abstract contract CoordinatorConstants {
     /// @notice Mock compute container ID
     string constant MOCK_CONTAINER_ID = "container";
 
+    /// @notice Mock compute container ID hashed
+    bytes32 constant HASHED_MOCK_CONTAINER_ID = keccak256(abi.encode(MOCK_CONTAINER_ID));
+
     /// @notice Mock container inputs
     bytes constant MOCK_CONTAINER_INPUTS = "inputs";
 
@@ -183,7 +186,7 @@ contract CoordinatorCallbackTest is CoordinatorTest {
         assertEq(sub.maxGasLimit, 100_000);
         assertEq(sub.frequency, 1);
         assertEq(sub.period, 0);
-        assertEq(sub.containerId, MOCK_CONTAINER_ID);
+        assertEq(sub.containerId, HASHED_MOCK_CONTAINER_ID);
         assertEq(sub.inputs, MOCK_CONTAINER_INPUTS);
     }
 

diff --git a/test/EIP712Coordinator.t.sol b/test/EIP712Coordinator.t.sol
@@ -106,7 +106,7 @@ contract EIP712CoordinatorTest is Test, CoordinatorConstants, ICoordinatorEvents
                 + uint32(COORDINATOR.DELIVERY_OVERHEAD_WEI()),
             frequency: 1,
             period: 0,
-            containerId: MOCK_CONTAINER_ID,
+            containerId: HASHED_MOCK_CONTAINER_ID,
             inputs: MOCK_CONTAINER_INPUTS
         });
     }
@@ -587,7 +587,7 @@ contract EIP712CoordinatorTest is Test, CoordinatorConstants, ICoordinatorEvents
 
         // Manually verifying the callstack is useful here to ensure that the overhead gas is being properly set
         // Measure direct delivery for creation + delivery
-        uint256 inputOverhead = 35_000 wei;
+        uint256 inputOverhead = 15_000 wei;
         uint256 gasExpected = CALLBACK_COST + COORDINATOR.DELEGATEE_OVERHEAD_CREATE_WEI()
             + COORDINATOR.DELIVERY_OVERHEAD_WEI() + inputOverhead;
         uint256 startingGas = gasleft();
@@ -603,8 +603,8 @@ contract EIP712CoordinatorTest is Test, CoordinatorConstants, ICoordinatorEvents
         endingGas = gasleft();
         uint256 gasUsedCached = startingGas - endingGas;
 
-        // Assert in ~approximate range (+/- 15K gas, actually copying calldata into memory is expensive)
-        uint256 delta = 15_000 wei;
+        // Assert in ~approximate range (+/- 16K gas, actually copying calldata into memory is expensive)
+        uint256 delta = 16_000 wei;
         assertApproxEqAbs(gasExpected, gasUsed, delta);
         assertApproxEqAbs(gasExpectedCached, gasUsedCached, delta);
     }

diff --git a/test/lib/LibSign.sol b/test/lib/LibSign.sol
@@ -13,12 +13,12 @@ library LibSign {
 
     /// @notice EIP-712 Coordinator.Subscription typeHash
     bytes32 private constant SUBSCRIPTION_TYPEHASH = keccak256(
-        "Subscription(address owner,uint32 activeAt,uint32 period,uint32 frequency,uint16 redundancy,uint48 maxGasPrice,uint32 maxGasLimit,string containerId,bytes inputs)"
+        "Subscription(address owner,uint32 activeAt,uint32 period,uint32 frequency,uint16 redundancy,uint48 maxGasPrice,uint32 maxGasLimit,bytes32 containerId,bytes inputs)"
     );
 
     /// @notice EIP-712 DelegateSubscription typeHash
     bytes32 private constant DELEGATE_SUBSCRIPTION_TYPEHASH = keccak256(
-        "DelegateSubscription(uint32 nonce,uint32 expiry,Subscription sub)Subscription(address owner,uint32 activeAt,uint32 period,uint32 frequency,uint16 redundancy,uint48 maxGasPrice,uint32 maxGasLimit,string containerId,bytes inputs)"
+        "DelegateSubscription(uint32 nonce,uint32 expiry,Subscription sub)Subscription(address owner,uint32 activeAt,uint32 period,uint32 frequency,uint16 redundancy,uint48 maxGasPrice,uint32 maxGasLimit,bytes32 containerId,bytes inputs)"
     );
 
     /*//////////////////////////////////////////////////////////////
@@ -61,8 +61,8 @@ library LibSign {
                 sub.redundancy,
                 sub.maxGasPrice,
                 sub.maxGasLimit,
+                sub.containerId,
                 // Dynamic values must be encoded as hash of contents
-                keccak256(bytes(sub.containerId)),
                 keccak256(sub.inputs)
             )
         );

diff --git a/test/lib/LibStruct.sol b/test/lib/LibStruct.sol
@@ -21,7 +21,7 @@ library LibStruct {
         uint32 maxGasLimit;
         uint32 frequency;
         uint32 period;
-        string containerId;
+        bytes32 containerId;
         bytes inputs;
     }
 
@@ -58,7 +58,7 @@ library LibStruct {
             uint16 redundancy,
             uint48 maxGasPrice,
             uint32 maxGasLimit,
-            string memory containerId,
+            bytes32 containerId,
             bytes memory inputs
         ) = coordinator.subscriptions(subscriptionId);
 

diff --git a/test/mocks/consumer/Callback.sol b/test/mocks/consumer/Callback.sol
@@ -55,7 +55,7 @@ contract MockCallbackConsumer is MockBaseConsumer, CallbackConsumer, StdAssertio
         assertEq(sub.maxGasLimit, maxGasLimit);
         assertEq(sub.frequency, 1);
         assertEq(sub.period, 0);
-        assertEq(sub.containerId, containerId);
+        assertEq(sub.containerId, keccak256(abi.encode(containerId)));
         assertEq(sub.inputs, inputs);
 
         // Explicitly return subscription ID

diff --git a/test/mocks/consumer/Subscription.sol b/test/mocks/consumer/Subscription.sol
@@ -64,7 +64,7 @@ contract MockSubscriptionConsumer is MockBaseConsumer, SubscriptionConsumer, Std
         assertEq(sub.maxGasLimit, maxGasLimit);
         assertEq(sub.frequency, frequency);
         assertEq(sub.period, period);
-        assertEq(sub.containerId, containerId);
+        assertEq(sub.containerId, keccak256(abi.encode(containerId)));
         assertEq(sub.inputs, "");
 
         // Explicitly return subscription ID