Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Serdnad
Copy link

@Serdnad Serdnad commented Nov 10, 2024

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 and anchor 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 that

Adds a new top-level, optional arg --max-retries to anchor 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.

Copy link

vercel bot commented Nov 10, 2024

@Serdnad is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@Serdnad
Copy link
Author

Serdnad commented Nov 10, 2024

Any other flags we should include by default? --use-quic maybe? And separately, would it make sense to also add --max-retries to anchor deploy?

Copy link
Collaborator

@acheroncrypto acheroncrypto left a 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 to anchor 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).

cli/src/lib.rs Show resolved Hide resolved
Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great other than the small clippy errors in CI.

Before we merge, could you also note this feature in the CHANGELOG?

cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
@Serdnad
Copy link
Author

Serdnad commented Nov 25, 2024

Sorry for the wait, fixed the clippy lints and updated the changelog

@@ -30,6 +30,7 @@ The minor version will be incremented upon a breaking change and the patch versi
- idl: Remove `anchor-syn` dependency ([#3030](https://github.com/coral-xyz/anchor/pull/3030)).
- lang: Add `const` of program ID to `declare_id!` and `declare_program!` ([#3019](https://github.com/coral-xyz/anchor/pull/3019)).
- idl: Add separate spec crate ([#3036](https://github.com/coral-xyz/anchor/pull/3036)).
- cli: Include recommended solana args by default and add new --max-retries ([#3354](https://github.com/coral-xyz/anchor/pull/3354)).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the wait, fixed the clippy lints and updated the changelog

Thanks but this is the incorrect one to update (v0.30.1 is already out, this will be included in v0.31.0). Please revert this change and update it in the place that was linked in #3354 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants