Jayant Krishnamurthy 8 miesięcy temu
rodzic
commit
98513c7e96

+ 7 - 8
target_chains/ethereum/contracts/contracts/pulse/Pulse.sol

@@ -70,9 +70,6 @@ abstract contract Pulse is IPulse, PulseState {
         //      Since tx.gasprice is used to calculate fees, allowing far-future requests would make
         //      the fee estimation unreliable.
         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(provider, callbackGasLimit, priceIds);
@@ -85,7 +82,7 @@ abstract contract Pulse is IPulse, PulseState {
         req.requester = msg.sender;
         req.provider = provider;
         req.fee = SafeCast.toUint128(msg.value - _state.pythFeeInWei);
-        req.priceIdsHash = keccak256(abi.encodePacked(priceIds));
+        req.priceIdsHash = keccak256(abi.encode(priceIds));
 
         _state.accruedFeesInWei += _state.pythFeeInWei;
 
@@ -112,10 +109,12 @@ abstract contract Pulse is IPulse, PulseState {
         }
 
         // Verify priceIds match
-        require(
-            req.priceIdsHash == keccak256(abi.encodePacked(priceIds)),
-            "Price IDs mismatch"
-        );
+        if (req.priceIdsHash != keccak256(abi.encode(priceIds))) {
+            revert InvalidPriceIds(
+                keccak256(abi.encode(priceIds)),
+                req.priceIdsHash
+            );
+        }
 
         // TODO: should this use parsePriceFeedUpdatesUnique? also, do we need to add 1 to maxPublishTime?
         IPyth pyth = IPyth(_state.pyth);

+ 0 - 1
target_chains/ethereum/contracts/contracts/pulse/PulseErrors.sol

@@ -12,4 +12,3 @@ error CallbackFailed();
 error InvalidPriceIds(bytes32 providedPriceIdsHash, bytes32 storedPriceIdsHash);
 error InvalidCallbackGasLimit(uint256 requested, uint256 stored);
 error ExceedsMaxPrices(uint32 requested, uint32 maxAllowed);
-error TooManyPriceIds(uint256 provided, uint256 maximum);

+ 2 - 251
target_chains/ethereum/contracts/forge-test/Pulse.t.sol

@@ -660,8 +660,8 @@ contract PulseTest is Test, PulseEvents, IPulseConsumer, PulseTestUtils {
         vm.expectRevert(
             abi.encodeWithSelector(
                 InvalidPriceIds.selector,
-                wrongPriceIds[0],
-                priceIds[0]
+                keccak256(abi.encode(wrongPriceIds)),
+                keccak256(abi.encode(priceIds))
             )
         );
         pulse.executeCallback(
@@ -672,33 +672,6 @@ contract PulseTest is Test, PulseEvents, IPulseConsumer, PulseTestUtils {
         );
     }
 
-    function testRevertOnTooManyPriceIds() public {
-        uint256 maxPriceIds = uint256(pulse.MAX_PRICE_IDS());
-        // Create array with MAX_PRICE_IDS + 1 price IDs
-        bytes32[] memory priceIds = new bytes32[](maxPriceIds + 1);
-        for (uint i = 0; i < priceIds.length; i++) {
-            priceIds[i] = bytes32(uint256(i + 1));
-        }
-
-        vm.deal(address(consumer), 1 gwei);
-        uint128 totalFee = calculateTotalFee();
-
-        vm.prank(address(consumer));
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                TooManyPriceIds.selector,
-                maxPriceIds + 1,
-                maxPriceIds
-            )
-        );
-        pulse.requestPriceUpdatesWithCallback{value: totalFee}(
-            defaultProvider,
-            SafeCast.toUint64(block.timestamp),
-            priceIds,
-            CALLBACK_GAS_LIMIT
-        );
-    }
-
     function testProviderRegistration() public {
         address provider = address(0x123);
         uint128 providerFee = 1000;
@@ -930,228 +903,6 @@ contract PulseTest is Test, PulseEvents, IPulseConsumer, PulseTestUtils {
         );
     }
 
-    function testGetFirstActiveRequests() public {
-        // Setup test data
-        (
-            bytes32[] memory priceIds,
-            bytes[] memory updateData
-        ) = setupTestData();
-        createTestRequests(priceIds);
-        completeRequests(updateData, priceIds);
-
-        testRequestScenarios(priceIds, updateData);
-    }
-
-    function setupTestData()
-        private
-        pure
-        returns (bytes32[] memory, bytes[] memory)
-    {
-        bytes32[] memory priceIds = new bytes32[](1);
-        priceIds[0] = bytes32(uint256(1));
-
-        bytes[] memory updateData = new bytes[](1);
-        return (priceIds, updateData);
-    }
-
-    function createTestRequests(bytes32[] memory priceIds) private {
-        uint64 publishTime = SafeCast.toUint64(block.timestamp);
-        for (uint i = 0; i < 5; i++) {
-            vm.deal(address(this), 1 ether);
-            pulse.requestPriceUpdatesWithCallback{value: 1 ether}(
-                defaultProvider,
-                publishTime,
-                priceIds,
-                1000000
-            );
-        }
-    }
-
-    function completeRequests(
-        bytes[] memory updateData,
-        bytes32[] memory priceIds
-    ) private {
-        // Create mock price feeds and setup Pyth response
-        PythStructs.PriceFeed[] memory priceFeeds = createMockPriceFeeds(
-            SafeCast.toUint64(block.timestamp)
-        );
-        mockParsePriceFeedUpdates(pyth, priceFeeds);
-        updateData = createMockUpdateData(priceFeeds);
-
-        vm.deal(defaultProvider, 2 ether); // Increase ETH allocation to prevent OutOfFunds
-        vm.startPrank(defaultProvider);
-        pulse.executeCallback{value: 1 ether}(
-            defaultProvider,
-            2,
-            updateData,
-            priceIds
-        );
-        pulse.executeCallback{value: 1 ether}(
-            defaultProvider,
-            4,
-            updateData,
-            priceIds
-        );
-        vm.stopPrank();
-    }
-
-    function testRequestScenarios(
-        bytes32[] memory priceIds,
-        bytes[] memory updateData
-    ) private {
-        // Test 1: Request more than available
-        checkMoreThanAvailable();
-
-        // Test 2: Request exact number
-        checkExactNumber();
-
-        // Test 3: Request fewer than available
-        checkFewerThanAvailable();
-
-        // Test 4: Request zero
-        checkZeroRequest();
-
-        // Test 5: Clear all and check empty
-        clearAllRequests(updateData, priceIds);
-        checkEmptyState();
-    }
-
-    // Split test scenarios into separate functions
-    function checkMoreThanAvailable() private {
-        (PulseState.Request[] memory requests, uint256 count) = pulse
-            .getFirstActiveRequests(10);
-        assertEq(count, 3, "Should find 3 active requests");
-        assertEq(requests.length, 3, "Array should be resized to 3");
-        assertEq(
-            requests[0].sequenceNumber,
-            1,
-            "First request should be oldest"
-        );
-        assertEq(requests[1].sequenceNumber, 3, "Second request should be #3");
-        assertEq(requests[2].sequenceNumber, 5, "Third request should be #5");
-    }
-
-    function checkExactNumber() private {
-        (PulseState.Request[] memory requests, uint256 count) = pulse
-            .getFirstActiveRequests(3);
-        assertEq(count, 3, "Should find 3 active requests");
-        assertEq(requests.length, 3, "Array should match requested size");
-    }
-
-    function checkFewerThanAvailable() private {
-        (PulseState.Request[] memory requests, uint256 count) = pulse
-            .getFirstActiveRequests(2);
-        assertEq(count, 2, "Should find 2 active requests");
-        assertEq(requests.length, 2, "Array should match requested size");
-        assertEq(
-            requests[0].sequenceNumber,
-            1,
-            "First request should be oldest"
-        );
-        assertEq(requests[1].sequenceNumber, 3, "Second request should be #3");
-    }
-
-    function checkZeroRequest() private {
-        (PulseState.Request[] memory requests, uint256 count) = pulse
-            .getFirstActiveRequests(0);
-        assertEq(count, 0, "Should find 0 active requests");
-        assertEq(requests.length, 0, "Array should be empty");
-    }
-
-    function clearAllRequests(
-        bytes[] memory updateData,
-        bytes32[] memory priceIds
-    ) private {
-        vm.deal(defaultProvider, 3 ether); // Increase ETH allocation
-        vm.startPrank(defaultProvider);
-        pulse.executeCallback{value: 1 ether}(
-            defaultProvider,
-            1,
-            updateData,
-            priceIds
-        );
-        pulse.executeCallback{value: 1 ether}(
-            defaultProvider,
-            3,
-            updateData,
-            priceIds
-        );
-        pulse.executeCallback{value: 1 ether}(
-            defaultProvider,
-            5,
-            updateData,
-            priceIds
-        );
-        vm.stopPrank();
-    }
-
-    function checkEmptyState() private {
-        (PulseState.Request[] memory requests, uint256 count) = pulse
-            .getFirstActiveRequests(10);
-        assertEq(count, 0, "Should find 0 active requests");
-        assertEq(requests.length, 0, "Array should be empty");
-    }
-
-    function testGetFirstActiveRequestsGasUsage() public {
-        // Setup test data
-        bytes32[] memory priceIds = new bytes32[](1);
-        priceIds[0] = bytes32(uint256(1));
-        uint64 publishTime = SafeCast.toUint64(block.timestamp);
-        uint256 callbackGasLimit = 1000000;
-
-        // Create mock price feeds and setup Pyth response
-        PythStructs.PriceFeed[] memory priceFeeds = createMockPriceFeeds(
-            publishTime
-        );
-        mockParsePriceFeedUpdates(pyth, priceFeeds);
-        bytes[] memory updateData = createMockUpdateData(priceFeeds);
-
-        // Create 20 requests with some gaps
-        for (uint i = 0; i < 20; i++) {
-            vm.deal(address(this), 1 ether);
-            pulse.requestPriceUpdatesWithCallback{value: 1 ether}(
-                defaultProvider,
-                publishTime,
-                priceIds,
-                callbackGasLimit
-            );
-
-            // Complete every third request to create gaps
-            if (i % 3 == 0) {
-                vm.deal(defaultProvider, 1 ether);
-                vm.prank(defaultProvider);
-                pulse.executeCallback{value: 1 ether}(
-                    defaultProvider,
-                    uint64(i + 1),
-                    updateData,
-                    priceIds
-                );
-            }
-        }
-
-        // Measure gas for different request counts
-        uint256 gas1 = gasleft();
-        pulse.getFirstActiveRequests(5);
-        uint256 gas1Used = gas1 - gasleft();
-
-        uint256 gas2 = gasleft();
-        pulse.getFirstActiveRequests(10);
-        uint256 gas2Used = gas2 - gasleft();
-
-        // Log gas usage for analysis
-        emit log_named_uint("Gas used for 5 requests", gas1Used);
-        emit log_named_uint("Gas used for 10 requests", gas2Used);
-
-        // Verify gas usage scales roughly linearly
-        // Allow 10% margin for other factors
-        assertApproxEqRel(
-            gas2Used,
-            gas1Used * 2,
-            0.1e18, // 10% tolerance
-            "Gas usage should scale roughly linearly"
-        );
-    }
-
     function getPulse() internal view override returns (address) {
         return address(pulse);
     }