Skip to content

feat(pulse): optimize gas while keeping requests on-chain #2519

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.

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

Merged
merged 7 commits into from
Apr 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 12 additions & 12 deletions target_chains/ethereum/contracts/contracts/pulse/IPulse.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ interface IPulse is PulseEvents {
address provider,
uint64 publishTime,
bytes32[] calldata priceIds,
uint256 callbackGasLimit
uint32 callbackGasLimit
) external payable returns (uint64 sequenceNumber);

/**
Expand All @@ -80,7 +80,7 @@ interface IPulse is PulseEvents {
* @dev This is a fixed fee per request that goes to the Pyth protocol, separate from gas costs
* @return pythFeeInWei The base fee in wei that every request must pay
*/
function getPythFeeInWei() external view returns (uint128 pythFeeInWei);
function getPythFeeInWei() external view returns (uint96 pythFeeInWei);

/**
* @notice Calculates the total fee required for a price update request
Expand All @@ -92,9 +92,9 @@ interface IPulse is PulseEvents {
*/
function getFee(
address provider,
uint256 callbackGasLimit,
uint32 callbackGasLimit,
bytes32[] calldata priceIds
) external view returns (uint128 feeAmount);
) external view returns (uint96 feeAmount);

function getAccruedPythFees()
external
Expand All @@ -116,16 +116,16 @@ interface IPulse is PulseEvents {
function withdrawAsFeeManager(address provider, uint128 amount) external;

function registerProvider(
uint128 baseFeeInWei,
uint128 feePerFeedInWei,
uint128 feePerGasInWei
uint96 baseFeeInWei,
uint96 feePerFeedInWei,
uint96 feePerGasInWei
) external;

function setProviderFee(
address provider,
uint128 newBaseFeeInWei,
uint128 newFeePerFeedInWei,
uint128 newFeePerGasInWei
uint96 newBaseFeeInWei,
uint96 newFeePerFeedInWei,
uint96 newFeePerGasInWei
) external;

function getProviderInfo(
Expand All @@ -136,9 +136,9 @@ interface IPulse is PulseEvents {

function setDefaultProvider(address provider) external;

function setExclusivityPeriod(uint256 periodSeconds) external;
function setExclusivityPeriod(uint32 periodSeconds) external;

function getExclusivityPeriod() external view returns (uint256);
function getExclusivityPeriod() external view returns (uint32);

/**
* @notice Gets the first N active requests
Expand Down
84 changes: 48 additions & 36 deletions target_chains/ethereum/contracts/contracts/pulse/Pulse.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import "./PulseErrors.sol";
abstract contract Pulse is IPulse, PulseState {
function _initialize(
address admin,
uint128 pythFeeInWei,
uint96 pythFeeInWei,
address pythAddress,
address defaultProvider,
bool prefillRequestStorage,
uint256 exclusivityPeriodSeconds
uint32 exclusivityPeriodSeconds
) internal {
require(admin != address(0), "admin is zero address");
require(pythAddress != address(0), "pyth is zero address");
Expand Down Expand Up @@ -44,11 +44,6 @@ 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);
}
}
}
}
Expand All @@ -58,7 +53,7 @@ abstract contract Pulse is IPulse, PulseState {
address provider,
uint64 publishTime,
bytes32[] calldata priceIds,
uint256 callbackGasLimit
uint32 callbackGasLimit
) external payable override returns (uint64 requestSequenceNumber) {
require(
_state.providers[provider].isRegistered,
Expand All @@ -77,21 +72,29 @@ abstract contract Pulse is IPulse, PulseState {
}
requestSequenceNumber = _state.currentSequenceNumber++;

uint128 requiredFee = getFee(provider, callbackGasLimit, priceIds);
uint96 requiredFee = getFee(provider, callbackGasLimit, priceIds);
if (msg.value < requiredFee) revert InsufficientFee();

Request storage req = allocRequest(requestSequenceNumber);
req.sequenceNumber = requestSequenceNumber;
req.publishTime = publishTime;
req.callbackGasLimit = callbackGasLimit;
req.requester = msg.sender;
req.numPriceIds = uint8(priceIds.length);
req.provider = provider;
req.fee = SafeCast.toUint128(msg.value - _state.pythFeeInWei);
req.fee = SafeCast.toUint96(msg.value - _state.pythFeeInWei);

// Copy price IDs to storage
// Create array with the right size
req.priceIdPrefixes = new bytes8[](priceIds.length);

// Copy only the first 8 bytes of each price ID to storage
for (uint8 i = 0; i < priceIds.length; i++) {
req.priceIds[i] = priceIds[i];
// Extract first 8 bytes of the price ID
bytes32 priceId = priceIds[i];
bytes8 prefix;
assembly {
prefix := priceId
}
req.priceIdPrefixes[i] = prefix;
}
_state.accruedFeesInWei += _state.pythFeeInWei;

Expand Down Expand Up @@ -119,12 +122,21 @@ abstract contract Pulse is IPulse, PulseState {

// Verify priceIds match
require(
priceIds.length == req.numPriceIds,
priceIds.length == req.priceIdPrefixes.length,
"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]);
for (uint8 i = 0; i < req.priceIdPrefixes.length; i++) {
// Extract first 8 bytes of the provided price ID
bytes32 priceId = priceIds[i];
bytes8 prefix;
assembly {
prefix := priceId
}

// Compare with stored prefix
if (prefix != req.priceIdPrefixes[i]) {
// Now we can directly use the bytes8 prefix in the error
revert InvalidPriceIds(priceIds[i], req.priceIdPrefixes[i]);
}
}

Expand Down Expand Up @@ -222,31 +234,31 @@ abstract contract Pulse is IPulse, PulseState {

function getFee(
address provider,
uint256 callbackGasLimit,
uint32 callbackGasLimit,
bytes32[] calldata priceIds
) public view override returns (uint128 feeAmount) {
uint128 baseFee = _state.pythFeeInWei; // Fixed fee to Pyth
) public view override returns (uint96 feeAmount) {
uint96 baseFee = _state.pythFeeInWei; // Fixed fee to Pyth
// Note: The provider needs to set its fees to include the fee charged by the Pyth contract.
// Ideally, we would be able to automatically compute the pyth fees from the priceIds, but the
// fee computation on IPyth assumes it has the full updated data.
uint128 providerBaseFee = _state.providers[provider].baseFeeInWei;
uint128 providerFeedFee = SafeCast.toUint128(
uint96 providerBaseFee = _state.providers[provider].baseFeeInWei;
uint96 providerFeedFee = SafeCast.toUint96(
priceIds.length * _state.providers[provider].feePerFeedInWei
);
uint128 providerFeeInWei = _state.providers[provider].feePerGasInWei; // Provider's per-gas rate
uint96 providerFeeInWei = _state.providers[provider].feePerGasInWei; // Provider's per-gas rate
uint256 gasFee = callbackGasLimit * providerFeeInWei; // Total provider fee based on gas
feeAmount =
baseFee +
providerBaseFee +
providerFeedFee +
SafeCast.toUint128(gasFee); // Total fee user needs to pay
SafeCast.toUint96(gasFee); // Total fee user needs to pay
}

function getPythFeeInWei()
public
view
override
returns (uint128 pythFeeInWei)
returns (uint96 pythFeeInWei)
{
pythFeeInWei = _state.pythFeeInWei;
}
Expand Down Expand Up @@ -367,9 +379,9 @@ abstract contract Pulse is IPulse, PulseState {
}

function registerProvider(
uint128 baseFeeInWei,
uint128 feePerFeedInWei,
uint128 feePerGasInWei
uint96 baseFeeInWei,
uint96 feePerFeedInWei,
uint96 feePerGasInWei
) external override {
ProviderInfo storage provider = _state.providers[msg.sender];
require(!provider.isRegistered, "Provider already registered");
Expand All @@ -382,9 +394,9 @@ abstract contract Pulse is IPulse, PulseState {

function setProviderFee(
address provider,
uint128 newBaseFeeInWei,
uint128 newFeePerFeedInWei,
uint128 newFeePerGasInWei
uint96 newBaseFeeInWei,
uint96 newFeePerFeedInWei,
uint96 newFeePerGasInWei
) external override {
require(
_state.providers[provider].isRegistered,
Expand All @@ -396,9 +408,9 @@ abstract contract Pulse is IPulse, PulseState {
"Only provider or fee manager can invoke this method"
);

uint128 oldBaseFee = _state.providers[provider].baseFeeInWei;
uint128 oldFeePerFeed = _state.providers[provider].feePerFeedInWei;
uint128 oldFeePerGas = _state.providers[provider].feePerGasInWei;
uint96 oldBaseFee = _state.providers[provider].baseFeeInWei;
uint96 oldFeePerFeed = _state.providers[provider].feePerFeedInWei;
uint96 oldFeePerGas = _state.providers[provider].feePerGasInWei;
_state.providers[provider].baseFeeInWei = newBaseFeeInWei;
_state.providers[provider].feePerFeedInWei = newFeePerFeedInWei;
_state.providers[provider].feePerGasInWei = newFeePerGasInWei;
Expand Down Expand Up @@ -437,7 +449,7 @@ abstract contract Pulse is IPulse, PulseState {
emit DefaultProviderUpdated(oldProvider, provider);
}

function setExclusivityPeriod(uint256 periodSeconds) external override {
function setExclusivityPeriod(uint32 periodSeconds) external override {
require(
msg.sender == _state.admin,
"Only admin can set exclusivity period"
Expand All @@ -447,7 +459,7 @@ abstract contract Pulse is IPulse, PulseState {
emit ExclusivityPeriodUpdated(oldPeriod, periodSeconds);
}

function getExclusivityPeriod() external view override returns (uint256) {
function getExclusivityPeriod() external view override returns (uint32) {
return _state.exclusivityPeriodSeconds;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ error InsufficientFee();
error Unauthorized();
error InvalidCallbackGas();
error CallbackFailed();
error InvalidPriceIds(bytes32 providedPriceIdsHash, bytes32 storedPriceIdsHash);
error InvalidPriceIds(bytes32 providedPriceId, bytes8 storedPriceId);
error InvalidCallbackGasLimit(uint256 requested, uint256 stored);
error ExceedsMaxPrices(uint32 requested, uint32 maxAllowed);
error TooManyPriceIds(uint256 provided, uint256 maximum);
14 changes: 7 additions & 7 deletions target_chains/ethereum/contracts/contracts/pulse/PulseEvents.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ interface PulseEvents {
address newFeeManager
);

event ProviderRegistered(address indexed provider, uint128 feeInWei);
event ProviderRegistered(address indexed provider, uint96 feeInWei);
event ProviderFeeUpdated(
address indexed provider,
uint128 oldBaseFee,
uint128 oldFeePerFeed,
uint128 oldFeePerGas,
uint128 newBaseFee,
uint128 newFeePerFeed,
uint128 newFeePerGas
uint96 oldBaseFee,
uint96 oldFeePerFeed,
uint96 oldFeePerGas,
uint96 newBaseFee,
uint96 newFeePerFeed,
uint96 newFeePerGas
);
event DefaultProviderUpdated(address oldProvider, address newProvider);

Expand Down
53 changes: 37 additions & 16 deletions target_chains/ethereum/contracts/contracts/pulse/PulseState.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,41 +10,62 @@ contract PulseState {
uint8 public constant MAX_PRICE_IDS = 10;

struct Request {
// Slot 1: 8 + 8 + 4 + 12 = 32 bytes
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;
uint32 callbackGasLimit;
uint96 fee;
// Slot 2: 20 + 12 = 32 bytes
address requester;
// 12 bytes padding

// Slot 3: 20 + 12 = 32 bytes
address provider;
uint128 fee;
// 12 bytes padding

// Dynamic array starts at its own slot
// Store only first 8 bytes of each price ID to save gas
bytes8[] priceIdPrefixes;
}

struct ProviderInfo {
uint128 baseFeeInWei;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this particular ordering if we can pack it even more?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think we can do better than 3 slots for ProviderInfo unless i'm missing something?

uint128 feePerFeedInWei;
uint128 feePerGasInWei;
// Slot 1: 12 + 12 + 8 = 32 bytes
uint96 baseFeeInWei;
uint96 feePerFeedInWei;
// 8 bytes padding

// Slot 2: 12 + 16 + 4 = 32 bytes
uint96 feePerGasInWei;
uint128 accruedFeesInWei;
// 4 bytes padding

// Slot 3: 20 + 1 + 11 = 32 bytes
address feeManager;
bool isRegistered;
// 11 bytes padding
}

struct State {
// Slot 1: 20 + 4 + 8 = 32 bytes
address admin;
uint128 pythFeeInWei;
uint128 accruedFeesInWei;
address pyth;
uint32 exclusivityPeriodSeconds;
uint64 currentSequenceNumber;
// Slot 2: 20 + 8 + 4 = 32 bytes
address pyth;
uint64 firstUnfulfilledSeq;
// 4 bytes padding

// Slot 3: 20 + 12 = 32 bytes
address defaultProvider;
uint256 exclusivityPeriodSeconds;
uint96 pythFeeInWei;
// Slot 4: 16 + 16 = 32 bytes
uint128 accruedFeesInWei;
// 16 bytes padding

// These take their own slots regardless of ordering
Request[NUM_REQUESTS] requests;
mapping(bytes32 => Request) requestsOverflow;
mapping(address => ProviderInfo) providers;
uint64 firstUnfulfilledSeq; // All sequences before this are fulfilled
}

State internal _state;
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ contract PulseUpgradeable is
function initialize(
address owner,
address admin,
uint128 pythFeeInWei,
uint96 pythFeeInWei,
address pythAddress,
address defaultProvider,
bool prefillRequestStorage,
uint256 exclusivityPeriodSeconds
uint32 exclusivityPeriodSeconds
) external initializer {
require(owner != address(0), "owner is zero address");
require(admin != address(0), "admin is zero address");
Expand Down
Loading
Loading