Skip to content

Commit 24c7ccd

Browse files
committed
Persist --hd-path on plain seed-phrase keys.
1 parent 343069b commit 24c7ccd

7 files changed

Lines changed: 218 additions & 12 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/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: 114 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
@@ -157,3 +181,89 @@ fn read_password(print: &Print, prompt: &str) -> Result<String, Error> {
157181
Ok(input)
158182
}
159183
}
184+
185+
#[cfg(test)]
186+
mod tests {
187+
use super::*;
188+
use crate::config::address::KeyName;
189+
190+
const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ";
191+
const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH";
192+
const TEST_SEED_PHRASE: &str =
193+
"depth decade power loud smile spatial sign movie judge february rate broccoli";
194+
195+
#[test]
196+
fn test_build_secret_persists_hd_path_on_seed_phrase() {
197+
let secret = build_secret(TEST_SEED_PHRASE, Some(5)).unwrap();
198+
match secret {
199+
Secret::SeedPhrase {
200+
seed_phrase,
201+
hd_path,
202+
} => {
203+
assert_eq!(seed_phrase, TEST_SEED_PHRASE);
204+
assert_eq!(hd_path, Some(5));
205+
}
206+
other => panic!("expected SeedPhrase variant, got {other:?}"),
207+
}
208+
}
209+
210+
#[test]
211+
fn test_build_secret_seed_phrase_without_hd_path() {
212+
let secret = build_secret(TEST_SEED_PHRASE, None).unwrap();
213+
match secret {
214+
Secret::SeedPhrase { hd_path, .. } => assert_eq!(hd_path, None),
215+
other => panic!("expected SeedPhrase variant, got {other:?}"),
216+
}
217+
}
218+
219+
#[test]
220+
fn test_build_secret_rejects_hd_path_with_secret_key() {
221+
let result = build_secret(TEST_SECRET_KEY, Some(5));
222+
assert!(matches!(result, Err(Error::HdPathNotSupportedForSecretKey)));
223+
}
224+
225+
#[test]
226+
fn test_build_secret_secret_key_without_hd_path() {
227+
let secret = build_secret(TEST_SECRET_KEY, None).unwrap();
228+
assert!(matches!(secret, Secret::SecretKey { .. }));
229+
}
230+
231+
fn cmd_with_public_key(public_key: &str, hd_path: Option<usize>) -> Cmd {
232+
let temp_dir = tempfile::tempdir().unwrap();
233+
Cmd {
234+
name: KeyName("test_name".to_string()),
235+
secrets: secret::Args {
236+
secret_key: false,
237+
seed_phrase: false,
238+
secure_store: false,
239+
},
240+
config_locator: locator::Args {
241+
config_dir: Some(temp_dir.path().to_path_buf()),
242+
},
243+
public_key: Some(public_key.to_string()),
244+
overwrite: false,
245+
hd_path,
246+
}
247+
}
248+
249+
#[test]
250+
fn test_run_rejects_hd_path_with_public_key() {
251+
let cmd = cmd_with_public_key(TEST_PUBLIC_KEY, Some(3));
252+
let global_args = global::Args {
253+
quiet: true,
254+
..Default::default()
255+
};
256+
let result = cmd.run(&global_args);
257+
assert!(matches!(result, Err(Error::HdPathNotSupportedForPublicKey)));
258+
}
259+
260+
#[test]
261+
fn test_run_accepts_public_key_without_hd_path() {
262+
let cmd = cmd_with_public_key(TEST_PUBLIC_KEY, None);
263+
let global_args = global::Args {
264+
quiet: true,
265+
..Default::default()
266+
};
267+
assert!(cmd.run(&global_args).is_ok());
268+
}
269+
}

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

Lines changed: 25 additions & 2 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

@@ -179,6 +185,23 @@ 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) = 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() {
184207
let (test_locator, mut cmd) = set_up_test();

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
@@ -176,7 +176,10 @@ mod test {
176176
#[test]
177177
fn secret_seed_phrase() {
178178
let seed_phrase = "singer swing mango apple singer swing mango apple singer swing mango apple singer swing mango apple".to_string();
179-
let secret = Secret::SeedPhrase { seed_phrase };
179+
let secret = Secret::SeedPhrase {
180+
seed_phrase,
181+
hd_path: None,
182+
};
180183
let key = Key::Secret(secret);
181184
round_trip(&key);
182185
}

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

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ pub enum Secret {
6262
},
6363
SeedPhrase {
6464
seed_phrase: String,
65+
// Persisted derivation index. Lets `--hd-path` set on `keys generate` /
66+
// `keys add` travel with the identity, so later commands derive the
67+
// intended account without re-passing the flag. Optional for backwards
68+
// compatibility with files written before this field existed.
69+
#[serde(default, skip_serializing_if = "Option::is_none")]
70+
hd_path: Option<usize>,
6571
},
6672
Ledger,
6773
SecureStore {
@@ -87,6 +93,7 @@ impl FromStr for Secret {
8793
} else if sep5::SeedPhrase::from_str(s).is_ok() {
8894
Ok(Secret::SeedPhrase {
8995
seed_phrase: s.to_string(),
96+
hd_path: None,
9097
})
9198
} else if s == "ledger" {
9299
Ok(Secret::Ledger)
@@ -120,6 +127,7 @@ impl From<SeedPhrase> for Secret {
120127
fn from(value: SeedPhrase) -> Self {
121128
Secret::SeedPhrase {
122129
seed_phrase: value.seed_phrase.into_phrase(),
130+
hd_path: None,
123131
}
124132
}
125133
}
@@ -128,9 +136,12 @@ impl Secret {
128136
pub fn private_key(&self, index: Option<usize>) -> Result<PrivateKey, Error> {
129137
Ok(match self {
130138
Secret::SecretKey { secret_key } => PrivateKey::from_string(secret_key)?,
131-
Secret::SeedPhrase { seed_phrase } => PrivateKey::from_payload(
139+
Secret::SeedPhrase {
140+
seed_phrase,
141+
hd_path,
142+
} => PrivateKey::from_payload(
132143
&sep5::SeedPhrase::from_str(seed_phrase)?
133-
.from_path_index(index.unwrap_or_default(), None)?
144+
.from_path_index(index.or(*hd_path).unwrap_or_default(), None)?
134145
.private()
135146
.0,
136147
)?,
@@ -347,4 +358,62 @@ mod tests {
347358
assert!(cached_public_key(Some("not-a-public-key"), None, None).is_none());
348359
assert!(cached_public_key(Some(""), None, None).is_none());
349360
}
361+
362+
#[test]
363+
fn test_seed_phrase_toml_round_trip_with_hd_path() {
364+
let secret = Secret::SeedPhrase {
365+
seed_phrase: TEST_SEED_PHRASE.to_string(),
366+
hd_path: Some(5),
367+
};
368+
let serialized = toml::to_string(&secret).unwrap();
369+
assert!(
370+
serialized.contains("hd_path"),
371+
"expected hd_path field in TOML, got: {serialized}"
372+
);
373+
let parsed: Secret = toml::from_str(&serialized).unwrap();
374+
assert_eq!(secret, parsed);
375+
}
376+
377+
#[test]
378+
fn test_seed_phrase_legacy_toml_parses_with_none_hd_path() {
379+
// Identity files written before this feature only contain seed_phrase.
380+
// They must still parse, with hd_path defaulting to None.
381+
let toml_str = format!("seed_phrase = \"{TEST_SEED_PHRASE}\"\n");
382+
let secret: Secret = toml::from_str(&toml_str).unwrap();
383+
match secret {
384+
Secret::SeedPhrase {
385+
seed_phrase,
386+
hd_path,
387+
} => {
388+
assert_eq!(seed_phrase, TEST_SEED_PHRASE);
389+
assert_eq!(hd_path, None);
390+
}
391+
other => panic!("expected SeedPhrase variant, got {other:?}"),
392+
}
393+
}
394+
395+
#[test]
396+
fn test_seed_phrase_uses_persisted_hd_path_when_caller_passes_none() {
397+
// When the caller passes None, the persisted hd_path should drive derivation.
398+
let secret = Secret::SeedPhrase {
399+
seed_phrase: TEST_SEED_PHRASE.to_string(),
400+
hd_path: Some(1),
401+
};
402+
let pk_at_0 = secret.public_key(Some(0)).unwrap();
403+
let pk_default = secret.public_key(None).unwrap();
404+
assert_ne!(pk_at_0.to_string(), pk_default.to_string());
405+
}
406+
407+
#[test]
408+
fn test_seed_phrase_caller_hd_path_overrides_persisted() {
409+
// Caller's explicit hd_path argument always wins over the persisted value.
410+
let secret = Secret::SeedPhrase {
411+
seed_phrase: TEST_SEED_PHRASE.to_string(),
412+
hd_path: Some(1),
413+
};
414+
let pk = secret.public_key(Some(0)).unwrap();
415+
let sk = secret.private_key(Some(0)).unwrap();
416+
assert_eq!(pk.to_string(), TEST_PUBLIC_KEY);
417+
assert_eq!(sk.to_string(), TEST_SECRET_KEY);
418+
}
350419
}

0 commit comments

Comments
 (0)