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

Implement nonblocking version of BlockhashQuery #27040

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

2501babe
Copy link
Member

@2501babe 2501babe commented Aug 9, 2022

Problem

token-cli is in the process of being updated to use the nonblocking rpc client, but we need a nonblocking version of BlockhashQuery to support offline signing options

Summary of Changes

this creates nonblocking versions of blockhash_query and nonce_utils

i wasnt able to wrap nonblocking calls in blocking ones, a la rpc_client, because these functions accept different RpcClients as parameters. everything in nonce_utils that doesnt have blocking/nonblocking versions was moved into the nonblocking version and reexported by the blocking one

i refrained from porting deprecated functions to the new blockhash_query and adapted tests to use the new interface. the only substantive change i made is that

pub enum BlockhashQuery {
    None(Hash),
    FeeCalculator(Source, Hash),
    All(Source),
}

becomes

pub enum BlockhashQuery {
    Static(Hash),
    Validated(Source, Hash),
    Rpc(Source),
}

in an attempt to better reflect its functionality now that the fee calculator is gone. the existing blocking interface is unchanged

closes #27043

@2501babe 2501babe marked this pull request as draft August 9, 2022 20:53
@2501babe 2501babe force-pushed the nonblocking-blockhash branch from 35b68d5 to 6f61f43 Compare August 9, 2022 20:57
@2501babe 2501babe marked this pull request as ready for review August 9, 2022 20:59
@2501babe
Copy link
Member Author

2501babe commented Aug 9, 2022

just had to clean up some imports

@2501babe 2501babe force-pushed the nonblocking-blockhash branch from 6f61f43 to 62a637d Compare August 9, 2022 21:22
@joncinque joncinque added the v1.11 label Aug 9, 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 great! Just a nit and a request to create a new issue. I've added the v1.11 tag to backport this, that way the token CLI can be fixed

client/src/nonblocking/blockhash_query.rs Show resolved Hide resolved
client/src/nonblocking/blockhash_query.rs Show resolved Hide resolved
client/src/nonblocking/blockhash_query.rs Show resolved Hide resolved
joncinque
joncinque previously approved these changes Aug 9, 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.

Thanks!

@2501babe 2501babe force-pushed the nonblocking-blockhash branch from 62a637d to db13a9f Compare August 9, 2022 22:18
@mergify mergify bot dismissed joncinque’s stale review August 9, 2022 22:18

Pull request has been modified.

@2501babe 2501babe force-pushed the nonblocking-blockhash branch from db13a9f to b49fc47 Compare August 10, 2022 00:17
@2501babe 2501babe added the CI Pull Request is ready to enter CI label Aug 10, 2022
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Aug 10, 2022
@2501babe 2501babe added the CI Pull Request is ready to enter CI label Aug 10, 2022
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Aug 10, 2022
@2501babe 2501babe closed this Aug 10, 2022
@2501babe 2501babe reopened this Aug 10, 2022
@2501babe 2501babe closed this Aug 10, 2022
@2501babe 2501babe reopened this Aug 10, 2022
@2501babe
Copy link
Member Author

(in case anyone is looking at this comedic history i have been trying to get ci working but it turns out to just be an interface issue https://buildkite.com/solana-labs/solana/builds/79465)

@2501babe 2501babe merged commit b9a5af0 into solana-labs:master Aug 10, 2022
@2501babe 2501babe deleted the nonblocking-blockhash branch August 10, 2022 03:18
mergify bot pushed a commit that referenced this pull request Aug 10, 2022
mergify bot added a commit that referenced this pull request Aug 10, 2022
…7047)

Implement nonblocking version of BlockhashQuery (#27040)

(cherry picked from commit b9a5af0)

Co-authored-by: hana <[email protected]>
xiangzhu70 pushed a commit to xiangzhu70/solana that referenced this pull request Aug 17, 2022
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.

Create nonblocking version of BlockhashQuery
3 participants