Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions FULL_HELP_DOCS.md
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,7 @@ Add a new identity (keypair, ledger, OS specific secure store)
This only supports seed phrases for now.

- `--public-key <PUBLIC_KEY>` — Add a public key, ed25519, or muxed account, e.g. G1.., M2..
- `--ledger` — Derive the address from a connected Ledger hardware wallet at `m/44'/148'/N'`, where `N` defaults to 0 and can be set with `--hd-path`. Persists the derived public key (and `--hd-path`, when provided) so later commands work without the device
- `--overwrite` — Overwrite existing identity if it already exists. When combined with --secure-store, also replaces the existing Secure Store entry
- `--hd-path <HD_PATH>` — When importing a seed phrase, which `hd_path` to derive the key at. Persisted on the identity so later commands derive the same account without re-passing the flag. Not valid with `--public-key` or a raw secret key

Expand Down
129 changes: 104 additions & 25 deletions cmd/soroban-cli/src/commands/keys/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ use crate::{
config::{
address::KeyName,
key, locator,
secret::{self, Secret},
secret::{self, HardwareKind, Secret},
},
print::Print,
signer::secure_store,
signer::{ledger, secure_store},
};

#[derive(thiserror::Error, Debug)]
Expand All @@ -28,6 +28,9 @@ pub enum Error {
#[error(transparent)]
SeedPhrase(#[from] sep5::error::Error),

#[error(transparent)]
Ledger(#[from] ledger::Error),

#[error("secret input error")]
PasswordRead,

Expand All @@ -42,6 +45,9 @@ pub enum Error {

#[error("--hd-path is not valid with a secret key; secret keys cannot be derived")]
HdPathNotSupportedForSecretKey,

#[error("--hd-path {0} is out of range for a Ledger account index")]
HdPathOutOfRange(usize),
}

#[derive(Debug, clap::Parser, Clone)]
Expand All @@ -61,10 +67,23 @@ pub struct Cmd {
long,
conflicts_with = "seed_phrase",
conflicts_with = "secret_key",
conflicts_with = "hd_path"
conflicts_with = "hd_path",
conflicts_with = "ledger"
)]
pub public_key: Option<String>,

/// Derive the address from a connected Ledger hardware wallet at
/// `m/44'/148'/N'`, where `N` defaults to 0 and can be set with
/// `--hd-path`. Persists the derived public key (and `--hd-path`,
/// when provided) so later commands work without the device.
#[arg(
long,
conflicts_with = "secret_key",
conflicts_with = "seed_phrase",
conflicts_with = "secure_store"
)]
pub ledger: bool,

/// Overwrite existing identity if it already exists. When combined with
/// --secure-store, also replaces the existing Secure Store entry.
#[arg(long)]
Expand All @@ -79,7 +98,7 @@ pub struct Cmd {
}

impl Cmd {
pub fn run(&self, global_args: &global::Args) -> Result<(), Error> {
pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> {
let print = Print::new(global_args.quiet);

if self.config_locator.read_identity(&self.name).is_ok() {
Expand All @@ -92,6 +111,8 @@ impl Cmd {

let key = if let Some(key) = self.public_key.as_ref() {
key::Key::parse_public_only(key)?
} else if self.ledger {
self.derive_ledger_secret().await?.into()
} else {
self.read_secret(&print)?.into()
};
Expand All @@ -103,6 +124,17 @@ impl Cmd {
Ok(())
}

async fn derive_ledger_secret(&self) -> Result<Secret, Error> {
let raw = self.hd_path.unwrap_or(0);
let index: u32 = raw.try_into().map_err(|_| Error::HdPathOutOfRange(raw))?;
let public_key = ledger::new(index).await?.public_key().await?;
Ok(Secret::Ledger {
hardware: HardwareKind::Ledger,
public_key: public_key.to_string(),
hd_path: self.hd_path,
})
}
Comment thread
fnando marked this conversation as resolved.

fn read_secret(&self, print: &Print) -> Result<Secret, Error> {
if self.secrets.secure_store {
if std::env::var("STELLAR_SECRET_KEY").is_ok() {
Expand Down Expand Up @@ -207,6 +239,7 @@ mod tests {
},
config_locator: locator.clone(),
public_key: None,
ledger: false,
overwrite: false,
hd_path: None,
};
Expand Down Expand Up @@ -286,61 +319,107 @@ mod tests {
}

#[test]
fn test_run_accepts_public_key_without_hd_path() {
let (_tmp, _locator, cmd) = cmd_with_public_key(PUBLIC_KEY, None);
assert!(cmd.run(&global_args()).is_ok());
fn test_clap_accepts_ledger_with_hd_path() {
use clap::Parser;

let cmd = Cmd::try_parse_from(["add", "test_name", "--ledger", "--hd-path", "5"])
.expect("--ledger + --hd-path must parse");
assert!(cmd.ledger);
assert_eq!(cmd.hd_path, Some(5));
}

#[test]
fn test_clap_rejects_ledger_with_public_key() {
use clap::Parser;

let err = Cmd::try_parse_from(["add", "test_name", "--ledger", "--public-key", PUBLIC_KEY])
.expect_err("clap must reject --ledger + --public-key");
assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict);
}

#[test]
fn test_clap_rejects_ledger_with_secret_key() {
use clap::Parser;

let err = Cmd::try_parse_from(["add", "test_name", "--ledger", "--secret-key"])
.expect_err("clap must reject --ledger + --secret-key");
assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict);
}

#[test]
fn test_clap_rejects_ledger_with_seed_phrase() {
use clap::Parser;

let err = Cmd::try_parse_from(["add", "test_name", "--ledger", "--seed-phrase"])
.expect_err("clap must reject --ledger + --seed-phrase");
assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict);
}

#[test]
fn public_key_flag_accepts_public_key() {
fn test_clap_rejects_ledger_with_secure_store() {
use clap::Parser;

let err = Cmd::try_parse_from(["add", "test_name", "--ledger", "--secure-store"])
.expect_err("clap must reject --ledger + --secure-store");
assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict);
}

#[tokio::test]
async fn test_run_accepts_public_key_without_hd_path() {
let (_tmp, _locator, cmd) = cmd_with_public_key(PUBLIC_KEY, None);
assert!(cmd.run(&global_args()).await.is_ok());
}

#[tokio::test]
async fn public_key_flag_accepts_public_key() {
let (_tmp, locator, mut cmd) = set_up_test();
cmd.public_key = Some(PUBLIC_KEY.to_string());
cmd.run(&global_args()).unwrap();
cmd.run(&global_args()).await.unwrap();
let stored = locator.read_identity("test_name").unwrap();
assert!(matches!(stored, Key::PublicKey(_)));
}

#[test]
fn public_key_flag_accepts_muxed_account() {
#[tokio::test]
async fn public_key_flag_accepts_muxed_account() {
let (_tmp, locator, mut cmd) = set_up_test();
cmd.public_key = Some(MUXED_ACCOUNT.to_string());
cmd.run(&global_args()).unwrap();
cmd.run(&global_args()).await.unwrap();
let stored = locator.read_identity("test_name").unwrap();
assert!(matches!(stored, Key::MuxedAccount(_)));
}

#[test]
fn public_key_flag_rejects_secret_key() {
#[tokio::test]
async fn public_key_flag_rejects_secret_key() {
let (_tmp, locator, mut cmd) = set_up_test();
cmd.public_key = Some(SECRET_KEY.to_string());
let err = cmd.run(&global_args()).unwrap_err();
let err = cmd.run(&global_args()).await.unwrap_err();
assert!(matches!(err, Error::Key(key_mod::Error::PublicKeyExpected)));
assert!(locator.read_identity("test_name").is_err());
}

#[test]
fn public_key_flag_rejects_seed_phrase() {
#[tokio::test]
async fn public_key_flag_rejects_seed_phrase() {
let (_tmp, locator, mut cmd) = set_up_test();
cmd.public_key = Some(SEED_PHRASE.to_string());
let err = cmd.run(&global_args()).unwrap_err();
let err = cmd.run(&global_args()).await.unwrap_err();
assert!(matches!(err, Error::Key(key_mod::Error::PublicKeyExpected)));
assert!(locator.read_identity("test_name").is_err());
}

#[test]
fn public_key_flag_rejects_ledger() {
#[tokio::test]
async fn public_key_flag_rejects_ledger() {
let (_tmp, locator, mut cmd) = set_up_test();
cmd.public_key = Some("ledger".to_string());
let err = cmd.run(&global_args()).unwrap_err();
assert!(matches!(err, Error::Key(key_mod::Error::PublicKeyExpected)));
let err = cmd.run(&global_args()).await.unwrap_err();
assert!(matches!(err, Error::Key(key_mod::Error::Parse)));
assert!(locator.read_identity("test_name").is_err());
}

#[test]
fn public_key_flag_rejects_secure_store() {
#[tokio::test]
async fn public_key_flag_rejects_secure_store() {
let (_tmp, locator, mut cmd) = set_up_test();
cmd.public_key = Some("secure_store:org.stellar.cli-alice".to_string());
let err = cmd.run(&global_args()).unwrap_err();
let err = cmd.run(&global_args()).await.unwrap_err();
assert!(matches!(err, Error::Key(key_mod::Error::PublicKeyExpected)));
assert!(locator.read_identity("test_name").is_err());
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/soroban-cli/src/commands/keys/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub enum Error {
impl Cmd {
pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> {
match self {
Cmd::Add(cmd) => cmd.run(global_args)?,
Cmd::Add(cmd) => cmd.run(global_args).await?,
Cmd::PublicKey(cmd) => cmd.run().await?,
Cmd::Fund(cmd) => cmd.run(global_args).await?,
Cmd::Generate(cmd) => cmd.run(global_args).await?,
Expand Down
17 changes: 0 additions & 17 deletions cmd/soroban-cli/src/commands/message/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,23 +312,6 @@ mod tests {
));
}

#[test]
fn test_verify_rejects_ledger_as_public_key() {
let cmd = super::Cmd {
message: Some("Hello, World!".to_string()),
base64: false,
signature: "fO5dbYhXUhBMhe6kId/cuVq/AfEnHRHEvsP8vXh03M1uLpi5e46yO2Q8rEBzu3feXQewcQE5GArp88u6ePK6BA==".to_string(),
public_key: "ledger".to_string(),
hd_path: None,
locator: setup_locator(),
};
let err = cmd.run(&global_args()).unwrap_err();
assert!(matches!(
err,
Error::Key(crate::config::key::Error::PublicKeyExpected)
));
}

#[test]
fn test_verify_rejects_secure_store_as_public_key() {
let cmd = super::Cmd {
Expand Down
5 changes: 0 additions & 5 deletions cmd/soroban-cli/src/config/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ pub enum Error {
InvalidKeyNameCharacters(String),
#[error("Invalid key name: {0}\n keys cannot exceed 250 characters")]
InvalidKeyNameLength(String),
#[error("Invalid key name: {0}\n keys cannot be the word \"ledger\"")]
InvalidKeyName(String),
#[error(transparent)]
Name(#[from] utils::Error),
}
Expand Down Expand Up @@ -107,9 +105,6 @@ impl std::str::FromStr for KeyName {
utils::Error::InvalidNameLength(s) => Error::InvalidKeyNameLength(s),
utils::Error::InvalidNameCharacters(s) => Error::InvalidKeyNameCharacters(s),
})?;
if s == "ledger" {
return Err(Error::InvalidKeyName(s.to_string()));
}
Ok(KeyName(s.to_string()))
}
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/soroban-cli/src/config/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ mod test {
#[test]
fn parse_public_only_rejects_ledger() {
let err = Key::parse_public_only("ledger").unwrap_err();
assert!(matches!(err, Error::PublicKeyExpected));
assert!(matches!(err, Error::Parse));
}

#[test]
Expand Down
Loading
Loading