diff --git a/qa/rpc-tests/wallet_orchard.py b/qa/rpc-tests/wallet_orchard.py index 5c73cf406..8a500e951 100755 --- a/qa/rpc-tests/wallet_orchard.py +++ b/qa/rpc-tests/wallet_orchard.py @@ -151,9 +151,25 @@ class WalletOrchardTest(BitcoinTestFramework): # un-mined and returned to the mempool assert_equal(set([rollback_tx]), set(self.nodes[2].getrawmempool())) + # our sole Sapling note is spent, so our confirmed balance is currently 0 + assert_equal( + {'pools': {}, 'minimum_confirmations': 1}, + self.nodes[2].z_getbalanceforaccount(acct2)) + + # our incoming change (unconfirmed, still in the mempool) is 9 zec + assert_equal( + {'pools': {'sapling': {'valueZat': Decimal('900000000')}}, 'minimum_confirmations': 0}, + self.nodes[2].z_getbalanceforaccount(acct2, 0)) + + # the transaction was un-mined, so we have no confirmed balance assert_equal( {'pools': {}, 'minimum_confirmations': 1}, self.nodes[3].z_getbalanceforaccount(acct3)) + # our unconfirmed balance is 1 zec + assert_equal( + {'pools': {'orchard': {'valueZat': Decimal('100000000')}}, 'minimum_confirmations': 0}, + self.nodes[3].z_getbalanceforaccount(acct3, 0)) + if __name__ == '__main__': WalletOrchardTest().main() diff --git a/src/rust/include/rust/orchard/wallet.h b/src/rust/include/rust/orchard/wallet.h index dc70926ec..e64eaff4a 100644 --- a/src/rust/include/rust/orchard/wallet.h +++ b/src/rust/include/rust/orchard/wallet.h @@ -83,9 +83,13 @@ bool orchard_wallet_rewind( * in the object destructor. */ struct RawOrchardActionIVK { - uint32_t actionIdx; + uint64_t actionIdx; OrchardIncomingViewingKeyPtr* ivk; }; +static_assert( + sizeof(RawOrchardActionIVK) == 16, + "RawOrchardActionIVK struct should have exactly a 128-bit in-memory representation."); +static_assert(alignof(RawOrchardActionIVK) == 8, "RawOrchardActionIVK struct alignment is not 64 bits."); typedef void (*push_action_ivk_callback_t)(void* resultCollection, const RawOrchardActionIVK actionIvk); diff --git a/src/rust/src/wallet.rs b/src/rust/src/wallet.rs index 42b89f4b9..28a358056 100644 --- a/src/rust/src/wallet.rs +++ b/src/rust/src/wallet.rs @@ -306,8 +306,9 @@ impl Wallet { } /// Add note data for those notes that are decryptable with one of this wallet's - /// incoming viewing keys to the wallet, and return a map from incoming viewing - /// key to the vector of action indices that that key decrypts. + /// incoming viewing keys to the wallet, and return a map from each decrypted + /// action's index to the incoming vieing key that successfully decrypted that + /// action. pub fn add_notes_from_bundle( &mut self, txid: &TxId, @@ -341,7 +342,7 @@ impl Wallet { for (action_idx, ivk, note, recipient, memo) in bundle.decrypt_outputs_for_keys(&keys) { assert!(self.add_decrypted_note(txid, action_idx, ivk.clone(), note, recipient, memo)); - result.entry(action_idx).or_insert(ivk); + result.insert(action_idx, ivk); } result @@ -354,23 +355,15 @@ impl Wallet { &mut self, txid: &TxId, bundle: &Bundle, - hints: &BTreeMap, + hints: BTreeMap, ) -> Result<(), BundleDecryptionError> { - for (action_idx, ivk) in hints.iter() { - if let Some((note, recipient, memo)) = bundle.decrypt_output_with_key(*action_idx, ivk) - { - if !self.add_decrypted_note( - txid, - *action_idx, - (*ivk).clone(), - note, - recipient, - memo, - ) { - return Err(BundleDecryptionError::FvkNotFound((*ivk).clone())); + for (action_idx, ivk) in hints.into_iter() { + if let Some((note, recipient, memo)) = bundle.decrypt_output_with_key(action_idx, ivk) { + if !self.add_decrypted_note(txid, action_idx, ivk.clone(), note, recipient, memo) { + return Err(BundleDecryptionError::FvkNotFound(ivk.clone())); } } else { - return Err(BundleDecryptionError::ActionDecryptionFailed(*action_idx)); + return Err(BundleDecryptionError::ActionDecryptionFailed(action_idx)); } } Ok(()) @@ -548,9 +541,9 @@ impl Wallet { // FFI // -/// A type alias for a pointer to a C++ vector used for collecting -/// return values across the FFI. -pub type VecObj = std::ptr::NonNull; +/// A type alias for a pointer to a C++ value that is the target of +/// a mutating action by a callback across the FFI +pub type FFICallbackReceiver = std::ptr::NonNull; #[no_mangle] pub extern "C" fn orchard_wallet_new() -> *mut Wallet { @@ -609,23 +602,24 @@ pub extern "C" fn orchard_wallet_rewind( /// A type used to pass action decryption metadata across the FFI boundary. /// This must have the same representation as `struct RawOrchardActionIVK` -/// in `rust/include/rust/orchard/wallet.h`. +/// in `rust/include/rust/orchard/wallet.h`. action_idx is #[repr(C)] pub struct FFIActionIvk { - action_idx: u32, + action_idx: u64, ivk_ptr: *mut IncomingViewingKey, } /// A C++-allocated function pointer that can push a FFIActionIVK value /// onto a C++ vector. -pub type ActionIvkPushCb = unsafe extern "C" fn(obj: Option, value: FFIActionIvk); +pub type ActionIvkPushCb = + unsafe extern "C" fn(obj: Option, value: FFIActionIvk); #[no_mangle] pub extern "C" fn orchard_wallet_add_notes_from_bundle( wallet: *mut Wallet, txid: *const [c_uchar; 32], bundle: *const Bundle, - result: Option, + result: Option, push_cb: Option, ) { let wallet = unsafe { wallet.as_mut() }.expect("Wallet pointer may not be null"); @@ -664,7 +658,7 @@ pub extern "C" fn orchard_wallet_restore_notes( ); } - match wallet.add_notes_from_bundle_with_hints(&txid, bundle, &hints) { + match wallet.add_notes_from_bundle_with_hints(&txid, bundle, hints) { Ok(_) => true, Err(e) => { error!("Failed to restore decrypted notes to wallet: {:?}", e); @@ -778,7 +772,7 @@ pub struct FFINoteMetadata { /// A C++-allocated function pointer that can push a FFINoteMetadata value /// onto a C++ vector. -pub type NotePushCb = unsafe extern "C" fn(obj: Option, meta: FFINoteMetadata); +pub type NotePushCb = unsafe extern "C" fn(obj: Option, meta: FFINoteMetadata); #[no_mangle] pub extern "C" fn orchard_wallet_get_filtered_notes( @@ -786,7 +780,7 @@ pub extern "C" fn orchard_wallet_get_filtered_notes( ivk: *const IncomingViewingKey, ignore_mined: bool, require_spending_key: bool, - result: Option, + result: Option, push_cb: Option, ) { let wallet = unsafe { wallet.as_ref() }.expect("Wallet pointer may not be null."); @@ -804,14 +798,14 @@ pub extern "C" fn orchard_wallet_get_filtered_notes( } } -pub type PushTxId = unsafe extern "C" fn(obj: Option, txid: *const [u8; 32]); +pub type PushTxId = unsafe extern "C" fn(obj: Option, txid: *const [u8; 32]); #[no_mangle] pub extern "C" fn orchard_wallet_get_potential_spends( wallet: *const Wallet, txid: *const [c_uchar; 32], action_idx: u32, - result: Option, + result: Option, push_cb: Option, ) { let wallet = unsafe { wallet.as_ref() }.expect("Wallet pointer may not be null."); diff --git a/src/wallet/orchard.h b/src/wallet/orchard.h index f20b0ced7..812c96c4a 100644 --- a/src/wallet/orchard.h +++ b/src/wallet/orchard.h @@ -114,11 +114,6 @@ public: * contains the full viewing key to the wallet, and return the * mapping from each decrypted Orchard action index to the IVK * that was used to decrypt that action's note. - * - * If hints are provided, this method will perform decryption of - * only the specified actions using the provided incoming viewing keys, - * rather than performing trial decryption on all the actions of the - * provided bundle. */ std::map AddNotesIfInvolvingMe(const CTransaction& tx) { std::map result; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bdec32140..a53ac73b8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -945,7 +945,7 @@ bool CWallet::LoadUnifiedCaches() } // Restore decrypted Orchard notes. - for (const auto& [txid, walletTx] : mapWallet) { + for (const auto& [_, walletTx] : mapWallet) { if (!walletTx.mapOrchardActionData.empty()) { if (!orchardWallet.RestoreDecryptedNotes(walletTx, walletTx.mapOrchardActionData)) { return false; @@ -2887,6 +2887,9 @@ bool CWallet::UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx) } bool unchangedOrchardFlag = (wtxIn.mapOrchardActionData.empty() || wtxIn.mapOrchardActionData == wtx.mapOrchardActionData); + if (!unchangedOrchardFlag) { + wtx.mapOrchardActionData = wtxIn.mapOrchardActionData; + } return !unchangedSproutFlag || !unchangedSaplingFlag || !unchangedOrchardFlag; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3f42ebbdd..5b133be80 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1059,7 +1059,7 @@ protected: // are empty. This covers transactions that have no Sprout or Sapling data // (i.e. are purely transparent), as well as shielding and unshielding // transactions in which we only have transparent addresses involved. - if (!(wtx.mapSproutNoteData.empty() && wtx.mapSaplingNoteData.empty() && wtx.mapOrchardActionData.empty())) { + if (!(wtx.mapSproutNoteData.empty() && wtx.mapSaplingNoteData.empty())) { if (!walletdb.WriteTx(wtx)) { LogPrintf("SetBestChain(): Failed to write CWalletTx, aborting atomic write\n"); walletdb.TxnAbort();