Skip to content
This repository was archived by the owner on Mar 11, 2025. It is now read-only.

Token client improvements #3139

Merged
merged 5 commits into from
May 5, 2022

Conversation

CriesofCarrots
Copy link
Contributor

It's not that easy to use spl-token-client with an RpcClient (to point at a test-validator or live cluster RPC).

Changes:

  • (First commit is purely cosmetic, and can be ignored in review)
  • Use the nonblocking RpcClient instead of the blocking one (we're already async here)
  • Arc-wrap the RpcClient in ProgramRpcClient so that it's easier to use for multiple calls without will not live long enough compile errors
  • Change RpcClient send impl method from send_transaction to send_and_confirm_transaction - I think this is the most controversial of the changes. This approach seems the most straightforward to me, but we could instead require the caller to do any confirming they need/want. If so, we probably need to rework some things in client/src/token.rs. For instance, Token::create_mint() doesn't return the Output (signature) from process_ixs(), so there isn't currently a way for a caller to confirm that signature.

@CriesofCarrots CriesofCarrots requested a review from joncinque May 4, 2022 22:11
joncinque
joncinque previously approved these changes May 4, 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! Changing to send_and_confirm_transaction is more aligned with process_transaction on the banks client anyway. If there's a need to send transactions without confirmation, we can implement a new trait for that with Output = Signature, right? This client is really forward-looking!

@CriesofCarrots
Copy link
Contributor Author

Here's a little example of using this client against a solana-test-validator, btw: https://gist.github.com/CriesofCarrots/0539fec5c5601db36d5b7a0c0ead385a

@CriesofCarrots
Copy link
Contributor Author

CI failure seems due to changes upstream in web3.js/transaction.ts (twotx patch), but I'll fix that before merging this

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants