zecfoundation
/
zcashd
mirror of https://github.com/ZcashFoundation/zcashd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Apply suggestions from code review 

Co-authored-by: str4d <thestr4d@gmail.com>
This commit is contained in:
Kris Nuttycombe 2022-03-09 13:15:13 -07:00 committed by Kris Nuttycombe
parent 98ee3bd733
commit ca34a97a37
6 changed files with 49 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
qa/rpc-tests/wallet_orchard.py
View File

@ -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()

6
src/rust/include/rust/orchard/wallet.h
View File

@ -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);

52
src/rust/src/wallet.rs
View File

@ -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<Authorized, Amount>,
hints: &BTreeMap<usize, &IncomingViewingKey>,
hints: BTreeMap<usize, &IncomingViewingKey>,
) -> 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<libc::c_void>;
/// 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<libc::c_void>;
#[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<VecObj>, value: FFIActionIvk);
pub type ActionIvkPushCb =
unsafe extern "C" fn(obj: Option<FFICallbackReceiver>, 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<Authorized, Amount>,
result: Option<VecObj>,
result: Option<FFICallbackReceiver>,
push_cb: Option<ActionIvkPushCb>,
) {
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<VecObj>, meta: FFINoteMetadata);
pub type NotePushCb = unsafe extern "C" fn(obj: Option<FFICallbackReceiver>, 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<VecObj>,
result: Option<FFICallbackReceiver>,
push_cb: Option<NotePushCb>,
) {
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<VecObj>, txid: *const [u8; 32]);
pub type PushTxId = unsafe extern "C" fn(obj: Option<FFICallbackReceiver>, 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<VecObj>,
result: Option<FFICallbackReceiver>,
push_cb: Option<PushTxId>,
) {
let wallet = unsafe { wallet.as_ref() }.expect("Wallet pointer may not be null.");

5
src/wallet/orchard.h
View File

@ -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<uint32_t, libzcash::OrchardIncomingViewingKey> AddNotesIfInvolvingMe(const CTransaction& tx) {
std::map<uint32_t, libzcash::OrchardIncomingViewingKey> result;

5
src/wallet/wallet.cpp
View File

@ -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;
}

2
src/wallet/wallet.h
View File

@ -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();