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

wallet airdrop: Fetch a new last_id to prevent DuplicateSignature errors during AccountInUse retries #2171

Merged
merged 1 commit into from
Dec 14, 2018

Conversation

mvines
Copy link
Member

@mvines mvines commented Dec 14, 2018

Attempts to fix the intermittent CI failure that occurred at https://buildkite.com/solana-labs/solana/builds/6169#ff62509f-f382-43a9-a886-538aff015182

@garious
Copy link
Contributor

garious commented Dec 14, 2018

Move that to thin_client.rs?

src/wallet.rs Outdated
@@ -783,15 +783,31 @@ fn request_and_confirm_airdrop(
};
match status {
RpcSignatureStatus::AccountInUse => {
last_id = get_last_id(rpc_client)?;
// Fetch a new last_id, to prevent the retry from getting rejected as a
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you implementing a DuplicateSignature handler in the AccountInUse handler?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not. When AccountInUse happens, we need to fetch a new last_id to prevent DuplicateSignature on the next retry

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, thanks!

@mvines mvines added the work in progress This isn't quite right yet label Dec 14, 2018
@mvines
Copy link
Member Author

mvines commented Dec 14, 2018

Bah this is wrong. request_airdrop_transaction() needs the new last_id

@mvines
Copy link
Member Author

mvines commented Dec 14, 2018

Move that to thin_client.rs?

Can't unfortunately, as request_airdrop_transaction(), which invokes the drone, needs a last_id so the drone can sign the transaction itself

@mvines
Copy link
Member Author

mvines commented Dec 14, 2018

@garious this PR is fixed up now if you care to take another look. It's not fancy stuff though

@mvines mvines added the automerge Merge this Pull Request automatically once CI passes label Dec 14, 2018
@solana-grimes solana-grimes merged commit 8fcb711 into solana-labs:master Dec 14, 2018
willhickey pushed a commit that referenced this pull request Jul 22, 2024
yihau pushed a commit to yihau/solana that referenced this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants