Skip to content

Commit f3ec247

Browse files
cctdanielcctdaniel
committed
address comments
1 parent 938c216 commit f3ec247

File tree

5 files changed

+67
-140
lines changed

5 files changed

+67
-140
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@ interface IPulse is PulseEvents {
2424

2525
function executeCallback(
2626
uint64 sequenceNumber,
27-
bytes32[] calldata priceIds,
2827
bytes[] calldata updateData,
29-
uint256 callbackGasLimit
28+
bytes32[] calldata priceIds
3029
) external payable;
3130

3231
// Getters

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

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ abstract contract Pulse is IPulse, PulseState {
4848
Request storage req = allocRequest(requestSequenceNumber);
4949
req.sequenceNumber = requestSequenceNumber;
5050
req.publishTime = publishTime;
51-
req.priceIds = priceIds;
51+
req.priceIdsHash = keccak256(abi.encode(priceIds));
5252
req.callbackGasLimit = callbackGasLimit;
5353
req.requester = msg.sender;
5454

@@ -59,24 +59,20 @@ abstract contract Pulse is IPulse, PulseState {
5959

6060
function executeCallback(
6161
uint64 sequenceNumber,
62-
bytes32[] calldata priceIds,
6362
bytes[] calldata updateData,
64-
uint256 callbackGasLimit
63+
bytes32[] calldata priceIds
6564
) external payable override {
6665
Request storage req = findActiveRequest(sequenceNumber);
66+
bytes32 providedPriceIdsHash = keccak256(abi.encode(priceIds));
67+
bytes32 storedPriceIdsHash = req.priceIdsHash;
6768

68-
if (
69-
keccak256(abi.encode(req.priceIds)) !=
70-
keccak256(abi.encode(priceIds))
71-
) {
72-
revert InvalidPriceIds(priceIds, req.priceIds);
69+
if (providedPriceIdsHash != storedPriceIdsHash) {
70+
revert InvalidPriceIds(providedPriceIdsHash, storedPriceIdsHash);
7371
}
7472

75-
if (req.callbackGasLimit != callbackGasLimit) {
76-
revert InvalidCallbackGasLimit(
77-
callbackGasLimit,
78-
req.callbackGasLimit
79-
);
73+
// Check if there's enough gas left for the callback
74+
if (gasleft() < req.callbackGasLimit) {
75+
revert InsufficientGas();
8076
}
8177

8278
PythStructs.PriceFeed[] memory priceFeeds = IPyth(_state.pyth)
@@ -90,12 +86,9 @@ abstract contract Pulse is IPulse, PulseState {
9086
uint256 publishTime = priceFeeds[0].price.publishTime;
9187

9288
try
93-
IPulseConsumer(req.requester).pulseCallback(
94-
sequenceNumber,
95-
msg.sender,
96-
publishTime,
97-
priceIds
98-
)
89+
IPulseConsumer(req.requester).pulseCallback{
90+
gas: req.callbackGasLimit
91+
}(sequenceNumber, msg.sender, publishTime, priceIds)
9992
{
10093
// Callback succeeded
10194
emitPriceUpdate(sequenceNumber, publishTime, priceIds, priceFeeds);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ error InsufficientFee();
88
error Unauthorized();
99
error InvalidCallbackGas();
1010
error CallbackFailed();
11-
error InvalidPriceIds(bytes32[] requested, bytes32[] stored);
11+
error InvalidPriceIds(bytes32 providedPriceIdsHash, bytes32 storedPriceIdsHash);
1212
error InvalidCallbackGasLimit(uint256 requested, uint256 stored);
1313
error ExceedsMaxPrices(uint32 requested, uint32 maxAllowed);
14+
error InsufficientGas();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ contract PulseState {
99
struct Request {
1010
uint64 sequenceNumber;
1111
uint256 publishTime;
12-
bytes32[] priceIds;
12+
bytes32 priceIdsHash;
1313
uint256 callbackGasLimit;
1414
address requester;
1515
}

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

Lines changed: 51 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ contract PulseTest is Test, PulseEvents {
190190
PulseState.Request memory expectedRequest = PulseState.Request({
191191
sequenceNumber: 1,
192192
publishTime: publishTime,
193-
priceIds: priceIds,
193+
priceIdsHash: keccak256(abi.encode(priceIds)),
194194
callbackGasLimit: CALLBACK_GAS_LIMIT,
195195
requester: address(consumer)
196196
});
@@ -209,10 +209,7 @@ contract PulseTest is Test, PulseEvents {
209209
PulseState.Request memory lastRequest = pulse.getRequest(1);
210210
assertEq(lastRequest.sequenceNumber, expectedRequest.sequenceNumber);
211211
assertEq(lastRequest.publishTime, expectedRequest.publishTime);
212-
assertEq(
213-
keccak256(abi.encode(lastRequest.priceIds)),
214-
keccak256(abi.encode(expectedRequest.priceIds))
215-
);
212+
assertEq(lastRequest.priceIdsHash, expectedRequest.priceIdsHash);
216213
assertEq(
217214
lastRequest.callbackGasLimit,
218215
expectedRequest.callbackGasLimit
@@ -245,7 +242,6 @@ contract PulseTest is Test, PulseEvents {
245242

246243
// Fund the consumer contract
247244
vm.deal(address(consumer), 1 gwei);
248-
249245
uint128 totalFee = calculateTotalFee();
250246

251247
// Step 1: Make the request as consumer
@@ -278,7 +274,7 @@ contract PulseTest is Test, PulseEvents {
278274
expectedPublishTimes[1] = publishTime;
279275

280276
// Expect the PriceUpdateExecuted event with all price data
281-
vm.expectEmit(true, true, false, true);
277+
vm.expectEmit();
282278
emit PriceUpdateExecuted(
283279
sequenceNumber,
284280
updater,
@@ -294,12 +290,7 @@ contract PulseTest is Test, PulseEvents {
294290
bytes[] memory updateData = createMockUpdateData(priceFeeds);
295291

296292
vm.prank(updater);
297-
pulse.executeCallback(
298-
sequenceNumber,
299-
priceIds,
300-
updateData,
301-
CALLBACK_GAS_LIMIT
302-
);
293+
pulse.executeCallback(sequenceNumber, updateData, priceIds);
303294

304295
// Verify callback was executed
305296
assertEq(consumer.lastSequenceNumber(), sequenceNumber);
@@ -321,7 +312,7 @@ contract PulseTest is Test, PulseEvents {
321312
mockParsePriceFeedUpdates(priceFeeds);
322313
bytes[] memory updateData = createMockUpdateData(priceFeeds);
323314

324-
vm.expectEmit(true, true, true, true);
315+
vm.expectEmit();
325316
emit PriceUpdateCallbackFailed(
326317
sequenceNumber,
327318
updater,
@@ -332,12 +323,7 @@ contract PulseTest is Test, PulseEvents {
332323
);
333324

334325
vm.prank(updater);
335-
pulse.executeCallback(
336-
sequenceNumber,
337-
priceIds,
338-
updateData,
339-
CALLBACK_GAS_LIMIT
340-
);
326+
pulse.executeCallback(sequenceNumber, updateData, priceIds);
341327
}
342328

343329
function testExecuteCallbackCustomErrorFailure() public {
@@ -355,7 +341,7 @@ contract PulseTest is Test, PulseEvents {
355341
mockParsePriceFeedUpdates(priceFeeds);
356342
bytes[] memory updateData = createMockUpdateData(priceFeeds);
357343

358-
vm.expectEmit(true, true, true, true);
344+
vm.expectEmit();
359345
emit PriceUpdateCallbackFailed(
360346
sequenceNumber,
361347
updater,
@@ -366,100 +352,28 @@ contract PulseTest is Test, PulseEvents {
366352
);
367353

368354
vm.prank(updater);
369-
pulse.executeCallback(
370-
sequenceNumber,
371-
priceIds,
372-
updateData,
373-
CALLBACK_GAS_LIMIT
374-
);
375-
}
376-
377-
// Test executing callback with mismatched price IDs
378-
function testExecuteCallbackWithMismatchedPriceIds() public {
379-
(
380-
uint64 sequenceNumber,
381-
bytes32[] memory originalPriceIds,
382-
uint256 publishTime
383-
) = setupConsumerRequest(address(consumer));
384-
385-
// Create different price IDs array
386-
bytes32[] memory differentPriceIds = new bytes32[](1);
387-
differentPriceIds[0] = bytes32(uint256(1)); // Different price ID
388-
389-
PythStructs.PriceFeed[] memory priceFeeds = createMockPriceFeeds(
390-
publishTime
391-
);
392-
mockParsePriceFeedUpdates(priceFeeds);
393-
bytes[] memory updateData = createMockUpdateData(priceFeeds);
394-
395-
vm.prank(updater);
396-
vm.expectRevert(
397-
abi.encodeWithSelector(
398-
InvalidPriceIds.selector,
399-
differentPriceIds,
400-
originalPriceIds
401-
)
402-
);
403-
pulse.executeCallback(
404-
sequenceNumber,
405-
differentPriceIds,
406-
updateData,
407-
CALLBACK_GAS_LIMIT
408-
);
355+
pulse.executeCallback(sequenceNumber, updateData, priceIds);
409356
}
410357

411358
function testExecuteCallbackWithInsufficientGas() public {
359+
// Setup request with 1M gas limit
412360
(
413361
uint64 sequenceNumber,
414362
bytes32[] memory priceIds,
415363
uint256 publishTime
416364
) = setupConsumerRequest(address(consumer));
417365

366+
// Setup mock data
418367
PythStructs.PriceFeed[] memory priceFeeds = createMockPriceFeeds(
419368
publishTime
420369
);
421370
mockParsePriceFeedUpdates(priceFeeds);
422371
bytes[] memory updateData = createMockUpdateData(priceFeeds);
423372

373+
// Try executing with only 10K gas when 1M is required
424374
vm.prank(updater);
425-
vm.expectRevert();
426-
pulse.executeCallback{gas: 10000}(
427-
sequenceNumber,
428-
priceIds,
429-
updateData,
430-
CALLBACK_GAS_LIMIT
431-
);
432-
}
433-
434-
function testExecuteCallbackWithInvalidGasLimit() public {
435-
(
436-
uint64 sequenceNumber,
437-
bytes32[] memory priceIds,
438-
uint256 publishTime
439-
) = setupConsumerRequest(address(consumer));
440-
441-
PythStructs.PriceFeed[] memory priceFeeds = createMockPriceFeeds(
442-
publishTime
443-
);
444-
mockParsePriceFeedUpdates(priceFeeds);
445-
bytes[] memory updateData = createMockUpdateData(priceFeeds);
446-
447-
// Try to execute with different gas limit than what was requested
448-
uint256 differentGasLimit = CALLBACK_GAS_LIMIT + 1000;
449-
vm.prank(updater);
450-
vm.expectRevert(
451-
abi.encodeWithSelector(
452-
InvalidCallbackGasLimit.selector,
453-
differentGasLimit,
454-
CALLBACK_GAS_LIMIT
455-
)
456-
);
457-
pulse.executeCallback(
458-
sequenceNumber,
459-
priceIds,
460-
updateData,
461-
differentGasLimit
462-
);
375+
vm.expectRevert(InsufficientGas.selector);
376+
pulse.executeCallback{gas: 10000}(sequenceNumber, updateData, priceIds); // Will fail because gasleft() < callbackGasLimit
463377
}
464378

465379
function testExecuteCallbackWithFutureTimestamp() public {
@@ -483,12 +397,7 @@ contract PulseTest is Test, PulseEvents {
483397

484398
vm.prank(updater);
485399
// Should succeed because we're simulating receiving future-dated price updates
486-
pulse.executeCallback(
487-
sequenceNumber,
488-
priceIds,
489-
updateData,
490-
CALLBACK_GAS_LIMIT
491-
);
400+
pulse.executeCallback(sequenceNumber, updateData, priceIds);
492401

493402
// Verify the callback was executed with future timestamp
494403
assertEq(consumer.lastPublishTime(), futureTime);
@@ -509,22 +418,12 @@ contract PulseTest is Test, PulseEvents {
509418

510419
// First execution
511420
vm.prank(updater);
512-
pulse.executeCallback(
513-
sequenceNumber,
514-
priceIds,
515-
updateData,
516-
CALLBACK_GAS_LIMIT
517-
);
421+
pulse.executeCallback(sequenceNumber, updateData, priceIds);
518422

519423
// Second execution should fail
520424
vm.prank(updater);
521425
vm.expectRevert(NoSuchRequest.selector);
522-
pulse.executeCallback(
523-
sequenceNumber,
524-
priceIds,
525-
updateData,
526-
CALLBACK_GAS_LIMIT
527-
);
426+
pulse.executeCallback(sequenceNumber, updateData, priceIds);
528427
}
529428

530429
function testGetFee() public {
@@ -661,4 +560,39 @@ contract PulseTest is Test, PulseEvents {
661560
vm.expectRevert("Insufficient balance");
662561
pulse.withdrawAsFeeManager(1 ether);
663562
}
563+
564+
// Add new test for invalid priceIds
565+
function testExecuteCallbackWithInvalidPriceIds() public {
566+
bytes32[] memory priceIds = createPriceIds();
567+
uint256 publishTime = block.timestamp;
568+
569+
// Setup request
570+
(uint64 sequenceNumber, , ) = setupConsumerRequest(address(consumer));
571+
572+
// Create different priceIds
573+
bytes32[] memory wrongPriceIds = new bytes32[](2);
574+
wrongPriceIds[0] = bytes32(uint256(1)); // Different price IDs
575+
wrongPriceIds[1] = bytes32(uint256(2));
576+
577+
PythStructs.PriceFeed[] memory priceFeeds = createMockPriceFeeds(
578+
publishTime
579+
);
580+
mockParsePriceFeedUpdates(priceFeeds);
581+
bytes[] memory updateData = createMockUpdateData(priceFeeds);
582+
583+
// Calculate hashes for both arrays
584+
bytes32 providedPriceIdsHash = keccak256(abi.encode(wrongPriceIds));
585+
bytes32 storedPriceIdsHash = keccak256(abi.encode(priceIds));
586+
587+
// Should revert when trying to execute with wrong priceIds
588+
vm.prank(updater);
589+
vm.expectRevert(
590+
abi.encodeWithSelector(
591+
InvalidPriceIds.selector,
592+
providedPriceIdsHash,
593+
storedPriceIdsHash
594+
)
595+
);
596+
pulse.executeCallback(sequenceNumber, updateData, wrongPriceIds);
597+
}
664598
}

0 commit comments

Comments
 (0)