From 72b27fb906622f669574f45d4c984901300f11ed Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 29 Mar 2022 18:58:13 -0600 Subject: [PATCH 1/3] Add independent wallet persistence tests. Co-authored by: @therealyingtong --- qa/pull-tester/rpc-tests.py | 3 +- qa/rpc-tests/wallet_orchard_persistence.py | 104 +++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100755 qa/rpc-tests/wallet_orchard_persistence.py diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index e40a91d20..194332266 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -56,6 +56,7 @@ BASE_SCRIPTS= [ 'wallet_listreceived.py', 'mempool_tx_expiry.py', 'finalsaplingroot.py', + 'wallet_orchard.py', 'wallet_overwintertx.py', 'wallet_persistence.py', 'wallet_listnotes.py', @@ -74,7 +75,7 @@ BASE_SCRIPTS= [ 'wallet_doublespend.py', 'wallet_import_export.py', 'wallet_isfromme.py', - 'wallet_orchard.py', + 'wallet_orchard_persistence.py', 'wallet_nullifiers.py', 'wallet_sapling.py', 'wallet_sendmany_any_taddr.py', diff --git a/qa/rpc-tests/wallet_orchard_persistence.py b/qa/rpc-tests/wallet_orchard_persistence.py new file mode 100755 index 000000000..0e5814bba --- /dev/null +++ b/qa/rpc-tests/wallet_orchard_persistence.py @@ -0,0 +1,104 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://www.opensource.org/licenses/mit-license.php . + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + NU5_BRANCH_ID, + assert_equal, + connect_nodes_bi, + get_coinbase_address, + nuparams, + start_nodes, + stop_nodes, + wait_bitcoinds, + wait_and_assert_operationid_status, +) + +from decimal import Decimal + +# Test wallet behaviour with the Orchard protocol +class WalletOrchardTest(BitcoinTestFramework): + def __init__(self): + super().__init__() + self.num_nodes = 4 + + def setup_nodes(self): + return start_nodes(self.num_nodes, self.options.tmpdir, [[ + nuparams(NU5_BRANCH_ID, 201), + ]] * self.num_nodes) + + def run_test(self): + # Sanity-check the test harness + assert_equal(self.nodes[0].getblockcount(), 200) + + # Send some sapling funds to node 2 for later spending after we split the network + acct0 = self.nodes[0].z_getnewaccount()['account'] + ua0 = self.nodes[0].z_getaddressforaccount(acct0, ['sapling', 'orchard'])['address'] + + recipients = [{"address": ua0, "amount": 10}] + myopid = self.nodes[0].z_sendmany(get_coinbase_address(self.nodes[0]), recipients, 1, 0, 'AllowRevealedSenders') + wait_and_assert_operationid_status(self.nodes[0], myopid) + + # Mine the tx & activate NU5 + self.sync_all() + self.nodes[0].generate(5) + self.sync_all() + + assert_equal( + {'pools': {'orchard': {'valueZat': 10_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[0].z_getbalanceforaccount(acct0)) + + # Send to a new orchard-only unified address + acct1 = self.nodes[1].z_getnewaccount()['account'] + ua1 = self.nodes[1].z_getaddressforaccount(acct1, ['orchard'])['address'] + + recipients = [{"address": ua1, "amount": 1}] + myopid = self.nodes[0].z_sendmany(ua0, recipients, 1, 0) + source_tx = wait_and_assert_operationid_status(self.nodes[0], myopid) + + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + assert_equal( + {'pools': {'orchard': {'valueZat': 9_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[0].z_getbalanceforaccount(acct0)) + assert_equal( + {'pools': {'orchard': {'valueZat': 1_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[1].z_getbalanceforaccount(acct1)) + + # Send another Orchard transaction from node 0 back to itself, so that the + # note commitment tree gets advanced. + recipients = [{"address": ua0, "amount": 1}] + myopid = self.nodes[0].z_sendmany(ua0, recipients, 1, 0) + source_tx = wait_and_assert_operationid_status(self.nodes[0], myopid) + + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + # Shut down the nodes, and restart so that we can check wallet load + stop_nodes(self.nodes); + wait_bitcoinds() + self.setup_network() + + assert_equal( + {'pools': {'orchard': {'valueZat': 9_0000_0000}}, 'minimum_confirmations': 1}, + self.nodes[0].z_getbalanceforaccount(acct0)) + + recipients = [{"address": ua0, "amount": Decimal('0.5')}] + myopid = self.nodes[1].z_sendmany(ua1, recipients, 1, 0) + txid = wait_and_assert_operationid_status(self.nodes[1], myopid) + + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + assert_equal( + {'pools': {'orchard': {'valueZat': 9_5000_0000}}, 'minimum_confirmations': 1}, + self.nodes[0].z_getbalanceforaccount(acct0)) + +if __name__ == '__main__': + WalletOrchardTest().main() From bc33ba5a9f0394495c4f7b88853958732c7b92ef Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Wed, 30 Mar 2022 20:36:59 +0800 Subject: [PATCH 2/3] Update FFI to use scoped APIs for viewing keys and addresses --- Cargo.lock | 3 +-- Cargo.toml | 1 + src/rust/src/orchard_keys_ffi.rs | 14 +++++--------- src/rust/src/wallet.rs | 6 +++--- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 37616ff3d..427f1593f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1260,8 +1260,7 @@ checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" [[package]] name = "orchard" version = "0.1.0-beta.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b48e6e124c3f7e9ed7f48b29f66069288235cecd16aa6053e98f8aff16efe827" +source = "git+https://github.com/zcash/orchard.git?rev=eaa0cfdbf6a52a023752ac8e29d10308318424a0#eaa0cfdbf6a52a023752ac8e29d10308318424a0" dependencies = [ "aes", "arrayvec 0.7.2", diff --git a/Cargo.toml b/Cargo.toml index afe54f261..5ed7955df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -93,3 +93,4 @@ zcash_history = { git = "https://github.com/zcash/librustzcash.git", rev = "9c1e zcash_note_encryption = { git = "https://github.com/zcash/librustzcash.git", rev = "9c1ed86c5aa8ae3b6d6dcc1478f2d6ba1264488f" } zcash_primitives = { git = "https://github.com/zcash/librustzcash.git", rev = "9c1ed86c5aa8ae3b6d6dcc1478f2d6ba1264488f" } zcash_proofs = { git = "https://github.com/zcash/librustzcash.git", rev = "9c1ed86c5aa8ae3b6d6dcc1478f2d6ba1264488f" } +orchard = { git = "https://github.com/zcash/orchard.git", rev = "eaa0cfdbf6a52a023752ac8e29d10308318424a0" } diff --git a/src/rust/src/orchard_keys_ffi.rs b/src/rust/src/orchard_keys_ffi.rs index 079de6439..f992a3759 100644 --- a/src/rust/src/orchard_keys_ffi.rs +++ b/src/rust/src/orchard_keys_ffi.rs @@ -3,7 +3,7 @@ use std::slice; use tracing::error; use orchard::{ - keys::{DiversifierIndex, FullViewingKey, IncomingViewingKey, OutgoingViewingKey, SpendingKey}, + keys::{DiversifierIndex, FullViewingKey, IncomingViewingKey, Scope, SpendingKey}, Address, }; @@ -268,7 +268,7 @@ pub extern "C" fn orchard_full_viewing_key_to_incoming_viewing_key( key: *const FullViewingKey, ) -> *mut IncomingViewingKey { unsafe { key.as_ref() } - .map(|key| Box::into_raw(Box::new(IncomingViewingKey::from(key)))) + .map(|key| Box::into_raw(Box::new(key.to_ivk(Scope::External)))) .unwrap_or(std::ptr::null_mut()) } @@ -277,10 +277,7 @@ pub extern "C" fn orchard_full_viewing_key_to_internal_incoming_viewing_key( fvk: *const FullViewingKey, ) -> *mut IncomingViewingKey { unsafe { fvk.as_ref() } - .map(|fvk| { - let internal_fvk = fvk.derive_internal(); - Box::into_raw(Box::new(IncomingViewingKey::from(&internal_fvk))) - }) + .map(|fvk| Box::into_raw(Box::new(fvk.to_ivk(Scope::Internal)))) .unwrap_or(std::ptr::null_mut()) } @@ -292,7 +289,7 @@ pub extern "C" fn orchard_full_viewing_key_to_external_outgoing_viewing_key( let fvk = unsafe { fvk.as_ref() }.expect("fvk must not be null"); let ovk_ret = unsafe { ovk_ret.as_mut() }.expect("ovk_ret must not be null"); - let ovk = OutgoingViewingKey::from(fvk); + let ovk = fvk.to_ovk(Scope::External); *ovk_ret = *ovk.as_ref(); } @@ -304,8 +301,7 @@ pub extern "C" fn orchard_full_viewing_key_to_internal_outgoing_viewing_key( let fvk = unsafe { fvk.as_ref() }.expect("fvk must not be null"); let ovk_ret = unsafe { ovk_ret.as_mut() }.expect("ovk_ret must not be null"); - let internal_fvk = fvk.derive_internal(); - let ovk = OutgoingViewingKey::from(&internal_fvk); + let ovk = fvk.to_ovk(Scope::Internal); *ovk_ret = *ovk.as_ref(); } diff --git a/src/rust/src/wallet.rs b/src/rust/src/wallet.rs index e2be5ed12..d1fc6ea18 100644 --- a/src/rust/src/wallet.rs +++ b/src/rust/src/wallet.rs @@ -16,7 +16,7 @@ use zcash_primitives::{ use orchard::{ bundle::Authorized, - keys::{FullViewingKey, IncomingViewingKey, OutgoingViewingKey, SpendingKey}, + keys::{FullViewingKey, IncomingViewingKey, OutgoingViewingKey, Scope, SpendingKey}, note::Nullifier, tree::{MerkleHashOrchard, MerklePath}, Address, Bundle, Note, @@ -92,8 +92,8 @@ impl KeyStore { pub fn add_full_viewing_key(&mut self, fvk: FullViewingKey) { // When we add a full viewing key, we need to add both the internal and external // incoming viewing keys. - let external_ivk = IncomingViewingKey::from(&fvk); - let internal_ivk = IncomingViewingKey::from(&fvk.derive_internal()); + let external_ivk = fvk.to_ivk(Scope::External); + let internal_ivk = fvk.to_ivk(Scope::Internal); self.viewing_keys.insert(external_ivk, fvk.clone()); self.viewing_keys.insert(internal_ivk, fvk); } From 53cc7ecceb237efb7c4d2498df21f719b849cc70 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 30 Mar 2022 10:08:00 -0600 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: str4d --- qa/rpc-tests/wallet_orchard_persistence.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/qa/rpc-tests/wallet_orchard_persistence.py b/qa/rpc-tests/wallet_orchard_persistence.py index 0e5814bba..827007655 100755 --- a/qa/rpc-tests/wallet_orchard_persistence.py +++ b/qa/rpc-tests/wallet_orchard_persistence.py @@ -7,7 +7,6 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( NU5_BRANCH_ID, assert_equal, - connect_nodes_bi, get_coinbase_address, nuparams, start_nodes, @@ -19,7 +18,7 @@ from test_framework.util import ( from decimal import Decimal # Test wallet behaviour with the Orchard protocol -class WalletOrchardTest(BitcoinTestFramework): +class WalletOrchardPersistenceTest(BitcoinTestFramework): def __init__(self): super().__init__() self.num_nodes = 4 @@ -33,7 +32,7 @@ class WalletOrchardTest(BitcoinTestFramework): # Sanity-check the test harness assert_equal(self.nodes[0].getblockcount(), 200) - # Send some sapling funds to node 2 for later spending after we split the network + # Send some Orchard funds to node 2 for later spending after we split the network acct0 = self.nodes[0].z_getnewaccount()['account'] ua0 = self.nodes[0].z_getaddressforaccount(acct0, ['sapling', 'orchard'])['address'] @@ -43,7 +42,7 @@ class WalletOrchardTest(BitcoinTestFramework): # Mine the tx & activate NU5 self.sync_all() - self.nodes[0].generate(5) + self.nodes[0].generate(1) self.sync_all() assert_equal( @@ -56,7 +55,7 @@ class WalletOrchardTest(BitcoinTestFramework): recipients = [{"address": ua1, "amount": 1}] myopid = self.nodes[0].z_sendmany(ua0, recipients, 1, 0) - source_tx = wait_and_assert_operationid_status(self.nodes[0], myopid) + wait_and_assert_operationid_status(self.nodes[0], myopid) self.sync_all() self.nodes[0].generate(1) @@ -73,7 +72,7 @@ class WalletOrchardTest(BitcoinTestFramework): # note commitment tree gets advanced. recipients = [{"address": ua0, "amount": 1}] myopid = self.nodes[0].z_sendmany(ua0, recipients, 1, 0) - source_tx = wait_and_assert_operationid_status(self.nodes[0], myopid) + wait_and_assert_operationid_status(self.nodes[0], myopid) self.sync_all() self.nodes[0].generate(1) @@ -90,7 +89,7 @@ class WalletOrchardTest(BitcoinTestFramework): recipients = [{"address": ua0, "amount": Decimal('0.5')}] myopid = self.nodes[1].z_sendmany(ua1, recipients, 1, 0) - txid = wait_and_assert_operationid_status(self.nodes[1], myopid) + wait_and_assert_operationid_status(self.nodes[1], myopid) self.sync_all() self.nodes[0].generate(1) @@ -101,4 +100,4 @@ class WalletOrchardTest(BitcoinTestFramework): self.nodes[0].z_getbalanceforaccount(acct0)) if __name__ == '__main__': - WalletOrchardTest().main() + WalletOrchardPersistenceTest().main()