Use staleness for pricing #87
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ali-bahjati
requested review from
jayantk,
m30m,
drozdziak1,
guibescos and
Reisen
m30m
reviewed
Nov 13, 2023
| // Get the estimated compute unit price and wrap it so it stays below the maximum total | ||
| // compute unit fee. We additionally divide such price by 2 to make sure a spike | ||
| // doesn't get propagated forever. | ||
| let estimated_price = (estimated_recent_price >> 1).min(maximum_unit_price); |
m30m
approved these changes
Nov 13, 2023
Reisen
approved these changes
Nov 13, 2023
jayantk
approved these changes
Nov 13, 2023
|
|
||
| // Use exponentially higher price if this publisher hasn't published in a while for the accounts | ||
| // in this batch. This will use the maximum total compute unit fee if the publisher | ||
| // hasn't updated for >= MAXIMUM_SLOT_GAP_FOR_DYNAMIC_COMPUTE_UNIT_PRICE slots. |
There was a problem hiding this comment.
this is pretty much guaranteed to use the maximum price when the market opens each day right?
I think that behavior is fine -- I think the max price isn't so high that we can't afford 1 of these transactions per day per price feed or whatever -- though probably worth a comment justifying it.
There was a problem hiding this comment.
Yeah it was a certain challenge. I think we should see how it will work with the other dynamic mechanism (which is decaying) and make sure it won't get very expensive.
src/agent/solana/exporter.rs
Outdated
| @@ -88,6 +87,7 @@ const UPDATE_PRICE_NO_FAIL_ON_ERROR: i32 = 13; | |||
| // measure while using dynamic compute price to prevent the exporter from paying too much for a | |||
| // single transaction | |||
| const MAXIMUM_TOTAL_COMPUTE_UNIT_FEE_MICRO_LAMPORTS: u64 = 1_000_000_000_000; | |||
| const MAXIMUM_SLOT_GAP_FOR_DYNAMIC_COMPUTE_UNIT_PRICE: u64 = 25; | |||
There was a problem hiding this comment.
I suggest putting both of these consts into the config. these are the kind of thing that we'll probably want to tune easily in an emergency.
This commit adds dynamic (exponential) pricing based on staleness of the feeds that the publishers publish. This feature, combined with the recent fees for the price updates causes the spike on fee to last some time before going away.
ali-bahjati
force-pushed
the
use-staleness-for-pricing
branch
from
d2a9808 to
ff26d55
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds dynamic (exponential) pricing based on staleness of the feeds that the publishers publish. This feature, combined with the recent fees for the price updates causes the spike on fee to last some time before going away.
The second commit contains a little refactoring to fix warnings. Please review commit-by-commit.