Skip to content

Conversation

grod220
Copy link
Member

@grod220 grod220 commented Mar 17, 2025

The first of a number of CLI/JS helpers for users of the program. First up is the CreateMint instruction.

Takes large inspiration from the CLI helpers and tests in spl-single-pool.

Walkthrough for testing locally:

  1. Build program
cargo build-sbf # generates .so file
cargo build # builds binary
  1. Start solana test validator
solana-test-validator \
  --bpf-program TwRapQCDhWkZRrDaHfZGuHxkZ91gHDRkyuzNqeU5MgR target/deploy/spl_token_wrap.so \
  --reset
  1. Create a new mint & token account
spl-token create-token
# Example will have G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb for this
spl-token create-account G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb
spl-token mint G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb 100000
  1. Run CreateMint token-wrap command
cargo run --bin spl-token-wrap create-mint \
    G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb \  # unwrapped_mint
    TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA \    # unwrapped_token_program
    TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb     # wrapped_token_program
  1. See expected output
Creating wrapped mint for G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb
Funding wrapped_mint_account 7KTYY4ZAsVG2Tn1V4119ZGRLWRPzo3r6nD2pde9rtLsV with 1461600 lamports for rent
Funding backpointer_account 7ohy6W6gxAHg42pAawrUPeivRLyCBPXALgnq17eNVYFF with 1113600 lamports for rent
Unwrapped mint address: G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb
Wrapped mint address: 7KTYY4ZAsVG2Tn1V4119ZGRLWRPzo3r6nD2pde9rtLsV
Wrapped backpointer address:: 7ohy6W6gxAHg42pAawrUPeivRLyCBPXALgnq17eNVYFF
Funded wrapped mint lamports: 1461600
Funded backpointer lamports: 1113600
Signature: 3T5yjnCgDwcPAafyYrjxfFXxDZfPFRFr2R8K4kcPbZnEukKjsYxiwJrdAJAFyCU7guq5Yye5QEwobLbsMkGZNKEe

@grod220 grod220 force-pushed the create-mint-cli branch 2 times, most recently from 76229ef to bbfc939 Compare March 17, 2025 21:06
@grod220 grod220 marked this pull request as ready for review March 17, 2025 21:39
Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Really small stuff from me. Looks great!

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 overall! Mostly small things

Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Lgtm, just a CI failure to address!

@grod220
Copy link
Member Author

grod220 commented Mar 20, 2025

Since swapping from ProgramClient to RpcClient in the tests I am hitting flakiness with a ~30% success rate on the cli test. I think it may be because the ProgramClient perhaps waits for confirmation and the other doesn't. Not sure, investigating.

Comment on lines +57 to +60
let starting_accounts = vec![(payer.pubkey(), payer_account), unwrapped_mint.clone()];

// Start the test validator with necessary programs
let validator = start_validator(starting_accounts).await;

Choose a reason for hiding this comment

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

Nice! 😊

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.

When using rpc_client.send_transaction, it only sends.

So rather than setting all of the data at startup to "fix" the race condition, the preference should be to use rpc_client.send_and_confirm_transaction to wait for a transaction to land before sending the next one, if the two transactions depend on each other.

You don't need to change setting the accounts into the validator, but can you at least use send_and_confirm_transaction in the tests? That also lets us find out if the transaction was successful or not.

Also, it's done now, but the workspace deps changes should have been done in a separate PR.

@grod220
Copy link
Member Author

grod220 commented Mar 21, 2025

the preference should be to use rpc_client.send_and_confirm_transaction to wait for a transaction to land before sending the next one, if the two transactions depend on each other.

When digging into the ProgramClient, I did manage to find send_and_confirm_transaction which fixed the issue. However, if all accounts are loaded on startup, that method isn't needed anymore and there is only the single command now:

 Command::new(TOKEN_WRAP_CLI_BIN).args(["create-mint", ...

Though it's still not clear to me what strategy is preferred for CLI tests. Is it better to A) load all accounts within the validator on startup and make the single command you are testing or B) have a fresh startup state and make the sequential transactions to build the state the command your testing needs. If A, there shouldn't be additional changes needed. If B, I can revert back and make use of send_and_confirm_transaction to fix that race condition.

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.

I typically lean towards building things up in tests with transactions rather than setting up the accounts by hand, since it's possible for the setup to be incorrect, as we've already seen in some of the program tests earlier, but there's no need to revert the work here.

And right, sorry, I thought there was still some usage of send_transaction somewhere. This is all good to me then!

@grod220
Copy link
Member Author

grod220 commented Mar 24, 2025

Helpful, in the next PR will probably re-establish the multiple-tx pattern again.

@grod220 grod220 merged commit 188d932 into main Mar 24, 2025
9 checks passed
@grod220 grod220 deleted the create-mint-cli branch March 24, 2025 08:01
@grod220 grod220 mentioned this pull request Mar 24, 2025
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.

3 participants