Skip to content

Add --ledger to stellar keys add.#2563

Merged
fnando merged 3 commits intomainfrom
add-ledger-keys
May 7, 2026
Merged

Add --ledger to stellar keys add.#2563
fnando merged 3 commits intomainfrom
add-ledger-keys

Conversation

@fnando
Copy link
Copy Markdown
Member

@fnando fnando commented May 6, 2026

What

Also persist public key and hd path to a config file, so we don't need to interact with the hardware on read-only operations.

Why

To match and have a uniform experience all around.

Known limitations

n/a

@fnando fnando self-assigned this May 6, 2026
Copilot AI review requested due to automatic review settings May 6, 2026 19:38
@fnando fnando added this to DevX May 6, 2026
@github-project-automation github-project-automation Bot moved this to Backlog (Not Ready) in DevX May 6, 2026
@fnando fnando moved this from Backlog (Not Ready) to Needs Review in DevX May 6, 2026
@fnando fnando requested review from leighmcculloch and mootz12 May 6, 2026 19:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends stellar keys add with a dedicated --ledger flag and updates config serialization to persist a Ledger-derived public key (and optional --hd-path) so read-only operations can work without a connected device.

Changes:

  • Add stellar keys add --ledger (async) to derive and persist a Ledger-backed identity (cached public_key + optional hd_path).
  • Update Secret::Ledger persistence format to include hardware, cached public_key, and optional hd_path, plus unit tests for TOML round-trips.
  • Adjust a few parsing/error-path tests and avoid attempting Ledger HID use in contract signer resolution.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
FULL_HELP_DOCS.md Documents the new --ledger option for stellar keys add.
cmd/soroban-cli/src/config/secret.rs Redefines Secret::Ledger to persist cached public_key/hd_path and adds related tests; updates key derivation/signing behavior.
cmd/soroban-cli/src/config/key.rs Updates a parsing test expectation now that "ledger" is no longer parsed as a secret literal.
cmd/soroban-cli/src/config/address.rs Removes the special-case rejection of the identity name "ledger".
cmd/soroban-cli/src/commands/message/verify.rs Updates a test expectation for "ledger" handling in public key resolution.
cmd/soroban-cli/src/commands/keys/mod.rs Makes keys add invocation await the now-async implementation.
cmd/soroban-cli/src/commands/keys/add.rs Implements --ledger flow (derivation + persistence), adds clap conflict tests, and converts affected tests to async.
cmd/soroban-cli/src/commands/contract/arg_parsing.rs Skips resolving a Ledger signer in the co-signer path to avoid HID transport conflicts.

Comment thread cmd/soroban-cli/src/config/secret.rs
Comment thread cmd/soroban-cli/src/config/secret.rs
Comment thread cmd/soroban-cli/src/commands/keys/add.rs
Comment thread cmd/soroban-cli/src/commands/message/verify.rs Outdated
Copy link
Copy Markdown
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

One concern, but defer to you on it.

Comment thread cmd/soroban-cli/src/commands/contract/arg_parsing.rs Outdated
@fnando fnando force-pushed the add-ledger-keys branch from 669cf7d to f669bf8 Compare May 7, 2026 17:54
@fnando fnando enabled auto-merge (squash) May 7, 2026 18:00
@fnando fnando merged commit 630619d into main May 7, 2026
211 checks passed
@fnando fnando deleted the add-ledger-keys branch May 7, 2026 18:11
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in DevX May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants