Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
111 changes: 75 additions & 36 deletions target_chains/ethereum/contracts/contracts/pulse/Scheduler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ abstract contract Scheduler is IScheduler, SchedulerState {
bytes[] calldata updateData,
bytes32[] calldata priceIds
) external override {
uint256 startGas = gasleft();

SubscriptionStatus storage status = _state.subscriptionStatuses[
subscriptionId
];
Expand All @@ -261,9 +263,12 @@ abstract contract Scheduler is IScheduler, SchedulerState {
revert InactiveSubscription();
}

// Verify price IDs match subscription
// Verify price IDs match subscription length
if (priceIds.length != params.priceIds.length) {
revert InvalidPriceIdsLength(priceIds[0], params.priceIds[0]);
revert InvalidPriceIdsLength(
priceIds.length,
params.priceIds.length
);
}

// Keepers must provide priceIds in the exact same order as defined in the subscription
Expand All @@ -277,26 +282,24 @@ abstract contract Scheduler is IScheduler, SchedulerState {
IPyth pyth = IPyth(_state.pyth);
uint256 pythFee = pyth.getUpdateFee(updateData);

// Check if subscription has enough balance
// If we don't have enough balance, revert
if (status.balanceInWei < pythFee) {
revert InsufficientBalance();
}

// Parse the price feed updates with an acceptable timestamp range of [-1h, +10s] from now.
// We will validate the trigger conditions ourselves.
uint64 curTime = SafeCast.toUint64(block.timestamp);
uint64 maxPublishTime = curTime + FUTURE_TIMESTAMP_MAX_VALIDITY_PERIOD;
uint64 minPublishTime = curTime > PAST_TIMESTAMP_MAX_VALIDITY_PERIOD
? curTime - PAST_TIMESTAMP_MAX_VALIDITY_PERIOD
: 0;
(
PythStructs.PriceFeed[] memory priceFeeds,
uint64[] memory slots
) = pyth.parsePriceFeedUpdatesWithSlots{value: pythFee}(
updateData,
priceIds,
minPublishTime,
maxPublishTime
curTime > PAST_TIMESTAMP_MAX_VALIDITY_PERIOD
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to inline this (and extract logic into helper functions) to avoid stack depth issues

? curTime - PAST_TIMESTAMP_MAX_VALIDITY_PERIOD
: 0,
curTime + FUTURE_TIMESTAMP_MAX_VALIDITY_PERIOD
);

// Verify all price feeds have the same Pythnet slot.
Expand All @@ -312,36 +315,21 @@ abstract contract Scheduler is IScheduler, SchedulerState {
// is more recent than latest stored update's. Reverts if not.
_validateShouldUpdatePrices(subscriptionId, params, status, priceFeeds);

// Store the price updates, update status, and emit event
_storePriceUpdatesAndStatus(
subscriptionId,
status,
priceFeeds,
pythFee
);
}

/**
* @notice Stores the price updates, updates subscription status, and emits event.
*/
function _storePriceUpdatesAndStatus(
uint256 subscriptionId,
SubscriptionStatus storage status,
PythStructs.PriceFeed[] memory priceFeeds,
uint256 pythFee
) internal {
// Store the price updates
// Update status and store the updates
uint256 latestPublishTime = 0; // Use the most recent publish time from the validated feeds
for (uint8 i = 0; i < priceFeeds.length; i++) {
_state.priceUpdates[subscriptionId][priceFeeds[i].id] = priceFeeds[
i
];
if (priceFeeds[i].price.publishTime > latestPublishTime) {
latestPublishTime = priceFeeds[i].price.publishTime;
}
}
status.priceLastUpdatedAt = priceFeeds[0].price.publishTime;
status.balanceInWei -= pythFee;
status.totalUpdates += 1;
status.totalSpent += pythFee;
status.priceLastUpdatedAt = latestPublishTime;
status.totalUpdates += priceFeeds.length;

emit PricesUpdated(subscriptionId, priceFeeds[0].price.publishTime);
_storePriceUpdates(subscriptionId, priceFeeds);

_processFeesAndPayKeeper(status, startGas, priceIds.length, pythFee);

emit PricesUpdated(subscriptionId, latestPublishTime);
}

/**
Expand Down Expand Up @@ -737,4 +725,55 @@ abstract contract Scheduler is IScheduler, SchedulerState {
_state.activeSubscriptionIndex[subscriptionId] = 0;
}
}

/**
* @notice Internal function to store the parsed price feeds.
* @param subscriptionId The ID of the subscription.
* @param priceFeeds The array of price feeds to store.
*/
function _storePriceUpdates(
uint256 subscriptionId,
PythStructs.PriceFeed[] memory priceFeeds
) internal {
for (uint8 i = 0; i < priceFeeds.length; i++) {
_state.priceUpdates[subscriptionId][priceFeeds[i].id] = priceFeeds[
i
];
}
}

/**
* @notice Internal function to calculate total fees, deduct from balance, and pay the keeper.
* @dev This function sends funds to `msg.sender`, so be sure that this is being called by a keeper.
* @param status Storage reference to the subscription's status.
* @param startGas Gas remaining at the start of the parent function call.
* @param numPriceIds Number of price IDs being updated.
* @param pythFee Fee paid to Pyth for the update.
*/
function _processFeesAndPayKeeper(
SubscriptionStatus storage status,
uint256 startGas,
uint256 numPriceIds,
uint256 pythFee
) internal {
// Calculate fee components
uint256 gasCost = (startGas - gasleft() + GAS_OVERHEAD) * tx.gasprice;
uint256 keeperSpecificFee = uint256(_state.singleUpdateKeeperFeeInWei) *
numPriceIds;
uint256 totalKeeperFee = gasCost + keeperSpecificFee;
uint256 totalFee = totalKeeperFee + pythFee; // pythFee is already paid in the parsePriceFeedUpdatesWithSlots call

// Check balance
if (status.balanceInWei < totalFee) {
revert InsufficientBalance();
}

// Update status and pay keeper
status.balanceInWei -= totalFee;
status.totalSpent += totalFee;
(bool sent, ) = msg.sender.call{value: totalKeeperFee}(""); // Pay only the keeper portion
Copy link
Collaborator

Choose a reason for hiding this comment

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

just double check that we have updated the state everywhere (to make sure there's no double entry attack)

Copy link
Contributor Author

@tejasbadadare tejasbadadare Apr 25, 2025

Choose a reason for hiding this comment

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

I better implemented the checks-effects-interactions pattern. Now we apply it independently to the pyth fee and the keeper fee. This keeps accounting accurate if either of the payments fail.

if (!sent) {
revert KeeperPaymentFailed();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ error CannotUpdatePermanentSubscription();

// Price feed errors
error InvalidPriceId(bytes32 providedPriceId, bytes32 expectedPriceId);
error InvalidPriceIdsLength(bytes32 providedLength, bytes32 expectedLength);
error InvalidPriceIdsLength(uint256 providedLength, uint256 expectedLength);
error EmptyPriceIds();
error TooManyPriceIds(uint256 provided, uint256 maximum);
error DuplicatePriceId(bytes32 priceId);
Expand All @@ -29,3 +29,6 @@ error TimestampOlderThanLastUpdate(
// Whitelist errors
error TooManyWhitelistedReaders(uint256 provided, uint256 maximum);
error DuplicateWhitelistAddress(address addr);

// Payment errors
error KeeperPaymentFailed();
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ contract SchedulerState {
/// Maximum time in the future (relative to current block timestamp)
/// for which a price update timestamp is considered valid
uint64 public constant FUTURE_TIMESTAMP_MAX_VALIDITY_PERIOD = 10 seconds;
/// Fixed gas overhead component used in keeper fee calculation.
/// This is a rough estimate of the tx overhead for a keeper to call updatePriceFeeds.
uint256 public constant GAS_OVERHEAD = 30000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

How did you get to this number?

Copy link
Contributor Author

@tejasbadadare tejasbadadare Apr 25, 2025

Choose a reason for hiding this comment

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

It's a rough estimate, we will probably want to set it per blockchain. The idea was to capture the gas used before the first gasleft() call, which would be the flat 21k gas for an external contract call, ~2.6k for the proxy's delegatecall, and some for the calldata. We can measure it more carefully when we do the math to ensure our pricing model works.


struct State {
/// Monotonically increasing counter for subscription IDs
Expand Down
Loading
Loading