cli: various improvements #393

2501babe · 2025-10-20T22:28:20Z

display --all now performs 4 total rpc calls total instead of 1 + (3 * number of pools)
determine if accounts are initialized based on their data rather than lamports
show main stake account excess lamports in display --verbose
do not create an associated token account for deposit --dry-run
correctly print that we cannot display deposit/withdraw values for --dry-run
check a vote-owned account is actually a vote account for initialize
remove ProgramClient from tests
add test for display --all
update README.md

part of #118
closes #119
closes #143
closes #234
closes #392

2501babe · 2025-10-21T03:02:36Z

clients/cli/src/main.rs

+    if pool_and_vote_addresses.len() > 100 {
+        return Err(
+            "Displaying more than 100 pools is not implemented; if you see \
+            this error, feel free to open an issue in the SVSP repo."
+                .into(),
+        );
+    }


getMultipleAccounts limit. mainnet has 40-something pools, so it wont be relevant for awhile. ill probably do the >100 chuncking and the onramp support in display at the same time, the two of them will make it complicated enough ill refactor all this stuff into its own module

2501babe · 2025-10-21T03:30:35Z

clients/cli/src/output.rs

+        let token_amount = if let Some(amount) = self.token_amount {
+            &amount.to_string()
+        } else {
+            "(cannot display in simulation)"
+        };
+        writeln_name_value(f, "Token amount:", token_amount)?;


after we upgrade to 3.0 (i want to do a final 2.2 program release first though) we should be able to get this off the collected balances

2501babe · 2025-10-21T03:33:25Z

clients/cli/tests/test.rs


-    // make clients
-    let rpc_client = Arc::new(validator.get_async_rpc_client());
-    let program_client: PClient = Arc::new(ProgramRpcClient::new(
-        rpc_client.clone(),
-        ProgramRpcClientSendTransaction,
+    // make client
+    let rpc_client = Arc::new(RpcClient::new_with_commitment(
+        validator.rpc_url(),
+        CommitmentConfig::confirmed(),
    ));


removed ProgramClient from tests just because we dont need it. planning on dropping token client from the cli itself when i do the 3.0 upgrade

2501babe · 2025-10-21T03:39:48Z

clients/cli/tests/test.rs

    loop {
        let epoch_info = rpc_client.get_epoch_info().await.unwrap();
-        if epoch_info.epoch > current_epoch {
+        if epoch_info.epoch > current_epoch && epoch_info.slot_index > 0 {
            return epoch_info.epoch;
        }


this was a frustrating but ultimately funny debugging experience, i was going nuts trying to figure out how my command_deposit() refactor introduced stake program error 10 (VoteAddressMismatch) in some tests and it took longer than id like to admit to notice it was actually error 0x10 (EpochRewardsActive)

i think something i changed made it slightly faster: before, by time the test transaction executed, a slot had already elapsed

Oof, that sounds frustrating

not important but i realized it was definitely the --dry-run fix, we used to create the ata in its own transaction which delayed enough to advance the slot, now we just prepend it to the deposit transaction

joncinque

Looks great to me! Just some tiny nits which you can take, leave, or fix in another PR

joncinque · 2025-10-28T11:53:30Z

README.md

+with:
+
+```console
+solana-verify build --library-name program


It looks like the build works with this command, but in my testing and looking at the source code, I think --library-name takes the crate name:

Suggested change

solana-verify build --library-name program

solana-verify build --library-name spl_single_pool

im not sure what you mean by "the build works with this command [but] --library-name takes the crate name." like both work but the latter is preferred for style? ive always built by giving it program

I mean that it'll build the program, but the CLI code is expecting spl_single_pool as the input, to show the build hash afterwards

39bf1d3

thats funny, im definitely not the only one because ive had people ask about the parse error solana-verify gives at the end and been like "oh dont worry it just does that for some reason, the build still worked"

joncinque · 2025-10-28T12:20:30Z

clients/cli/tests/test.rs

    loop {
        let epoch_info = rpc_client.get_epoch_info().await.unwrap();
-        if epoch_info.epoch > current_epoch {
+        if epoch_info.epoch > current_epoch && epoch_info.slot_index > 0 {
            return epoch_info.epoch;
        }


Oof, that sounds frustrating

joncinque · 2025-10-28T12:21:42Z

clients/cli/tests/test.rs

+    let output = Command::new(SVSP_CLI)
+        .args(["display", "-C", &env.config_file_path, "--all", "--verbose"])
+        .output()
+        .unwrap();
+    assert!(output.status.success());
+
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    let pools = stdout
+        .lines()
+        .filter(|line| line.starts_with("  Pool address:"))
+        .count();
+    assert_eq!(pools, 3);


nit: up to you, but since this part is checking the --verbose flag, do you want to check for an additional output not part of the normal display command?

joncinque · 2025-10-28T12:24:04Z

clients/cli/Cargo.toml

 solana-transaction = "2.2"
 solana-transaction-status = "2.3.4"
 solana-vote-program = "2.2"
+spl-associated-token-account = { version = "7.0", features = ["no-entrypoint"] }


nit: not a big deal since this is a CLI crate, but spl-associated-token-account-interface has what you need with fewer deps

joncinque · 2025-10-28T12:29:32Z

clients/cli/src/main.rs

+    let stake_addresses = pool_and_vote_addresses
+        .iter()
+        .map(|(pool_address, _)| find_pool_stake_address(&spl_single_pool::id(), pool_address))
+        .collect::<Vec<_>>();


Is the idea to add the onramp accounts here in the future when the onramp account will be integrated into more instructions?

yes, i was imagining doing it in another pr later. i havent decided how to present the onramp info yet since onramp stake is disregarded for token accounted but i want to do a v2 of token math soon-ish

joncinque

Looks great to me!

2501babe added 8 commits October 20, 2025 06:29

better init checks

cac1b03

dont create token account if --dry-run

dc40186

call minimum delegation once

0a126fa

test multi-display, get pclient out of tests

842e185

make DisplayAll like 1000x faste rlol

2025fc2

add excess lamps to display

84cc620

actually do the dryrun right

640dcda

update readme

f0fff83

2501babe self-assigned this Oct 20, 2025

wait for rewards period to end

c7f2c69

2501babe commented Oct 21, 2025

View reviewed changes

2501babe marked this pull request as ready for review October 21, 2025 03:43

2501babe requested a review from joncinque October 21, 2025 03:43

2501babe mentioned this pull request Oct 21, 2025

upgrade to sdk 3.0 #397

Closed

joncinque previously approved these changes Oct 28, 2025

View reviewed changes

2501babe added 2 commits October 28, 2025 11:20

use ata interface

619fda0

test verbose display all

2a1316d

2501babe dismissed joncinque’s stale review via 2a1316d October 28, 2025 18:28

improve verify command

39bf1d3

joncinque approved these changes Oct 28, 2025

View reviewed changes

2501babe merged commit b23776b into solana-program:main Oct 28, 2025
18 checks passed

2501babe deleted the cli-cleaning branch October 28, 2025 20:35

2501babe mentioned this pull request Nov 4, 2025

interface: include onramp in deposit/withdraw #396

Merged

	solana-verify build --library-name program
	solana-verify build --library-name spl_single_pool

cli: various improvements #393

cli: various improvements #393

Uh oh!

Conversation

2501babe commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

2501babe Oct 21, 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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

joncinque left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

2501babe commented Oct 20, 2025 •

edited

Loading

2501babe Oct 21, 2025 •

edited

Loading