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
bot: Force refresh the blockhash more often while sending txs #160
Conversation
bot/src/rpc_client_utils.rs
Outdated
// every 100 transactions, get a new blockhash | ||
if num_transactions >= 100 { | ||
info!("refreshing blockhash"); | ||
blockhash = rpc_client.get_recent_blockhash()?.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.
Doesn't this break the blockhash expiration checks below since we now have multiple live blockhashes in play, but only account for the most recent
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.
Kind of -- I've been confused about that logic for a bit.
The blockhash check is done and transactions are resent before we've actually checked the transaction status. Which means that you can take time to send 100 transactions, and if the blockhash expires right after that, you'll just resend them all without ever checking if the transactions even errored. The resend logic should go after the check, and only if the status is status.err.unwrap() == TransactionError::BlockhashNotFound
, no?
As I was thinking about better ways to do this, I thought: "oh, then we'd probably need a hashmap. Trent probably already thought that, which means that it's probably in his PR. I'll just read his PR"
475a62e
to
42c2f65
Compare
So this does a Slightly Nicer Thing ™️. Now, we actually refresh the blockhash and retry a transaction if the simulation or transaction has a blockhash error. So far, this has done well in testing until getting timeouts from testnet. I'll keep testing this to see how well it recovers from blockhash expirations. |
This has changed a lot, so I'll close and re-open |
I feel like we should just flush
|
Yeah I was thinking about that too... I do have the changes done using rpc client, so maybe we go the TpuClient route next time everything falls over? |
Up to you. I found moving to the TpuClient pretty easy, should be the same here if you just lift my work from |
All right sounds good, I'll give it a shot! |
While running the stake-o-matic for testnet stake pools, I kept running into the same issue often reported on buildkite:
So as a little patch / Ugly Thing ™️ , this force refreshes the blockhash.
This is not intended as a replacement for the work happening in #143, but as a band-aid. Note that the number of 100 was chosen based on my local machine. 200 still resulted in some blockhashes expiring, so I went with this safe value, which consistently manages to push all transactions out. We may be able to bump it up for the actual runs.