-
Notifications
You must be signed in to change notification settings - Fork 59
shredstream added local address table lookup and check vote txn #262
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
base: main
Are you sure you want to change the base?
Conversation
why not merge? pretty nice solution. the only thing i'd like to change is splitting
to not force other users to work with ALTs the same strategy can be applied to the votes, if you wish |
Ok, I am also considering a feature that would send a notification after a set of entries have been processed. |
83fa4be
to
427bd42
Compare
tokio_util::sync::CancellationToken, | ||
}; | ||
|
||
const VOTE_ID: Pubkey = solana_sdk::pubkey!("Vote111111111111111111111111111111111111111"); | ||
type LocalAddresseTables = Arc<RwLock<HashMap<Pubkey, [Pubkey; 255]>>>; |
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.
You could also use the concurrent hashmap from scc
dependency which is also used for the dedup cache
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.
As an input parameter, there is no need to introduce too many new dependencies unless there are special circumstances.
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.
just use
solana_sdk_ids::vote::id()
this package exports only program constants
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.
I am referring to the import of the ssc
package.
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 scc
package is already in the dependencies (HashCache
comes from scc
)
https://docs.rs/scc/latest/scc/#hashmap
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.
I know that if HashCache
is used here, the dependency scc
must be added when instantiating it. If this is optional, we think HashMap
is more convenient.
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.
I mean as scc
is already in dependencies, and provides scc::HashMap
which is a concurrent map, it could be interesting to use it here as this is a hot path and using it here instead of RwLock + std HashMap
can give significant performance improvements.
// Skip vote transaction. | ||
// Voting transactions are not sent along with normal transactions. | ||
// NOTE: These conclusions are based on limited testing. |
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.
2025-04-22T09:36:13.334393Z INFO example: New entry: 4ZvoMhH1EkFTZUkF9fGY7QzwkE1Zrvh1iBHdtdvf5qZM | votes 2/7
2025-04-22T09:36:32.154228Z INFO example: New entry: 4W1BuLCVFXUEK6qxS4ct5hicgfjsRTBbYmVJ87n6abep | votes 1/4
2025-04-22T09:36:43.450979Z INFO example: New entry: ELyJZmzJ7Djr7nFUYePhKpxF1YaAi6baQTcZtt7ZaVZW | votes 2/4
unfortunately not always
if dedup_cache.contains(&entry.hash) { | ||
duplicate_entries += 1; | ||
continue; | ||
} | ||
let _ = dedup_cache.put(entry.hash, ()); | ||
|
||
for transaction in entry.transactions { | ||
let accounts = transaction.message.static_account_keys(); | ||
let is_vote = accounts.len() == 3 && accounts[2].eq(&VOTE_ID); |
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.
there is solana_sdk::vote::program::ID
instead of redefining VOTE_ID
or
let is_vote = accounts.len() == 3 && accounts[2].eq(&VOTE_ID); | |
let is_vote = accounts.len() == 3 && solana_sdk::vote::program::check_id(&accounts[2]); |
if you prefer
no need to merge, this is my suggestion for jito shredstream!