solana-cli: add a Squads vault path to shreds validator-client-rewards claim

[elitegreg]

Resolves: #4185

Summary of Changes

• shreds validator-client-rewards claim gains a Squads vault path. With --multisig (and optional --vault-index), the vault stands in for the wallet as manager, no keypair is loaded, and the command prints base58 payloads for import into Squads instead of signing and sending. Each payload carries CheckCliVersion and the claim, and no compute budget instructions, since Squads sets the budget on its execute transaction.
• The vault path keeps the manager check, against the vault key, so a payload the multisig can never execute is refused before it consumes an approval round. The mismatch message now names whichever actor was checked.
• The default destination on the vault path is the vault's associated token account. When it is missing, every payload prepends create_idempotent, so each payload stays independently executable, and the output says the vault pays the rent. An explicit --destination-token-account that does not exist is refused. The direct path still requires the destination to exist.
• MAX_CLAIM_EPOCHS_PER_TX is retired. Each transaction grows one holding at a time while a trial still fits. The direct path measures the v0 transaction the wallet sends, always counting the fee payer and both compute budget instructions so packing does not shift with the flags. The vault path measures the legacy message the base58 string carries against the Squads payload budget. Measured capacities: 19 holdings per direct transaction, 16 with a distinct --fee-payer, 12 per vault payload into an existing destination, 10 when every payload creates it.
• New --max-transactions N caps how many transactions one invocation produces. The output reports how many were withheld and how to reach them.
• Vault output is written for a human copying payloads by hand: every payload is numbered, states its holdings, epochs and pre-claim total, keeps the base58 alone on its own line, and the command closes with a recap and the note that payloads are independent and a re-run discovers only what is still outstanding. The destination token account's authority is printed on both paths.
• solana-client-tools: add squads::try_write_vault_transaction, a writer-taking form that try_print_vault_transaction now delegates to, and Wallet::try_transaction_size, which measures the transaction new_transaction would build with the same signer set.

Diff Breakdown

Category	Files	Lines (+/-)	Net
Core logic	1	+538 / -192	+346
Scaffolding	2	+47 / -23	+24
Tests	3	+299 / -11	+288
Docs	3	+8 / -0	+8
Total	6	+892 / -226	+666

Test and core lines are split by test module within each Rust file, so the per-category numbers are approximate. Roughly half the change is the claim command itself and a third is new tests.

Key files (click to expand)

• offchain/crates/solana-cli/src/command/shreds/validator_client_rewards/claim.rs — actor selection (wallet or vault), destination resolution and ATA creation, measured packing, --max-transactions, and the vault output
• offchain/crates/solana-client-tools/src/payer.rs — Wallet::try_transaction_size and the shared signer list it measures with
• offchain/crates/solana-client-tools/src/squads.rs — try_write_vault_transaction, with the printing form delegating to it

Testing Verification

• Unit tests pin the measured packing capacities (19, 16, 12, 10 holdings) and check that every packed batch fits as sent, with and without a compute unit price, while one more holding does not. The vault batches are also run through the checked Squads encoder, which accepts each batch and refuses each batch plus one holding.
• Tests cover the growth loop itself (grows, splits, refuses a holding that fits nowhere), the --max-transactions withheld note, the create-ATA instruction appearing in every payload, --vault-index requiring --multisig, --max-transactions 0 being rejected, and the manager mismatch message naming the wallet or the vault.
• A Wallet with a distinct fee payer measures exactly 96 bytes larger than one without, and the writer-taking Squads printer puts the base58 alone on its own line.
• Manually on Solana devnet, with no keypair available to the command: --multisig pointed at a non-Squads account fails with the "not the Squads v4 program" message before anything is built, and a real devnet multisig gets through the multisig check to the validator client rewards lookup. No devnet client is managed by a Squads vault, so no end-to-end payload was produced against a live multisig.

[Copilot]

🟡 Changes recommended

Wallet::signers can now include duplicate signers when fee_payer equals signer, which can break transaction construction for callers that build Wallet directly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This pull request adds a Squads multisig vault execution path to the shreds validator-client-rewards claim Solana CLI command so a vault-managed manager can claim rewards without a local keypair, while keeping transaction sizing accurate for both the direct-send and Squads-import flows.

Changes:

• Add --multisig and --vault-index support to emit numbered base58 payloads for Squads import, including destination ATA creation when missing and clearer manager-mismatch messaging.
• Replace the fixed “holdings per tx” cap with measured packing that grows each claim until it no longer fits, plus a new --max-transactions cap.
• Extend solana-client-tools with squads::try_write_vault_transaction (writer-based output) and Wallet::try_transaction_size for consistent sizing with the actual signer set.

File summaries

File	Description
offchain/crates/solana-cli/src/command/shreds/validator_client_rewards/claim.rs	Add vault actor mode, destination resolution and ATA creation for vault, measured batching, and --max-transactions output behavior.
offchain/crates/solana-client-tools/src/payer.rs	Add Wallet::try_transaction_size and refactor signer selection into a helper.
offchain/crates/solana-client-tools/src/squads.rs	Add try_write_vault_transaction that writes to an impl Write, and delegate the stdout printer to it.
offchain/crates/solana-client-tools/README.md	Document the new writer-based Squads payload output helper.
offchain/crates/solana-client-tools/CHANGELOG.md	Record the new Squads writer helper and wallet sizing helper.
offchain/crates/solana-cli/CHANGELOG.md	Record the new --multisig claim workflow, packing behavior, and --max-transactions.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[nikw9944]

Two things to change before merge: vault payloads are packed to within one holding of the size ceiling, so a memo typed at import is refused — squads.rs names that hazard exactly; and an existing --destination-token-account is accepted on the vault path without checking its authority, while the missing-destination branch already enforces the vault-only rule. Four smaller items on the epoch range, the pre-claim total under --max-transactions, CheckCliVersion in a deferred payload, and the unbounded execute CU.

[bgm-malbeclabs]

Good shape overall. Measured packing is the right call over the fixed cap, and the trial that always counts the fee payer and both compute budget instructions means the sent transaction is never larger than what was measured. Checking the manager against the vault before anything gets built saves a wasted approval round. The tests that pin the capacities and then re-run every packed batch through the checked encoder are the right shape, not just the number.

Two blockers inline: the Pre-claim total line reports money this run did not claim, and the vault payloads pack to within 36 bytes of the Squads budget, which a memo typed at import spends. Five non-blocking notes after those.

Minor, no comment needed: pack_holdings treats any fits error as fatal, so a build failure aborts the claim instead of ending the batch; last_executed is really any_executed; and the header's "N more transaction(s) withheld" duplicates write_withheld_note.

[elitegreg]

Pushed 029f19d, replies on each thread above.

On the three from the review body: pack_holdings still treats a fits error as fatal on purpose, since a build failure there is a bug rather than a size signal and ending the batch on it would quietly emit short transactions. last_executed is now any_executed. The header no longer duplicates write_withheld_note.

Two things worth a second look. Vault payload capacity moved to 11 holdings into an existing destination and 9 where every payload creates it, down one each: the 72-byte memo reserve costs more than that, and dropping CheckCliVersion from the payload gives most of it back. And the execute compute budget is surfaced rather than solved, since each payload now prints the compute units it needs but the Squads app's own limit is still unconfirmed from here.

[nikw9944]

All six findings from the last round are fixed in 029f19de and verified in the code, with tests covering the memo-reserve packing budget, the vault-authority check on an existing destination, the sparse epoch list, and the emitted-only totals. The execute compute is now reported per payload rather than bounded — the Squads app's own limit stays unconfirmed, which the vault output says plainly. lgtm.

Re reviewed