From a42144cb3c3ad0353d313fb0734ff33cbf7e2f76 Mon Sep 17 00:00:00 2001 From: Sergio Chouhy Date: Fri, 17 Apr 2026 19:45:30 -0300 Subject: [PATCH] minor refactor. update ffi --- integration_tests/tests/wallet_ffi.rs | 151 +++++++----------- .../src/key_management/key_tree/mod.rs | 4 +- key_protocol/src/key_protocol_core/mod.rs | 10 +- wallet-ffi/src/account.rs | 38 +++-- wallet-ffi/wallet_ffi.h | 52 +++--- wallet/src/cli/account.rs | 8 +- wallet/src/lib.rs | 4 +- 7 files changed, 127 insertions(+), 140 deletions(-) diff --git a/integration_tests/tests/wallet_ffi.rs b/integration_tests/tests/wallet_ffi.rs index 00c68085..50965883 100644 --- a/integration_tests/tests/wallet_ffi.rs +++ b/integration_tests/tests/wallet_ffi.rs @@ -48,11 +48,13 @@ unsafe extern "C" { out_account_id: *mut FfiBytes32, ) -> error::WalletFfiError; - fn wallet_ffi_create_account_private( + fn wallet_ffi_create_private_accounts_key( handle: *mut WalletHandle, - out_account_id: *mut FfiBytes32, + out_keys: *mut FfiPrivateAccountKeys, ) -> error::WalletFfiError; + fn wallet_ffi_free_private_account_keys(keys: *mut FfiPrivateAccountKeys); + fn wallet_ffi_list_accounts( handle: *mut WalletHandle, out_list: *mut FfiAccountList, @@ -262,33 +264,32 @@ fn wallet_ffi_create_public_accounts() -> Result<()> { fn wallet_ffi_create_private_accounts() -> Result<()> { let password = "password_for_tests"; let n_accounts = 10; - // Create `n_accounts` private accounts with wallet FFI - let new_private_account_ids_ffi = unsafe { - let mut account_ids = Vec::new(); + // Create `n_accounts` receiving keys with wallet FFI + let new_npks_ffi = unsafe { + let mut npks = Vec::new(); let wallet_ffi_handle = new_wallet_ffi_with_default_config(password)?; for _ in 0..n_accounts { - let mut out_account_id = FfiBytes32::from_bytes([0; 32]); - wallet_ffi_create_account_private(wallet_ffi_handle, &raw mut out_account_id); - account_ids.push(out_account_id.data); + let mut out_keys = FfiPrivateAccountKeys::default(); + wallet_ffi_create_private_accounts_key(wallet_ffi_handle, &raw mut out_keys); + npks.push(out_keys.nullifier_public_key.data); + wallet_ffi_free_private_account_keys(&raw mut out_keys); } wallet_ffi_destroy(wallet_ffi_handle); - account_ids + npks }; - // All returned IDs must be unique and non-zero - assert_eq!(new_private_account_ids_ffi.len(), n_accounts); - let unique: HashSet<_> = new_private_account_ids_ffi.iter().collect(); + // All returned NPKs must be unique and non-zero + assert_eq!(new_npks_ffi.len(), n_accounts); + let unique: HashSet<_> = new_npks_ffi.iter().collect(); assert_eq!( unique.len(), n_accounts, - "Duplicate private account IDs returned" + "Duplicate NPKs returned" ); assert!( - new_private_account_ids_ffi - .iter() - .all(|id| *id != [0_u8; 32]), - "Zero account ID returned" + new_npks_ffi.iter().all(|id| *id != [0_u8; 32]), + "Zero NPK returned" ); Ok(()) @@ -296,47 +297,34 @@ fn wallet_ffi_create_private_accounts() -> Result<()> { #[test] fn wallet_ffi_save_and_load_persistent_storage() -> Result<()> { let ctx = BlockingTestContext::new()?; - let mut out_private_account_id = FfiBytes32::from_bytes([0; 32]); let home = tempfile::tempdir()?; + let mut first_npk = [0u8; 32]; - // Create a private account with the wallet FFI and save it + // Create a receiving key and save unsafe { let wallet_ffi_handle = new_wallet_ffi_with_test_context_config(&ctx, home.path())?; - wallet_ffi_create_account_private(wallet_ffi_handle, &raw mut out_private_account_id); - + let mut out_keys = FfiPrivateAccountKeys::default(); + wallet_ffi_create_private_accounts_key(wallet_ffi_handle, &raw mut out_keys); + first_npk = out_keys.nullifier_public_key.data; + wallet_ffi_free_private_account_keys(&raw mut out_keys); wallet_ffi_save(wallet_ffi_handle); wallet_ffi_destroy(wallet_ffi_handle); } - let private_account_keys = unsafe { + // After loading, creating a new key should yield a different NPK (state was persisted) + let second_npk = unsafe { let wallet_ffi_handle = load_existing_ffi_wallet(home.path())?; - - let mut private_account = FfiAccount::default(); - - let result = wallet_ffi_get_account_private( - wallet_ffi_handle, - &raw const out_private_account_id, - &raw mut private_account, - ); - assert_eq!(result, error::WalletFfiError::Success); - let mut out_keys = FfiPrivateAccountKeys::default(); - let result = wallet_ffi_get_private_account_keys( - wallet_ffi_handle, - &raw const out_private_account_id, - &raw mut out_keys, - ); - assert_eq!(result, error::WalletFfiError::Success); - + wallet_ffi_create_private_accounts_key(wallet_ffi_handle, &raw mut out_keys); + let npk = out_keys.nullifier_public_key.data; + wallet_ffi_free_private_account_keys(&raw mut out_keys); wallet_ffi_destroy(wallet_ffi_handle); - - out_keys + npk }; - assert_eq!( - nssa::AccountId::from((&private_account_keys.npk(), 0)), - out_private_account_id.into() - ); + assert_ne!(first_npk, [0u8; 32], "First NPK should be non-zero"); + assert_ne!(second_npk, [0u8; 32], "Second NPK should be non-zero"); + assert_ne!(first_npk, second_npk, "Keys should differ after state was persisted"); Ok(()) } @@ -351,15 +339,17 @@ fn test_wallet_ffi_list_accounts() -> Result<()> { let mut public_ids: Vec<[u8; 32]> = Vec::new(); let mut private_ids: Vec<[u8; 32]> = Vec::new(); - // Create 5 public accounts and 5 private accounts, recording their IDs + // Create 5 public accounts and 5 receiving keys for _ in 0..5 { let mut out_account_id = FfiBytes32::from_bytes([0; 32]); wallet_ffi_create_account_public(handle, &raw mut out_account_id); public_ids.push(out_account_id.data); - wallet_ffi_create_account_private(handle, &raw mut out_account_id); - private_ids.push(out_account_id.data); + let mut out_keys = FfiPrivateAccountKeys::default(); + wallet_ffi_create_private_accounts_key(handle, &raw mut out_keys); + wallet_ffi_free_private_account_keys(&raw mut out_keys); } + let _ = private_ids; (handle, public_ids, private_ids) }; @@ -396,19 +386,13 @@ fn test_wallet_ffi_list_accounts() -> Result<()> { "Created public account not found in list with is_public=true" ); } - for id in &created_private_ids { - assert!( - listed_private_ids.contains(id), - "Created private account not found in list with is_public=false" - ); - } - - // Total listed accounts must be at least the number we created + // Total listed accounts must be at least the number of public accounts created + // (receiving keys without synced accounts don't appear in the list) assert!( - wallet_ffi_account_list.count >= created_public_ids.len() + created_private_ids.len(), - "Listed account count ({}) is less than the number of created accounts ({})", + wallet_ffi_account_list.count >= created_public_ids.len(), + "Listed account count ({}) is less than the number of created public accounts ({})", wallet_ffi_account_list.count, - created_public_ids.len() + created_private_ids.len() + created_public_ids.len() ); unsafe { @@ -712,25 +696,16 @@ fn wallet_ffi_init_private_account_auth_transfer() -> Result<()> { let home = tempfile::tempdir()?; let wallet_ffi_handle = new_wallet_ffi_with_test_context_config(&ctx, home.path())?; - // Create a new uninitialized public account - let mut out_account_id = FfiBytes32::from_bytes([0; 32]); - unsafe { - wallet_ffi_create_account_private(wallet_ffi_handle, &raw mut out_account_id); - } - - // Check its program owner is the default program id - let account: Account = unsafe { - let mut out_account = FfiAccount::default(); - wallet_ffi_get_account_private( - wallet_ffi_handle, - &raw const out_account_id, - &raw mut out_account, - ); - (&out_account).try_into().unwrap() + // Create a new receiving key and derive account_id for identifier=0 + let out_account_id: FfiBytes32 = unsafe { + let mut out_keys = FfiPrivateAccountKeys::default(); + wallet_ffi_create_private_accounts_key(wallet_ffi_handle, &raw mut out_keys); + let account_id = nssa::AccountId::from((&out_keys.npk(), 0_u128)); + wallet_ffi_free_private_account_keys(&raw mut out_keys); + (&account_id).into() }; - assert_eq!(account.program_owner, DEFAULT_PROGRAM_ID); - // Call the init funciton + // Call the init function let mut transfer_result = FfiTransferResult::default(); unsafe { wallet_ffi_register_private_account( @@ -834,15 +809,11 @@ fn test_wallet_ffi_transfer_shielded() -> Result<()> { let wallet_ffi_handle = new_wallet_ffi_with_test_context_config(&ctx, home.path())?; let from: FfiBytes32 = (&ctx.ctx().existing_public_accounts()[0]).into(); let (to, to_keys) = unsafe { - let mut out_account_id = FfiBytes32::default(); let mut out_keys = FfiPrivateAccountKeys::default(); - wallet_ffi_create_account_private(wallet_ffi_handle, &raw mut out_account_id); - wallet_ffi_get_private_account_keys( - wallet_ffi_handle, - &raw const out_account_id, - &raw mut out_keys, - ); - (out_account_id, out_keys) + wallet_ffi_create_private_accounts_key(wallet_ffi_handle, &raw mut out_keys); + let account_id = nssa::AccountId::from((&out_keys.npk(), 0_u128)); + let to: FfiBytes32 = (&account_id).into(); + (to, out_keys) }; let amount: [u8; 16] = 100_u128.to_le_bytes(); @@ -970,15 +941,11 @@ fn test_wallet_ffi_transfer_private() -> Result<()> { let from: FfiBytes32 = (&ctx.ctx().existing_private_accounts()[0]).into(); let (to, to_keys) = unsafe { - let mut out_account_id = FfiBytes32::default(); let mut out_keys = FfiPrivateAccountKeys::default(); - wallet_ffi_create_account_private(wallet_ffi_handle, &raw mut out_account_id); - wallet_ffi_get_private_account_keys( - wallet_ffi_handle, - &raw const out_account_id, - &raw mut out_keys, - ); - (out_account_id, out_keys) + wallet_ffi_create_private_accounts_key(wallet_ffi_handle, &raw mut out_keys); + let account_id = nssa::AccountId::from((&out_keys.npk(), 0_u128)); + let to: FfiBytes32 = (&account_id).into(); + (to, out_keys) }; let amount: [u8; 16] = 100_u128.to_le_bytes(); diff --git a/key_protocol/src/key_management/key_tree/mod.rs b/key_protocol/src/key_management/key_tree/mod.rs index 4ecdcf75..d9ba6ea2 100644 --- a/key_protocol/src/key_management/key_tree/mod.rs +++ b/key_protocol/src/key_management/key_tree/mod.rs @@ -256,11 +256,11 @@ impl KeyTree { } impl KeyTree { - pub fn generate_new_private_node(&mut self, parent_cci: &ChainIndex) -> Option { + pub fn create_private_accounts_key_node(&mut self, parent_cci: &ChainIndex) -> Option { self.generate_new_node(parent_cci) } - pub fn generate_new_private_node_layered(&mut self) -> Option { + pub fn create_private_accounts_key_node_layered(&mut self) -> Option { self.generate_new_node_layered() } diff --git a/key_protocol/src/key_protocol_core/mod.rs b/key_protocol/src/key_protocol_core/mod.rs index fb0943d9..b32a86ab 100644 --- a/key_protocol/src/key_protocol_core/mod.rs +++ b/key_protocol/src/key_protocol_core/mod.rs @@ -121,19 +121,19 @@ impl NSSAUserData { .or_else(|| self.public_key_tree.get_node(account_id).map(Into::into)) } - /// Generates a new private key node and returns its `ChainIndex`. - pub fn generate_new_privacy_preserving_transaction_key_chain( + /// Creates a new receiving key node and returns its `ChainIndex`. + pub fn create_private_accounts_key( &mut self, parent_cci: Option, ) -> ChainIndex { match parent_cci { Some(parent_cci) => self .private_key_tree - .generate_new_private_node(&parent_cci) + .create_private_accounts_key_node(&parent_cci) .expect("Parent must be present in a tree"), None => self .private_key_tree - .generate_new_private_node_layered() + .create_private_accounts_key_node_layered() .expect("Search for new node slot failed"), } } @@ -222,7 +222,7 @@ mod tests { let mut user_data = NSSAUserData::default(); let chain_index = user_data - .generate_new_privacy_preserving_transaction_key_chain(Some(ChainIndex::root())); + .create_private_accounts_key(Some(ChainIndex::root())); let is_key_chain_generated = user_data.private_key_tree.key_map.contains_key(&chain_index); assert!(is_key_chain_generated); diff --git a/wallet-ffi/src/account.rs b/wallet-ffi/src/account.rs index 1762d73b..9f16a3e2 100644 --- a/wallet-ffi/src/account.rs +++ b/wallet-ffi/src/account.rs @@ -7,7 +7,7 @@ use nssa::AccountId; use crate::{ block_on, error::{print_error, WalletFfiError}, - types::{FfiAccount, FfiAccountList, FfiAccountListEntry, FfiBytes32, WalletHandle}, + types::{FfiAccount, FfiAccountList, FfiAccountListEntry, FfiBytes32, FfiPrivateAccountKeys, WalletHandle}, wallet::get_wallet, }; @@ -61,32 +61,36 @@ pub unsafe extern "C" fn wallet_ffi_create_account_public( /// Create a new private key node. /// -/// Returns the nullifier public key (npk) to share with senders. Account IDs are -/// discovered later via sync when senders initialize accounts under this key. +/// Returns the nullifier public key (npk) and viewing public key (vpk) to share with +/// senders. Account IDs are discovered later via sync when senders initialize accounts +/// under this key. /// /// # Parameters /// - `handle`: Valid wallet handle -/// - `out_npk`: Output pointer for the nullifier public key (32 bytes) +/// - `out_keys`: Output pointer for the key data (npk + vpk) /// /// # Returns /// - `Success` on successful creation /// - Error code on failure /// +/// # Memory +/// The keys structure must be freed with `wallet_ffi_free_private_account_keys()`. +/// /// # Safety /// - `handle` must be a valid wallet handle from `wallet_ffi_create_new` or `wallet_ffi_open` -/// - `out_npk` must be a valid pointer to a `FfiBytes32` struct +/// - `out_keys` must be a valid pointer to a `FfiPrivateAccountKeys` struct #[no_mangle] -pub unsafe extern "C" fn wallet_ffi_create_account_private( +pub unsafe extern "C" fn wallet_ffi_create_private_accounts_key( handle: *mut WalletHandle, - out_npk: *mut FfiBytes32, + out_keys: *mut FfiPrivateAccountKeys, ) -> WalletFfiError { let wrapper = match get_wallet(handle) { Ok(w) => w, Err(e) => return e, }; - if out_npk.is_null() { - print_error("Null output pointer for npk"); + if out_keys.is_null() { + print_error("Null output pointer for keys"); return WalletFfiError::NullPointer; } @@ -98,7 +102,7 @@ pub unsafe extern "C" fn wallet_ffi_create_account_private( } }; - let chain_index = wallet.create_new_account_private(None); + let chain_index = wallet.create_private_accounts_key(None); let node = wallet .storage() @@ -108,8 +112,20 @@ pub unsafe extern "C" fn wallet_ffi_create_account_private( .get(&chain_index) .expect("Node was just inserted"); + let key_chain = &node.value.0; + let npk_bytes = key_chain.nullifier_public_key.0; + let vpk_bytes = key_chain.viewing_public_key.to_bytes(); + let vpk_len = vpk_bytes.len(); + #[expect( + clippy::as_conversions, + reason = "We need to convert the boxed slice into a raw pointer for FFI" + )] + let vpk_ptr = Box::into_raw(vpk_bytes.to_vec().into_boxed_slice()) as *const u8; + unsafe { - (*out_npk).data = node.value.0.nullifier_public_key.0; + (*out_keys).nullifier_public_key.data = npk_bytes; + (*out_keys).viewing_public_key = vpk_ptr; + (*out_keys).viewing_public_key_len = vpk_len; } WalletFfiError::Success diff --git a/wallet-ffi/wallet_ffi.h b/wallet-ffi/wallet_ffi.h index c469c9ee..d0bdc60a 100644 --- a/wallet-ffi/wallet_ffi.h +++ b/wallet-ffi/wallet_ffi.h @@ -126,6 +126,24 @@ typedef struct FfiBytes32 { uint8_t data[32]; } FfiBytes32; +/** + * Public keys for a private account (safe to expose). + */ +typedef struct FfiPrivateAccountKeys { + /** + * Nullifier public key (32 bytes). + */ + struct FfiBytes32 nullifier_public_key; + /** + * viewing public key (compressed secp256k1 point). + */ + const uint8_t *viewing_public_key; + /** + * Length of viewing public key (typically 33 bytes). + */ + uintptr_t viewing_public_key_len; +} FfiPrivateAccountKeys; + /** * Single entry in the account list. */ @@ -189,24 +207,6 @@ typedef struct FfiPublicAccountKey { struct FfiBytes32 public_key; } FfiPublicAccountKey; -/** - * Public keys for a private account (safe to expose). - */ -typedef struct FfiPrivateAccountKeys { - /** - * Nullifier public key (32 bytes). - */ - struct FfiBytes32 nullifier_public_key; - /** - * viewing public key (compressed secp256k1 point). - */ - const uint8_t *viewing_public_key; - /** - * Length of viewing public key (typically 33 bytes). - */ - uintptr_t viewing_public_key_len; -} FfiPrivateAccountKeys; - /** * Result of a transfer operation. */ @@ -245,23 +245,27 @@ enum WalletFfiError wallet_ffi_create_account_public(struct WalletHandle *handle /** * Create a new private key node. * - * Returns the nullifier public key (npk) to share with senders. Account IDs are - * discovered later via sync when senders initialize accounts under this key. + * Returns the nullifier public key (npk) and viewing public key (vpk) to share with + * senders. Account IDs are discovered later via sync when senders initialize accounts + * under this key. * * # Parameters * - `handle`: Valid wallet handle - * - `out_npk`: Output pointer for the nullifier public key (32 bytes) + * - `out_keys`: Output pointer for the key data (npk + vpk) * * # Returns * - `Success` on successful creation * - Error code on failure * + * # Memory + * The keys structure must be freed with `wallet_ffi_free_private_account_keys()`. + * * # Safety * - `handle` must be a valid wallet handle from `wallet_ffi_create_new` or `wallet_ffi_open` - * - `out_npk` must be a valid pointer to a `FfiBytes32` struct + * - `out_keys` must be a valid pointer to a `FfiPrivateAccountKeys` struct */ -enum WalletFfiError wallet_ffi_create_account_private(struct WalletHandle *handle, - struct FfiBytes32 *out_npk); +enum WalletFfiError wallet_ffi_create_private_accounts_key(struct WalletHandle *handle, + struct FfiPrivateAccountKeys *out_keys); /** * List all accounts in the wallet. diff --git a/wallet/src/cli/account.rs b/wallet/src/cli/account.rs index 21b8654a..6c7cd75a 100644 --- a/wallet/src/cli/account.rs +++ b/wallet/src/cli/account.rs @@ -82,8 +82,8 @@ pub enum NewSubcommand { /// Label to assign to the new account. label: Option, }, - /// Register new private account. - Private { + /// Create a new receiving key (npk + vpk) to share with senders. + PrivateAccountsKey { #[arg(long)] /// Chain index of a parent node. cci: Option, @@ -133,8 +133,8 @@ impl WalletSubcommand for NewSubcommand { Ok(SubcommandReturnValue::RegisterAccount { account_id }) } - Self::Private { cci } => { - let chain_index = wallet_core.create_new_account_private(cci); + Self::PrivateAccountsKey { cci } => { + let chain_index = wallet_core.create_private_accounts_key(cci); let node = wallet_core .storage diff --git a/wallet/src/lib.rs b/wallet/src/lib.rs index b2458390..cbed292c 100644 --- a/wallet/src/lib.rs +++ b/wallet/src/lib.rs @@ -256,13 +256,13 @@ impl WalletCore { .generate_new_public_transaction_private_key(chain_index) } - pub fn create_new_account_private( + pub fn create_private_accounts_key( &mut self, chain_index: Option, ) -> ChainIndex { self.storage .user_data - .generate_new_privacy_preserving_transaction_key_chain(chain_index) + .create_private_accounts_key(chain_index) } /// Get account balance.