Skip to content

Commit 98513c7

Browse files
jayantkjayantk
committed
fix
1 parent ff7c2f2 commit 98513c7

File tree

3 files changed

+9
-260
lines changed

3 files changed

+9
-260
lines changed

‎target_chains/ethereum/contracts/contracts/pulse/Pulse.sol‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,6 @@ abstract contract Pulse is IPulse, PulseState {
7070
// Since tx.gasprice is used to calculate fees, allowing far-future requests would make
7171
// the fee estimation unreliable.
7272
require(publishTime <= block.timestamp + 60, "Too far in future");
73-
if (priceIds.length > MAX_PRICE_IDS) {
74-
revert TooManyPriceIds(priceIds.length, MAX_PRICE_IDS);
75-
}
7673
requestSequenceNumber = _state.currentSequenceNumber++;
7774

7875
uint128 requiredFee = getFee(provider, callbackGasLimit, priceIds);
@@ -85,7 +82,7 @@ abstract contract Pulse is IPulse, PulseState {
8582
req.requester = msg.sender;
8683
req.provider = provider;
8784
req.fee = SafeCast.toUint128(msg.value - _state.pythFeeInWei);
88-
req.priceIdsHash = keccak256(abi.encodePacked(priceIds));
85+
req.priceIdsHash = keccak256(abi.encode(priceIds));
8986

9087
_state.accruedFeesInWei += _state.pythFeeInWei;
9188

@@ -112,10 +109,12 @@ abstract contract Pulse is IPulse, PulseState {
112109
}
113110

114111
// Verify priceIds match
115-
require(
116-
req.priceIdsHash == keccak256(abi.encodePacked(priceIds)),
117-
"Price IDs mismatch"
118-
);
112+
if (req.priceIdsHash != keccak256(abi.encode(priceIds))) {
113+
revert InvalidPriceIds(
114+
keccak256(abi.encode(priceIds)),
115+
req.priceIdsHash
116+
);
117+
}
119118

120119
// TODO: should this use parsePriceFeedUpdatesUnique? also, do we need to add 1 to maxPublishTime?
121120
IPyth pyth = IPyth(_state.pyth);

‎target_chains/ethereum/contracts/contracts/pulse/PulseErrors.sol‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,3 @@ error CallbackFailed();
1212
error InvalidPriceIds(bytes32 providedPriceIdsHash, bytes32 storedPriceIdsHash);
1313
error InvalidCallbackGasLimit(uint256 requested, uint256 stored);
1414
error ExceedsMaxPrices(uint32 requested, uint32 maxAllowed);
15-
error TooManyPriceIds(uint256 provided, uint256 maximum);

‎target_chains/ethereum/contracts/forge-test/Pulse.t.sol‎

Lines changed: 2 additions & 251 deletions
Original file line numberDiff line numberDiff line change
@@ -660,8 +660,8 @@ contract PulseTest is Test, PulseEvents, IPulseConsumer, PulseTestUtils {
660660
vm.expectRevert(
661661
abi.encodeWithSelector(
662662
InvalidPriceIds.selector,
663-
wrongPriceIds[0],
664-
priceIds[0]
663+
keccak256(abi.encode(wrongPriceIds)),
664+
keccak256(abi.encode(priceIds))
665665
)
666666
);
667667
pulse.executeCallback(
@@ -672,33 +672,6 @@ contract PulseTest is Test, PulseEvents, IPulseConsumer, PulseTestUtils {
672672
);
673673
}
674674

675-
function testRevertOnTooManyPriceIds() public {
676-
uint256 maxPriceIds = uint256(pulse.MAX_PRICE_IDS());
677-
// Create array with MAX_PRICE_IDS + 1 price IDs
678-
bytes32[] memory priceIds = new bytes32[](maxPriceIds + 1);
679-
for (uint i = 0; i < priceIds.length; i++) {
680-
priceIds[i] = bytes32(uint256(i + 1));
681-
}
682-
683-
vm.deal(address(consumer), 1 gwei);
684-
uint128 totalFee = calculateTotalFee();
685-
686-
vm.prank(address(consumer));
687-
vm.expectRevert(
688-
abi.encodeWithSelector(
689-
TooManyPriceIds.selector,
690-
maxPriceIds + 1,
691-
maxPriceIds
692-
)
693-
);
694-
pulse.requestPriceUpdatesWithCallback{value: totalFee}(
695-
defaultProvider,
696-
SafeCast.toUint64(block.timestamp),
697-
priceIds,
698-
CALLBACK_GAS_LIMIT
699-
);
700-
}
701-
702675
function testProviderRegistration() public {
703676
address provider = address(0x123);
704677
uint128 providerFee = 1000;
@@ -930,228 +903,6 @@ contract PulseTest is Test, PulseEvents, IPulseConsumer, PulseTestUtils {
930903
);
931904
}
932905

933-
function testGetFirstActiveRequests() public {
934-
// Setup test data
935-
(
936-
bytes32[] memory priceIds,
937-
bytes[] memory updateData
938-
) = setupTestData();
939-
createTestRequests(priceIds);
940-
completeRequests(updateData, priceIds);
941-
942-
testRequestScenarios(priceIds, updateData);
943-
}
944-
945-
function setupTestData()
946-
private
947-
pure
948-
returns (bytes32[] memory, bytes[] memory)
949-
{
950-
bytes32[] memory priceIds = new bytes32[](1);
951-
priceIds[0] = bytes32(uint256(1));
952-
953-
bytes[] memory updateData = new bytes[](1);
954-
return (priceIds, updateData);
955-
}
956-
957-
function createTestRequests(bytes32[] memory priceIds) private {
958-
uint64 publishTime = SafeCast.toUint64(block.timestamp);
959-
for (uint i = 0; i < 5; i++) {
960-
vm.deal(address(this), 1 ether);
961-
pulse.requestPriceUpdatesWithCallback{value: 1 ether}(
962-
defaultProvider,
963-
publishTime,
964-
priceIds,
965-
1000000
966-
);
967-
}
968-
}
969-
970-
function completeRequests(
971-
bytes[] memory updateData,
972-
bytes32[] memory priceIds
973-
) private {
974-
// Create mock price feeds and setup Pyth response
975-
PythStructs.PriceFeed[] memory priceFeeds = createMockPriceFeeds(
976-
SafeCast.toUint64(block.timestamp)
977-
);
978-
mockParsePriceFeedUpdates(pyth, priceFeeds);
979-
updateData = createMockUpdateData(priceFeeds);
980-
981-
vm.deal(defaultProvider, 2 ether); // Increase ETH allocation to prevent OutOfFunds
982-
vm.startPrank(defaultProvider);
983-
pulse.executeCallback{value: 1 ether}(
984-
defaultProvider,
985-
2,
986-
updateData,
987-
priceIds
988-
);
989-
pulse.executeCallback{value: 1 ether}(
990-
defaultProvider,
991-
4,
992-
updateData,
993-
priceIds
994-
);
995-
vm.stopPrank();
996-
}
997-
998-
function testRequestScenarios(
999-
bytes32[] memory priceIds,
1000-
bytes[] memory updateData
1001-
) private {
1002-
// Test 1: Request more than available
1003-
checkMoreThanAvailable();
1004-
1005-
// Test 2: Request exact number
1006-
checkExactNumber();
1007-
1008-
// Test 3: Request fewer than available
1009-
checkFewerThanAvailable();
1010-
1011-
// Test 4: Request zero
1012-
checkZeroRequest();
1013-
1014-
// Test 5: Clear all and check empty
1015-
clearAllRequests(updateData, priceIds);
1016-
checkEmptyState();
1017-
}
1018-
1019-
// Split test scenarios into separate functions
1020-
function checkMoreThanAvailable() private {
1021-
(PulseState.Request[] memory requests, uint256 count) = pulse
1022-
.getFirstActiveRequests(10);
1023-
assertEq(count, 3, "Should find 3 active requests");
1024-
assertEq(requests.length, 3, "Array should be resized to 3");
1025-
assertEq(
1026-
requests[0].sequenceNumber,
1027-
1,
1028-
"First request should be oldest"
1029-
);
1030-
assertEq(requests[1].sequenceNumber, 3, "Second request should be #3");
1031-
assertEq(requests[2].sequenceNumber, 5, "Third request should be #5");
1032-
}
1033-
1034-
function checkExactNumber() private {
1035-
(PulseState.Request[] memory requests, uint256 count) = pulse
1036-
.getFirstActiveRequests(3);
1037-
assertEq(count, 3, "Should find 3 active requests");
1038-
assertEq(requests.length, 3, "Array should match requested size");
1039-
}
1040-
1041-
function checkFewerThanAvailable() private {
1042-
(PulseState.Request[] memory requests, uint256 count) = pulse
1043-
.getFirstActiveRequests(2);
1044-
assertEq(count, 2, "Should find 2 active requests");
1045-
assertEq(requests.length, 2, "Array should match requested size");
1046-
assertEq(
1047-
requests[0].sequenceNumber,
1048-
1,
1049-
"First request should be oldest"
1050-
);
1051-
assertEq(requests[1].sequenceNumber, 3, "Second request should be #3");
1052-
}
1053-
1054-
function checkZeroRequest() private {
1055-
(PulseState.Request[] memory requests, uint256 count) = pulse
1056-
.getFirstActiveRequests(0);
1057-
assertEq(count, 0, "Should find 0 active requests");
1058-
assertEq(requests.length, 0, "Array should be empty");
1059-
}
1060-
1061-
function clearAllRequests(
1062-
bytes[] memory updateData,
1063-
bytes32[] memory priceIds
1064-
) private {
1065-
vm.deal(defaultProvider, 3 ether); // Increase ETH allocation
1066-
vm.startPrank(defaultProvider);
1067-
pulse.executeCallback{value: 1 ether}(
1068-
defaultProvider,
1069-
1,
1070-
updateData,
1071-
priceIds
1072-
);
1073-
pulse.executeCallback{value: 1 ether}(
1074-
defaultProvider,
1075-
3,
1076-
updateData,
1077-
priceIds
1078-
);
1079-
pulse.executeCallback{value: 1 ether}(
1080-
defaultProvider,
1081-
5,
1082-
updateData,
1083-
priceIds
1084-
);
1085-
vm.stopPrank();
1086-
}
1087-
1088-
function checkEmptyState() private {
1089-
(PulseState.Request[] memory requests, uint256 count) = pulse
1090-
.getFirstActiveRequests(10);
1091-
assertEq(count, 0, "Should find 0 active requests");
1092-
assertEq(requests.length, 0, "Array should be empty");
1093-
}
1094-
1095-
function testGetFirstActiveRequestsGasUsage() public {
1096-
// Setup test data
1097-
bytes32[] memory priceIds = new bytes32[](1);
1098-
priceIds[0] = bytes32(uint256(1));
1099-
uint64 publishTime = SafeCast.toUint64(block.timestamp);
1100-
uint256 callbackGasLimit = 1000000;
1101-
1102-
// Create mock price feeds and setup Pyth response
1103-
PythStructs.PriceFeed[] memory priceFeeds = createMockPriceFeeds(
1104-
publishTime
1105-
);
1106-
mockParsePriceFeedUpdates(pyth, priceFeeds);
1107-
bytes[] memory updateData = createMockUpdateData(priceFeeds);
1108-
1109-
// Create 20 requests with some gaps
1110-
for (uint i = 0; i < 20; i++) {
1111-
vm.deal(address(this), 1 ether);
1112-
pulse.requestPriceUpdatesWithCallback{value: 1 ether}(
1113-
defaultProvider,
1114-
publishTime,
1115-
priceIds,
1116-
callbackGasLimit
1117-
);
1118-
1119-
// Complete every third request to create gaps
1120-
if (i % 3 == 0) {
1121-
vm.deal(defaultProvider, 1 ether);
1122-
vm.prank(defaultProvider);
1123-
pulse.executeCallback{value: 1 ether}(
1124-
defaultProvider,
1125-
uint64(i + 1),
1126-
updateData,
1127-
priceIds
1128-
);
1129-
}
1130-
}
1131-
1132-
// Measure gas for different request counts
1133-
uint256 gas1 = gasleft();
1134-
pulse.getFirstActiveRequests(5);
1135-
uint256 gas1Used = gas1 - gasleft();
1136-
1137-
uint256 gas2 = gasleft();
1138-
pulse.getFirstActiveRequests(10);
1139-
uint256 gas2Used = gas2 - gasleft();
1140-
1141-
// Log gas usage for analysis
1142-
emit log_named_uint("Gas used for 5 requests", gas1Used);
1143-
emit log_named_uint("Gas used for 10 requests", gas2Used);
1144-
1145-
// Verify gas usage scales roughly linearly
1146-
// Allow 10% margin for other factors
1147-
assertApproxEqRel(
1148-
gas2Used,
1149-
gas1Used * 2,
1150-
0.1e18, // 10% tolerance
1151-
"Gas usage should scale roughly linearly"
1152-
);
1153-
}
1154-
1155906
function getPulse() internal view override returns (address) {
1156907
return address(pulse);
1157908
}

0 commit comments

Comments
 (0)