CLI helper: Wrap #38

grod220 · 2025-03-24T16:38:21Z

The second CLI helper (after #33) for the Wrap instruction. Note, it only supports single signer at the moment.

Walkthrough for testing locally:

Build program

cargo build-sbf # generates .so file
cargo build # builds binary

Start solana test validator

solana-test-validator \
  --bpf-program TwRapQCDhWkZRrDaHfZGuHxkZ91gHDRkyuzNqeU5MgR target/deploy/spl_token_wrap.so \
  --reset

Create a new mint & token account

spl-token create-token
# mint addr -> G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb
spl-token create-account G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb 
# token account -> EYUCh7xP4sxnNBr1VFcGXfHm1piEA6ERci5prNykSV4G
spl-token mint G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb 100000

Run CreateMint token-wrap command

cargo run --bin spl-token-wrap create-mint \
    G85DDQCoHchhDFWtUFT2MDkfUPYQt8BvViFyjGv167xb \  # unwrapped_mint
    TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA \    # unwrapped_token_program
    TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb     # wrapped_token_program

# Keep note of console output
# Unwrapped mint address: 8eZ2rk3GcKopnKrAmVibV7zLW1sszNo2GdbhtxRhrZRi
# Wrapped mint address: 3xnGREX1Mi7zcuXJAYoDGk9xjpugZnfRhAKLPHdfEPPq
# Wrapped mint authority: A2LJVqpdm8kQUpw9x6PBZNVKzG9KesxF7gpbbvHxe8m

Create an escrow token account and set its owner to the mint_authority

spl-token create-account 8eZ2rk3GcKopnKrAmVibV7zLW1sszNo2GdbhtxRhrZRi --owner A2LJVqpdm8kQUpw9x6PBZNVKzG9KesxF7gpbbvHxe8m --fee-payer /Users/gabe/.config/solana/id.json

Create a recipient token account of that new wrapped mint

spl-token create-account 3xnGREX1Mi7zcuXJAYoDGk9xjpugZnfRhAKLPHdfEPPq

Issue Wrap command

spl-token-wrap wrap <UNWRAPPED_TOKEN_PROGRAM> <UNWRAPPED_MINT> <UNWRAPPED_TOKEN_ACCOUNT> <ESCROW_ACCOUNT> <WRAPPED_TOKEN_PROGRAM> <RECIPIENT_TOKEN_ACCOUNT> <AMOUNT>

cargo run --bin spl-token-wrap wrap TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA 8eZ2rk3GcKopnKrAmVibV7zLW1sszNo2GdbhtxRhrZRi EYUCh7xP4sxnNBr1VFcGXfHm1piEA6ERci5prNykSV4G 27QMHK3W6qYni1abKz7tK7A6Sm69jvLaBv8zataRppEy TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb 6v5uYPBuubQqDwFU2uT6uQYC4P89H8iHzJgA4q7pvPHR 55

See console output

Wrapping 55 tokens from mint 8eZ2rk3GcKopnKrAmVibV7zLW1sszNo2GdbhtxRhrZRi
Unwrapped mint address: 8eZ2rk3GcKopnKrAmVibV7zLW1sszNo2GdbhtxRhrZRi
Wrapped mint address: 3xnGREX1Mi7zcuXJAYoDGk9xjpugZnfRhAKLPHdfEPPq
Unwrapped token account: EYUCh7xP4sxnNBr1VFcGXfHm1piEA6ERci5prNykSV4G
Recipient wrapped token account: 6v5uYPBuubQqDwFU2uT6uQYC4P89H8iHzJgA4q7pvPHR
Escrow account: 27QMHK3W6qYni1abKz7tK7A6Sm69jvLaBv8zataRppEy
Amount: 55
Signature: FRbCJmutYYFiGtnXzMFSgHucMRpdkHBHh4SCPX984qVQZRTfsFkvyKnnkbjW7M1nw1ZAKiTBC1igMnRSGg1H4x4

grod220 · 2025-03-24T21:33:58Z

clients/cli/src/create_mint.rs

    #[serde_as(as = "DisplayFromStr")]
    pub wrapped_mint_address: Pubkey,
    #[serde_as(as = "DisplayFromStr")]
+    pub wrapped_mint_authority: Pubkey,


Found myself needing the wrapped mint authority from the previous step (CreateMint) for the wrap command. In fact, will have another PR to expose CLI utilities for getting access to stuff like this at any time.

Do you need this in production, or only tests? Because it looks to me like you're deriving it.

Was mistaken here. There is not a need to pass the wrapped mint authority to the wrap command. Will delete.

clients/cli/tests/helpers.rs

buffalojoec

Looking great, just a few questions from my side, really.

buffalojoec · 2025-03-25T12:24:32Z

clients/cli/src/create_mint.rs

    #[serde_as(as = "DisplayFromStr")]
    pub wrapped_mint_address: Pubkey,
    #[serde_as(as = "DisplayFromStr")]
+    pub wrapped_mint_authority: Pubkey,


Do you need this in production, or only tests? Because it looks to me like you're deriving it.

clients/cli/src/wrap.rs

buffalojoec · 2025-03-25T12:35:56Z

clients/cli/tests/helpers.rs

-    };
-    let mut data = vec![0u8; spl_token::state::Mint::LEN];
-    state.pack_into_slice(&mut data);
+pub async fn create_unwrapped_mint(


For what it's worth, I prefer the startup account fixtures approach you had here originally. I saw the back-and-forth over here, so I'll leave it up to you to choose which you prefer, and it seems you have.

Do you have a reason for that preference?

We've had false positives in the past due to improper setup in tests, so I lean towards the more complicated setup to be sure we're actually testing a real-world situation.

We can certainly stick with fixtures for unit tests, but for a CLI, an end-to-end test is more useful to test the whole flow, but maybe I'm missing some context.

Do you have a reason for that preference?

I have formed a bias toward fixtures for a number of reasons, but it spawned from unit testing the RPC API for @solana/kit and has been hardened through writing program unit tests with Mollusk. It was also the motivation for building the latter.

The crux of the fixtures/mocks approach is the belief that your unit tests should be testing a single unit in isolation. If you're trying to test function C and its setup depends on functions A and B, now your tests for C are all coupled to functions A and B.

What if, all this time, CreateMint was doing something slightly erroneous, and we were getting all false positives in Wrap and Unwrap? It might sound ridiculous, but it happens.

Fixtures/mocks also let you set up a wide array of edge cases, which might be complex to set up through transactions (perhaps they would require a custom program, for example).

I also think if we're going to write unit tests that use separate test-validator instances, the faster they spin up and then back down, the better. That means less "send and confirm".

We can certainly stick with fixtures for unit tests, but for a CLI, an end-to-end test is more useful to test the whole flow, but maybe I'm missing some context.

I suppose I would just set up the per-command unit tests in isolation and then have one or more aptly-labeled end-to-end tests for the whole flow. I wouldn't make every CLI test an end-to-end test.

We've had false positives in the past due to improper setup in tests, so I lean towards the more complicated setup to be sure we're actually testing a real-world situation.

Without knowing the context of these false positives I don't want to make any assumptions, but I generally think this is an acceptable risk compared to coupling too many things within each test.

Anyway, that's just my two cents! If you both disagree with me, you can do what you think is best! If we're only testing for success within the CLI tests, and nothing else, maybe it doesn't matter anyway.

In my experience, there's a much higher chance of false positives with unit-testing than end-to-end testing because you can setup any set of inputs. As one example, remember all of the firedancer conformance fixtures that were checking impossible situations?

On the flip side, I'll definitely admit that highly-coupled tests are a lot more work to maintain. But I still stick by these words: Write tests. Not too many. Mostly integration

For now, since we don't have integration tests otherwise, I lean towards doing them here in the CLI. But we can let Gabe be the tie-breaker 😅

Definitely can see the two sides to this. One benefit I've seen with these CLI tests making the actual calls is that it's shown me the real-life process the end-user will need to go through in order to make use of it. This inspired #40 because I ran into a user flow I wasn't fully supporting.

One thing to note is in the main program code with mollusk, the processor methods were tested in isolation with pre-made accounts passed in, which feels like it worked well given the variety of cases those tests covered. Given that, I think this program probably could use the "integration coverage" by continuing along the path of this PR. Will go that route for these CLI tests.

joncinque

Looks great overall! Mostly small things

joncinque · 2025-03-25T12:48:13Z

clients/cli/tests/helpers.rs

-    };
-    let mut data = vec![0u8; spl_token::state::Mint::LEN];
-    state.pack_into_slice(&mut data);
+pub async fn create_unwrapped_mint(


Do you have a reason for that preference?

We've had false positives in the past due to improper setup in tests, so I lean towards the more complicated setup to be sure we're actually testing a real-world situation.

We can certainly stick with fixtures for unit tests, but for a CLI, an end-to-end test is more useful to test the whole flow, but maybe I'm missing some context.

clients/cli/tests/test_wrap.rs

clients/cli/src/wrap.rs

joncinque · 2025-03-25T12:59:21Z

clients/cli/src/wrap.rs

+    /// The address of the escrow account that will hold the unwrapped tokens
+    #[clap(value_parser = parse_pubkey)]
+    pub escrow_account: Pubkey,


Doesn't need to be done here, but we could eventually have a helper to create an escrow account, probably the wrapped mint authority's ATA for the unwrapped token mint would be good.

That way, we could make this optional too and default to that ATA

Good to know. Helpful for a later PR.

clients/cli/src/wrap.rs

joncinque

Just a few last nits, then this can go in from my side

clients/cli/Cargo.toml

clients/cli/src/wrap.rs

clients/cli/tests/helpers.rs

joncinque

Looks great!

buffalojoec

Ship it!

grod220 force-pushed the cli-wrap branch from 49a5ff8 to 5ddb3e3 Compare March 24, 2025 21:31

grod220 commented Mar 24, 2025

View reviewed changes

clients/cli/tests/helpers.rs Show resolved Hide resolved

grod220 marked this pull request as ready for review March 24, 2025 21:49

grod220 requested review from joncinque and buffalojoec March 24, 2025 21:49

buffalojoec reviewed Mar 25, 2025

View reviewed changes

joncinque reviewed Mar 25, 2025

View reviewed changes

grod220 added 2 commits March 25, 2025 19:23

Wrap CLI (single signer)

6dcb2ff

Review updates

01e0013

grod220 force-pushed the cli-wrap branch from 5ddb3e3 to 01e0013 Compare March 25, 2025 18:35

grod220 requested review from buffalojoec and joncinque March 25, 2025 18:56

joncinque reviewed Mar 25, 2025

View reviewed changes

grod220 force-pushed the cli-wrap branch from 0c8a0e4 to 55b9089 Compare March 26, 2025 11:17

Review updates

d8f8ddd

grod220 force-pushed the cli-wrap branch from 55b9089 to d8f8ddd Compare March 26, 2025 11:18

grod220 requested a review from joncinque March 26, 2025 11:34

joncinque approved these changes Mar 26, 2025

View reviewed changes

buffalojoec approved these changes Mar 27, 2025

View reviewed changes

grod220 merged commit 5a8b26d into main Mar 27, 2025
10 checks passed

grod220 deleted the cli-wrap branch March 27, 2025 13:00

grod220 mentioned this pull request Mar 31, 2025

JS client helper: Wrap #45

Merged

CLI helper: Wrap #38

CLI helper: Wrap #38

Uh oh!

Conversation

grod220 commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough for testing locally:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

buffalojoec left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

buffalojoec Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joncinque left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joncinque left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joncinque left a comment

Choose a reason for hiding this comment

Uh oh!

buffalojoec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

grod220 commented Mar 24, 2025 •

edited

Loading

buffalojoec Mar 25, 2025 •

edited

Loading