Skip to content

Conversation

@tejasbadadare
Copy link
Contributor

@tejasbadadare tejasbadadare commented Apr 25, 2025

Summary

Add keeper payment in updatePriceFeeds

Rationale

Keepers are permissionless and we pay them in-band if they successfully call updatePriceFeeds. If the trigger condition isn't satisfied or the subscription doesn't have enough funds, they won't get paid.

This is a temporary payment model, the long term goal is to add a monthly subscription to make it easy for users to consume the service, rather than keeping their balance topped up. However, the basic mechanism of paying the keeper for successful updates will be the same, just the fee calculation will change.

How has this been tested?

  • Current tests cover my changes
  • Added new tests
  • Manually tested the code

@vercel
Copy link

vercel bot commented Apr 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
entropy-debugger ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2025 9:12pm
insights ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2025 9:12pm
4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) Visit Preview Apr 25, 2025 9:12pm
component-library ⬜️ Ignored (Inspect) Visit Preview Apr 25, 2025 9:12pm
proposals ⬜️ Ignored (Inspect) Visit Preview Apr 25, 2025 9:12pm
staking ⬜️ Ignored (Inspect) Visit Preview Apr 25, 2025 9:12pm

@tejasbadadare tejasbadadare marked this pull request as ready for review April 25, 2025 02:49
@tejasbadadare tejasbadadare changed the title feat: keeper payment feat(pulse): keeper payment Apr 25, 2025
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

// 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.

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.

@tejasbadadare tejasbadadare merged commit 8d7bba7 into main Apr 25, 2025
10 checks passed
@tejasbadadare tejasbadadare deleted the tb/pulse/keeper-payment branch April 25, 2025 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants