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

add withdraw durable nonce #26829

Merged
merged 2 commits into from
Jul 30, 2022

Conversation

KirillLykov
Copy link
Contributor

@KirillLykov KirillLykov commented Jul 28, 2022

Problem

Adds function which withdraws durable nonce accounts.
This is part of the series of PRs adding durable nonce accounts to bench-tps: #25759

Summary of Changes

@KirillLykov KirillLykov force-pushed the benchRefactorCreate_fu7 branch from 0c783c9 to 05ff1b6 Compare July 28, 2022 16:17
@KirillLykov KirillLykov force-pushed the benchRefactorCreate_fu7 branch from 05ff1b6 to cca2bec Compare July 28, 2022 20:00
@@ -109,17 +109,35 @@ pub fn generate_durable_nonce_accounts<T: 'static + BenchTpsClient + Send + Sync
let (mut nonce_keypairs, _extra) = generate_keypairs(seed_keypair, count as u64);
nonce_keypairs.truncate(count);

let to_fund: Vec<(&Keypair, &Keypair)> = authority_keypairs
let to_fund: Vec<NonceCreateSigners> = authority_keypairs
Copy link
Contributor Author

@KirillLykov KirillLykov Jul 29, 2022

Choose a reason for hiding this comment

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

I replace pair to struct in order to have two different types for Withdraw and Create accounts. It is required because for Create I need two signatures and for Withdraw only one (Sliceable::to_slice method returns array of signing Keypairs).

joncinque
joncinque previously approved these changes Jul 29, 2022
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good! Just the one nit, which I don't really feel too strongly about, so feel free to go forward with this

&authority.pubkey(),
nonce_rent,
);
struct NonceWithdrawSigners<'a>(&'a Keypair, &'a Keypair);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It would be clearer for the second parameter to be a Pubkey since it's not signing, but I think that'll make things more complicated in the implementation, like (&'a Keypair, Pubkey), which is probably worse. I imagine you already considered this tradeoff 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this is really good point! And it happens that it is quite easy to implement

Before &Keypair was passed although it is not necessary because nonce
doesn't sign withdraw account transactions anyways.
@mergify mergify bot dismissed joncinque’s stale review July 30, 2022 09:23

Pull request has been modified.

@KirillLykov KirillLykov merged commit ddfa64d into solana-labs:master Jul 30, 2022
@KirillLykov KirillLykov deleted the benchRefactorCreate_fu7 branch August 15, 2022 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants