feat: add Soroban vault operations CLI#465
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughOverview
Key locations
Notable functional changes
Security & safety highlights (implemented)
Critical items for human review (prioritise)
Smart-contract-specific checklist
Minor/operational notes
Recommendation
WalkthroughRefactors proxy-curator governance ABI and TTL behavior, standardizes deploy artifact to the optimized runtime WASM, and adds a new soroban-vault-cli crate (CLI, manifest, profile, Stellar integration, artifact management, tests, Docker, and docs). ChangesSoroban proxy contract refactoring and Soroban Vault CLI implementation
Sequence Diagram(s)sequenceDiagram
participant UserCLI as tmplr-soroban-vault (CLI)
participant Manifest as Manifest (state.json)
participant Stellar as stellar CLI
participant RPC as Soroban RPC
UserCLI->>Manifest: load_or_new / read manifest
UserCLI->>Stellar: build_package / upload (or invoke)
Stellar->>RPC: submit transaction / fetch tx
RPC-->>Stellar: tx result / wasm hash
Stellar-->>UserCLI: stdout/stderr with tx hash
UserCLI->>Manifest: save updated artifact/contract records
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
carrion256
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
carrion256
marked this pull request as ready for review
June 10, 2026 16:13
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f69cd1942
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
carrion256
force-pushed
the
codex/soroban-vault-cli
branch
from
June 10, 2026 16:19
8f69cd1 to
c00b91b
Compare
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contract/proxy-curator-soroban/src/contract.rs`:
- Around line 262-265: The extend_ttl function currently calls
extend_instance_ttl(&env) permissively, allowing permissionless TTL extension
prior to initialization; update extend_ttl to check the contract's initialized
state (the same condition used in initialize/initialize-related checks) before
calling extend_instance_ttl and return an appropriate ContractError if the
instance is not initialized; locate and reuse the existing initialized flag or
helper used by initialize() (and the init-gating logic around
initialize/extend_instance_ttl) so only initialized instances may have their TTL
extended.
In `@contract/vault/deployments/tTUSDC/mainnet/manifest.json`:
- Line 79: The manifest.json entry for the governance contract sets
"timelock_ns": "0" which violates the repo safety guidance; either change the
manifest's timelock_ns to a non-zero value appropriate for mainnet governance
(e.g., a conservative seconds->nanoseconds duration) in the same manifest that
contains the "Test Templar USDC" metadata, or if this is intentionally a test
deployment, add an explicit note to the mainnet README.md documenting that this
deployment is experimental (reference "Test Templar USDC"), that it uses
SOROBAN_GOV_TIMELOCK_NS=0, and that SOROBAN_ALLOW_ZERO_GOV_TIMELOCK=1 must be
set to permit zero timelock; ensure the README callout explains the risk to user
funds and the temporary/test-only status so reviewers can verify intent.
- Line 43: The manifest's wasm_path currently points to
"templar_soroban_runtime.deploy.wasm" which no longer matches the new build
convention used by the CLI (see tools/soroban-vault-cli/src/artifacts.rs:55);
update the wasm_path value in manifest.json to "templar_soroban_runtime.wasm" so
it aligns with the new workflow and the hardcoded CLI lookup, or alternatively
add a brief note in the mainnet README explaining the historical ".deploy"
suffix and how to verify this manifest with the new tooling; modify the
wasm_path key (in manifest.json) or add the README comment accordingly to ensure
manifest verification works with the updated CLI.
In `@tools/soroban-vault-cli/Dockerfile`:
- Line 77: The Dockerfile currently uses a broad COPY --chown=templar:templar .
/workspace which bakes the entire build context (including .git, target, .env,
editor temp files) into the image; either add a repository-level .dockerignore
to exclude .git, target, .env, *.swp, .DS_Store etc., or replace the single COPY
with selective copies of only the build inputs (e.g., Cargo.toml, Cargo.lock,
relevant source directories like contract/ and tools/) so only necessary files
are included; if the full workspace truly is required, add a clear comment above
the COPY and ensure .dockerignore at minimum excludes .git and secrets.
- Around line 79-84: The Dockerfile creates runtime directories
(/home/templar/.cargo, /home/templar/.config/stellar,
/workspace/contract/vault/soroban/.deploy-state, /workspace/target) but later
declares a VOLUME for /workspace/target and possibly
/workspace/contract/vault/soroban/.deploy-state, which will hide any build-time
artifacts copied into /workspace; update the Dockerfile around the RUN mkdir -p
and the VOLUME declaration to add a clear comment documenting that VOLUME mounts
will shadow the image’s /workspace/target and
/workspace/contract/vault/soroban/.deploy-state (and instruct operators to mount
local paths to persist prebuilt artifacts if needed), or alternatively avoid
declaring those directories as VOLUME so prebuilt artifacts remain available at
runtime—reference the RUN mkdir -p block and the VOLUME entries to make the
change.
In `@tools/soroban-vault-cli/src/stellar.rs`:
- Line 263: The temporary-file removal call currently swallows errors with let _
= fs::remove_file(&out_file); — change it to check the Result and log failures
so cleanup errors (permissions, disk, open handle) are visible; locate the place
where out_file is removed (the fs::remove_file(&out_file) call) and replace the
ignore with an error-aware branch (e.g., if let Err(e) =
fs::remove_file(&out_file) { /* log e with context mentioning out_file and the
operation */ }) using the existing logging facility or eprintln! if none is
available.
- Around line 476-477: fetch_transaction_status currently treats
transaction_status_from_output(&output.stdout).unwrap_or(TransactionConfirmationStatus::Success)
which masks parse failures as successes; change fetch_transaction_status to
treat a None result as an error (or return an explicit
TransactionConfirmationStatus::Unknown/Unconfirmed) instead of defaulting to
Success, and include a short excerpt of output.stdout (e.g., first N bytes or
trimmed snippet) in the error message to aid diagnosis; update any call sites
expecting Success on parse failure to handle the new error/Unknown variant and
adjust signatures/errors accordingly (refer to fetch_transaction_status,
transaction_status_from_output, output.stdout, and
TransactionConfirmationStatus).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 459e1bd6-a2bf-4142-bf22-7cb8a297efca
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
.gitignoreCargo.tomlcontract/proxy-curator-soroban/Cargo.tomlcontract/proxy-curator-soroban/src/contract.rscontract/proxy-curator-soroban/src/governance_abi.rscontract/proxy-curator-soroban/src/lib.rscontract/proxy-curator-soroban/src/tests.rscontract/vault/deployments/tTUSDC/mainnet/README.mdcontract/vault/deployments/tTUSDC/mainnet/manifest.jsoncontract/vault/soroban/AGENTS.mdcontract/vault/soroban/README.mdcontract/vault/soroban/SIZE_BUDGET.mdcontract/vault/soroban/justfiletools/soroban-vault-cli/Cargo.tomltools/soroban-vault-cli/Dockerfiletools/soroban-vault-cli/README.mdtools/soroban-vault-cli/schema/output.schema.jsontools/soroban-vault-cli/src/artifacts.rstools/soroban-vault-cli/src/cli.rstools/soroban-vault-cli/src/commands.rstools/soroban-vault-cli/src/lib.rstools/soroban-vault-cli/src/main.rstools/soroban-vault-cli/src/manifest.rstools/soroban-vault-cli/src/profile.rstools/soroban-vault-cli/src/stellar.rstools/soroban-vault-cli/src/types.rs
💤 Files with no reviewable changes (1)
- contract/proxy-curator-soroban/Cargo.toml
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
♻️ Duplicate comments (1)
contract/proxy-curator-soroban/src/contract.rs (1)
252-259:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate TTL extension on initialized state.
Line 257 extends proxy instance TTL permissively without checking initialization, allowing anyone to keep an uninitialized proxy alive indefinitely. Combined with Line 701 extending TTL before verifying initialization in
require_initialized, this widens the front-running window forinitialize().Based on learnings,
initialize()is highly sensitive because front-running can seize governance/curator control; pre-init TTL extension should stay explicit and constrained.Suggested fix
pub fn extend_ttl(env: Env) -> Result<(), ContractError> { + require_initialized(&env)?; extend_instance_ttl(&env); Ok(()) }And in
require_initialized:pub(crate) fn require_initialized(env: &Env) -> Result<(), ContractError> { - extend_instance_ttl(env); - is_initialized(env) - .then_some(()) - .ok_or(ContractError::NotInitialized) + if !is_initialized(env) { + return Err(ContractError::NotInitialized); + } + extend_instance_ttl(env); + Ok(()) }Also applies to: 700-705
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contract/proxy-curator-soroban/src/contract.rs` around lines 252 - 259, The extend_ttl path currently calls extend_instance_ttl unconditionally, allowing anyone to prolong an uninitialized proxy; modify pub fn extend_ttl(env: Env) to first verify the contract is initialized (reuse the require_initialized or the same check it performs) and only call extend_instance_ttl when initialized, returning an appropriate ContractError otherwise; also move the TTL-extension call in require_initialized (around the code referenced at lines ~700) so the extension happens after the initialization check/success, not before, ensuring extend_instance_ttl is only invoked for already-initialized instances and preventing pre-init TTL extensions that widen the initialize() front-running window.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/soroban-vault-cli/src/lib.rs`:
- Around line 18-51: Add a regression test that ensures profile-injected values
starting with '-' can't be parsed as extra flags: create a test that uses
profile::expand_args (or expand_args_with_dir/push_profile_arg) to inject a
ProfileConfig with a value like network = "--allow-mainnet-write" into raw args,
then attempt to parse with cli::Cli::try_parse_from and assert the parser either
fails safely (returns an error) or that the hazardous flag was not set on the
resulting Cli (i.e., parsing did not treat the injected value as a separate
option). Locate the test next to other profile tests and reference
profile::expand_args, expand_args_with_dir, and push_profile_arg when
constructing the scenario.
In `@tools/soroban-vault-cli/src/manifest.rs`:
- Around line 41-63: Add a short inline comment above the line
`manifest.source_account = None;` in the `load_or_new` function of `manifest.rs`
explaining that this explicitly clears any persisted secret/source account info
to avoid writing sensitive data to disk (security: scrub secrets from manifest
on load) so future maintainers understand this is intentional and not a bug.
---
Duplicate comments:
In `@contract/proxy-curator-soroban/src/contract.rs`:
- Around line 252-259: The extend_ttl path currently calls extend_instance_ttl
unconditionally, allowing anyone to prolong an uninitialized proxy; modify pub
fn extend_ttl(env: Env) to first verify the contract is initialized (reuse the
require_initialized or the same check it performs) and only call
extend_instance_ttl when initialized, returning an appropriate ContractError
otherwise; also move the TTL-extension call in require_initialized (around the
code referenced at lines ~700) so the extension happens after the initialization
check/success, not before, ensuring extend_instance_ttl is only invoked for
already-initialized instances and preventing pre-init TTL extensions that widen
the initialize() front-running window.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97faf711-edfb-4c70-af3b-3c64f4b5d074
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
.gitignoreCargo.tomlcontract/proxy-curator-soroban/Cargo.tomlcontract/proxy-curator-soroban/src/contract.rscontract/proxy-curator-soroban/src/governance_abi.rscontract/proxy-curator-soroban/src/lib.rscontract/proxy-curator-soroban/src/tests.rscontract/vault/deployments/tTUSDC/mainnet/README.mdcontract/vault/deployments/tTUSDC/mainnet/manifest.jsoncontract/vault/soroban/AGENTS.mdcontract/vault/soroban/README.mdcontract/vault/soroban/SIZE_BUDGET.mdcontract/vault/soroban/justfiletools/soroban-vault-cli/Cargo.tomltools/soroban-vault-cli/Dockerfiletools/soroban-vault-cli/README.mdtools/soroban-vault-cli/schema/output.schema.jsontools/soroban-vault-cli/src/artifacts.rstools/soroban-vault-cli/src/cli.rstools/soroban-vault-cli/src/commands.rstools/soroban-vault-cli/src/lib.rstools/soroban-vault-cli/src/main.rstools/soroban-vault-cli/src/manifest.rstools/soroban-vault-cli/src/profile.rstools/soroban-vault-cli/src/stellar.rstools/soroban-vault-cli/src/types.rs
💤 Files with no reviewable changes (1)
- contract/proxy-curator-soroban/Cargo.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new
tmplr-soroban-vaultCLI crate for deploying and operating Soroban vault deployments through the Stellar CLI.The CLI includes typed deployment, curator, user, governance, share-token, and Blend adapter operations. It records deployment state in a manifest, reuses uploaded WASM by hash where possible, supports dry-run and machine-readable output modes, and adds a deployment-wide
extend-ttlcommand for every TTL-capable contract in the manifest.This also adds a Docker operator image at
tools/soroban-vault-cli/Dockerfile. The image includestmplr-soroban-vault,stellar-cliv26, Python for the runtime contractspec-strip step, and Rust toolchains/targets forstellar contract buildso operators can run deployment flows from a container.The curator proxy TTL gap is fixed: the proxy now has its own permissionless
extend_ttlentrypoint and refreshes its instance TTL when initialized config is read, matching the 4626 proxy liveness pattern.The CLI no longer requires or defaults a source account CLI argument. Operators can use the Stellar keystore/default identity via
stellar keys use <identity>, or pass a non-secret identity alias/public account with--source-account/SOROBAN_IDENTITY. Explicit source-account overrides are provided to childstellarcommands through a redactedSTELLAR_ACCOUNTenvironment override instead of--source-accountargv.The CLI rejects obvious raw secret keys and seed phrases passed to
--source-account, redacts source-account environment overrides from dry-run output and errors, zeroizes in-process override buffers after use, and scrubs source-account values from deployment manifests. If rawSTELLAR_ACCOUNTis set directly for signing, the CLI refuses to derive admin/caller addresses from it because that Stellar subcommand would require placing the secret in child argv; operators must pass explicit public--admin/--callervalues or use a keystore/default identity for that flow.This update also adds safer operator and automation surfaces:
--assets 1.25 --asset-decimals 7,--shares 10 --share-decimals manifest, and curator--amount 1.25 --asset-decimals 7convert to raw contract units without floating point.--assets-raw,--shares-raw, and--amount-raw.profile init <name>and--profile <name>can carry network/RPC/passphrase/state/workspace/config-dir/default public admin/caller/operator values without storing secrets.completions zsh|bash|fishandmangenerate shell completions and a roff manpage.--yesor interactive confirmation; JSON modes remain non-interactive.--jsonnow emits a stable envelope withtype,ok,network,manifest,commands,tx_hashes,warnings, and command-specificdataor structurederror.--json-linesemits the same newline-delimited envelope shape for automation that consumes long-running flows.missing_manifest_contract,mainnet_guard,secret_in_argv,invalid_args, andcommand_failed, including JSON parse-error output before clap exits.tools/soroban-vault-cli/schema/output.schema.json.This update also adds operator UX helpers:
doctorchecks Stellar CLI availability, configured network/passphrase/RPC, source identity availability without printing secrets, manifest writability, WASM artifact/build readiness, Docker mount health, and mainnet guard status.deploy plan stackanddeploy plan adaptersemit reuse/deploy/upload/manifest decisions and redacted Stellar command shapes without network writes or manifest changes.plan-accept,plan-submit-set-supply-queue, andplan-submit-set-timelock.queue,explain,accept-ready, andsubmit-and-waitflows with conservative decoding of pending proposal readiness.Operator impact
doctor.Validation
cargo fmt --package templar-curator-proxy-soroban --package templar-soroban-vault-cli --checkcargo fmt --package templar-soroban-vault-cli --checkcargo test -p templar-curator-proxy-soroban -- --nocapture(13 unit + 3 integration passed)cargo test -p templar-soroban-vault-cli -- --nocapture(45 passed)cargo clippy -p templar-curator-proxy-soroban --all-targets -- -D warningscargo clippy -p templar-soroban-vault-cli --all-targets -- -D warnings(exits 0; repo-levelunknown lint: clippy::ignore_without_reasonwarning remains)cargo run -q -p templar-soroban-vault-cli -- --json --source-account SC36XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX status(expected exit 2; emitssecret_in_argvJSON envelope)TEMPLAR_SOROBAN_VAULT_PROFILE_DIR=$(mktemp -d) cargo run -q -p templar-soroban-vault-cli -- profile init testnetcargo run -q -p templar-soroban-vault-cli -- completions bash > <tempfile>cargo run -q -p templar-soroban-vault-cli -- man > <tempfile>docker build --check -f tools/soroban-vault-cli/Dockerfile .docker build --network=host -f tools/soroban-vault-cli/Dockerfile -t templar/soroban-vault-cli:codex .docker run --rm --network=none templar/soroban-vault-cli:codex --helpsize-budget-checkpassed (123720 bytes)This change is