Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

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
Contributor

@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
Contributor 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
Contributor Author

mvines commented Dec 14, 2018

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

@mvines
Copy link
Contributor 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 mvines removed the work in progress This isn't quite right yet label Dec 14, 2018
@mvines
Copy link
Contributor 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 subscribe to this conversation on GitHub. Already have an account? Sign in.
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