Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(pulse): Gas optimizations #2492

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat(pulse): Gas optimizations #2492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
85c3921
with optimizations
jayantk Mar 9, 2025
a4b172d
fix
jayantk Mar 17, 2025
d9b75f0
cleanup
jayantk Mar 17, 2025
ff7c2f2
comments
jayantk Mar 17, 2025
98513c7
fix
jayantk Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions target_chains/ethereum/contracts/contracts/pulse/IPulse.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,23 +139,4 @@ interface IPulse is PulseEvents {
function setExclusivityPeriod(uint256 periodSeconds) external;

function getExclusivityPeriod() external view returns (uint256);

/**
* @notice Gets the first N active requests
* @param count Maximum number of active requests to return
* @return requests Array of active requests, ordered from oldest to newest
* @return actualCount Number of active requests found (may be less than count)
* @dev Gas Usage: This function's gas cost scales linearly with the number of requests
* between firstUnfulfilledSeq and currentSequenceNumber. Each iteration costs approximately:
* - 2100 gas for cold storage reads, 100 gas for warm storage reads (SLOAD)
* - Additional gas for array operations
* The function starts from firstUnfulfilledSeq (all requests before this are fulfilled)
* and scans forward until it finds enough active requests or reaches currentSequenceNumber.
*/
function getFirstActiveRequests(
uint256 count
)
external
view
returns (PulseState.Request[] memory requests, uint256 actualCount);
}
76 changes: 10 additions & 66 deletions target_chains/ethereum/contracts/contracts/pulse/Pulse.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,9 @@ 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);
}
req.priceIdsHash = bytes32(uint256(1));
req.fee = 1;
req.provider = address(1);
}
}
}
Expand All @@ -72,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);
Expand All @@ -83,16 +78,12 @@ abstract contract Pulse is IPulse, PulseState {
Request storage req = allocRequest(requestSequenceNumber);
req.sequenceNumber = requestSequenceNumber;
req.publishTime = publishTime;
req.callbackGasLimit = callbackGasLimit;
req.callbackGasLimit = SafeCast.toUint128(callbackGasLimit);
req.requester = msg.sender;
req.numPriceIds = uint8(priceIds.length);
req.provider = provider;
req.fee = SafeCast.toUint128(msg.value - _state.pythFeeInWei);
req.priceIdsHash = keccak256(abi.encode(priceIds));

// Copy price IDs to storage
for (uint8 i = 0; i < priceIds.length; i++) {
req.priceIds[i] = priceIds[i];
}
_state.accruedFeesInWei += _state.pythFeeInWei;

emit PriceUpdateRequested(req, priceIds);
Expand All @@ -118,14 +109,11 @@ abstract contract Pulse is IPulse, PulseState {
}

// Verify priceIds match
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]);
}
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?
Expand All @@ -151,16 +139,6 @@ abstract contract Pulse is IPulse, PulseState {

clearRequest(sequenceNumber);

// TODO: I'm pretty sure this is going to use a lot of gas because it's doing a storage lookup for each sequence number.
// a better solution would be a doubly-linked list of active requests.
// After successful callback, update firstUnfulfilledSeq if needed
while (
_state.firstUnfulfilledSeq < _state.currentSequenceNumber &&
!isActive(findRequest(_state.firstUnfulfilledSeq))
) {
_state.firstUnfulfilledSeq++;
}

try
IPulseConsumer(req.requester)._pulseCallback{
gas: req.callbackGasLimit
Expand Down Expand Up @@ -450,38 +428,4 @@ abstract contract Pulse is IPulse, PulseState {
function getExclusivityPeriod() external view override returns (uint256) {
return _state.exclusivityPeriodSeconds;
}

function getFirstActiveRequests(
uint256 count
)
external
view
override
returns (Request[] memory requests, uint256 actualCount)
{
requests = new Request[](count);
actualCount = 0;

// Start from the first unfulfilled sequence and work forwards
uint64 currentSeq = _state.firstUnfulfilledSeq;

// Continue until we find enough active requests or reach current sequence
while (
actualCount < count && currentSeq < _state.currentSequenceNumber
) {
Request memory req = findRequest(currentSeq);
if (isActive(req)) {
requests[actualCount] = req;
actualCount++;
}
currentSeq++;
}

// If we found fewer requests than asked for, resize the array
if (actualCount < count) {
assembly {
mstore(requests, actualCount)
}
}
}
}
1 change: 0 additions & 1 deletion target_chains/ethereum/contracts/contracts/pulse/PulseErrors.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
34 changes: 21 additions & 13 deletions target_chains/ethereum/contracts/contracts/pulse/PulseState.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,22 @@ pragma solidity ^0.8.0;
contract PulseState {
uint8 public constant NUM_REQUESTS = 32;
bytes1 public constant NUM_REQUESTS_MASK = 0x1f;
// Maximum number of price feeds per request. This limit keeps gas costs predictable and reasonable. 10 is a reasonable number for most use cases.
// Requests with more than 10 price feeds should be split into multiple requests
uint8 public constant MAX_PRICE_IDS = 10;

struct Request {
uint64 sequenceNumber;
uint64 publishTime;
// TODO: this is going to absolutely explode gas costs. Need to do something smarter here.
// possible solution is to hash the price ids and store the hash instead.
// The ids themselves can be retrieved from the event.
bytes32[MAX_PRICE_IDS] priceIds;
uint8 numPriceIds; // Actual number of price IDs used
uint256 callbackGasLimit;
// Storage slot 1 //
address requester;
uint64 sequenceNumber;
// 4 bytes unused
// Storage slot 2 //
address provider;
uint64 publishTime;
// 4 bytes unused
// Storage slot 3 //
// Hash of the array of price ids that should be provided to fulfill this request.
// The hash is order-sensitive.
bytes32 priceIdsHash;
// Storage slot 4 //
uint128 callbackGasLimit;
uint128 fee;
}

Expand All @@ -33,17 +34,24 @@ contract PulseState {
}

struct State {
address admin;
// Storage slot 1 //
uint128 pythFeeInWei;
uint128 accruedFeesInWei;
// Storage slot 2 //
address pyth;
uint64 currentSequenceNumber;
// 4 bytes unused
// Storage slot 3 //
address defaultProvider;
// 12 bytes unused
// Storage slot 4 //
uint256 exclusivityPeriodSeconds;
// Storage slot 5 //
address admin;
// 12 bytes unused
Request[NUM_REQUESTS] requests;
mapping(bytes32 => Request) requestsOverflow;
mapping(address => ProviderInfo) providers;
uint64 firstUnfulfilledSeq; // All sequences before this are fulfilled
}

State internal _state;
Expand Down
Loading
Loading