clarify bip32 derivation paths (#800)

Co-authored-by: ardocrat <ardocrat@gri.mw>

controller/src/display.rs

@@ -16,11 +16,12 @@ use crate::core::core::FeeFields;
 use crate::core::core::{self, amount_to_hr_string};
 use crate::core::global;
 use crate::libwallet::{
-	AcctPathMapping, Error, OutputCommitMapping, OutputStatus, SlatepackAddress, TxLogEntry,
-	ViewWallet, WalletInfo,
+	address, AcctPathMapping, Error, OutputCommitMapping, OutputStatus, SlatepackAddress,
+	TxLogEntry, ViewWallet, WalletInfo,
 };
 use crate::util::ToHex;
 use prettytable;
+use prettytable::format::{FormatBuilder, LinePosition, LineSeparator};
 use std::io::prelude::Write;
 use term;
 
@@ -537,15 +538,29 @@ pub fn accounts(acct_mappings: Vec<AcctPathMapping>) {
 
 	table.set_titles(row![
 		mMG->"Name",
-		bMG->"Parent BIP-32 Derivation Path",
+		bMG->"Parent Output Key",
+		bMG->"Slatepack Address (Index 0)",
 	]);
 	for m in acct_mappings {
+		let slatepack_path = address::address_derivation_path(&m.path, 0).to_bip_32_string();
 		table.add_row(row![
 			bFC->m.label,
 			bGC->m.path.to_bip_32_string(),
+			bGC->slatepack_path,
 		]);
 	}
-	table.set_format(*prettytable::format::consts::FORMAT_NO_BORDER_LINE_SEPARATOR);
+	table.set_format(
+		FormatBuilder::new()
+			.column_separator('|')
+			.separators(
+				&[LinePosition::Top, LinePosition::Title],
+				LineSeparator::new('-', '+', '+', '+'),
+			)
+			.padding(1, 1)
+			.build(),
+	);
+	let width = table.to_string().find('\n').unwrap_or(0);
+	println!("{:^1$}", "BIP-32 Derivation Path", width);
 	table.printstd();
 	println!();
 }

controller/tests/accounts.rs

@@ -175,6 +175,8 @@ fn accounts_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
 		mask1,
 		PathBuf::from(test_dir),
 		|api, m| {
+			let labels: Vec<_> = api.accounts(m)?.into_iter().map(|a| a.label).collect();
+			assert_eq!(labels, ["account1", "default", "account2", "account3"]);
 			// check last confirmed height on this account is different from above (should be 0)
 			let (_, wallet1_info) = api.retrieve_summary_info(m, false, 1)?;
 			assert_eq!(wallet1_info.last_confirmed_height, 0);

libwallet/src/address.rs

@@ -21,6 +21,23 @@ use grin_keychain::{ChildNumber, Identifier, Keychain, SwitchCommitmentType};
 
 use crate::blake2::blake2b::blake2b;
 
+/// Derive a Slatepack address path from an output parent path and index.
+///
+/// Output paths:
+/// - account m/0: m/0/0/0, m/0/0/1
+/// - account m/1: m/1/0/0, m/1/0/1
+///
+/// Slatepack address paths:
+/// - account m/0: m/0/1/0, m/0/1/1
+/// - account m/1: m/1/1/0, m/1/1/1
+pub fn address_derivation_path(parent_key_id: &Identifier, index: u32) -> Identifier {
+	let mut key_path = parent_key_id.to_path();
+	key_path.path[1] = ChildNumber::from(1);
+	key_path.depth += 1;
+	key_path.path[key_path.depth as usize - 1] = ChildNumber::from(index);
+	Identifier::from_path(&key_path)
+}
+
 /// Derive a secret key given a derivation path and index
 pub fn address_from_derivation_path<K>(
 	keychain: &K,
@@ -30,16 +47,7 @@ pub fn address_from_derivation_path<K>(
 where
 	K: Keychain,
 {
-	let mut key_path = parent_key_id.to_path();
-	// An output derivation for acct m/0
-	// is m/0/0/0, m/0/0/1 (for instance), m/1 is m/1/0/0, m/1/0/1
-	// Address generation path should be
-	// for m/0: m/0/1/0, m/0/1/1
-	// for m/1: m/1/1/0, m/1/1/1
-	key_path.path[1] = ChildNumber::from(1);
-	key_path.depth += 1;
-	key_path.path[key_path.depth as usize - 1] = ChildNumber::from(index);
-	let key_id = Identifier::from_path(&key_path);
+	let key_id = address_derivation_path(parent_key_id, index);
 	let sec_key = keychain.derive_key(0, &key_id, SwitchCommitmentType::None)?;
 	let hashed = blake2b(32, &[], &sec_key.0[..]);
 	Ok(SecretKey::from_slice(
@@ -47,3 +55,16 @@ where
 		&hashed.as_bytes()[..],
 	)?)
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use grin_keychain::ExtKeychain;
+
+	#[test]
+	fn slatepack_address_path() {
+		let parent = ExtKeychain::derive_key_id(2, 2, 0, 0, 0);
+		let expected = ExtKeychain::derive_key_id(3, 2, 1, 3, 0);
+		assert_eq!(address_derivation_path(&parent, 3), expected);
+	}
+}

libwallet/src/api_impl/owner.rs

@@ -57,7 +57,11 @@ where
 	C: NodeClient,
 	K: Keychain,
 {
-	keys::accounts(w)
+	let mut accounts = keys::accounts(w)?;
+	accounts.sort_by(|a, b| a.path.cmp(&b.path));
+	// Put active account on top.
+	accounts.sort_by_key(|k| k.path != w.parent_key_id());
+	Ok(accounts)
 }
 
 /// new account path