-
Notifications
You must be signed in to change notification settings - Fork 798
feat(scheduler-bindings): Organize bindings flags better #8821
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
Conversation
ffa4612 to
29ce6af
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8821 +/- ##
=======================================
Coverage 83.2% 83.2%
=======================================
Files 864 864
Lines 375226 375276 +50
=======================================
+ Hits 312291 312345 +54
+ Misses 62935 62931 -4 🚀 New features to boost your workflow:
|
scheduler-bindings/src/lib.rs
Outdated
| /// Fee-payer balance should be fetched for transactions. | ||
| pub const GET_FEE_PAYER_BALANCE: u16 = 1 << 2; | ||
|
|
||
| /// Transactions should have ATL pubkeys resolved and returned. | ||
| pub const RESOLVE_PUBKEYS: u16 = 1 << 3; |
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.
could call these LOAD_FEE_PAYER and LOAD_ALTS to focus on the cost of the action + standardize naming?
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.
scheduler-bindings/src/lib.rs
Outdated
| pub const MESSAGE_PROCESSED: u8 = 0; | ||
| /// The message was not processed because the message was invalid. | ||
| pub const INVALID_MESSAGE: u8 = 1; |
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.
Could consider dropping message prefix/postfix as all the variants relate to messages.
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.
scheduler-bindings/src/lib.rs
Outdated
| /// Flag set if status checks failed due to the transaction being | ||
| /// too old. | ||
| pub const TOO_OLD: u8 = 1 << 3; |
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.
Technically a randomly generated blockhash would not be too old but just simply "invalid" right? I assume this error is for blockhash failure on TXs that are not using durable nonce?
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.
Yeah, now that i think with error we don't even differentiate between nonce/blockhash. probably best to just call this invalid_lifetime or something like that to encapsulate any error pertaining to the recent_blockhash field whether blockhash, rnadom, or nonce
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.
ahh yep, i had assumed you had a way of splitting those out. if nonce is getting mixed in would do something like the invalid lifetime name
| success: agave_scheduler_bindings::worker_message_types::RESOLVE_SUCCESS, | ||
| slot, | ||
| Ok(Some((resolved_pubkeys, min_alt_deactivation_slot))) => CheckResponse { | ||
| parsing_and_sanitization_flags: 0, |
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.
parsing_and_sanitization_flags: needs to be set to success now right?
23ae290 to
225d762
Compare
Problem
Summary of Changes
Fixes #