From dd35c89ca1bd5b43c419c91b18f9693d5489a63d Mon Sep 17 00:00:00 2001 From: Pravdyvy Date: Mon, 1 Jun 2026 13:59:41 +0300 Subject: [PATCH] fix(wallet_ffi): suggestions fix 1 --- wallet-ffi/src/generic_transaction.rs | 87 ++++++------------ wallet-ffi/src/keys.rs | 20 +++-- wallet-ffi/src/types.rs | 125 +++++++++++++++++++++++++- wallet-ffi/wallet_ffi.h | 16 ++-- wallet/src/account_manager.rs | 2 +- 5 files changed, 175 insertions(+), 75 deletions(-) diff --git a/wallet-ffi/src/generic_transaction.rs b/wallet-ffi/src/generic_transaction.rs index 1351a7be..9ec3db33 100644 --- a/wallet-ffi/src/generic_transaction.rs +++ b/wallet-ffi/src/generic_transaction.rs @@ -126,7 +126,7 @@ pub struct FfiTransactionResult { /// Whether the transaction succeeded. pub success: bool, pub secrets_data: *const FfiBytes32, - /// Public transaction have 0 secrets. + /// Public transactions have 0 secrets. pub secrets_size: usize, } @@ -222,12 +222,12 @@ pub unsafe extern "C" fn wallet_ffi_send_generic_public_transaction( }; if account_identities.is_null() { - print_error("Null output pointer for account identities list"); + print_error("Null input pointer for account identities list"); return WalletFfiError::NullPointer; } if instruction_words.is_null() { - print_error("Null output pointer for instruction data"); + print_error("Null input pointer for instruction data"); return WalletFfiError::NullPointer; } @@ -244,34 +244,19 @@ pub unsafe extern "C" fn wallet_ffi_send_generic_public_transaction( } }; + let accounts_ffi = std::slice::from_raw_parts(account_identities, account_identities_size); + let instruction_data = std::slice::from_raw_parts(instruction_words, instruction_words_size); + let mut accounts = Vec::with_capacity(account_identities_size); - let mut instruction_data = Vec::with_capacity(instruction_words_size); - // Alignment will be different, we need to read elements one-by-one - for i in 0..account_identities_size { - accounts.push( - match match unsafe { account_identities.add(i).as_ref() } - .ok_or(WalletFfiError::NullPointer) - { - Ok(v) => v, - Err(err) => { - print_error( - "account_identities_size does not match actual size of account_identities", - ); - return err; - } + for ffi_acc in accounts_ffi { + match ffi_acc.try_into() { + Ok(v) => accounts.push(v), + Err(err) => { + print_error("Failed to convert FfiAccountIdentity into AccountIdentity"); + return err; } - .try_into() - { - Ok(v) => v, - Err(err) => return err, - }, - ); - } - - // Alignment will be different, we need to read elements one-by-one - for i in 0..instruction_words_size { - instruction_data.push(unsafe { *instruction_words.add(i) }); + } } let program = match unsafe { &*program_with_dependencies }.try_into() { @@ -279,7 +264,7 @@ pub unsafe extern "C" fn wallet_ffi_send_generic_public_transaction( Err(err) => return err, }; - match block_on(wallet.send_pub_tx(accounts, instruction_data, &program)) { + match block_on(wallet.send_pub_tx(accounts, instruction_data.to_vec(), &program)) { Ok(tx_hash) => { let tx_hash = CString::new(tx_hash.to_string()) .map_or(std::ptr::null_mut(), std::ffi::CString::into_raw); @@ -334,12 +319,12 @@ pub unsafe extern "C" fn wallet_ffi_send_generic_private_transaction( }; if account_identities.is_null() { - print_error("Null output pointer for account identities list"); + print_error("Null input pointer for account identities list"); return WalletFfiError::NullPointer; } if instruction_words.is_null() { - print_error("Null output pointer for instruction data"); + print_error("Null input pointer for instruction data"); return WalletFfiError::NullPointer; } @@ -356,34 +341,19 @@ pub unsafe extern "C" fn wallet_ffi_send_generic_private_transaction( } }; + let accounts_ffi = std::slice::from_raw_parts(account_identities, account_identities_size); + let instruction_data = std::slice::from_raw_parts(instruction_words, instruction_words_size); + let mut accounts = Vec::with_capacity(account_identities_size); - let mut instruction_data = Vec::with_capacity(instruction_words_size); - // Alignment will be different, we need to read elements one-by-one - for i in 0..account_identities_size { - accounts.push( - match match unsafe { account_identities.add(i).as_ref() } - .ok_or(WalletFfiError::NullPointer) - { - Ok(v) => v, - Err(err) => { - print_error( - "account_identities_size does not match actual size of account_identities", - ); - return err; - } + for ffi_acc in accounts_ffi { + match ffi_acc.try_into() { + Ok(v) => accounts.push(v), + Err(err) => { + print_error("Failed to convert FfiAccountIdentity into AccountIdentity"); + return err; } - .try_into() - { - Ok(v) => v, - Err(err) => return err, - }, - ); - } - - // Alignment will be different, we need to read elements one-by-one - for i in 0..instruction_words_size { - instruction_data.push(unsafe { *instruction_words.add(i) }); + } } let program = match unsafe { &*program_with_dependencies }.try_into() { @@ -391,7 +361,8 @@ pub unsafe extern "C" fn wallet_ffi_send_generic_private_transaction( Err(err) => return err, }; - match block_on(wallet.send_privacy_preserving_tx(accounts, instruction_data, &program)) { + match block_on(wallet.send_privacy_preserving_tx(accounts, instruction_data.to_vec(), &program)) + { Ok((tx_hash, secrets)) => { let tx_hash = CString::new(tx_hash.to_string()) .map_or(std::ptr::null_mut(), std::ffi::CString::into_raw); @@ -414,7 +385,7 @@ pub unsafe extern "C" fn wallet_ffi_send_generic_private_transaction( WalletFfiError::Success } Err(e) => { - print_error(format!("Public send failed: {e:?}")); + print_error(format!("Private send failed: {e:?}")); unsafe { *out_result = FfiTransactionResult::default(); } diff --git a/wallet-ffi/src/keys.rs b/wallet-ffi/src/keys.rs index 65aa5ef0..ae7fdc36 100644 --- a/wallet-ffi/src/keys.rs +++ b/wallet-ffi/src/keys.rs @@ -257,20 +257,25 @@ pub unsafe extern "C" fn wallet_ffi_account_id_from_base58( /// /// # Parameters /// - `account_id`: 32 bytes of the public account ID -/// - `needs_sign`: does account needs signing +/// - `needs_sign`: whether the account needs signing /// - `out_account_identity`: valid pointer, where output will be written /// /// # Returns /// - `Success` on successful retrieval /// /// # Safety -/// - `out_account_identity` must be a valid pointer to a `FfiAccountManagerAccountIdentity` struct +/// - `out_account_identity` must be a valid pointer to a `FfiAccountIdentity` struct #[no_mangle] pub unsafe extern "C" fn wallet_ffi_resolve_public_account( account_id: FfiBytes32, needs_sign: bool, out_account_identity: *mut FfiAccountIdentity, ) -> WalletFfiError { + if out_account_identity.is_null() { + print_error("Null pointer argument"); + return WalletFfiError::NullPointer; + } + let resolved_account = if needs_sign { AccountIdentity::Public(account_id.into()) } else { @@ -293,18 +298,23 @@ pub unsafe extern "C" fn wallet_ffi_resolve_public_account( /// /// # Returns /// - `Success` on successful retrieval -/// - `InternalError` if wailed to lock wallet -/// - `AccountNotFound` if failed to found account +/// - `InternalError` if failed to lock wallet +/// - `AccountNotFound` if the account is not found /// /// # Safety /// - `handle` must be a valid wallet handle from `wallet_ffi_create_new` or `wallet_ffi_open` -/// - `out_account_identity` must be a valid pointer to a `FfiAccountManagerAccountIdentity` struct +/// - `out_account_identity` must be a valid pointer to a `FfiAccountIdentity` struct #[no_mangle] pub unsafe extern "C" fn wallet_ffi_resolve_private_account( handle: *mut WalletHandle, account_id: FfiBytes32, out_account_identity: *mut FfiAccountIdentity, ) -> WalletFfiError { + if out_account_identity.is_null() { + print_error("Null pointer argument"); + return WalletFfiError::NullPointer; + } + let wrapper = match get_wallet(handle) { Ok(w) => w, Err(e) => return e, diff --git a/wallet-ffi/src/types.rs b/wallet-ffi/src/types.rs index 149b646b..4ad72285 100644 --- a/wallet-ffi/src/types.rs +++ b/wallet-ffi/src/types.rs @@ -179,8 +179,9 @@ impl FfiPrivateAccountKeys { } } -/// Enumeration to represent kinds of `FfiAccountManagerAccountIdentity`. +/// Enumeration to represent kinds of `FfiAccountIdentity`. #[repr(C)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum FfiAccountIdentityKind { Public = 0, PublicNoSign = 1, @@ -192,7 +193,7 @@ pub enum FfiAccountIdentityKind { PrivatePdaShared = 7, } -/// Struct representing of account identity, given to `AccountManager` at intialization. +/// Struct representing an account identity, given to `AccountManager` at intialization. #[repr(C)] pub struct FfiAccountIdentity { pub kind: FfiAccountIdentityKind, @@ -432,7 +433,7 @@ impl From for FfiAccountIdentity { }; Self { - kind: FfiAccountIdentityKind::PrivateShared, + kind: FfiAccountIdentityKind::PrivatePdaShared, account_id: account_id.into(), nullifier_secret_key: nsk.into(), nullifier_public_key: npk.0.into(), @@ -539,3 +540,121 @@ impl TryFrom<&FfiAccountIdentity> for AccountIdentity { } } } + +#[cfg(test)] +mod tests { + use nssa::{AccountId, PrivateKey, PublicKey}; + use nssa_core::{encryption::ViewingPublicKey, program::PdaSeed, PrivateAccountKind}; + use wallet::AccountIdentity; + + use crate::{FfiAccountIdentity, FfiAccountIdentityKind}; + + #[test] + fn account_identity_roundtrip() { + let private_key = PrivateKey::try_new([42; 32]).unwrap(); + let public_key = PublicKey::new_from_private_key(&private_key); + let pub_acc_id = (&public_key).into(); + + let nsk = [43; 32]; + let vpk = ViewingPublicKey::from_scalar([44; 32]); + let npk = (&nsk).into(); + let identifier = u128::from_le_bytes([45; 16]); + + let private_reg_acc_id = + AccountId::for_private_account(&npk, &PrivateAccountKind::Regular(identifier)); + let private_pda_acc_id = AccountId::for_private_account( + &npk, + &PrivateAccountKind::Pda { + program_id: [46; 8], + seed: PdaSeed::new([47; 32]), + identifier, + }, + ); + + let acc_identity_1 = AccountIdentity::Public(pub_acc_id); + let acc_identity_2 = AccountIdentity::PublicNoSign(pub_acc_id); + let acc_identity_3 = AccountIdentity::PrivateOwned(private_reg_acc_id); + let acc_identity_4 = AccountIdentity::PrivateForeign { + npk, + vpk: vpk.clone(), + identifier, + }; + let acc_identity_5 = AccountIdentity::PrivatePdaOwned(private_pda_acc_id); + let acc_identity_6 = AccountIdentity::PrivatePdaForeign { + account_id: private_pda_acc_id, + npk, + vpk: vpk.clone(), + identifier, + }; + let acc_identity_7 = AccountIdentity::PrivateShared { + nsk, + npk, + vpk: vpk.clone(), + identifier, + }; + let acc_identity_8 = AccountIdentity::PrivatePdaShared { + account_id: private_pda_acc_id, + nsk, + npk, + vpk, + identifier, + }; + + let ffi_acc_identity_1: FfiAccountIdentity = acc_identity_1.clone().into(); + let ffi_acc_identity_2: FfiAccountIdentity = acc_identity_2.clone().into(); + let ffi_acc_identity_3: FfiAccountIdentity = acc_identity_3.clone().into(); + let ffi_acc_identity_4: FfiAccountIdentity = acc_identity_4.clone().into(); + let ffi_acc_identity_5: FfiAccountIdentity = acc_identity_5.clone().into(); + let ffi_acc_identity_6: FfiAccountIdentity = acc_identity_6.clone().into(); + let ffi_acc_identity_7: FfiAccountIdentity = acc_identity_7.clone().into(); + let ffi_acc_identity_8: FfiAccountIdentity = acc_identity_8.clone().into(); + + assert_eq!(ffi_acc_identity_1.kind, FfiAccountIdentityKind::Public); + assert_eq!( + ffi_acc_identity_2.kind, + FfiAccountIdentityKind::PublicNoSign + ); + assert_eq!( + ffi_acc_identity_3.kind, + FfiAccountIdentityKind::PrivateOwned + ); + assert_eq!( + ffi_acc_identity_4.kind, + FfiAccountIdentityKind::PrivateForeign + ); + assert_eq!( + ffi_acc_identity_5.kind, + FfiAccountIdentityKind::PrivatePdaOwned + ); + assert_eq!( + ffi_acc_identity_6.kind, + FfiAccountIdentityKind::PrivatePdaForeign + ); + assert_eq!( + ffi_acc_identity_7.kind, + FfiAccountIdentityKind::PrivateShared + ); + assert_eq!( + ffi_acc_identity_8.kind, + FfiAccountIdentityKind::PrivatePdaShared + ); + + let acc_identity_res_1: AccountIdentity = (&ffi_acc_identity_1).try_into().unwrap(); + let acc_identity_res_2: AccountIdentity = (&ffi_acc_identity_2).try_into().unwrap(); + let acc_identity_res_3: AccountIdentity = (&ffi_acc_identity_3).try_into().unwrap(); + let acc_identity_res_4: AccountIdentity = (&ffi_acc_identity_4).try_into().unwrap(); + let acc_identity_res_5: AccountIdentity = (&ffi_acc_identity_5).try_into().unwrap(); + let acc_identity_res_6: AccountIdentity = (&ffi_acc_identity_6).try_into().unwrap(); + let acc_identity_res_7: AccountIdentity = (&ffi_acc_identity_7).try_into().unwrap(); + let acc_identity_res_8: AccountIdentity = (&ffi_acc_identity_8).try_into().unwrap(); + + assert_eq!(acc_identity_res_1, acc_identity_1); + assert_eq!(acc_identity_res_2, acc_identity_2); + assert_eq!(acc_identity_res_3, acc_identity_3); + assert_eq!(acc_identity_res_4, acc_identity_4); + assert_eq!(acc_identity_res_5, acc_identity_5); + assert_eq!(acc_identity_res_6, acc_identity_6); + assert_eq!(acc_identity_res_7, acc_identity_7); + assert_eq!(acc_identity_res_8, acc_identity_8); + } +} diff --git a/wallet-ffi/wallet_ffi.h b/wallet-ffi/wallet_ffi.h index 03b139d7..3b31516a 100644 --- a/wallet-ffi/wallet_ffi.h +++ b/wallet-ffi/wallet_ffi.h @@ -114,7 +114,7 @@ typedef enum WalletFfiError { } WalletFfiError; /** - * Enumeration to represent kinds of `FfiAccountManagerAccountIdentity`. + * Enumeration to represent kinds of `FfiAccountIdentity`. */ typedef enum FfiAccountIdentityKind { PUBLIC = 0, @@ -225,7 +225,7 @@ typedef struct SerializationHelperResult { } SerializationHelperResult; /** - * Struct representing of account identity, given to `AccountManager` at intialization. + * Struct representing an account identity, given to `AccountManager` at intialization. */ typedef struct FfiAccountIdentity { enum FfiAccountIdentityKind kind; @@ -268,7 +268,7 @@ typedef struct FfiTransactionResult { bool success; const struct FfiBytes32 *secrets_data; /** - * Public transaction have 0 secrets. + * Public transactions have 0 secrets. */ uintptr_t secrets_size; } FfiTransactionResult; @@ -710,14 +710,14 @@ enum WalletFfiError wallet_ffi_account_id_from_base58(const char *base58_str, * * # Parameters * - `account_id`: 32 bytes of the public account ID - * - `needs_sign`: does account needs signing + * - `needs_sign`: whether the account needs signing * - `out_account_identity`: valid pointer, where output will be written * * # Returns * - `Success` on successful retrieval * * # Safety - * - `out_account_identity` must be a valid pointer to a `FfiAccountManagerAccountIdentity` struct + * - `out_account_identity` must be a valid pointer to a `FfiAccountIdentity` struct */ enum WalletFfiError wallet_ffi_resolve_public_account(struct FfiBytes32 account_id, bool needs_sign, @@ -733,12 +733,12 @@ enum WalletFfiError wallet_ffi_resolve_public_account(struct FfiBytes32 account_ * * # Returns * - `Success` on successful retrieval - * - `InternalError` if wailed to lock wallet - * - `AccountNotFound` if failed to found account + * - `InternalError` if failed to lock wallet + * - `AccountNotFound` if the account is not found * * # Safety * - `handle` must be a valid wallet handle from `wallet_ffi_create_new` or `wallet_ffi_open` - * - `out_account_identity` must be a valid pointer to a `FfiAccountManagerAccountIdentity` struct + * - `out_account_identity` must be a valid pointer to a `FfiAccountIdentity` struct */ enum WalletFfiError wallet_ffi_resolve_private_account(struct WalletHandle *handle, struct FfiBytes32 account_id, diff --git a/wallet/src/account_manager.rs b/wallet/src/account_manager.rs index 5caf2b22..3934a2a5 100644 --- a/wallet/src/account_manager.rs +++ b/wallet/src/account_manager.rs @@ -10,7 +10,7 @@ use nssa_core::{ use crate::{ExecutionFailureKind, WalletCore}; -#[derive(Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum AccountIdentity { Public(AccountId), /// A public account without signing. Would not try to sign, even if account is owned.