Skip to content

Commit b66d0fa

Browse files
committed
Persist --hd-path on plain seed-phrase and Secure Store keys.
Plain seed-phrase identities now persist --hd-path and honor it as a default for later derivations. Secure Store identities did the same on disk but ignored the persisted hd_path at derivation time, falling back to index 0 unless the user re-passed --hd-path; Secret::SecureStore now also falls back to the persisted value when the caller passes None, matching the behavior of Secret::SeedPhrase. Caller-supplied indices still take precedence.
1 parent 2af8896 commit b66d0fa

9 files changed

Lines changed: 292 additions & 24 deletions

File tree

FULL_HELP_DOCS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,7 +1138,7 @@ Add a new identity (keypair, ledger, OS specific secure store)
11381138

11391139
- `--public-key <PUBLIC_KEY>` — Add a public key, ed25519, or muxed account, e.g. G1.., M2..
11401140
- `--overwrite` — Overwrite existing identity if it already exists. When combined with --secure-store, also replaces the existing Secure Store entry
1141-
- `--hd-path <HD_PATH>` — When importing a seed phrase into the Secure Store, which `hd_path` to derive the key at
1141+
- `--hd-path <HD_PATH>` — When importing a seed phrase, which `hd_path` to derive the key at. Persisted on the identity (or its Secure Store entry) so later commands derive the same account without re-passing the flag. Not valid with `--public-key` or a raw secret key
11421142

11431143
###### **Options (Global):**
11441144

@@ -1207,7 +1207,7 @@ Generate a new identity using a 24-word seed phrase The seed phrase can be store
12071207

12081208
On Mac this uses Keychain, on Windows it is Secure Store Service, and on \*nix platforms it uses a combination of the kernel keyutils and DBus-based Secret Service.
12091209

1210-
- `--hd-path <HD_PATH>`With `--as-secret` or `--secure-store`, which `hd_path` to derive the key at from the seed phrase
1210+
- `--hd-path <HD_PATH>`Which `hd_path` to derive the key at from the seed phrase. Honored across all storage modes: with `--as-secret` it picks which derived key is stored, with `--secure-store` or plain seed-phrase storage it is persisted on the identity so later commands derive the same account without re-passing the flag
12111211
- `--fund` — Fund generated key pair
12121212

12131213
Default value: `false`

