From 7cd83289ebbcfa9bf9030e4193ecea1e27509444 Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Wed, 8 Jan 2025 12:08:07 +0900 Subject: [PATCH] fix: use fixed-length array for priceIds in req --- .../contracts/contracts/pulse/Pulse.sol | 27 +++++++++++++++---- .../contracts/contracts/pulse/PulseErrors.sol | 1 + .../contracts/contracts/pulse/PulseState.sol | 4 ++- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/target_chains/ethereum/contracts/contracts/pulse/Pulse.sol b/target_chains/ethereum/contracts/contracts/pulse/Pulse.sol index 9c10b10e49..64ccecaed3 100644 --- a/target_chains/ethereum/contracts/contracts/pulse/Pulse.sol +++ b/target_chains/ethereum/contracts/contracts/pulse/Pulse.sol @@ -31,6 +31,11 @@ abstract contract Pulse is IPulse, PulseState { req.publishTime = 1; req.callbackGasLimit = 1; req.requester = address(1); + req.numPriceIds = 0; + // Pre-warm the priceIds array storage + for (uint8 j = 0; j < MAX_PRICE_IDS; j++) { + req.priceIds[j] = bytes32(0); + } } } } @@ -41,6 +46,9 @@ abstract contract Pulse is IPulse, PulseState { uint256 callbackGasLimit ) external payable override returns (uint64 requestSequenceNumber) { require(publishTime <= block.timestamp + 60, "Too far in future"); + if (priceIds.length > MAX_PRICE_IDS) { + revert TooManyPriceIds(priceIds.length, MAX_PRICE_IDS); + } requestSequenceNumber = _state.currentSequenceNumber++; uint128 requiredFee = getFee(callbackGasLimit); @@ -49,9 +57,14 @@ abstract contract Pulse is IPulse, PulseState { Request storage req = allocRequest(requestSequenceNumber); req.sequenceNumber = requestSequenceNumber; req.publishTime = publishTime; - req.priceIdsHash = keccak256(abi.encode(priceIds)); req.callbackGasLimit = callbackGasLimit; req.requester = msg.sender; + req.numPriceIds = uint8(priceIds.length); + + // Copy price IDs to storage + for (uint8 i = 0; i < priceIds.length; i++) { + req.priceIds[i] = priceIds[i]; + } _state.accruedFeesInWei += SafeCast.toUint128(msg.value); @@ -66,10 +79,14 @@ abstract contract Pulse is IPulse, PulseState { Request storage req = findActiveRequest(sequenceNumber); // Verify priceIds match - bytes32 providedPriceIdsHash = keccak256(abi.encode(priceIds)); - bytes32 storedPriceIdsHash = req.priceIdsHash; - if (providedPriceIdsHash != storedPriceIdsHash) { - revert InvalidPriceIds(providedPriceIdsHash, storedPriceIdsHash); + require( + priceIds.length == req.numPriceIds, + "Price IDs length mismatch" + ); + for (uint8 i = 0; i < req.numPriceIds; i++) { + if (priceIds[i] != req.priceIds[i]) { + revert InvalidPriceIds(priceIds[i], req.priceIds[i]); + } } // Parse price feeds first to measure gas usage diff --git a/target_chains/ethereum/contracts/contracts/pulse/PulseErrors.sol b/target_chains/ethereum/contracts/contracts/pulse/PulseErrors.sol index c2fe41ccb6..aacb123ba5 100644 --- a/target_chains/ethereum/contracts/contracts/pulse/PulseErrors.sol +++ b/target_chains/ethereum/contracts/contracts/pulse/PulseErrors.sol @@ -12,3 +12,4 @@ error InvalidPriceIds(bytes32 providedPriceIdsHash, bytes32 storedPriceIdsHash); error InvalidCallbackGasLimit(uint256 requested, uint256 stored); error ExceedsMaxPrices(uint32 requested, uint32 maxAllowed); error InsufficientGas(); +error TooManyPriceIds(uint256 provided, uint256 maximum); diff --git a/target_chains/ethereum/contracts/contracts/pulse/PulseState.sol b/target_chains/ethereum/contracts/contracts/pulse/PulseState.sol index 6b0b48fc61..82c1fa7967 100644 --- a/target_chains/ethereum/contracts/contracts/pulse/PulseState.sol +++ b/target_chains/ethereum/contracts/contracts/pulse/PulseState.sol @@ -5,11 +5,13 @@ pragma solidity ^0.8.0; contract PulseState { uint8 public constant NUM_REQUESTS = 32; bytes1 public constant NUM_REQUESTS_MASK = 0x1f; + uint8 public constant MAX_PRICE_IDS = 10; struct Request { uint64 sequenceNumber; uint256 publishTime; - bytes32 priceIdsHash; + bytes32[MAX_PRICE_IDS] priceIds; + uint8 numPriceIds; // Actual number of price IDs used uint256 callbackGasLimit; address requester; }