diff --git a/src/Coordinator.sol b/src/Coordinator.sol index da1a0c4..185bd96 100644 --- 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 index 859824d..bf871dc 100644 --- a/src/EIP712Coordinator.sol +++ b/src/EIP712Coordinator.sol @@ -27,13 +27,13 @@ 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 @@ -41,7 +41,7 @@ contract EIP712Coordinator is EIP712, Coordinator { /// @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 index dff6d73..1238738 100644 --- 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 index 9b2811d..6c8480b 100644 --- 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 index 465f7a1..e0e4f5b 100644 --- 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 index ca6d984..6f93abb 100644 --- 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 index c879913..9859cb5 100644 --- 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 index 779d887..3785b1b 100644 --- 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