Skip to content

Commit 13c3967

Browse files
cctdanielcctdaniel
committed
address comments
1 parent f3ec247 commit 13c3967

File tree

4 files changed

+60
-35
lines changed

4 files changed

+60
-35
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22

33
pragma solidity ^0.8.0;
44

5+
import "@pythnetwork/pyth-sdk-solidity/IPyth.sol";
56
import "./PulseEvents.sol";
67
import "./PulseState.sol";
78

89
interface IPulseConsumer {
910
function pulseCallback(
1011
uint64 sequenceNumber,
1112
address updater,
12-
uint256 publishTime,
13-
bytes32[] calldata priceIds
13+
PythStructs.PriceFeed[] memory priceFeeds
1414
) external;
1515
}
1616

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,15 @@ abstract contract Pulse is IPulse, PulseState {
6363
bytes32[] calldata priceIds
6464
) external payable override {
6565
Request storage req = findActiveRequest(sequenceNumber);
66+
67+
// Verify priceIds match
6668
bytes32 providedPriceIdsHash = keccak256(abi.encode(priceIds));
6769
bytes32 storedPriceIdsHash = req.priceIdsHash;
68-
6970
if (providedPriceIdsHash != storedPriceIdsHash) {
7071
revert InvalidPriceIds(providedPriceIdsHash, storedPriceIdsHash);
7172
}
7273

73-
// Check if there's enough gas left for the callback
74-
if (gasleft() < req.callbackGasLimit) {
75-
revert InsufficientGas();
76-
}
77-
74+
// Parse price feeds first to measure gas usage
7875
PythStructs.PriceFeed[] memory priceFeeds = IPyth(_state.pyth)
7976
.parsePriceFeedUpdates(
8077
updateData,
@@ -83,21 +80,23 @@ abstract contract Pulse is IPulse, PulseState {
8380
SafeCast.toUint64(req.publishTime)
8481
);
8582

86-
uint256 publishTime = priceFeeds[0].price.publishTime;
83+
// Check if enough gas remains for the callback
84+
if (gasleft() < req.callbackGasLimit) {
85+
revert InsufficientGas();
86+
}
8787

8888
try
8989
IPulseConsumer(req.requester).pulseCallback{
9090
gas: req.callbackGasLimit
91-
}(sequenceNumber, msg.sender, publishTime, priceIds)
91+
}(sequenceNumber, msg.sender, priceFeeds)
9292
{
9393
// Callback succeeded
94-
emitPriceUpdate(sequenceNumber, publishTime, priceIds, priceFeeds);
94+
emitPriceUpdate(sequenceNumber, priceIds, priceFeeds);
9595
} catch Error(string memory reason) {
9696
// Explicit revert/require
9797
emit PriceUpdateCallbackFailed(
9898
sequenceNumber,
9999
msg.sender,
100-
publishTime,
101100
priceIds,
102101
req.requester,
103102
reason
@@ -107,7 +106,6 @@ abstract contract Pulse is IPulse, PulseState {
107106
emit PriceUpdateCallbackFailed(
108107
sequenceNumber,
109108
msg.sender,
110-
publishTime,
111109
priceIds,
112110
req.requester,
113111
"low-level error (possibly out of gas)"
@@ -119,7 +117,6 @@ abstract contract Pulse is IPulse, PulseState {
119117

120118
function emitPriceUpdate(
121119
uint64 sequenceNumber,
122-
uint256 publishTime,
123120
bytes32[] memory priceIds,
124121
PythStructs.PriceFeed[] memory priceFeeds
125122
) internal {
@@ -138,7 +135,6 @@ abstract contract Pulse is IPulse, PulseState {
138135
emit PriceUpdateExecuted(
139136
sequenceNumber,
140137
msg.sender,
141-
publishTime,
142138
priceIds,
143139
prices,
144140
conf,

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ interface PulseEvents {
99
event PriceUpdateExecuted(
1010
uint64 indexed sequenceNumber,
1111
address indexed updater,
12-
uint256 publishTime,
1312
bytes32[] priceIds,
1413
int64[] prices,
1514
uint64[] conf,
@@ -22,7 +21,6 @@ interface PulseEvents {
2221
event PriceUpdateCallbackFailed(
2322
uint64 indexed sequenceNumber,
2423
address indexed updater,
25-
uint256 publishTime,
2624
bytes32[] priceIds,
2725
address requester,
2826
string reason

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

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
pragma solidity ^0.8.0;
44

55
import "forge-std/Test.sol";
6+
import "@pythnetwork/pyth-sdk-solidity/IPyth.sol";
67
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
78
import "../contracts/pulse/PulseUpgradeable.sol";
89
import "../contracts/pulse/IPulse.sol";
@@ -13,28 +14,34 @@ import "../contracts/pulse/PulseErrors.sol";
1314
contract MockPulseConsumer is IPulseConsumer {
1415
uint64 public lastSequenceNumber;
1516
address public lastUpdater;
16-
uint256 public lastPublishTime;
17-
bytes32[] public lastPriceIds;
17+
PythStructs.PriceFeed[] private _lastPriceFeeds;
1818

1919
function pulseCallback(
2020
uint64 sequenceNumber,
2121
address updater,
22-
uint256 publishTime,
23-
bytes32[] calldata priceIds
22+
PythStructs.PriceFeed[] memory priceFeeds
2423
) external override {
2524
lastSequenceNumber = sequenceNumber;
2625
lastUpdater = updater;
27-
lastPublishTime = publishTime;
28-
lastPriceIds = priceIds;
26+
for (uint i = 0; i < priceFeeds.length; i++) {
27+
_lastPriceFeeds.push(priceFeeds[i]);
28+
}
29+
}
30+
31+
function lastPriceFeeds()
32+
external
33+
view
34+
returns (PythStructs.PriceFeed[] memory)
35+
{
36+
return _lastPriceFeeds;
2937
}
3038
}
3139

3240
contract FailingPulseConsumer is IPulseConsumer {
3341
function pulseCallback(
3442
uint64,
3543
address,
36-
uint256,
37-
bytes32[] calldata
44+
PythStructs.PriceFeed[] memory
3845
) external pure override {
3946
revert("callback failed");
4047
}
@@ -46,8 +53,7 @@ contract CustomErrorPulseConsumer is IPulseConsumer {
4653
function pulseCallback(
4754
uint64,
4855
address,
49-
uint256,
50-
bytes32[] calldata
56+
PythStructs.PriceFeed[] memory
5157
) external pure override {
5258
revert CustomError("callback failed");
5359
}
@@ -278,7 +284,6 @@ contract PulseTest is Test, PulseEvents {
278284
emit PriceUpdateExecuted(
279285
sequenceNumber,
280286
updater,
281-
publishTime,
282287
priceIds,
283288
expectedPrices,
284289
expectedConf,
@@ -294,7 +299,22 @@ contract PulseTest is Test, PulseEvents {
294299

295300
// Verify callback was executed
296301
assertEq(consumer.lastSequenceNumber(), sequenceNumber);
297-
assertEq(consumer.lastPublishTime(), publishTime);
302+
303+
// Compare price feeds array length
304+
PythStructs.PriceFeed[] memory lastFeeds = consumer.lastPriceFeeds();
305+
assertEq(lastFeeds.length, priceFeeds.length);
306+
307+
// Compare each price feed
308+
for (uint i = 0; i < priceFeeds.length; i++) {
309+
assertEq(lastFeeds[i].id, priceFeeds[i].id);
310+
assertEq(lastFeeds[i].price.price, priceFeeds[i].price.price);
311+
assertEq(lastFeeds[i].price.conf, priceFeeds[i].price.conf);
312+
assertEq(lastFeeds[i].price.expo, priceFeeds[i].price.expo);
313+
assertEq(
314+
lastFeeds[i].price.publishTime,
315+
priceFeeds[i].price.publishTime
316+
);
317+
}
298318
}
299319

300320
function testExecuteCallbackFailure() public {
@@ -316,7 +336,6 @@ contract PulseTest is Test, PulseEvents {
316336
emit PriceUpdateCallbackFailed(
317337
sequenceNumber,
318338
updater,
319-
publishTime,
320339
priceIds,
321340
address(failingConsumer),
322341
"callback failed"
@@ -345,7 +364,6 @@ contract PulseTest is Test, PulseEvents {
345364
emit PriceUpdateCallbackFailed(
346365
sequenceNumber,
347366
updater,
348-
publishTime,
349367
priceIds,
350368
address(failingConsumer),
351369
"low-level error (possibly out of gas)"
@@ -370,10 +388,14 @@ contract PulseTest is Test, PulseEvents {
370388
mockParsePriceFeedUpdates(priceFeeds);
371389
bytes[] memory updateData = createMockUpdateData(priceFeeds);
372390

373-
// Try executing with only 10K gas when 1M is required
391+
// Try executing with only 100K gas when 1M is required
374392
vm.prank(updater);
375393
vm.expectRevert(InsufficientGas.selector);
376-
pulse.executeCallback{gas: 10000}(sequenceNumber, updateData, priceIds); // Will fail because gasleft() < callbackGasLimit
394+
pulse.executeCallback{gas: 100000}(
395+
sequenceNumber,
396+
updateData,
397+
priceIds
398+
); // Will fail because gasleft() < callbackGasLimit
377399
}
378400

379401
function testExecuteCallbackWithFutureTimestamp() public {
@@ -399,8 +421,17 @@ contract PulseTest is Test, PulseEvents {
399421
// Should succeed because we're simulating receiving future-dated price updates
400422
pulse.executeCallback(sequenceNumber, updateData, priceIds);
401423

402-
// Verify the callback was executed with future timestamp
403-
assertEq(consumer.lastPublishTime(), futureTime);
424+
// Compare price feeds array length
425+
PythStructs.PriceFeed[] memory lastFeeds = consumer.lastPriceFeeds();
426+
assertEq(lastFeeds.length, priceFeeds.length);
427+
428+
// Compare each price feed publish time
429+
for (uint i = 0; i < priceFeeds.length; i++) {
430+
assertEq(
431+
lastFeeds[i].price.publishTime,
432+
priceFeeds[i].price.publishTime
433+
);
434+
}
404435
}
405436

406437
function testDoubleExecuteCallback() public {

0 commit comments

Comments
 (0)