-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
cli: Include recommended solana args by default and add new --max-retries #3354
base: master
Are you sure you want to change the base?
Conversation
@Serdnad is attempting to deploy a commit to the coral-xyz Team on Vercel. A member of the Team first needs to authorize it. |
Any other flags we should include by default? |
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.
Any other flags we should include by default?
--use-quic
maybe? And separately, would it make sense to also add--max-retries
toanchor deploy
Afaik --use-rpc
is more consistent but also depends on the RPC provider. I think increasing maximum retries makes sense though (changing the max sign attempts to 30 like you've done should take care of it).
/// Max times to retry on failure. | ||
#[clap(long, default_value = "0")] | ||
max_retries: u32, |
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 --max-sign-attempts
take care of this? Any failure other than blockhash expiration is likely to fail again e.g. not enough balance.
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.
In my experience, there's 2 common issues that consistently just require another attempt: expired blockhash, and failed write transactions. Occasionally, I've also infrequently seen other issues, like say, a stray 503 from my RPC provider, that went away with another run.
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.
Here's a recording using this feature with the anchor program that inspired me to open this PR. I got "lucky" here and hit the trifecta: invalid blockhash, failed write transactions, and an entirely unrelated error I'm not actually sure I've seen before, that went away on its own.
This PR includes the two following changes. They were closely related and small enough that 1 PR seemed reasonable, but happy to split them up if requested.
Updates
anchor deploy
andanchor upgrade
to use default values for the following args, if not supplied:--with-compute-unit-price
: defaults to the median unit price of recent successful transaction--max-sign-attempts
: a somewhat arbitrary, but fairly practical value of 30--buffer
: creates a new buffer keypair in the OS' default tmp directory on first use, and reuses thatAdds a new top-level, optional arg
--max-retries
toanchor upgrade
, which will retry the upgrade with the same args. This is primarily useful for inconsistent issues like "Invalid Blockhash" or "N write transactions failed", which often just require another attempt.