Improve Keeper Reliability #19
Conversation
ebatsell
force-pushed
the
improve-keeper-reliability
branch
2 times, most recently
from
a31ef10 to
b773fdb
Compare
ebatsell
force-pushed
the
improve-keeper-reliability
branch
from
65d8d4b to
b6822c1
Compare
keepers/keeper-core/src/lib.rs
Outdated
| RetryError(Vec<Instruction>), | ||
| #[error("Transactions failed to execute after multiple retries")] | ||
| TransactionRetryError(Vec<Vec<Instruction>>), | ||
| TransactionRetryError(Vec<(Vec<Instruction>, String)>), |
There was a problem hiding this comment.
TransactionError instead of string?
There was a problem hiding this comment.
there's other transaction types baked into the ClientError like IoError, ReqwestError, SigningError, etc that I feel is useful to propogate. and the only way to bubble it up is to cast to string since the ClientError type doesn't implement the Clone trait we need. If we saved only the transaction errors you'd end up with Vec<Result<(), Option<TransactionError>>>. Can switch that out if it's preferred but we'd be losing information with that
keepers/keeper-core/src/lib.rs
Outdated
| .flat_map(|i| instruction_batches[i]) | ||
| .cloned() | ||
| .collect::<Vec<_>>(); | ||
| // Re-sign transactions with fresh blockhash |
There was a problem hiding this comment.
going to send duplicates if always resigning. should just need to resign once the blockhash you previously signed txs with expires based on processed commit level. there's a is_blockhash_valid_with_config method
There was a problem hiding this comment.
added logic to do less re-signing
| &mut executed_signatures, | ||
| ) | ||
| .await?; | ||
| while retries < retry_count { |
There was a problem hiding this comment.
imo max retries should prolly be incremented on blockhash increment? or related to some time.
retry_count of 3 may be fine for 100 txs, but won't be fine for 2k txs
There was a problem hiding this comment.
using a retry count of 10, maybe you were looking at a older commit?
There was a problem hiding this comment.
added logic to fetch blockhash and increment retries only when it's expired
| #[error("RPC Client error: {0:?}")] | ||
| TransactionClientError(String, Vec<Vec<Instruction>>), | ||
| TransactionClientError(String, Vec<Result<(), SendTransactionError>>), |
There was a problem hiding this comment.
this is kinda messy return type
There was a problem hiding this comment.
what does vec correspond to?
There was a problem hiding this comment.
this error is for the case where we're submitting transactions and one of the calls to get blockhash fails, and we may have partially-executed transactions in the list - the vec corresponds to the transactions passed into parallel_execute_transactions
keepers/keeper-core/src/lib.rs
Outdated
| executed_signatures: &mut HashMap<Signature, usize>, | ||
| ) { | ||
| // Confirmes TXs in batches of 256 (max allowed by RPC method) | ||
| executed_signatures: HashMap<Signature, usize>, |
There was a problem hiding this comment.
nit: you could pass in hashset here then return hashset of confirmed and worry about the index in caller. but this is fine
Improves keeper reliability for landing transactions by:
The first part has already been running on mainnet for a week, and for the last 3 epochs, all vote accounts have been updated each epoch (as measured by: same number of commissions tracked as stakes).