cmd/crates/soroban-test/tests/it/integration/hello_world.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ async fn invoke_contract() {
7171
let dir = sandbox.config_dir();
7272
let seed_phrase = std::fs::read_to_string(dir.join("identity/test.toml")).unwrap();
7373
let s = toml::from_str::<secret::Secret>(&seed_phrase).unwrap();
74-
let secret::Secret::SeedPhrase { seed_phrase } = s else {
74+
let secret::Secret::SeedPhrase { seed_phrase, .. } = s else {
7575
panic!("Expected seed phrase")
7676
};
7777

cmd/crates/soroban-test/tests/it/integration/keys.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,79 @@ async fn rm_with_force_skips_confirmation() {
351351
.failure();
352352
}
353353

354+
// `keys generate --hd-path N` (plain seed-phrase storage) must persist N so
355+
// that later `keys address` calls without `--hd-path` derive at index N rather
356+
// than the default. Guards the user-visible contract from #2538 across CLI
357+
// parsing, identity-file I/O, and key derivation.
358+
#[tokio::test]
359+
async fn hd_path_persists_for_keys_generate() {
360+
let sandbox = &TestEnv::new();
361+
sandbox
362+
.new_assert_cmd("keys")
363+
.args(["generate", "hd-gen", "--hd-path", "5"])
364+
.assert()
365+
.success();
366+
367+
let address_default = pubkey_for_identity(sandbox, "hd-gen");
368+
let address_explicit = sandbox
369+
.new_assert_cmd("keys")
370+
.args(["address", "hd-gen", "--hd-path", "5"])
371+
.assert()
372+
.success()
373+
.stdout_as_str();
374+
let address_zero = sandbox
375+
.new_assert_cmd("keys")
376+
.args(["address", "hd-gen", "--hd-path", "0"])
377+
.assert()
378+
.success()
379+
.stdout_as_str();
380+
381+
assert_eq!(
382+
address_default, address_explicit,
383+
"expected `keys address hd-gen` (no flag) to derive at the persisted hd_path 5"
384+
);
385+
assert_ne!(
386+
address_default, address_zero,
387+
"expected hd_path 5 derivation to differ from hd_path 0"
388+
);
389+
}
390+
391+
#[tokio::test]
392+
async fn hd_path_persists_for_keys_add_seed_phrase() {
393+
let sandbox = &TestEnv::new();
394+
let seed_phrase = "aisle reflect depart add safe fury dress artist bronze abuse warrior clap inquiry ask mandate deputy view trade debate flip priority boy depart recipe";
395+
396+
sandbox
397+
.new_assert_cmd("keys")
398+
.write_stdin(format!("{seed_phrase}\n"))
399+
.args(["add", "hd-add", "--hd-path", "5"])
400+
.assert()
401+
.success();
402+
403+
let address_default = pubkey_for_identity(sandbox, "hd-add");
404+
let address_explicit = sandbox
405+
.new_assert_cmd("keys")
406+
.args(["address", "hd-add", "--hd-path", "5"])
407+
.assert()
408+
.success()
409+
.stdout_as_str();
410+
let address_zero = sandbox
411+
.new_assert_cmd("keys")
412+
.args(["address", "hd-add", "--hd-path", "0"])
413+
.assert()
414+
.success()
415+
.stdout_as_str();
416+
417+
assert_eq!(
418+
address_default, address_explicit,
419+
"expected `keys address hd-add` (no flag) to derive at the persisted hd_path 5"
420+
);
421+
assert_ne!(
422+
address_default, address_zero,
423+
"expected hd_path 5 derivation to differ from hd_path 0"
424+
);
425+
}
426+
354427
#[tokio::test]
355428
async fn rm_nonexistent_key() {
356429
let sandbox = &TestEnv::new();

cmd/crates/soroban-test/tests/it/util.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub fn add_key(dir: &Path, name: &str, kind: SecretKind, data: &str) {
1818
let secret = match kind {
1919
SecretKind::Seed => Secret::SeedPhrase {
2020
seed_phrase: data.to_string(),
21+
hd_path: None,
2122
},
2223
SecretKind::Key => Secret::SecretKey {
2324
secret_key: data.to_string(),

cmd/soroban-cli/src/commands/keys/add.rs

Lines changed: 87 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ pub enum Error {
3939
unset STELLAR_SECRET_KEY or provide a seed phrase instead"
4040
)]
4141
SecureStoreRequiresSeedPhrase,
42+
43+
#[error("--hd-path is not valid with a secret key; secret keys cannot be derived")]
44+
HdPathNotSupportedForSecretKey,
45+
46+
#[error("--hd-path is not valid with --public-key; public keys cannot be derived")]
47+
HdPathNotSupportedForPublicKey,
4248
}
4349

4450
#[derive(Debug, clap::Parser, Clone)]
@@ -62,7 +68,9 @@ pub struct Cmd {
6268
#[arg(long)]
6369
pub overwrite: bool,
6470

65-
/// When importing a seed phrase into the Secure Store, which `hd_path` to derive the key at.
71+
/// When importing a seed phrase, which `hd_path` to derive the key at. Persisted on
72+
/// the identity (or its Secure Store entry) so later commands derive the same account
73+
/// without re-passing the flag. Not valid with `--public-key` or a raw secret key.
6674
#[arg(long)]
6775
pub hd_path: Option<usize>,
6876
}
@@ -71,6 +79,10 @@ impl Cmd {
7179
pub fn run(&self, global_args: &global::Args) -> Result<(), Error> {
7280
let print = Print::new(global_args.quiet);
7381

82+
if self.public_key.is_some() && self.hd_path.is_some() {
83+
return Err(Error::HdPathNotSupportedForPublicKey);
84+
}
85+
7486
if self.config_locator.read_identity(&self.name).is_ok() {
7587
if !self.overwrite {
7688
return Err(Error::IdentityAlreadyExists(self.name.to_string()));
@@ -98,7 +110,7 @@ impl Cmd {
98110
return Err(Error::SecureStoreRequiresSeedPhrase);
99111
}
100112
} else if let Ok(secret_key) = std::env::var("STELLAR_SECRET_KEY") {
101-
return Ok(secret_key.parse()?);
113+
return build_secret(&secret_key, self.hd_path);
102114
}
103115

104116
if self.secrets.secure_store {
@@ -123,8 +135,8 @@ impl Cmd {
123135
} else {
124136
let prompt = "Type a secret key or 12/24 word seed phrase:";
125137
let secret_key = read_password(print, prompt)?;
126-
let secret = secret_key.parse()?;
127-
if let Secret::SeedPhrase { seed_phrase } = &secret {
138+
let secret = build_secret(&secret_key, self.hd_path)?;
139+
if let Secret::SeedPhrase { seed_phrase, .. } = &secret {
128140
if seed_phrase.split_whitespace().count() < 24 {
129141
print.warnln("The provided seed phrase lacks sufficient entropy and should be avoided. Using a 24-word seed phrase is a safer option.".to_string());
130142
print.warnln(
@@ -138,6 +150,18 @@ impl Cmd {
138150
}
139151
}
140152

153+
fn build_secret(input: &str, hd_path: Option<usize>) -> Result<Secret, Error> {
154+
let secret: Secret = input.parse()?;
155+
match (secret, hd_path) {
156+
(Secret::SecretKey { .. }, Some(_)) => Err(Error::HdPathNotSupportedForSecretKey),
157+
(Secret::SeedPhrase { seed_phrase, .. }, hd_path) => Ok(Secret::SeedPhrase {
158+
seed_phrase,
159+
hd_path,
160+
}),
161+
(secret, _) => Ok(secret),
162+
}
163+
}
164+
141165
fn read_password(print: &Print, prompt: &str) -> Result<String, Error> {
142166
if std::io::stdin().is_terminal() {
143167
// Interactive: prompt and read from TTY
@@ -190,13 +214,72 @@ mod tests {
190214
(temp_dir, locator, cmd)
191215
}
192216

217+
fn cmd_with_public_key(
218+
public_key: &str,
219+
hd_path: Option<usize>,
220+
) -> (tempfile::TempDir, locator::Args, Cmd) {
221+
let (temp_dir, locator, mut cmd) = set_up_test();
222+
cmd.public_key = Some(public_key.to_string());
223+
cmd.hd_path = hd_path;
224+
(temp_dir, locator, cmd)
225+
}
226+
193227
fn global_args() -> global::Args {
194228
global::Args {
195229
quiet: true,
196230
..Default::default()
197231
}
198232
}
199233

234+
#[test]
235+
fn test_build_secret_persists_hd_path_on_seed_phrase() {
236+
let secret = build_secret(SEED_PHRASE, Some(5)).unwrap();
237+
match secret {
238+
Secret::SeedPhrase {
239+
seed_phrase,
240+
hd_path,
241+
} => {
242+
assert_eq!(seed_phrase, SEED_PHRASE);
243+
assert_eq!(hd_path, Some(5));
244+
}
245+
other => panic!("expected SeedPhrase variant, got {other:?}"),
246+
}
247+
}
248+
249+
#[test]
250+
fn test_build_secret_seed_phrase_without_hd_path() {
251+
let secret = build_secret(SEED_PHRASE, None).unwrap();
252+
match secret {
253+
Secret::SeedPhrase { hd_path, .. } => assert_eq!(hd_path, None),
254+
other => panic!("expected SeedPhrase variant, got {other:?}"),
255+
}
256+
}
257+
258+
#[test]
259+
fn test_build_secret_rejects_hd_path_with_secret_key() {
260+
let result = build_secret(SECRET_KEY, Some(5));
261+
assert!(matches!(result, Err(Error::HdPathNotSupportedForSecretKey)));
262+
}
263+
264+
#[test]
265+
fn test_build_secret_secret_key_without_hd_path() {
266+
let secret = build_secret(SECRET_KEY, None).unwrap();
267+
assert!(matches!(secret, Secret::SecretKey { .. }));
268+
}
269+
270+
#[test]
271+
fn test_run_rejects_hd_path_with_public_key() {
272+
let (_tmp, _locator, cmd) = cmd_with_public_key(PUBLIC_KEY, Some(3));
273+
let result = cmd.run(&global_args());
274+
assert!(matches!(result, Err(Error::HdPathNotSupportedForPublicKey)));
275+
}
276+
277+
#[test]
278+
fn test_run_accepts_public_key_without_hd_path() {
279+
let (_tmp, _locator, cmd) = cmd_with_public_key(PUBLIC_KEY, None);
280+
assert!(cmd.run(&global_args()).is_ok());
281+
}
282+
200283
#[test]
201284
fn public_key_flag_accepts_public_key() {
202285
let (_tmp, locator, mut cmd) = set_up_test();

cmd/soroban-cli/src/commands/keys/generate.rs

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ pub struct Cmd {
5050
#[command(flatten)]
5151
pub config_locator: locator::Args,
5252

53-
/// With `--as-secret` or `--secure-store`, which `hd_path` to derive the key at from the seed phrase.
53+
/// Which `hd_path` to derive the key at from the seed phrase. Honored across all
54+
/// storage modes: with `--as-secret` it picks which derived key is stored, with
55+
/// `--secure-store` or plain seed-phrase storage it is persisted on the identity
56+
/// so later commands derive the same account without re-passing the flag.
5457
#[arg(long)]
5558
pub hd_path: Option<usize>,
5659

@@ -127,7 +130,10 @@ impl Cmd {
127130
let secret: Secret = seed_phrase.into();
128131
Ok(secret.private_key(self.hd_path)?.into())
129132
} else {
130-
Ok(seed_phrase.into())
133+
Ok(Secret::SeedPhrase {
134+
seed_phrase: seed_phrase.seed_phrase.into_phrase(),
135+
hd_path: self.hd_path,
136+
})
131137
}
132138
}
133139

@@ -140,7 +146,7 @@ impl Cmd {
140146
mod tests {
141147
use crate::config::{address::KeyName, key::Key, secret::Secret};
142148

143-
fn set_up_test() -> (super::locator::Args, super::Cmd) {
149+
fn set_up_test() -> (super::locator::Args, super::Cmd, tempfile::TempDir) {
144150
let temp_dir = tempfile::tempdir().unwrap();
145151
let locator = super::locator::Args {
146152
config_dir: Some(temp_dir.path().to_path_buf()),
@@ -158,7 +164,7 @@ mod tests {
158164
overwrite: false,
159165
};
160166

161-
(locator, cmd)
167+
(locator, cmd, temp_dir)
162168
}
163169

164170
fn global_args() -> super::global::Args {
@@ -170,7 +176,7 @@ mod tests {
170176

171177
#[tokio::test]
172178
async fn test_storing_secret_as_a_seed_phrase() {
173-
let (test_locator, cmd) = set_up_test();
179+
let (test_locator, cmd, _temp_dir) = set_up_test();
174180
let global_args = global_args();
175181

176182
let result = cmd.run(&global_args).await;
@@ -179,9 +185,26 @@ mod tests {
179185
assert!(matches!(identity, Key::Secret(Secret::SeedPhrase { .. })));
180186
}
181187

188+
#[tokio::test]
189+
async fn test_generate_seed_phrase_persists_hd_path() {
190+
let (test_locator, mut cmd, _temp_dir) = set_up_test();
191+
cmd.hd_path = Some(7);
192+
let global_args = global_args();
193+
194+
cmd.run(&global_args).await.unwrap();
195+
196+
let identity = test_locator.read_identity("test_name").unwrap();
197+
match identity {
198+
Key::Secret(Secret::SeedPhrase { hd_path, .. }) => {
199+
assert_eq!(hd_path, Some(7));
200+
}
201+
other => panic!("expected SeedPhrase variant, got {other:?}"),
202+
}
203+
}
204+
182205
#[tokio::test]
183206
async fn test_storing_secret_as_a_secret_key() {
184-
let (test_locator, mut cmd) = set_up_test();
207+
let (test_locator, mut cmd, _temp_dir) = set_up_test();
185208
cmd.as_secret = true;
186209
let global_args = global_args();
187210

@@ -196,7 +219,7 @@ mod tests {
196219
async fn test_storing_secret_in_secure_store() {
197220
use keyring::{mock, set_default_credential_builder};
198221
set_default_credential_builder(mock::default_credential_builder());
199-
let (test_locator, mut cmd) = set_up_test();
222+
let (test_locator, mut cmd, _temp_dir) = set_up_test();
200223
cmd.secure_store = true;
201224
let global_args = global_args();
202225

@@ -211,7 +234,7 @@ mod tests {
211234
async fn test_generate_secure_store_caches_public_key_on_disk() {
212235
use keyring::{mock, set_default_credential_builder};
213236
set_default_credential_builder(mock::default_credential_builder());
214-
let (test_locator, mut cmd) = set_up_test();
237+
let (test_locator, mut cmd, _temp_dir) = set_up_test();
215238
cmd.secure_store = true;
216239
let global_args = global_args();
217240

@@ -237,7 +260,7 @@ mod tests {
237260
#[cfg(not(feature = "additional-libs"))]
238261
#[tokio::test]
239262
async fn test_storing_in_secure_store_returns_error_when_additional_libs_not_enabled() {
240-
let (test_locator, mut cmd) = set_up_test();
263+
let (test_locator, mut cmd, _temp_dir) = set_up_test();
241264
cmd.secure_store = true;
242265
let global_args = global_args();
243266

cmd/soroban-cli/src/commands/keys/secret.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ impl Cmd {
4949
pub fn seed_phrase(&self) -> Result<String, Error> {
5050
let key = self.locator.read_identity(&self.name)?;
5151

52-
if let Key::Secret(Secret::SeedPhrase { seed_phrase }) = key {
52+
if let Key::Secret(Secret::SeedPhrase { seed_phrase, .. }) = key {
5353
Ok(seed_phrase)
5454
} else {
5555
Err(Error::UnknownSeedPhrase)

cmd/soroban-cli/src/config/key.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,10 @@ mod test {
185185
#[test]
186186
fn secret_seed_phrase() {
187187
let seed_phrase = "singer swing mango apple singer swing mango apple singer swing mango apple singer swing mango apple".to_string();
188-
let secret = Secret::SeedPhrase { seed_phrase };
188+
let secret = Secret::SeedPhrase {
189+
seed_phrase,
190+
hd_path: None,
191+
};
189192
let key = Key::Secret(secret);
190193
round_trip(&key);
191194
}

0 commit comments

Comments
 (0)