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

token-cli: --blockhash does nothing #3442

Closed
2501babe opened this issue Aug 9, 2022 · 7 comments
Closed

token-cli: --blockhash does nothing #3442

2501babe opened this issue Aug 9, 2022 · 7 comments
Assignees

Comments

@2501babe
Copy link
Member

2501babe commented Aug 9, 2022

this is a regression introduced when the cli was converted to async. BlockhashQuery is a client wrapper that can be instantiated with a static blockhash which it always returns, a client that fetches a blockhash, or a static blockhash that it verifies before returning. it was removed for the nonblocking client and an unconditional call to get_latest_blockhash

i found this by accident when dumping a transaction, and im opening an issue instead of fixing it because:

  • do we even need this? im not sure if there is a usecase for producing offline non-nonce transactions considering theyre good for like 30 seconds
  • a quick fix here is pointless because if we decide to keep supporting it im going to have to work it into the client pr im working on anyway

@joncinque thoughts?

@joncinque
Copy link
Contributor

Thanks for writing this up, I definitely broke this 😓

We definitely need this, since even offline nonce transactions require a blockhash provided from the outside. Check out the example and writeup here: https://spl.solana.com/token#example-offline-signing-with-multisig

Let's fix this the right way, by adding a nonblocking version of BlockhashQuery https://github.com/solana-labs/solana/blob/master/client/src/blockhash_query.rs -- since the implementation is pretty small, and a lot of the functions are deprecated, I think we're fine mostly copy-pasting with async and await wherever needed.

Alternatively, if you want to do it the DRY way, you can reuse the runtime from within the blocking RPC client in the blocking BlockhashQuery, and on the inside call the nonblocking BlockhashQuery, ie https://github.com/solana-labs/solana/blob/b6d38aad6979e7aca8fd431904af3cb90d87ebec/client/src/tpu_client.rs#L141-L146

But that seems like overkill. What do you think?

@2501babe
Copy link
Member Author

2501babe commented Aug 9, 2022

cooool, im gonna look into copying the nonblocking client approach for a nonblocking BlockhashQuery then

this is another thing we are gonna have to pass in to create_mint et al so maybe we should have the complicated interface that the cli needs and then a wrapper with defaults (no nonce/offline stuff, at least) for normal consumers and expose them both...

@joncinque
Copy link
Contributor

Hm, might be worth implementing that no-op client to make this a bit simpler... I can get a draft PR up with it to show what I mean.

@2501babe
Copy link
Member Author

2501babe commented Aug 9, 2022

#[derive(Debug, PartialEq, Eq)]
pub enum BlockhashQuery {
    None(Hash),
    FeeCalculator(Source, Hash),
    All(Source),
}

should we rename these? all the fee calculator stuff is going away so the only difference in functionality now is:

  • returns a static hash unconditionally
  • validates the static hash before returning it
  • rpc passthrough

none/fee/all referred to which of the hash/fee pair it fetched. maybe Static Validated Rpc?

edit: by "rename" i mean in the nonblocking version, existing api will be unchanged

@2501babe
Copy link
Member Author

update, i landed this: solana-labs/solana#27040 in the monorepo to provide a nonblocking blockhash client. its been backported to 1.11 so once we have a next release of that i can update versions and do this

@Poletgole

This comment was marked as abuse.

@2501babe
Copy link
Member Author

fixed by #3449

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

No branches or pull requests

3 participants