-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Geyser: add starting entry to ReplicaEntryInfo(V2) #33963
Geyser: add starting entry to ReplicaEntryInfo(V2) #33963
Conversation
Codecov Report
@@ Coverage Diff @@
## master #33963 +/- ##
=========================================
- Coverage 81.9% 81.9% -0.1%
=========================================
Files 811 811
Lines 219329 219344 +15
=========================================
+ Hits 179696 179703 +7
- Misses 39633 39641 +8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I found our doc on geyser is quite outdated https://github.com/solana-labs/solana/blob/master/docs/src/developing/plugins/geyser-plugins.md. We need to update it with these new interfaces.
/// A wrapper to future-proof ReplicaEntryInfo handling. To make a change to the structure of | ||
/// ReplicaEntryInfo, add an new enum variant wrapping a newer version, which will force plugin | ||
/// implementations to handle the change. | ||
#[repr(u32)] | ||
pub enum ReplicaEntryInfoVersions<'a> { | ||
V0_0_1(&'a ReplicaEntryInfo<'a>), | ||
V0_0_2(&'a ReplicaEntryInfoV2<'a>), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does the upgrade path look like for this? People just pull in the new struct and we expect their plugins to handle the new fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notifications themselves use the version enums. @lijunwangs , correct me if I'm wrong, but I think the intent is that people use sensible catch-all logic for the enums, so they can update to the new variant and fields at their leisure. Although thinking about it more carefully, I guess they can really only use that catch-all as an alert...
Problem
In order to verify an Entry, geyser consumers currently have to walk all the previous entries in the block and sum the executed tx counts to figure out which transactions are included in the current entry. However, the node knows the transaction index an Entry starts with, so we can save geyser consumers quite a few operations by emitting this index too.
Summary of Changes
starting_transaction_index