From ebf4c0671e617f0cbbf59967e65eb7da758058cf Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Sep 2018 09:27:28 +0100 Subject: [PATCH 01/31] net: Check against the current epoch's version when rejecting nodes --- src/main.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 54928d3f1..9b0ce308d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -5016,15 +5016,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, return false; } - // When Overwinter is active, reject incoming connections from non-Overwinter nodes + // Reject incoming connections from nodes that don't know about the current epoch const Consensus::Params& params = Params().GetConsensus(); - if (NetworkUpgradeActive(GetHeight(), params, Consensus::UPGRADE_OVERWINTER) - && pfrom->nVersion < params.vUpgrades[Consensus::UPGRADE_OVERWINTER].nProtocolVersion) + auto currentEpoch = CurrentEpoch(GetHeight(), params); + if (pfrom->nVersion < params.vUpgrades[currentEpoch].nProtocolVersion) { LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->id, pfrom->nVersion); pfrom->PushMessage("reject", strCommand, REJECT_OBSOLETE, strprintf("Version must be %d or greater", - params.vUpgrades[Consensus::UPGRADE_OVERWINTER].nProtocolVersion)); + params.vUpgrades[currentEpoch].nProtocolVersion)); pfrom->fDisconnect = true; return false; } From 07d3f947ec3725640bf07bdd0de34f910aaca349 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Sep 2018 10:07:44 +0100 Subject: [PATCH 02/31] Extract a helper method for finding the next epoch --- src/consensus/upgrades.cpp | 20 ++++++++++++++------ src/consensus/upgrades.h | 6 ++++++ src/gtest/test_upgrades.cpp | 29 +++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/consensus/upgrades.cpp b/src/consensus/upgrades.cpp index 95bde7925..d346f90b1 100644 --- a/src/consensus/upgrades.cpp +++ b/src/consensus/upgrades.cpp @@ -114,20 +114,28 @@ bool IsActivationHeightForAnyUpgrade( return false; } -boost::optional NextActivationHeight( - int nHeight, - const Consensus::Params& params) -{ +boost::optional NextEpoch(int nHeight, const Consensus::Params& params) { if (nHeight < 0) { return boost::none; } - // Don't count Sprout as an activation height + // Sprout is never pending for (auto idx = Consensus::BASE_SPROUT + 1; idx < Consensus::MAX_NETWORK_UPGRADES; idx++) { if (NetworkUpgradeState(nHeight, params, Consensus::UpgradeIndex(idx)) == UPGRADE_PENDING) { - return params.vUpgrades[idx].nActivationHeight; + return idx; } } return boost::none; } + +boost::optional NextActivationHeight( + int nHeight, + const Consensus::Params& params) +{ + auto idx = NextEpoch(nHeight, params); + if (idx) { + return params.vUpgrades[idx.get()].nActivationHeight; + } + return boost::none; +} diff --git a/src/consensus/upgrades.h b/src/consensus/upgrades.h index 620dc94c4..913a316cb 100644 --- a/src/consensus/upgrades.h +++ b/src/consensus/upgrades.h @@ -79,6 +79,12 @@ bool IsActivationHeightForAnyUpgrade( int nHeight, const Consensus::Params& params); +/** + * Returns the index of the next upgrade after the given block height, or + * boost::none if there are no more known upgrades. + */ +boost::optional NextEpoch(int nHeight, const Consensus::Params& params); + /** * Returns the activation height for the next upgrade after the given block height, * or boost::none if there are no more known upgrades. diff --git a/src/gtest/test_upgrades.cpp b/src/gtest/test_upgrades.cpp index 1066a28d0..54fa48832 100644 --- a/src/gtest/test_upgrades.cpp +++ b/src/gtest/test_upgrades.cpp @@ -143,6 +143,35 @@ TEST_F(UpgradesTest, IsActivationHeightForAnyUpgrade) { EXPECT_FALSE(IsActivationHeightForAnyUpgrade(1000000, params)); } +TEST_F(UpgradesTest, NextEpoch) { + SelectParams(CBaseChainParams::REGTEST); + const Consensus::Params& params = Params().GetConsensus(); + + // Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT + EXPECT_EQ(NextEpoch(-1, params), boost::none); + EXPECT_EQ(NextEpoch(0, params), boost::none); + EXPECT_EQ(NextEpoch(1, params), boost::none); + EXPECT_EQ(NextEpoch(1000000, params), boost::none); + + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_TESTDUMMY, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + + EXPECT_EQ(NextEpoch(-1, params), boost::none); + EXPECT_EQ(NextEpoch(0, params), boost::none); + EXPECT_EQ(NextEpoch(1, params), boost::none); + EXPECT_EQ(NextEpoch(1000000, params), boost::none); + + int nActivationHeight = 100; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_TESTDUMMY, nActivationHeight); + + EXPECT_EQ(NextEpoch(-1, params), boost::none); + EXPECT_EQ(NextEpoch(0, params), static_cast(Consensus::UPGRADE_TESTDUMMY)); + EXPECT_EQ(NextEpoch(1, params), static_cast(Consensus::UPGRADE_TESTDUMMY)); + EXPECT_EQ(NextEpoch(nActivationHeight - 1, params), static_cast(Consensus::UPGRADE_TESTDUMMY)); + EXPECT_EQ(NextEpoch(nActivationHeight, params), boost::none); + EXPECT_EQ(NextEpoch(nActivationHeight + 1, params), boost::none); + EXPECT_EQ(NextEpoch(1000000, params), boost::none); +} + TEST_F(UpgradesTest, NextActivationHeight) { SelectParams(CBaseChainParams::REGTEST); const Consensus::Params& params = Params().GetConsensus(); From feee210af6f756ffdd84e1261ce4cdda7e5d7f22 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Sep 2018 10:10:22 +0100 Subject: [PATCH 03/31] net: Check against the next epoch's version when evicting peers --- src/net.cpp | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 1269c8c25..475643199 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -820,22 +820,26 @@ static bool AttemptToEvictConnection(bool fPreferNewConnection) { } const Consensus::Params& params = Params().GetConsensus(); - int nActivationHeight = params.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight; + auto nextEpoch = NextEpoch(height, params); + if (nextEpoch) { + auto idx = nextEpoch.get(); + int nActivationHeight = params.vUpgrades[idx].nActivationHeight; - if (nActivationHeight > 0 && - height < nActivationHeight && - height >= nActivationHeight - NETWORK_UPGRADE_PEER_PREFERENCE_BLOCK_PERIOD) - { - // Find any nodes which don't support Overwinter protocol version - BOOST_FOREACH(const CNodeRef &node, vEvictionCandidates) { - if (node->nVersion < params.vUpgrades[Consensus::UPGRADE_SAPLING].nProtocolVersion) { - vTmpEvictionCandidates.push_back(node); + if (nActivationHeight > 0 && + height < nActivationHeight && + height >= nActivationHeight - NETWORK_UPGRADE_PEER_PREFERENCE_BLOCK_PERIOD) + { + // Find any nodes which don't support the protocol version for the next upgrade + for (const CNodeRef &node : vEvictionCandidates) { + if (node->nVersion < params.vUpgrades[idx].nProtocolVersion) { + vTmpEvictionCandidates.push_back(node); + } } - } - // Prioritize these nodes by replacing eviction set with them - if (vTmpEvictionCandidates.size() > 0) { - vEvictionCandidates = vTmpEvictionCandidates; + // Prioritize these nodes by replacing eviction set with them + if (vTmpEvictionCandidates.size() > 0) { + vEvictionCandidates = vTmpEvictionCandidates; + } } } From c861137624166b2564a91c6f2f6c5644128b30f1 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Sep 2018 10:17:33 +0100 Subject: [PATCH 04/31] net: Check against the current epoch's version when disconnecting peers --- src/main.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 9b0ce308d..ecc5645da 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -5149,15 +5149,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // Disconnect existing peer connection when: // 1. The version message has been received - // 2. Overwinter is active - // 3. Peer version is pre-Overwinter - else if (NetworkUpgradeActive(GetHeight(), chainparams.GetConsensus(), Consensus::UPGRADE_OVERWINTER) - && (pfrom->nVersion < chainparams.GetConsensus().vUpgrades[Consensus::UPGRADE_OVERWINTER].nProtocolVersion)) + // 2. Peer version is below the minimum version for the current epoch + else if (pfrom->nVersion < chainparams.GetConsensus().vUpgrades[ + CurrentEpoch(GetHeight(), chainparams.GetConsensus())].nProtocolVersion) { LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->id, pfrom->nVersion); pfrom->PushMessage("reject", strCommand, REJECT_OBSOLETE, strprintf("Version must be %d or greater", - chainparams.GetConsensus().vUpgrades[Consensus::UPGRADE_OVERWINTER].nProtocolVersion)); + chainparams.GetConsensus().vUpgrades[ + CurrentEpoch(GetHeight(), chainparams.GetConsensus())].nProtocolVersion)); pfrom->fDisconnect = true; return false; } From 8e057ad9ee89611227c7b180c61ebd4de8d9a541 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 12 Sep 2018 12:11:10 +0100 Subject: [PATCH 05/31] qa: Test both Overwinter and Sapling peer management --- qa/pull-tester/rpc-tests.sh | 2 +- qa/rpc-tests/overwinter_peer_management.py | 118 ------------- qa/rpc-tests/p2p_nu_peer_management.py | 192 +++++++++++++++++++++ qa/rpc-tests/test_framework/mininode.py | 3 +- 4 files changed, 195 insertions(+), 120 deletions(-) delete mode 100755 qa/rpc-tests/overwinter_peer_management.py create mode 100755 qa/rpc-tests/p2p_nu_peer_management.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 25ecc409a..0bc6fefaa 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -57,7 +57,7 @@ testScripts=( 'getblocktemplate.py' 'bip65-cltv-p2p.py' 'bipdersig-p2p.py' - 'overwinter_peer_management.py' + 'p2p_nu_peer_management.py' 'rewind_index.py' 'p2p_txexpiry_dos.py' 'p2p_node_bloom.py' diff --git a/qa/rpc-tests/overwinter_peer_management.py b/qa/rpc-tests/overwinter_peer_management.py deleted file mode 100755 index a5017b8b0..000000000 --- a/qa/rpc-tests/overwinter_peer_management.py +++ /dev/null @@ -1,118 +0,0 @@ -#!/usr/bin/env python2 -# Copyright (c) 2018 The Zcash developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -from test_framework.mininode import NodeConn, NodeConnCB, NetworkThread, \ - msg_ping, SPROUT_PROTO_VERSION, OVERWINTER_PROTO_VERSION -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import initialize_chain_clean, start_nodes, \ - p2p_port, assert_equal - -import time - -# -# In this test we connect Sprout and Overwinter mininodes to a Zcashd node -# which will activate Overwinter at block 10. -# -# We test: -# 1. the mininodes stay connected to Zcash with Sprout consensus rules -# 2. when Overwinter activates, the Sprout mininodes are dropped -# 3. new Overwinter nodes can connect to Zcash -# 4. new Sprout nodes cannot connect to Zcash -# -# This test *does not* verify that prior to Overwinter activation, the Zcashd -# node will prefer connections with Overwinter nodes, with an eviction process -# that prioritizes Sprout connections. -# - - -class TestManager(NodeConnCB): - def __init__(self): - NodeConnCB.__init__(self) - self.create_callback_map() - - def on_close(self, conn): - pass - - def on_reject(self, conn, message): - conn.rejectMessage = message - - -class OverwinterPeerManagementTest(BitcoinTestFramework): - - def setup_chain(self): - print "Initializing test directory "+self.options.tmpdir - initialize_chain_clean(self.options.tmpdir, 1) - - def setup_network(self): - self.nodes = start_nodes(1, self.options.tmpdir, - extra_args=[['-nuparams=5ba81b19:10', '-debug', '-whitelist=127.0.0.1']]) - - def run_test(self): - test = TestManager() - - # Launch 10 Sprout and 10 Overwinter mininodes - nodes = [] - for x in xrange(10): - nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], - test, "regtest", SPROUT_PROTO_VERSION)) - nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], - test, "regtest", OVERWINTER_PROTO_VERSION)) - - # Start up network handling in another thread - NetworkThread().start() - - # Sprout consensus rules apply at block height 9 - self.nodes[0].generate(9) - assert_equal(9, self.nodes[0].getblockcount()) - - # Verify mininodes are still connected to zcashd node - peerinfo = self.nodes[0].getpeerinfo() - versions = [x["version"] for x in peerinfo] - assert_equal(10, versions.count(SPROUT_PROTO_VERSION)) - assert_equal(10, versions.count(OVERWINTER_PROTO_VERSION)) - - # Overwinter consensus rules activate at block height 10 - self.nodes[0].generate(1) - assert_equal(10, self.nodes[0].getblockcount()) - - # Mininodes send ping message to zcashd node. - pingCounter = 1 - for node in nodes: - node.send_message(msg_ping(pingCounter)) - pingCounter = pingCounter + 1 - - time.sleep(3) - - # Verify Sprout mininodes have been dropped and Overwinter mininodes are still connected. - peerinfo = self.nodes[0].getpeerinfo() - versions = [x["version"] for x in peerinfo] - assert_equal(0, versions.count(SPROUT_PROTO_VERSION)) - assert_equal(10, versions.count(OVERWINTER_PROTO_VERSION)) - - # Extend the Overwinter chain with another block. - self.nodes[0].generate(1) - - # Connect a new Overwinter mininode to the zcashd node, which is accepted. - nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test, "regtest", OVERWINTER_PROTO_VERSION)) - time.sleep(3) - assert_equal(11, len(self.nodes[0].getpeerinfo())) - - # Try to connect a new Sprout mininode to the zcashd node, which is rejected. - sprout = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test, "regtest", SPROUT_PROTO_VERSION) - nodes.append(sprout) - time.sleep(3) - assert("Version must be 170003 or greater" in str(sprout.rejectMessage)) - - # Verify that only Overwinter mininodes are connected. - peerinfo = self.nodes[0].getpeerinfo() - versions = [x["version"] for x in peerinfo] - assert_equal(0, versions.count(SPROUT_PROTO_VERSION)) - assert_equal(11, versions.count(OVERWINTER_PROTO_VERSION)) - - for node in nodes: - node.disconnect_node() - -if __name__ == '__main__': - OverwinterPeerManagementTest().main() diff --git a/qa/rpc-tests/p2p_nu_peer_management.py b/qa/rpc-tests/p2p_nu_peer_management.py new file mode 100755 index 000000000..6cedf66bb --- /dev/null +++ b/qa/rpc-tests/p2p_nu_peer_management.py @@ -0,0 +1,192 @@ +#!/usr/bin/env python2 +# Copyright (c) 2018 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.mininode import ( + NodeConn, + NodeConnCB, + NetworkThread, + msg_ping, + SPROUT_PROTO_VERSION, + OVERWINTER_PROTO_VERSION, + SAPLING_PROTO_VERSION, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import initialize_chain_clean, start_nodes, \ + p2p_port, assert_equal + +import time + +# +# In this test we connect Sprout, Overwinter, and Sapling mininodes to a Zcashd +# node which will activate Overwinter at block 10 and Sapling at block 15. +# +# We test: +# 1. the mininodes stay connected to Zcash with Sprout consensus rules +# 2. when Overwinter activates, the Sprout mininodes are dropped +# 3. new Overwinter and Sapling nodes can connect to Zcash +# 4. new Sprout nodes cannot connect to Zcash +# 5. when Sapling activates, the Overwinter mininodes are dropped +# 6. new Sapling nodes can connect to Zcash +# 7. new Sprout and Overwinter nodes cannot connect to Zcash +# +# This test *does not* verify that prior to each activation, the Zcashd +# node will prefer connections with NU-aware nodes, with an eviction process +# that prioritizes non-NU-aware connections. +# + + +class TestManager(NodeConnCB): + def __init__(self): + NodeConnCB.__init__(self) + self.create_callback_map() + + def on_close(self, conn): + pass + + def on_reject(self, conn, message): + conn.rejectMessage = message + + +class NUPeerManagementTest(BitcoinTestFramework): + + def setup_chain(self): + print "Initializing test directory "+self.options.tmpdir + initialize_chain_clean(self.options.tmpdir, 1) + + def setup_network(self): + self.nodes = start_nodes(1, self.options.tmpdir, extra_args=[[ + '-nuparams=5ba81b19:10', # Overwinter + '-nuparams=76b809bb:15', # Sapling + '-debug', + '-whitelist=127.0.0.1', + ]]) + + def run_test(self): + test = TestManager() + + # Launch Sprout, Overwinter, and Sapling mininodes + nodes = [] + for x in xrange(10): + nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], + test, "regtest", SPROUT_PROTO_VERSION)) + nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], + test, "regtest", OVERWINTER_PROTO_VERSION)) + nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], + test, "regtest", SAPLING_PROTO_VERSION)) + + # Start up network handling in another thread + NetworkThread().start() + + # Sprout consensus rules apply at block height 9 + self.nodes[0].generate(9) + assert_equal(9, self.nodes[0].getblockcount()) + + # Verify mininodes are still connected to zcashd node + peerinfo = self.nodes[0].getpeerinfo() + versions = [x["version"] for x in peerinfo] + assert_equal(10, versions.count(SPROUT_PROTO_VERSION)) + assert_equal(10, versions.count(OVERWINTER_PROTO_VERSION)) + assert_equal(10, versions.count(SAPLING_PROTO_VERSION)) + + # Overwinter consensus rules activate at block height 10 + self.nodes[0].generate(1) + assert_equal(10, self.nodes[0].getblockcount()) + print('Overwinter active') + + # Mininodes send ping message to zcashd node. + pingCounter = 1 + for node in nodes: + node.send_message(msg_ping(pingCounter)) + pingCounter = pingCounter + 1 + + time.sleep(3) + + # Verify Sprout mininodes have been dropped, while Overwinter and + # Sapling mininodes are still connected. + peerinfo = self.nodes[0].getpeerinfo() + versions = [x["version"] for x in peerinfo] + assert_equal(0, versions.count(SPROUT_PROTO_VERSION)) + assert_equal(10, versions.count(OVERWINTER_PROTO_VERSION)) + assert_equal(10, versions.count(SAPLING_PROTO_VERSION)) + + # Extend the Overwinter chain with another block. + self.nodes[0].generate(1) + + # Connect a new Overwinter mininode to the zcashd node, which is accepted. + nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test, "regtest", OVERWINTER_PROTO_VERSION)) + time.sleep(3) + assert_equal(21, len(self.nodes[0].getpeerinfo())) + + # Connect a new Sapling mininode to the zcashd node, which is accepted. + nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test, "regtest", SAPLING_PROTO_VERSION)) + time.sleep(3) + assert_equal(22, len(self.nodes[0].getpeerinfo())) + + # Try to connect a new Sprout mininode to the zcashd node, which is rejected. + sprout = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test, "regtest", SPROUT_PROTO_VERSION) + nodes.append(sprout) + time.sleep(3) + assert("Version must be 170003 or greater" in str(sprout.rejectMessage)) + + # Verify that only Overwinter and Sapling mininodes are connected. + peerinfo = self.nodes[0].getpeerinfo() + versions = [x["version"] for x in peerinfo] + assert_equal(0, versions.count(SPROUT_PROTO_VERSION)) + assert_equal(11, versions.count(OVERWINTER_PROTO_VERSION)) + assert_equal(11, versions.count(SAPLING_PROTO_VERSION)) + + # Sapling consensus rules activate at block height 15 + self.nodes[0].generate(4) + assert_equal(15, self.nodes[0].getblockcount()) + print('Sapling active') + + # Mininodes send ping message to zcashd node. + pingCounter = 1 + for node in nodes: + node.send_message(msg_ping(pingCounter)) + pingCounter = pingCounter + 1 + + time.sleep(3) + + # Verify Sprout and Overwinter mininodes have been dropped, while + # Sapling mininodes are still connected. + peerinfo = self.nodes[0].getpeerinfo() + versions = [x["version"] for x in peerinfo] + assert_equal(0, versions.count(SPROUT_PROTO_VERSION)) + assert_equal(0, versions.count(OVERWINTER_PROTO_VERSION)) + assert_equal(11, versions.count(SAPLING_PROTO_VERSION)) + + # Extend the Sapling chain with another block. + self.nodes[0].generate(1) + + # Connect a new Sapling mininode to the zcashd node, which is accepted. + nodes.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test, "regtest", SAPLING_PROTO_VERSION)) + time.sleep(3) + assert_equal(12, len(self.nodes[0].getpeerinfo())) + + # Try to connect a new Sprout mininode to the zcashd node, which is rejected. + sprout = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test, "regtest", SPROUT_PROTO_VERSION) + nodes.append(sprout) + time.sleep(3) + assert("Version must be 170006 or greater" in str(sprout.rejectMessage)) + + # Try to connect a new Overwinter mininode to the zcashd node, which is rejected. + sprout = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test, "regtest", OVERWINTER_PROTO_VERSION) + nodes.append(sprout) + time.sleep(3) + assert("Version must be 170006 or greater" in str(sprout.rejectMessage)) + + # Verify that only Sapling mininodes are connected. + peerinfo = self.nodes[0].getpeerinfo() + versions = [x["version"] for x in peerinfo] + assert_equal(0, versions.count(SPROUT_PROTO_VERSION)) + assert_equal(0, versions.count(OVERWINTER_PROTO_VERSION)) + assert_equal(12, versions.count(SAPLING_PROTO_VERSION)) + + for node in nodes: + node.disconnect_node() + +if __name__ == '__main__': + NUPeerManagementTest().main() diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index 39b634a5e..cd29c1791 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -42,6 +42,7 @@ from .equihash import ( OVERWINTER_PROTO_VERSION = 170003 BIP0031_VERSION = 60000 SPROUT_PROTO_VERSION = 170002 # past bip-31 for ping/pong +SAPLING_PROTO_VERSION = 170006 MY_SUBVERSION = "/python-mininode-tester:0.0.1/" OVERWINTER_VERSION_GROUP_ID = 0x03C48270 @@ -1415,7 +1416,7 @@ class NodeConn(asyncore.dispatcher): vt.addrFrom.port = 0 self.send_message(vt, True) print 'MiniNode: Connecting to Bitcoin Node IP # ' + dstaddr + ':' \ - + str(dstport) + + str(dstport) + ' using version ' + str(protocol_version) try: self.connect((dstaddr, dstport)) From 6206d86237e6056d9618ea51032bfbc42af255f1 Mon Sep 17 00:00:00 2001 From: Larry Ruane Date: Wed, 8 Aug 2018 16:19:12 -0600 Subject: [PATCH 06/31] Test peer banning logic in both pre- and post-initial block download states --- src/gtest/test_checktransaction.cpp | 15 ++++++++++++--- src/main.cpp | 12 +++++++++--- src/main.h | 3 ++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 8e82f3850..c6f66cc64 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -506,8 +506,11 @@ TEST(checktransaction_tests, bad_txns_invalid_joinsplit_signature) { CTransaction tx(mtx); MockCValidationState state; + // during initial block download, DoS ban score should be zero, else 100 EXPECT_CALL(state, DoS(0, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false)).Times(1); - ContextualCheckTransaction(tx, state, 0, 100); + ContextualCheckTransaction(tx, state, 0, 100, []() { return true; }); + EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false)).Times(1); + ContextualCheckTransaction(tx, state, 0, 100, []() { return false; }); } TEST(checktransaction_tests, non_canonical_ed25519_signature) { @@ -539,8 +542,11 @@ TEST(checktransaction_tests, non_canonical_ed25519_signature) { CTransaction tx(mtx); MockCValidationState state; + // during initial block download, DoS ban score should be zero, else 100 EXPECT_CALL(state, DoS(0, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false)).Times(1); - ContextualCheckTransaction(tx, state, 0, 100); + ContextualCheckTransaction(tx, state, 0, 100, []() { return true; }); + EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false)).Times(1); + ContextualCheckTransaction(tx, state, 0, 100, []() { return false; }); } TEST(checktransaction_tests, OverwinterConstructors) { @@ -829,8 +835,11 @@ TEST(checktransaction_tests, OverwinterNotActive) { CTransaction tx(mtx); MockCValidationState state; + // during initial block download, DoS ban score should be zero, else 100 EXPECT_CALL(state, DoS(0, false, REJECT_INVALID, "tx-overwinter-not-active", false)).Times(1); - ContextualCheckTransaction(tx, state, 1, 100); + ContextualCheckTransaction(tx, state, 1, 100, []() { return true; }); + EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "tx-overwinter-not-active", false)).Times(1); + ContextualCheckTransaction(tx, state, 1, 100, []() { return false; }); } // This tests a transaction without the fOverwintered flag set, against the Overwinter consensus rule set. diff --git a/src/main.cpp b/src/main.cpp index 54928d3f1..2fe8a9206 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -877,8 +877,14 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in * 1. AcceptToMemoryPool calls CheckTransaction and this function. * 2. ProcessNewBlock calls AcceptBlock, which calls CheckBlock (which calls CheckTransaction) * and ContextualCheckBlock (which calls this function). + * 3. The isInitBlockDownload argument is only to assist with testing. */ -bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, const int nHeight, const int dosLevel) +bool ContextualCheckTransaction( + const CTransaction& tx, + CValidationState &state, + const int nHeight, + const int dosLevel, + bool (*isInitBlockDownload)()) { bool overwinterActive = NetworkUpgradeActive(nHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER); bool saplingActive = NetworkUpgradeActive(nHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING); @@ -886,7 +892,7 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, // If Sprout rules apply, reject transactions which are intended for Overwinter and beyond if (isSprout && tx.fOverwintered) { - return state.DoS(IsInitialBlockDownload() ? 0 : dosLevel, + return state.DoS(isInitBlockDownload() ? 0 : dosLevel, error("ContextualCheckTransaction(): overwinter is not active yet"), REJECT_INVALID, "tx-overwinter-not-active"); } @@ -987,7 +993,7 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, dataToBeSigned.begin(), 32, tx.joinSplitPubKey.begin() ) != 0) { - return state.DoS(IsInitialBlockDownload() ? 0 : 100, + return state.DoS(isInitBlockDownload() ? 0 : 100, error("CheckTransaction(): invalid joinsplit signature"), REJECT_INVALID, "bad-txns-invalid-joinsplit-signature"); } diff --git a/src/main.h b/src/main.h index 9481a6090..2b3c70111 100644 --- a/src/main.h +++ b/src/main.h @@ -343,7 +343,8 @@ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, cons std::vector *pvChecks = NULL); /** Check a transaction contextually against a set of consensus rules */ -bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, int nHeight, int dosLevel); +bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, int nHeight, int dosLevel, + bool (*isInitBlockDownload)() = IsInitialBlockDownload); /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); From d6b31d59b50cd20b8416e46f01f1700af67865d3 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Mon, 27 Aug 2018 15:26:52 -0600 Subject: [PATCH 07/31] Rename map to include sprout --- src/wallet/gtest/test_wallet_zkeys.cpp | 6 +++--- src/wallet/rpcdump.cpp | 6 +++--- src/wallet/wallet.cpp | 16 ++++++++-------- src/wallet/wallet.h | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index 37f4b618a..8a2ecd8d3 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -116,7 +116,7 @@ TEST(wallet_zkeys_tests, store_and_load_zkeys) { ASSERT_TRUE(wallet.LoadZKeyMetadata(addr, meta)); // check metadata is the same - CKeyMetadata m= wallet.mapZKeyMetadata[addr]; + CKeyMetadata m= wallet.mapSproutZKeyMetadata[addr]; ASSERT_EQ(m.nCreateTime, now); } @@ -215,7 +215,7 @@ TEST(wallet_zkeys_tests, write_zkey_direct_to_db) { ASSERT_EQ(1, addrs.size()); // wallet should have default metadata for addr with null createtime - CKeyMetadata m = wallet.mapZKeyMetadata[addr]; + CKeyMetadata m = wallet.mapSproutZKeyMetadata[addr]; ASSERT_EQ(m.nCreateTime, 0); ASSERT_NE(m.nCreateTime, now); @@ -235,7 +235,7 @@ TEST(wallet_zkeys_tests, write_zkey_direct_to_db) { ASSERT_EQ(2, addrs.size()); // check metadata is now the same - m = wallet.mapZKeyMetadata[addr]; + m = wallet.mapSproutZKeyMetadata[addr]; ASSERT_EQ(m.nCreateTime, now); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 50d2ce78c..f6bc6528f 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -314,7 +314,7 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys continue; } // Successfully imported zaddr. Now import the metadata. - pwalletMain->mapZKeyMetadata[addr].nCreateTime = nTime; + pwalletMain->mapSproutZKeyMetadata[addr].nCreateTime = nTime; continue; } else { LogPrint("zrpc", "Importing detected an error: invalid spending key. Trying as a transparent key...\n"); @@ -537,7 +537,7 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) for (auto addr : addresses ) { libzcash::SproutSpendingKey key; if (pwalletMain->GetSproutSpendingKey(addr, key)) { - std::string strTime = EncodeDumpTime(pwalletMain->mapZKeyMetadata[addr].nCreateTime); + std::string strTime = EncodeDumpTime(pwalletMain->mapSproutZKeyMetadata[addr].nCreateTime); file << strprintf("%s %s # zaddr=%s\n", EncodeSpendingKey(key), strTime, EncodePaymentAddress(addr)); } } @@ -571,7 +571,7 @@ public: throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); } - m_wallet->mapZKeyMetadata[addr].nCreateTime = 1; + m_wallet->mapSproutZKeyMetadata[addr].nCreateTime = 1; return false; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d931f7384..f4e692219 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -83,7 +83,7 @@ const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const // Generate a new spending key and return its public payment address libzcash::PaymentAddress CWallet::GenerateNewZKey() { - AssertLockHeld(cs_wallet); // mapZKeyMetadata + AssertLockHeld(cs_wallet); // mapSproutZKeyMetadata // TODO: Add Sapling support auto k = SproutSpendingKey::random(); auto addr = k.address(); @@ -94,7 +94,7 @@ libzcash::PaymentAddress CWallet::GenerateNewZKey() // Create new metadata int64_t nCreationTime = GetTime(); - mapZKeyMetadata[addr] = CKeyMetadata(nCreationTime); + mapSproutZKeyMetadata[addr] = CKeyMetadata(nCreationTime); if (!AddZKey(k)) throw std::runtime_error("CWallet::GenerateNewZKey(): AddZKey failed"); @@ -171,7 +171,7 @@ bool CWallet::AddSaplingZKey( // Add spending key to keystore and persist to disk bool CWallet::AddZKey(const libzcash::SproutSpendingKey &key) { - AssertLockHeld(cs_wallet); // mapZKeyMetadata + AssertLockHeld(cs_wallet); // mapSproutZKeyMetadata auto addr = key.address(); if (!CCryptoKeyStore::AddSproutSpendingKey(key)) @@ -187,7 +187,7 @@ bool CWallet::AddZKey(const libzcash::SproutSpendingKey &key) if (!IsCrypted()) { return CWalletDB(strWalletFile).WriteZKey(addr, key, - mapZKeyMetadata[addr]); + mapSproutZKeyMetadata[addr]); } return true; } @@ -278,12 +278,12 @@ bool CWallet::AddCryptedSproutSpendingKey( return pwalletdbEncryption->WriteCryptedZKey(address, rk, vchCryptedSecret, - mapZKeyMetadata[address]); + mapSproutZKeyMetadata[address]); } else { return CWalletDB(strWalletFile).WriteCryptedZKey(address, rk, vchCryptedSecret, - mapZKeyMetadata[address]); + mapSproutZKeyMetadata[address]); } } return false; @@ -315,8 +315,8 @@ bool CWallet::LoadKeyMetadata(const CPubKey &pubkey, const CKeyMetadata &meta) bool CWallet::LoadZKeyMetadata(const SproutPaymentAddress &addr, const CKeyMetadata &meta) { - AssertLockHeld(cs_wallet); // mapZKeyMetadata - mapZKeyMetadata[addr] = meta; + AssertLockHeld(cs_wallet); // mapSproutZKeyMetadata + mapSproutZKeyMetadata[addr] = meta; return true; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index dda1e4dfe..e660c82df 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -845,7 +845,7 @@ public: std::set setKeyPool; std::map mapKeyMetadata; - std::map mapZKeyMetadata; + std::map mapSproutZKeyMetadata; std::map mapSaplingZKeyMetadata; typedef std::map MasterKeyMap; From 5e360fb29f1f3b8447d693c3b3bf720cc5e03006 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Wed, 12 Sep 2018 05:01:08 -0600 Subject: [PATCH 08/31] Add sapling spending keys to z_exportwallet --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/wallet_import_export.py | 54 ++++++++++++++++++++++++++++ src/keystore.cpp | 10 ++++++ src/keystore.h | 5 +++ src/wallet/rpcdump.cpp | 21 ++++++++--- src/wallet/wallet.cpp | 11 ++---- 6 files changed, 90 insertions(+), 12 deletions(-) create mode 100755 qa/rpc-tests/wallet_import_export.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 25ecc409a..fd71877f2 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -16,6 +16,7 @@ testScripts=( 'wallet_treestate.py' 'wallet_anchorfork.py' 'wallet_changeindicator.py' + 'wallet_import_export.py' 'wallet_protectcoinbase.py' 'wallet_shieldcoinbase.py' 'wallet_mergetoaddress.py' diff --git a/qa/rpc-tests/wallet_import_export.py b/qa/rpc-tests/wallet_import_export.py new file mode 100755 index 000000000..b23143c9e --- /dev/null +++ b/qa/rpc-tests/wallet_import_export.py @@ -0,0 +1,54 @@ +#!/usr/bin/env python2 +# Copyright (c) 2018 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_true, start_nodes + +class WalletImportExportTest (BitcoinTestFramework): + def setup_network(self, split=False): + extra_args = [["-exportdir={}/export{}".format(self.options.tmpdir, i)] for i in range(2)] + self.nodes = start_nodes(2, self.options.tmpdir, extra_args) + + def run_test(self): + sprout_address0 = self.nodes[0].z_getnewaddress('sprout') + sapling_address0 = self.nodes[0].z_getnewaddress('sapling') + + # node 0 should have the keys + dump_path = self.nodes[0].z_exportwallet('walletdump') + (t_keys0, sprout_keys0, sapling_keys0) = parse_wallet_file(dump_path) + + assert_true(sprout_address0 in sprout_keys0) + assert_true(sapling_address0 in sapling_keys0) + + # node 1 should not + dump_path = self.nodes[1].z_exportwallet('walletdump') + (t_keys1, sprout_keys1, sapling_keys1) = parse_wallet_file(dump_path) + + assert_true(sprout_address0 not in sprout_keys1) + assert_true(sapling_address0 not in sapling_keys1) + +# Helper functions +def parse_wallet_file(dump_path): + file_lines = open(dump_path, "r").readlines() + + (t_keys, i) = parse_wallet_file_lines(file_lines, 0) + (sprout_keys, i) = parse_wallet_file_lines(file_lines, i) + (sapling_keys, i) = parse_wallet_file_lines(file_lines, i) + + return (t_keys, sprout_keys, sapling_keys) + +def parse_wallet_file_lines(file_lines, i): + keys = [] + # skip blank lines and comments + while i < len(file_lines) and (file_lines[i] == '\n' or file_lines[i].startswith("#")): + i += 1 + # add keys until we hit another blank line or comment + while i < len(file_lines) and not (file_lines[i] == '\n' or file_lines[i].startswith("#")): + keys.append(file_lines[i]) + i += 1 + return ("".join(keys), i) + +if __name__ == '__main__': + WalletImportExportTest().main() \ No newline at end of file diff --git a/src/keystore.cpp b/src/keystore.cpp index 8262c8aab..fd42f7634 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -226,3 +226,13 @@ bool CBasicKeyStore::GetSaplingIncomingViewingKey(const libzcash::SaplingPayment } return false; } + +bool CBasicKeyStore::GetSaplingExtendedSpendingKey(const libzcash::SaplingPaymentAddress &addr, + libzcash::SaplingExtendedSpendingKey &extskOut) const { + libzcash::SaplingIncomingViewingKey ivk; + libzcash::SaplingFullViewingKey fvk; + + return GetSaplingIncomingViewingKey(addr, ivk) && + GetSaplingFullViewingKey(ivk, fvk) && + GetSaplingSpendingKey(fvk, extskOut); +} diff --git a/src/keystore.h b/src/keystore.h index a8a8f62f5..5a80384bb 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -273,6 +273,11 @@ public: virtual bool GetSaplingIncomingViewingKey( const libzcash::SaplingPaymentAddress &addr, libzcash::SaplingIncomingViewingKey& ivkOut) const; + + bool GetSaplingExtendedSpendingKey( + const libzcash::SaplingPaymentAddress &addr, + libzcash::SaplingExtendedSpendingKey &extskOut) const; + void GetSaplingPaymentAddresses(std::set &setAddress) const { setAddress.clear(); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index f6bc6528f..87d29a6ec 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -318,7 +318,7 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys continue; } else { LogPrint("zrpc", "Importing detected an error: invalid spending key. Trying as a transparent key...\n"); - // Not a valid spending key, so carry on and see if it's a Zcash style address. + // Not a valid spending key, so carry on and see if it's a Zcash style t-address. } } @@ -529,18 +529,31 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) file << "\n"; if (fDumpZKeys) { - std::set addresses; - pwalletMain->GetSproutPaymentAddresses(addresses); + std::set sproutAddresses; + pwalletMain->GetSproutPaymentAddresses(sproutAddresses); file << "\n"; file << "# Zkeys\n"; file << "\n"; - for (auto addr : addresses ) { + for (auto addr : sproutAddresses) { libzcash::SproutSpendingKey key; if (pwalletMain->GetSproutSpendingKey(addr, key)) { std::string strTime = EncodeDumpTime(pwalletMain->mapSproutZKeyMetadata[addr].nCreateTime); file << strprintf("%s %s # zaddr=%s\n", EncodeSpendingKey(key), strTime, EncodePaymentAddress(addr)); } } + std::set saplingAddresses; + pwalletMain->GetSaplingPaymentAddresses(saplingAddresses); + file << "\n"; + file << "# Sapling keys\n"; + file << "\n"; + for (auto addr : saplingAddresses) { + libzcash::SaplingExtendedSpendingKey extsk; + if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) { + auto ivk = extsk.expsk.full_viewing_key().in_viewing_key(); + std::string strTime = EncodeDumpTime(pwalletMain->mapSaplingZKeyMetadata[ivk].nCreateTime); + file << strprintf("%s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, EncodePaymentAddress(addr)); + } + } file << "\n"; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f4e692219..abd064501 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4553,14 +4553,9 @@ boost::optional GetSpendingKeyForPaymentAddress::operator boost::optional GetSpendingKeyForPaymentAddress::operator()( const libzcash::SaplingPaymentAddress &zaddr) const { - libzcash::SaplingIncomingViewingKey ivk; - libzcash::SaplingFullViewingKey fvk; - libzcash::SaplingExtendedSpendingKey sk; - - if (m_wallet->GetSaplingIncomingViewingKey(zaddr, ivk) && - m_wallet->GetSaplingFullViewingKey(ivk, fvk) && - m_wallet->GetSaplingSpendingKey(fvk, sk)) { - return libzcash::SpendingKey(sk); + libzcash::SaplingExtendedSpendingKey extsk; + if (m_wallet->GetSaplingExtendedSpendingKey(zaddr, extsk)) { + return libzcash::SpendingKey(extsk); } else { return boost::none; } From a0783bb957b4782b49b60ec8992b9547eaaa421a Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Wed, 12 Sep 2018 05:02:09 -0600 Subject: [PATCH 09/31] Rename AddZKey to include sprout --- src/wallet/gtest/test_wallet_zkeys.cpp | 4 ++-- src/wallet/rpcdump.cpp | 4 ++-- src/wallet/wallet.cpp | 6 +++--- src/wallet/wallet.h | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index 8a2ecd8d3..e4a8eeeb4 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -63,7 +63,7 @@ TEST(wallet_zkeys_tests, store_and_load_sapling_zkeys) { /** * This test covers methods on CWallet * GenerateNewZKey() - * AddZKey() + * AddSproutZKey() * LoadZKey() * LoadZKeyMetadata() */ @@ -89,7 +89,7 @@ TEST(wallet_zkeys_tests, store_and_load_zkeys) { // manually add new spending key to wallet auto sk = libzcash::SproutSpendingKey::random(); - ASSERT_TRUE(wallet.AddZKey(sk)); + ASSERT_TRUE(wallet.AddSproutZKey(sk)); // verify wallet did add it addr = sk.address(); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 87d29a6ec..c2f4f510f 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -308,7 +308,7 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys } int64_t nTime = DecodeDumpTime(vstr[1]); LogPrint("zrpc", "Importing zaddr %s...\n", EncodePaymentAddress(addr)); - if (!pwalletMain->AddZKey(key)) { + if (!pwalletMain->AddSproutZKey(key)) { // Something went wrong fGood = false; continue; @@ -580,7 +580,7 @@ public: } else { m_wallet->MarkDirty(); - if (!m_wallet-> AddZKey(sk)) { + if (!m_wallet-> AddSproutZKey(sk)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index abd064501..e39c687c3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -96,8 +96,8 @@ libzcash::PaymentAddress CWallet::GenerateNewZKey() int64_t nCreationTime = GetTime(); mapSproutZKeyMetadata[addr] = CKeyMetadata(nCreationTime); - if (!AddZKey(k)) - throw std::runtime_error("CWallet::GenerateNewZKey(): AddZKey failed"); + if (!AddSproutZKey(k)) + throw std::runtime_error("CWallet::GenerateNewZKey(): AddSproutZKey failed"); return addr; } @@ -169,7 +169,7 @@ bool CWallet::AddSaplingZKey( // Add spending key to keystore and persist to disk -bool CWallet::AddZKey(const libzcash::SproutSpendingKey &key) +bool CWallet::AddSproutZKey(const libzcash::SproutSpendingKey &key) { AssertLockHeld(cs_wallet); // mapSproutZKeyMetadata auto addr = key.address(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e660c82df..1b7331d0c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1031,7 +1031,7 @@ public: //! Generates a new zaddr libzcash::PaymentAddress GenerateNewZKey(); //! Adds spending key to the store, and saves it to disk - bool AddZKey(const libzcash::SproutSpendingKey &key); + bool AddSproutZKey(const libzcash::SproutSpendingKey &key); //! Adds spending key to the store, without saving it to disk (used by LoadWallet) bool LoadZKey(const libzcash::SproutSpendingKey &key); //! Load spending key metadata (used by LoadWallet) From fcab001b1ed9696f050e0f47b4808eac17766a27 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Fri, 14 Sep 2018 15:44:03 -0600 Subject: [PATCH 10/31] Move AddSpendingKeyToWallet --- src/wallet/rpcdump.cpp | 61 ------------------------------------------ src/wallet/wallet.cpp | 51 +++++++++++++++++++++++++++++++++++ src/wallet/wallet.h | 15 +++++++++++ 3 files changed, 66 insertions(+), 61 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index c2f4f510f..e9ce51bcb 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -563,67 +563,6 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) return exportfilepath.string(); } -class AddSpendingKeyToWallet : public boost::static_visitor -{ -private: - CWallet *m_wallet; - const Consensus::Params ¶ms; -public: - AddSpendingKeyToWallet(CWallet *wallet, const Consensus::Params ¶ms) : - m_wallet(wallet), params(params) {} - - bool operator()(const libzcash::SproutSpendingKey &sk) const { - auto addr = sk.address(); - // Don't throw error in case a key is already there - if (m_wallet->HaveSproutSpendingKey(addr)) { - return true; - } else { - m_wallet->MarkDirty(); - - if (!m_wallet-> AddSproutZKey(sk)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); - } - - m_wallet->mapSproutZKeyMetadata[addr].nCreateTime = 1; - - return false; - } - } - - bool operator()(const libzcash::SaplingExtendedSpendingKey &sk) const { - auto fvk = sk.expsk.full_viewing_key(); - auto ivk = fvk.in_viewing_key(); - auto addr = sk.DefaultAddress(); - { - // Don't throw error in case a key is already there - if (m_wallet->HaveSaplingSpendingKey(fvk)) { - return true; - } else { - m_wallet->MarkDirty(); - - if (!m_wallet-> AddSaplingZKey(sk, addr)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); - } - - // Sapling addresses can't have been used in transactions prior to activation. - if (params.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight == Consensus::NetworkUpgrade::ALWAYS_ACTIVE) { - m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = 1; - } else { - // Friday, 26 October 2018 00:00:00 GMT - definitely before Sapling activates - m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = 1540512000; - } - - return false; - } - } - } - - bool operator()(const libzcash::InvalidEncoding& no) const { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid spending key"); - } - -}; - UniValue z_importkey(const UniValue& params, bool fHelp) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e39c687c3..dc2670f9d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -14,6 +14,7 @@ #include "key_io.h" #include "main.h" #include "net.h" +#include "rpc/protocol.h" #include "script/script.h" #include "script/sign.h" #include "timedata.h" @@ -4567,3 +4568,53 @@ boost::optional GetSpendingKeyForPaymentAddress::operator // Defaults to InvalidEncoding return libzcash::SpendingKey(); } + +bool AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKey &sk) const { + auto addr = sk.address(); + // Don't throw error in case a key is already there + if (m_wallet->HaveSproutSpendingKey(addr)) { + return true; + } else { + m_wallet->MarkDirty(); + + if (!m_wallet-> AddSproutZKey(sk)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); + } + + m_wallet->mapSproutZKeyMetadata[addr].nCreateTime = 1; + + return false; + } +} + +bool AddSpendingKeyToWallet::operator()(const libzcash::SaplingExtendedSpendingKey &sk) const { + auto fvk = sk.expsk.full_viewing_key(); + auto ivk = fvk.in_viewing_key(); + auto addr = sk.DefaultAddress(); + { + // Don't throw error in case a key is already there + if (m_wallet->HaveSaplingSpendingKey(fvk)) { + return true; + } else { + m_wallet->MarkDirty(); + + if (!m_wallet-> AddSaplingZKey(sk, addr)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); + } + + // Sapling addresses can't have been used in transactions prior to activation. + if (params.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight == Consensus::NetworkUpgrade::ALWAYS_ACTIVE) { + m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = 1; + } else { + // Friday, 26 October 2018 00:00:00 GMT - definitely before Sapling activates + m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = 1540512000; + } + + return false; + } + } +} + +bool AddSpendingKeyToWallet::operator()(const libzcash::InvalidEncoding& no) const { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid spending key"); +} diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1b7331d0c..c0b36d88d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1376,4 +1376,19 @@ public: boost::optional operator()(const libzcash::InvalidEncoding& no) const; }; +class AddSpendingKeyToWallet : public boost::static_visitor +{ +private: + CWallet *m_wallet; + const Consensus::Params ¶ms; +public: + AddSpendingKeyToWallet(CWallet *wallet, const Consensus::Params ¶ms) : + m_wallet(wallet), params(params) {} + + bool operator()(const libzcash::SproutSpendingKey &sk) const; + bool operator()(const libzcash::SaplingExtendedSpendingKey &sk) const; + bool operator()(const libzcash::InvalidEncoding& no) const; +}; + + #endif // BITCOIN_WALLET_WALLET_H From 0f03de55364218c4915928ebb00ebfe207e5013f Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Wed, 12 Sep 2018 05:03:13 -0600 Subject: [PATCH 11/31] Return more information when adding a spending key --- src/wallet/rpcdump.cpp | 8 ++++++-- src/wallet/wallet.cpp | 29 +++++++++++------------------ src/wallet/wallet.h | 14 ++++++++++---- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e9ce51bcb..9dc7f69fc 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -635,11 +635,15 @@ UniValue z_importkey(const UniValue& params, bool fHelp) } // Sapling support - auto keyAlreadyExists = boost::apply_visitor( + auto addResult = boost::apply_visitor( AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus()), spendingkey); - if (keyAlreadyExists && fIgnoreExistingKey) { + if (addResult == KeyAlreadyExists && fIgnoreExistingKey) { return NullUniValue; } + pwalletMain->MarkDirty(); + if (addResult == KeyNotAdded) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); + } // whenever a key is imported, we need to scan the whole chain pwalletMain->nTimeFirstKey = 1; // 0 would be considered 'no value' diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dc2670f9d..e4695b301 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4569,37 +4569,30 @@ boost::optional GetSpendingKeyForPaymentAddress::operator return libzcash::SpendingKey(); } -bool AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKey &sk) const { +SpendingKeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKey &sk) const { auto addr = sk.address(); // Don't throw error in case a key is already there if (m_wallet->HaveSproutSpendingKey(addr)) { - return true; - } else { - m_wallet->MarkDirty(); - - if (!m_wallet-> AddSproutZKey(sk)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); - } - + return KeyAlreadyExists; + } else if (m_wallet-> AddSproutZKey(sk)) { m_wallet->mapSproutZKeyMetadata[addr].nCreateTime = 1; - - return false; + return KeyAdded; + } else { + return KeyNotAdded; } } -bool AddSpendingKeyToWallet::operator()(const libzcash::SaplingExtendedSpendingKey &sk) const { +SpendingKeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SaplingExtendedSpendingKey &sk) const { auto fvk = sk.expsk.full_viewing_key(); auto ivk = fvk.in_viewing_key(); auto addr = sk.DefaultAddress(); { // Don't throw error in case a key is already there if (m_wallet->HaveSaplingSpendingKey(fvk)) { - return true; + return KeyAlreadyExists; } else { - m_wallet->MarkDirty(); - if (!m_wallet-> AddSaplingZKey(sk, addr)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); + return KeyNotAdded; } // Sapling addresses can't have been used in transactions prior to activation. @@ -4610,11 +4603,11 @@ bool AddSpendingKeyToWallet::operator()(const libzcash::SaplingExtendedSpendingK m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = 1540512000; } - return false; + return KeyAdded; } } } -bool AddSpendingKeyToWallet::operator()(const libzcash::InvalidEncoding& no) const { +SpendingKeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::InvalidEncoding& no) const { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid spending key"); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c0b36d88d..0032138e1 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1376,7 +1376,13 @@ public: boost::optional operator()(const libzcash::InvalidEncoding& no) const; }; -class AddSpendingKeyToWallet : public boost::static_visitor +enum SpendingKeyAddResult { + KeyAlreadyExists, + KeyAdded, + KeyNotAdded, +}; + +class AddSpendingKeyToWallet : public boost::static_visitor { private: CWallet *m_wallet; @@ -1385,9 +1391,9 @@ public: AddSpendingKeyToWallet(CWallet *wallet, const Consensus::Params ¶ms) : m_wallet(wallet), params(params) {} - bool operator()(const libzcash::SproutSpendingKey &sk) const; - bool operator()(const libzcash::SaplingExtendedSpendingKey &sk) const; - bool operator()(const libzcash::InvalidEncoding& no) const; + SpendingKeyAddResult operator()(const libzcash::SproutSpendingKey &sk) const; + SpendingKeyAddResult operator()(const libzcash::SaplingExtendedSpendingKey &sk) const; + SpendingKeyAddResult operator()(const libzcash::InvalidEncoding& no) const; }; From 9bcf90e2de13d96f0209b61b9d8bb5be15686d12 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Wed, 12 Sep 2018 05:03:45 -0600 Subject: [PATCH 12/31] Add sapling support to z_importwallet --- qa/rpc-tests/wallet_import_export.py | 20 +++++++++++++++----- src/wallet/rpcdump.cpp | 20 ++++++-------------- src/wallet/wallet.cpp | 17 +++++++++++------ src/wallet/wallet.h | 6 +++++- 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/qa/rpc-tests/wallet_import_export.py b/qa/rpc-tests/wallet_import_export.py index b23143c9e..6577acb53 100755 --- a/qa/rpc-tests/wallet_import_export.py +++ b/qa/rpc-tests/wallet_import_export.py @@ -16,19 +16,29 @@ class WalletImportExportTest (BitcoinTestFramework): sapling_address0 = self.nodes[0].z_getnewaddress('sapling') # node 0 should have the keys - dump_path = self.nodes[0].z_exportwallet('walletdump') - (t_keys0, sprout_keys0, sapling_keys0) = parse_wallet_file(dump_path) + dump_path0 = self.nodes[0].z_exportwallet('walletdump') + (t_keys0, sprout_keys0, sapling_keys0) = parse_wallet_file(dump_path0) assert_true(sprout_address0 in sprout_keys0) assert_true(sapling_address0 in sapling_keys0) - # node 1 should not - dump_path = self.nodes[1].z_exportwallet('walletdump') - (t_keys1, sprout_keys1, sapling_keys1) = parse_wallet_file(dump_path) + # node 1 should not have the keys + dump_path1 = self.nodes[1].z_exportwallet('walletdumpbefore') + (t_keys1, sprout_keys1, sapling_keys1) = parse_wallet_file(dump_path1) assert_true(sprout_address0 not in sprout_keys1) assert_true(sapling_address0 not in sapling_keys1) + # import wallet to node 1 + self.nodes[1].z_importwallet(dump_path0) + + # node 1 should now have the keys + dump_path1 = self.nodes[1].z_exportwallet('walletdumpafter') + (t_keys1, sprout_keys1, sapling_keys1) = parse_wallet_file(dump_path1) + + assert_true(sprout_address0 in sprout_keys1) + assert_true(sapling_address0 in sapling_keys1) + # Helper functions def parse_wallet_file(dump_path): file_lines = open(dump_path, "r").readlines() diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 9dc7f69fc..42023de4a 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -297,24 +297,16 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys // Let's see if the address is a valid Zcash spending key if (fImportZKeys) { auto spendingkey = DecodeSpendingKey(vstr[0]); + int64_t nTime = DecodeDumpTime(vstr[1]); if (IsValidSpendingKey(spendingkey)) { - // TODO: Add Sapling support. For now, ensure we can freely convert. - assert(boost::get(&spendingkey) != nullptr); - auto key = boost::get(spendingkey); - auto addr = key.address(); - if (pwalletMain->HaveSproutSpendingKey(addr)) { - LogPrint("zrpc", "Skipping import of zaddr %s (key already present)\n", EncodePaymentAddress(addr)); - continue; - } - int64_t nTime = DecodeDumpTime(vstr[1]); - LogPrint("zrpc", "Importing zaddr %s...\n", EncodePaymentAddress(addr)); - if (!pwalletMain->AddSproutZKey(key)) { + auto addResult = boost::apply_visitor( + AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus(), nTime, true), spendingkey); + if (addResult == KeyAlreadyExists){ + LogPrint("zrpc", "Skipping import of zaddr (key already present)\n"); + } else if (addResult == KeyNotAdded) { // Something went wrong fGood = false; - continue; } - // Successfully imported zaddr. Now import the metadata. - pwalletMain->mapSproutZKeyMetadata[addr].nCreateTime = nTime; continue; } else { LogPrint("zrpc", "Importing detected an error: invalid spending key. Trying as a transparent key...\n"); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e4695b301..7fd20d69b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4570,12 +4570,14 @@ boost::optional GetSpendingKeyForPaymentAddress::operator } SpendingKeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKey &sk) const { - auto addr = sk.address(); - // Don't throw error in case a key is already there + auto addr = sk.address(); + if (log){ + LogPrint("zrpc", "Importing zaddr %s...\n", EncodePaymentAddress(addr)); + } if (m_wallet->HaveSproutSpendingKey(addr)) { return KeyAlreadyExists; } else if (m_wallet-> AddSproutZKey(sk)) { - m_wallet->mapSproutZKeyMetadata[addr].nCreateTime = 1; + m_wallet->mapSproutZKeyMetadata[addr].nCreateTime = nTime; return KeyAdded; } else { return KeyNotAdded; @@ -4587,6 +4589,9 @@ SpendingKeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SaplingE auto ivk = fvk.in_viewing_key(); auto addr = sk.DefaultAddress(); { + if (log){ + LogPrint("zrpc", "Importing zaddr %s...\n", EncodePaymentAddress(addr)); + } // Don't throw error in case a key is already there if (m_wallet->HaveSaplingSpendingKey(fvk)) { return KeyAlreadyExists; @@ -4597,10 +4602,10 @@ SpendingKeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SaplingE // Sapling addresses can't have been used in transactions prior to activation. if (params.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight == Consensus::NetworkUpgrade::ALWAYS_ACTIVE) { - m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = 1; + m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = nTime; } else { - // Friday, 26 October 2018 00:00:00 GMT - definitely before Sapling activates - m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = 1540512000; + // 154051200 seconds from epoch is Friday, 26 October 2018 00:00:00 GMT - definitely before Sapling activates + m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = std::max((int64_t) 154051200, nTime); } return KeyAdded; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0032138e1..5b2d6731d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1387,9 +1387,13 @@ class AddSpendingKeyToWallet : public boost::static_visitor Date: Sat, 9 Jul 2016 11:41:01 +0200 Subject: [PATCH 13/31] [Wallet] extend CKeyMetadata with HD keypath Zcash: modified for zip32 --- src/wallet/wallet.cpp | 5 ++++- src/wallet/walletdb.h | 13 ++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7fd20d69b..be1b27036 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -117,18 +117,21 @@ SaplingPaymentAddress CWallet::GenerateNewSaplingZKey() throw std::runtime_error("CWallet::GenerateNewSaplingZKey(): HD seed not found"); auto m = libzcash::SaplingExtendedSpendingKey::Master(seed); + uint32_t bip44CoinType = Params().BIP44CoinType(); // We use a fixed keypath scheme of m/32'/coin_type'/account' // Derive m/32' auto m_32h = m.Derive(32 | ZIP32_HARDENED_KEY_LIMIT); // Derive m/32'/coin_type' - auto m_32h_cth = m_32h.Derive(Params().BIP44CoinType() | ZIP32_HARDENED_KEY_LIMIT); + auto m_32h_cth = m_32h.Derive(bip44CoinType | ZIP32_HARDENED_KEY_LIMIT); // Derive account key at next index, skip keys already known to the wallet libzcash::SaplingExtendedSpendingKey xsk; do { xsk = m_32h_cth.Derive(hdChain.saplingAccountCounter | ZIP32_HARDENED_KEY_LIMIT); + metadata.hdKeypath = "m/32'/" + std::to_string(bip44CoinType) + "'/" + std::to_string(hdChain.saplingAccountCounter) + "'"; + metadata.seedFp = hdChain.seedFp; // Increment childkey index hdChain.saplingAccountCounter++; } while (HaveSaplingSpendingKey(xsk.expsk.full_viewing_key())); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 7cd446e9b..a42bb937c 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -77,9 +77,13 @@ public: class CKeyMetadata { public: - static const int CURRENT_VERSION=1; + static const int VERSION_BASIC=1; + static const int VERSION_WITH_HDDATA=10; + static const int CURRENT_VERSION=VERSION_WITH_HDDATA; int nVersion; int64_t nCreateTime; // 0 means unknown + std::string hdKeypath; //optional HD/zip32 keypath + uint256 seedFp; CKeyMetadata() { @@ -89,6 +93,7 @@ public: { nVersion = CKeyMetadata::CURRENT_VERSION; nCreateTime = nCreateTime_; + hdKeypath.clear(); } ADD_SERIALIZE_METHODS; @@ -97,12 +102,18 @@ public: inline void SerializationOp(Stream& s, Operation ser_action) { READWRITE(this->nVersion); READWRITE(nCreateTime); + if (this->nVersion >= VERSION_WITH_HDDATA) + { + READWRITE(hdKeypath); + READWRITE(seedFp); + } } void SetNull() { nVersion = CKeyMetadata::CURRENT_VERSION; nCreateTime = 0; + hdKeypath.clear(); } }; From 82e71233b0a62ef895e6c6f870077518ca630c3b Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Sat, 9 Jul 2016 11:25:02 +0200 Subject: [PATCH 14/31] [Wallet] print hd masterkeyid in getwalletinfo Zcash: modified for zip32 --- src/wallet/rpcwallet.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a9dfbfb0e..39d019b92 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2280,6 +2280,7 @@ UniValue getwalletinfo(const UniValue& params, bool fHelp) " \"keypoolsize\": xxxx, (numeric) how many new keys are pre-generated\n" " \"unlocked_until\": ttt, (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked\n" " \"paytxfee\": x.xxxx, (numeric) the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB\n" + " \"seedfp\": \"uint256\", (string) the BLAKE2b-256 hash of the HD seed\n" "}\n" "\nExamples:\n" + HelpExampleCli("getwalletinfo", "") @@ -2299,6 +2300,9 @@ UniValue getwalletinfo(const UniValue& params, bool fHelp) if (pwalletMain->IsCrypted()) obj.push_back(Pair("unlocked_until", nWalletUnlockTime)); obj.push_back(Pair("paytxfee", ValueFromAmount(payTxFee.GetFeePerK()))); + uint256 seedFp = pwalletMain->GetHDChain().seedFp; + if (!seedFp.IsNull()) + obj.push_back(Pair("seedfp", seedFp.GetHex())); return obj; } From 002753ae644d798470cde5e36482ed8e96b1e5ec Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Fri, 15 Jul 2016 10:33:25 +0200 Subject: [PATCH 15/31] [Wallet] ensure CKeyMetadata.hdMasterKeyID will be cleared during SetNull() Zcash: modified for zip32 --- src/wallet/walletdb.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index a42bb937c..dc58adb65 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -91,9 +91,8 @@ public: } CKeyMetadata(int64_t nCreateTime_) { - nVersion = CKeyMetadata::CURRENT_VERSION; + SetNull(); nCreateTime = nCreateTime_; - hdKeypath.clear(); } ADD_SERIALIZE_METHODS; @@ -114,6 +113,7 @@ public: nVersion = CKeyMetadata::CURRENT_VERSION; nCreateTime = 0; hdKeypath.clear(); + seedFp.SetNull(); } }; From ae807af41344192af599046298a07ff69118bc98 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Fri, 14 Sep 2018 17:26:02 -0600 Subject: [PATCH 16/31] Export comment on HDSeed and fingerprint with wallet --- qa/rpc-tests/wallet_import_export.py | 4 +++- src/wallet/rpcdump.cpp | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/wallet_import_export.py b/qa/rpc-tests/wallet_import_export.py index 6577acb53..355c16d05 100755 --- a/qa/rpc-tests/wallet_import_export.py +++ b/qa/rpc-tests/wallet_import_export.py @@ -42,7 +42,9 @@ class WalletImportExportTest (BitcoinTestFramework): # Helper functions def parse_wallet_file(dump_path): file_lines = open(dump_path, "r").readlines() - + # WE expect information about the HDSeed and fingerpring in the header + assert_true("HDSeed" in file_lines[4], "Expected HDSeed") + assert_true("fingerprint" in file_lines[4], "Expected fingerprint") (t_keys, i) = parse_wallet_file_lines(file_lines, 0) (sprout_keys, i) = parse_wallet_file_lines(file_lines, i) (sapling_keys, i) = parse_wallet_file_lines(file_lines, i) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 42023de4a..446f3bc41 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -502,6 +502,12 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) file << strprintf("# * Created on %s\n", EncodeDumpTime(GetTime())); file << strprintf("# * Best block at time of backup was %i (%s),\n", chainActive.Height(), chainActive.Tip()->GetBlockHash().ToString()); file << strprintf("# mined on %s\n", EncodeDumpTime(chainActive.Tip()->GetBlockTime())); + { + HDSeed hdSeed; + pwalletMain->GetHDSeed(hdSeed); + file << strprintf("# HDSeed=%s fingerprint=%s", pwalletMain->GetHDChain().seedFp.GetHex(), hdSeed.Fingerprint().GetHex()); + file << "\n"; + } file << "\n"; for (std::vector >::const_iterator it = vKeyBirth.begin(); it != vKeyBirth.end(); it++) { const CKeyID &keyid = it->second; From 2fe39561ec8845cbf812c9b95e45648190df0e46 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Fri, 14 Sep 2018 17:27:20 -0600 Subject: [PATCH 17/31] Export zip32 metadata with sapling keys --- qa/rpc-tests/wallet_import_export.py | 11 +++++++++-- src/wallet/rpcdump.cpp | 12 +++++++----- src/wallet/wallet.cpp | 11 +++++++++-- src/wallet/wallet.h | 15 ++++++++++++--- 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/qa/rpc-tests/wallet_import_export.py b/qa/rpc-tests/wallet_import_export.py index 355c16d05..8169e36ba 100755 --- a/qa/rpc-tests/wallet_import_export.py +++ b/qa/rpc-tests/wallet_import_export.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_true, start_nodes +from test_framework.util import assert_equal, assert_true, start_nodes class WalletImportExportTest (BitcoinTestFramework): def setup_network(self, split=False): @@ -18,6 +18,9 @@ class WalletImportExportTest (BitcoinTestFramework): # node 0 should have the keys dump_path0 = self.nodes[0].z_exportwallet('walletdump') (t_keys0, sprout_keys0, sapling_keys0) = parse_wallet_file(dump_path0) + + for sapling_key0 in sapling_keys0.splitlines(): + assert_equal(4, len(sapling_key0.split(' #')[0].split()), "Should have 4 parameters before ' #'") assert_true(sprout_address0 in sprout_keys0) assert_true(sapling_address0 in sapling_keys0) @@ -39,6 +42,10 @@ class WalletImportExportTest (BitcoinTestFramework): assert_true(sprout_address0 in sprout_keys1) assert_true(sapling_address0 in sapling_keys1) + # make sure we have perserved the metadata + for sapling_key0 in sapling_keys0.splitlines(): + assert_true(sapling_key0 in sapling_keys1) + # Helper functions def parse_wallet_file(dump_path): file_lines = open(dump_path, "r").readlines() @@ -60,7 +67,7 @@ def parse_wallet_file_lines(file_lines, i): while i < len(file_lines) and not (file_lines[i] == '\n' or file_lines[i].startswith("#")): keys.append(file_lines[i]) i += 1 - return ("".join(keys), i) + return ("\n".join(keys), i) if __name__ == '__main__': WalletImportExportTest().main() \ No newline at end of file diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 446f3bc41..e6bd0fe3e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -298,9 +298,11 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys if (fImportZKeys) { auto spendingkey = DecodeSpendingKey(vstr[0]); int64_t nTime = DecodeDumpTime(vstr[1]); + boost::optional hdKeypath = (vstr.size() > 2) ? boost::optional(vstr[2]) : boost::none; + boost::optional seedFpStr = (vstr.size() > 3) ? boost::optional(vstr[3]) : boost::none; if (IsValidSpendingKey(spendingkey)) { auto addResult = boost::apply_visitor( - AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus(), nTime, true), spendingkey); + AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus(), nTime, hdKeypath, seedFpStr, true), spendingkey); if (addResult == KeyAlreadyExists){ LogPrint("zrpc", "Skipping import of zaddr (key already present)\n"); } else if (addResult == KeyNotAdded) { @@ -548,8 +550,9 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) libzcash::SaplingExtendedSpendingKey extsk; if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) { auto ivk = extsk.expsk.full_viewing_key().in_viewing_key(); - std::string strTime = EncodeDumpTime(pwalletMain->mapSaplingZKeyMetadata[ivk].nCreateTime); - file << strprintf("%s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, EncodePaymentAddress(addr)); + CKeyMetadata keyMeta = pwalletMain->mapSaplingZKeyMetadata[ivk]; + std::string strTime = EncodeDumpTime(keyMeta.nCreateTime); + file << strprintf("%s %s %s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, keyMeta.hdKeypath, keyMeta.seedFp.GetHex(), EncodePaymentAddress(addr)); } } file << "\n"; @@ -633,8 +636,7 @@ UniValue z_importkey(const UniValue& params, bool fHelp) } // Sapling support - auto addResult = boost::apply_visitor( - AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus()), spendingkey); + auto addResult = boost::apply_visitor(AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus()), spendingkey); if (addResult == KeyAlreadyExists && fIgnoreExistingKey) { return NullUniValue; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index be1b27036..6e2631db3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4573,7 +4573,7 @@ boost::optional GetSpendingKeyForPaymentAddress::operator } SpendingKeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKey &sk) const { - auto addr = sk.address(); + auto addr = sk.address(); if (log){ LogPrint("zrpc", "Importing zaddr %s...\n", EncodePaymentAddress(addr)); } @@ -4610,7 +4610,14 @@ SpendingKeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SaplingE // 154051200 seconds from epoch is Friday, 26 October 2018 00:00:00 GMT - definitely before Sapling activates m_wallet->mapSaplingZKeyMetadata[ivk].nCreateTime = std::max((int64_t) 154051200, nTime); } - + if (hdKeypath) { + m_wallet->mapSaplingZKeyMetadata[ivk].hdKeypath = hdKeypath.get(); + } + if (seedFpStr) { + uint256 seedFp; + seedFp.SetHex(seedFpStr.get()); + m_wallet->mapSaplingZKeyMetadata[ivk].seedFp = seedFp; + } return KeyAdded; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5b2d6731d..50acb2394 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1388,12 +1388,21 @@ private: CWallet *m_wallet; const Consensus::Params ¶ms; int64_t nTime; + boost::optional hdKeypath; // currently sapling only + boost::optional seedFpStr; // currently sapling only bool log; public: AddSpendingKeyToWallet(CWallet *wallet, const Consensus::Params ¶ms) : - m_wallet(wallet), params(params), nTime(1), log(false) {} - AddSpendingKeyToWallet(CWallet *wallet, const Consensus::Params ¶ms, int64_t _nTime, bool _log) : - m_wallet(wallet), params(params), nTime(_nTime), log(_log) {} + m_wallet(wallet), params(params), nTime(1), hdKeypath(boost::none), seedFpStr(boost::none), log(false) {} + AddSpendingKeyToWallet( + CWallet *wallet, + const Consensus::Params ¶ms, + int64_t _nTime, + boost::optional _hdKeypath, + boost::optional _seedFp, + bool _log + ) : m_wallet(wallet), params(params), nTime(_nTime), hdKeypath(_hdKeypath), seedFpStr(_seedFp), log(_log) {} + SpendingKeyAddResult operator()(const libzcash::SproutSpendingKey &sk) const; SpendingKeyAddResult operator()(const libzcash::SaplingExtendedSpendingKey &sk) const; From b37dc4e22fa2702af53e71082620cfe0b3d63ce4 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Fri, 14 Sep 2018 18:36:24 -0600 Subject: [PATCH 18/31] Don't export empty zip32 metadata --- qa/rpc-tests/wallet_import_export.py | 23 ++++++++++++++++------- src/wallet/rpcdump.cpp | 10 ++++++++-- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/qa/rpc-tests/wallet_import_export.py b/qa/rpc-tests/wallet_import_export.py index 8169e36ba..ee9cc42ce 100755 --- a/qa/rpc-tests/wallet_import_export.py +++ b/qa/rpc-tests/wallet_import_export.py @@ -8,10 +8,15 @@ from test_framework.util import assert_equal, assert_true, start_nodes class WalletImportExportTest (BitcoinTestFramework): def setup_network(self, split=False): - extra_args = [["-exportdir={}/export{}".format(self.options.tmpdir, i)] for i in range(2)] - self.nodes = start_nodes(2, self.options.tmpdir, extra_args) + num_nodes = 3 + extra_args = [["-exportdir={}/export{}".format(self.options.tmpdir, i)] for i in range(num_nodes)] + self.nodes = start_nodes(num_nodes, self.options.tmpdir, extra_args) def run_test(self): + sapling_address2 = self.nodes[2].z_getnewaddress('sapling') + privkey2 = self.nodes[2].z_exportkey(sapling_address2) + self.nodes[0].z_importkey(privkey2) + sprout_address0 = self.nodes[0].z_getnewaddress('sprout') sapling_address0 = self.nodes[0].z_getnewaddress('sapling') @@ -19,11 +24,14 @@ class WalletImportExportTest (BitcoinTestFramework): dump_path0 = self.nodes[0].z_exportwallet('walletdump') (t_keys0, sprout_keys0, sapling_keys0) = parse_wallet_file(dump_path0) - for sapling_key0 in sapling_keys0.splitlines(): - assert_equal(4, len(sapling_key0.split(' #')[0].split()), "Should have 4 parameters before ' #'") - + sapling_line_lengths = [len(sapling_key0.split(' #')[0].split()) for sapling_key0 in sapling_keys0.splitlines()] + assert_equal(2, len(sapling_line_lengths), "Should have 2 sapling keys") + assert_true(2 in sapling_line_lengths, "Should have a key with 2 parameters") + assert_true(4 in sapling_line_lengths, "Should have a key with 4 parameters") + assert_true(sprout_address0 in sprout_keys0) assert_true(sapling_address0 in sapling_keys0) + assert_true(sapling_address2 in sapling_keys0) # node 1 should not have the keys dump_path1 = self.nodes[1].z_exportwallet('walletdumpbefore') @@ -41,6 +49,7 @@ class WalletImportExportTest (BitcoinTestFramework): assert_true(sprout_address0 in sprout_keys1) assert_true(sapling_address0 in sapling_keys1) + assert_true(sapling_address2 in sapling_keys1) # make sure we have perserved the metadata for sapling_key0 in sapling_keys0.splitlines(): @@ -49,7 +58,7 @@ class WalletImportExportTest (BitcoinTestFramework): # Helper functions def parse_wallet_file(dump_path): file_lines = open(dump_path, "r").readlines() - # WE expect information about the HDSeed and fingerpring in the header + # We expect information about the HDSeed and fingerpring in the header assert_true("HDSeed" in file_lines[4], "Expected HDSeed") assert_true("fingerprint" in file_lines[4], "Expected fingerprint") (t_keys, i) = parse_wallet_file_lines(file_lines, 0) @@ -67,7 +76,7 @@ def parse_wallet_file_lines(file_lines, i): while i < len(file_lines) and not (file_lines[i] == '\n' or file_lines[i].startswith("#")): keys.append(file_lines[i]) i += 1 - return ("\n".join(keys), i) + return ("".join(keys), i) if __name__ == '__main__': WalletImportExportTest().main() \ No newline at end of file diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e6bd0fe3e..81e2969d5 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -298,7 +298,8 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys if (fImportZKeys) { auto spendingkey = DecodeSpendingKey(vstr[0]); int64_t nTime = DecodeDumpTime(vstr[1]); - boost::optional hdKeypath = (vstr.size() > 2) ? boost::optional(vstr[2]) : boost::none; + // Only include hdKeypath and seedFpStr if we have both + boost::optional hdKeypath = (vstr.size() > 3) ? boost::optional(vstr[2]) : boost::none; boost::optional seedFpStr = (vstr.size() > 3) ? boost::optional(vstr[3]) : boost::none; if (IsValidSpendingKey(spendingkey)) { auto addResult = boost::apply_visitor( @@ -552,7 +553,12 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) auto ivk = extsk.expsk.full_viewing_key().in_viewing_key(); CKeyMetadata keyMeta = pwalletMain->mapSaplingZKeyMetadata[ivk]; std::string strTime = EncodeDumpTime(keyMeta.nCreateTime); - file << strprintf("%s %s %s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, keyMeta.hdKeypath, keyMeta.seedFp.GetHex(), EncodePaymentAddress(addr)); + // Keys imported with z_importkey do not have zip32 metadata + if (keyMeta.hdKeypath.empty() || keyMeta.seedFp.IsNull()) { + file << strprintf("%s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, EncodePaymentAddress(addr)); + } else { + file << strprintf("%s %s %s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, keyMeta.hdKeypath, keyMeta.seedFp.GetHex(), EncodePaymentAddress(addr)); + } } } file << "\n"; From 611f93244bf8075b49bbbf446af689f750d60878 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Tue, 21 Aug 2018 13:54:17 -0600 Subject: [PATCH 19/31] Allow more information to be returned when an async rpc fails --- qa/rpc-tests/test_framework/util.py | 45 ++++++++++++++++------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index 79130c9bd..10391864a 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -385,29 +385,34 @@ def assert_raises(exc, fun, *args, **kwds): # Returns txid if operation was a success or None def wait_and_assert_operationid_status(node, myopid, in_status='success', in_errormsg=None): print('waiting for async operation {}'.format(myopid)) - opids = [] - opids.append(myopid) - timeout = 300 - status = None - errormsg = None - txid = None - for x in xrange(1, timeout): - results = node.z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - if status == "failed": - errormsg = results[0]['error']['message'] - elif status == "success": - txid = results[0]['result']['txid'] + result = None + for x in xrange(1, 300): # 300 is the timeout + results = node.z_getoperationresult([myopid]) + if len(results) > 0: + result = results[0] break + time.sleep(1) + + assert_true(result is not None, "timeout occured") + status = result['status'] + + txid = None + errormsg = None + if status == "failed": + errormsg = result['error']['message'] + elif status == "success": + txid = result['result']['txid'] + if os.getenv("PYTHON_DEBUG", ""): print('...returned status: {}'.format(status)) if errormsg is not None: print('...returned error: {}'.format(errormsg)) - assert_equal(in_status, status) + + assert_equal(in_status, status, "Operation returned mismatched status. Error Message: {}".format(errormsg)) + if errormsg is not None: - assert(in_errormsg is not None) - assert_equal(in_errormsg in errormsg, True) - return txid + assert_true(in_errormsg is not None, "No error retured. Expected: {}".format(errormsg)) + assert_true(in_errormsg in errormsg, "Error returned: {}. Error expected: {}".format(errormsg, in_errormsg)) + return result # if there was an error return the result + else: + return txid # otherwise return the txid From f081d9cb0229463b5289ef99339fc27c0d9fbc9f Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Tue, 21 Aug 2018 12:12:26 -0600 Subject: [PATCH 20/31] Use utility method to wait for async operations --- qa/rpc-tests/mempool_tx_input_limit.py | 16 +----- qa/rpc-tests/wallet.py | 35 ++---------- qa/rpc-tests/wallet_nullifiers.py | 76 +++----------------------- qa/rpc-tests/wallet_protectcoinbase.py | 49 ++++------------- 4 files changed, 27 insertions(+), 149 deletions(-) diff --git a/qa/rpc-tests/mempool_tx_input_limit.py b/qa/rpc-tests/mempool_tx_input_limit.py index 201b82998..06b51f3eb 100755 --- a/qa/rpc-tests/mempool_tx_input_limit.py +++ b/qa/rpc-tests/mempool_tx_input_limit.py @@ -54,22 +54,8 @@ class MempoolTxInputLimitTest(BitcoinTestFramework): recipients.append({"address":node0_zaddr, "amount":Decimal('30.0')-Decimal('0.0001')}) # utxo amount less fee myopid = self.nodes[0].z_sendmany(node0_taddr, recipients) - opids = [] - opids.append(myopid) - # Spend should fail due to -mempooltxinputlimit - timeout = 120 - status = None - for x in xrange(1, timeout): - results = self.nodes[0].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - msg = results[0]["error"]["message"] - assert_equal("failed", status) - assert_equal("Too many transparent inputs 3 > limit 2", msg) - break + wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Too many transparent inputs 3 > limit 2") # Mempool should be empty. assert_equal(set(self.nodes[0].getrawmempool()), set()) diff --git a/qa/rpc-tests/wallet.py b/qa/rpc-tests/wallet.py index 642590b81..3f538c844 100755 --- a/qa/rpc-tests/wallet.py +++ b/qa/rpc-tests/wallet.py @@ -8,7 +8,8 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.authproxy import JSONRPCException from test_framework.util import assert_equal, assert_greater_than, \ initialize_chain_clean, start_nodes, start_node, connect_nodes_bi, \ - stop_nodes, sync_blocks, sync_mempools, wait_bitcoinds + stop_nodes, sync_blocks, sync_mempools, wait_and_assert_operationid_status, \ + wait_bitcoinds import time from decimal import Decimal @@ -348,23 +349,9 @@ class WalletTest (BitcoinTestFramework): # send node 2 taddr to zaddr recipients = [] recipients.append({"address":myzaddr, "amount":7}) - myopid = self.nodes[2].z_sendmany(mytaddr, recipients) - opids = [] - opids.append(myopid) + mytxid = wait_and_assert_operationid_status(self.nodes[2], self.nodes[2].z_sendmany(mytaddr, recipients)) - timeout = 300 - status = None - for x in xrange(1, timeout): - results = self.nodes[2].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - mytxid = results[0]["result"]["txid"] - break - - assert_equal("success", status) self.sync_all() self.nodes[2].generate(1) self.sync_all() @@ -399,7 +386,6 @@ class WalletTest (BitcoinTestFramework): assert("randomSeed" in myjoinsplit.keys()) assert("ciphertexts" in myjoinsplit.keys()) - # send from private note to node 0 and node 2 node0balance = self.nodes[0].getbalance() # 25.99794745 node2balance = self.nodes[2].getbalance() # 16.99790000 @@ -407,20 +393,9 @@ class WalletTest (BitcoinTestFramework): recipients = [] recipients.append({"address":self.nodes[0].getnewaddress(), "amount":1}) recipients.append({"address":self.nodes[2].getnewaddress(), "amount":1.0}) - myopid = self.nodes[2].z_sendmany(myzaddr, recipients) + + wait_and_assert_operationid_status(self.nodes[2], self.nodes[2].z_sendmany(myzaddr, recipients)) - status = None - opids = [] - opids.append(myopid) - for x in xrange(1, timeout): - results = self.nodes[2].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - break - - assert_equal("success", status) self.sync_all() self.nodes[2].generate(1) self.sync_all() diff --git a/qa/rpc-tests/wallet_nullifiers.py b/qa/rpc-tests/wallet_nullifiers.py index 72e26fab8..f26e986a6 100755 --- a/qa/rpc-tests/wallet_nullifiers.py +++ b/qa/rpc-tests/wallet_nullifiers.py @@ -5,8 +5,8 @@ from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_true, start_node, \ - start_nodes, connect_nodes_bi, bitcoind_processes +from test_framework.util import assert_equal, assert_true, bitcoind_processes, \ + connect_nodes_bi, start_node, start_nodes, wait_and_assert_operationid_status import time from decimal import Decimal @@ -25,22 +25,8 @@ class WalletNullifiersTest (BitcoinTestFramework): mytaddr = self.nodes[0].getnewaddress(); recipients = [] recipients.append({"address":myzaddr0, "amount":Decimal('10.0')-Decimal('0.0001')}) # utxo amount less fee - myopid = self.nodes[0].z_sendmany(mytaddr, recipients) - - opids = [] - opids.append(myopid) - - timeout = 120 - status = None - for x in xrange(1, timeout): - results = self.nodes[0].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - assert_equal("success", status) - mytxid = results[0]["result"]["txid"] - break + + wait_and_assert_operationid_status(self.nodes[0], self.nodes[0].z_sendmany(mytaddr, recipients)) self.sync_all() self.nodes[0].generate(1) @@ -66,22 +52,8 @@ class WalletNullifiersTest (BitcoinTestFramework): # send node 0 zaddr to note 2 zaddr recipients = [] recipients.append({"address":myzaddr, "amount":7.0}) - myopid = self.nodes[0].z_sendmany(myzaddr0, recipients) - - opids = [] - opids.append(myopid) - - timeout = 120 - status = None - for x in xrange(1, timeout): - results = self.nodes[0].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - assert_equal("success", status) - mytxid = results[0]["result"]["txid"] - break + + wait_and_assert_operationid_status(self.nodes[0], self.nodes[0].z_sendmany(myzaddr0, recipients)) self.sync_all() self.nodes[0].generate(1) @@ -98,22 +70,8 @@ class WalletNullifiersTest (BitcoinTestFramework): # send node 2 zaddr to note 3 zaddr recipients = [] recipients.append({"address":myzaddr3, "amount":2.0}) - myopid = self.nodes[2].z_sendmany(myzaddr, recipients) - opids = [] - opids.append(myopid) - - timeout = 120 - status = None - for x in xrange(1, timeout): - results = self.nodes[2].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - assert_equal("success", status) - mytxid = results[0]["result"]["txid"] - break + wait_and_assert_operationid_status(self.nodes[2], self.nodes[2].z_sendmany(myzaddr, recipients)) self.sync_all() self.nodes[2].generate(1) @@ -139,23 +97,8 @@ class WalletNullifiersTest (BitcoinTestFramework): mytaddr1 = self.nodes[1].getnewaddress(); recipients = [] recipients.append({"address":mytaddr1, "amount":1.0}) - myopid = self.nodes[1].z_sendmany(myzaddr, recipients) - - opids = [] - opids.append(myopid) - - timeout = 120 - status = None - for x in xrange(1, timeout): - results = self.nodes[1].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - assert_equal("success", status) - mytxid = results[0]["result"]["txid"] - [mytxid] # hush pyflakes - break + + wait_and_assert_operationid_status(self.nodes[1], self.nodes[1].z_sendmany(myzaddr, recipients)) self.sync_all() self.nodes[1].generate(1) @@ -206,7 +149,6 @@ class WalletNullifiersTest (BitcoinTestFramework): if key != 'change': assert_equal(received2[key], received3[key]) - # Node 3's balances should be unchanged without explicitly requesting # to include watch-only balances assert_equal({k: Decimal(v) for k, v in self.nodes[3].z_gettotalbalance().items()}, { diff --git a/qa/rpc-tests/wallet_protectcoinbase.py b/qa/rpc-tests/wallet_protectcoinbase.py index a27896df7..fbba2286b 100755 --- a/qa/rpc-tests/wallet_protectcoinbase.py +++ b/qa/rpc-tests/wallet_protectcoinbase.py @@ -82,50 +82,25 @@ class WalletProtectCoinbaseTest (BitcoinTestFramework): self.nodes[3].importaddress(mytaddr) recipients= [{"address":myzaddr, "amount": Decimal('1')}] myopid = self.nodes[3].z_sendmany(mytaddr, recipients) - errorString="" - status = None - opids = [myopid] - timeout = 10 - for x in xrange(1, timeout): - results = self.nodes[3].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - errorString = results[0]["error"]["message"] - break - assert_equal("failed", status) - assert_equal("no UTXOs found for taddr from address" in errorString, True) + + wait_and_assert_operationid_status(self.nodes[3], myopid, "failed", "no UTXOs found for taddr from address") # This send will fail because our wallet does not allow any change when protecting a coinbase utxo, # as it's currently not possible to specify a change address in z_sendmany. recipients = [] recipients.append({"address":myzaddr, "amount":Decimal('1.23456789')}) - errorString = "" + myopid = self.nodes[0].z_sendmany(mytaddr, recipients) - opids = [] - opids.append(myopid) - timeout = 10 - status = None - for x in xrange(1, timeout): - results = self.nodes[0].z_getoperationresult(opids) - if len(results)==0: - time.sleep(1) - else: - status = results[0]["status"] - errorString = results[0]["error"]["message"] + error_result = wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "wallet does not allow any change") - # Test that the returned status object contains a params field with the operation's input parameters - assert_equal(results[0]["method"], "z_sendmany") - params =results[0]["params"] - assert_equal(params["fee"], Decimal('0.0001')) # default - assert_equal(params["minconf"], Decimal('1')) # default - assert_equal(params["fromaddress"], mytaddr) - assert_equal(params["amounts"][0]["address"], myzaddr) - assert_equal(params["amounts"][0]["amount"], Decimal('1.23456789')) - break - assert_equal("failed", status) - assert_equal("wallet does not allow any change" in errorString, True) + # Test that the returned status object contains a params field with the operation's input parameters + assert_equal(error_result["method"], "z_sendmany") + params = error_result["params"] + assert_equal(params["fee"], Decimal('0.0001')) # default + assert_equal(params["minconf"], Decimal('1')) # default + assert_equal(params["fromaddress"], mytaddr) + assert_equal(params["amounts"][0]["address"], myzaddr) + assert_equal(params["amounts"][0]["amount"], Decimal('1.23456789')) # Add viewing key for myzaddr to Node 3 myviewingkey = self.nodes[0].z_exportviewingkey(myzaddr) From 4d89d020a6609f29ad3f3fe922105893b77455eb Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Tue, 21 Aug 2018 14:00:04 -0600 Subject: [PATCH 21/31] Remove unneeded semicolons --- qa/rpc-tests/mempool_tx_input_limit.py | 2 +- qa/rpc-tests/test_framework/util.py | 14 ++++++------ qa/rpc-tests/wallet.py | 30 +++++++++++++------------- qa/rpc-tests/wallet_nullifiers.py | 4 ++-- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/qa/rpc-tests/mempool_tx_input_limit.py b/qa/rpc-tests/mempool_tx_input_limit.py index 06b51f3eb..098daae2d 100755 --- a/qa/rpc-tests/mempool_tx_input_limit.py +++ b/qa/rpc-tests/mempool_tx_input_limit.py @@ -49,7 +49,7 @@ class MempoolTxInputLimitTest(BitcoinTestFramework): node0_zaddr = self.nodes[0].z_getnewaddress() # Send three inputs from node 0 taddr to zaddr to get out of coinbase - node0_taddr = self.nodes[0].getnewaddress(); + node0_taddr = self.nodes[0].getnewaddress() recipients = [] recipients.append({"address":node0_zaddr, "amount":Decimal('30.0')-Decimal('0.0001')}) # utxo amount less fee myopid = self.nodes[0].z_sendmany(node0_taddr, recipients) diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index 10391864a..07d56204e 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -76,13 +76,13 @@ def initialize_datadir(dirname, n): if not os.path.isdir(datadir): os.makedirs(datadir) with open(os.path.join(datadir, "zcash.conf"), 'w') as f: - f.write("regtest=1\n"); - f.write("showmetrics=0\n"); - f.write("rpcuser=rt\n"); - f.write("rpcpassword=rt\n"); - f.write("port="+str(p2p_port(n))+"\n"); - f.write("rpcport="+str(rpc_port(n))+"\n"); - f.write("listenonion=0\n"); + f.write("regtest=1\n") + f.write("showmetrics=0\n") + f.write("rpcuser=rt\n") + f.write("rpcpassword=rt\n") + f.write("port="+str(p2p_port(n))+"\n") + f.write("rpcport="+str(rpc_port(n))+"\n") + f.write("listenonion=0\n") return datadir def initialize_chain(test_dir): diff --git a/qa/rpc-tests/wallet.py b/qa/rpc-tests/wallet.py index 3f538c844..099facc58 100755 --- a/qa/rpc-tests/wallet.py +++ b/qa/rpc-tests/wallet.py @@ -220,7 +220,7 @@ class WalletTest (BitcoinTestFramework): for uTx in unspentTxs: if uTx['txid'] == zeroValueTxid: found = True - assert_equal(uTx['amount'], Decimal('0.00000000')); + assert_equal(uTx['amount'], Decimal('0.00000000')) assert(found) #do some -walletbroadcast tests @@ -232,13 +232,13 @@ class WalletTest (BitcoinTestFramework): connect_nodes_bi(self.nodes,0,2) self.sync_all() - txIdNotBroadcasted = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2); + txIdNotBroadcasted = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2) txObjNotBroadcasted = self.nodes[0].gettransaction(txIdNotBroadcasted) self.sync_all() self.nodes[1].generate(1) #mine a block, tx should not be in there self.sync_all() - assert_equal(self.nodes[2].getbalance(), Decimal('9.99800000')); #should not be changed because tx was not broadcasted - assert_equal(self.nodes[2].getbalance("*"), Decimal('9.99800000')); #should not be changed because tx was not broadcasted + assert_equal(self.nodes[2].getbalance(), Decimal('9.99800000')) #should not be changed because tx was not broadcasted + assert_equal(self.nodes[2].getbalance("*"), Decimal('9.99800000')) #should not be changed because tx was not broadcasted #now broadcast from another node, mine a block, sync, and check the balance self.nodes[1].sendrawtransaction(txObjNotBroadcasted['hex']) @@ -246,11 +246,11 @@ class WalletTest (BitcoinTestFramework): self.nodes[1].generate(1) self.sync_all() txObjNotBroadcasted = self.nodes[0].gettransaction(txIdNotBroadcasted) - assert_equal(self.nodes[2].getbalance(), Decimal('11.99800000')); #should not be - assert_equal(self.nodes[2].getbalance("*"), Decimal('11.99800000')); #should not be + assert_equal(self.nodes[2].getbalance(), Decimal('11.99800000')) #should not be + assert_equal(self.nodes[2].getbalance("*"), Decimal('11.99800000')) #should not be #create another tx - txIdNotBroadcasted = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2); + txIdNotBroadcasted = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 2) #restart the nodes with -walletbroadcast=1 stop_nodes(self.nodes) @@ -265,18 +265,18 @@ class WalletTest (BitcoinTestFramework): sync_blocks(self.nodes) #tx should be added to balance because after restarting the nodes tx should be broadcastet - assert_equal(self.nodes[2].getbalance(), Decimal('13.99800000')); #should not be - assert_equal(self.nodes[2].getbalance("*"), Decimal('13.99800000')); #should not be + assert_equal(self.nodes[2].getbalance(), Decimal('13.99800000')) #should not be + assert_equal(self.nodes[2].getbalance("*"), Decimal('13.99800000')) #should not be # send from node 0 to node 2 taddr - mytaddr = self.nodes[2].getnewaddress(); - mytxid = self.nodes[0].sendtoaddress(mytaddr, 10.0); + mytaddr = self.nodes[2].getnewaddress() + mytxid = self.nodes[0].sendtoaddress(mytaddr, 10.0) self.sync_all() self.nodes[0].generate(1) self.sync_all() mybalance = self.nodes[2].z_getbalance(mytaddr) - assert_equal(mybalance, Decimal('10.0')); + assert_equal(mybalance, Decimal('10.0')) mytxdetails = self.nodes[2].gettransaction(mytxid) myvjoinsplits = mytxdetails["vjoinsplit"] @@ -365,7 +365,7 @@ class WalletTest (BitcoinTestFramework): assert_equal(self.nodes[2].getbalance("*"), node2utxobalance) # check zaddr balance - assert_equal(self.nodes[2].z_getbalance(myzaddr), zsendmanynotevalue); + assert_equal(self.nodes[2].z_getbalance(myzaddr), zsendmanynotevalue) # check via z_gettotalbalance resp = self.nodes[2].z_gettotalbalance() @@ -428,7 +428,7 @@ class WalletTest (BitcoinTestFramework): except JSONRPCException,e: errorString = e.error['message'] - assert_equal("Invalid amount" in errorString, True); + assert_equal("Invalid amount" in errorString, True) errorString = "" try: @@ -436,7 +436,7 @@ class WalletTest (BitcoinTestFramework): except JSONRPCException,e: errorString = e.error['message'] - assert_equal("not an integer" in errorString, True); + assert_equal("not an integer" in errorString, True) myzaddr = self.nodes[0].z_getnewaddress() recipients = [ {"address": myzaddr, "amount": Decimal('0.0') } ] diff --git a/qa/rpc-tests/wallet_nullifiers.py b/qa/rpc-tests/wallet_nullifiers.py index f26e986a6..0cd0f4b43 100755 --- a/qa/rpc-tests/wallet_nullifiers.py +++ b/qa/rpc-tests/wallet_nullifiers.py @@ -22,7 +22,7 @@ class WalletNullifiersTest (BitcoinTestFramework): myzaddr0 = self.nodes[0].z_getnewaddress() # send node 0 taddr to zaddr to get out of coinbase - mytaddr = self.nodes[0].getnewaddress(); + mytaddr = self.nodes[0].getnewaddress() recipients = [] recipients.append({"address":myzaddr0, "amount":Decimal('10.0')-Decimal('0.0001')}) # utxo amount less fee @@ -94,7 +94,7 @@ class WalletNullifiersTest (BitcoinTestFramework): # This requires that node 1 be unlocked, which triggers caching of # uncached nullifiers. self.nodes[1].walletpassphrase("test", 600) - mytaddr1 = self.nodes[1].getnewaddress(); + mytaddr1 = self.nodes[1].getnewaddress() recipients = [] recipients.append({"address":mytaddr1, "amount":1.0}) From e39f0e16c2e5554c6df1e1f965ff347fa45737c3 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Thu, 30 Aug 2018 09:43:09 -0600 Subject: [PATCH 22/31] Remove unused imports --- qa/rpc-tests/mempool_tx_input_limit.py | 1 - qa/rpc-tests/wallet.py | 1 - qa/rpc-tests/wallet_nullifiers.py | 1 - qa/rpc-tests/wallet_protectcoinbase.py | 1 - 4 files changed, 4 deletions(-) diff --git a/qa/rpc-tests/mempool_tx_input_limit.py b/qa/rpc-tests/mempool_tx_input_limit.py index 098daae2d..f91d9b52d 100755 --- a/qa/rpc-tests/mempool_tx_input_limit.py +++ b/qa/rpc-tests/mempool_tx_input_limit.py @@ -8,7 +8,6 @@ from test_framework.authproxy import JSONRPCException from test_framework.util import assert_equal, initialize_chain_clean, \ start_node, connect_nodes, wait_and_assert_operationid_status -import time from decimal import Decimal # Test -mempooltxinputlimit diff --git a/qa/rpc-tests/wallet.py b/qa/rpc-tests/wallet.py index 099facc58..5d221a28c 100755 --- a/qa/rpc-tests/wallet.py +++ b/qa/rpc-tests/wallet.py @@ -11,7 +11,6 @@ from test_framework.util import assert_equal, assert_greater_than, \ stop_nodes, sync_blocks, sync_mempools, wait_and_assert_operationid_status, \ wait_bitcoinds -import time from decimal import Decimal class WalletTest (BitcoinTestFramework): diff --git a/qa/rpc-tests/wallet_nullifiers.py b/qa/rpc-tests/wallet_nullifiers.py index 0cd0f4b43..4bd765ba9 100755 --- a/qa/rpc-tests/wallet_nullifiers.py +++ b/qa/rpc-tests/wallet_nullifiers.py @@ -8,7 +8,6 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_true, bitcoind_processes, \ connect_nodes_bi, start_node, start_nodes, wait_and_assert_operationid_status -import time from decimal import Decimal class WalletNullifiersTest (BitcoinTestFramework): diff --git a/qa/rpc-tests/wallet_protectcoinbase.py b/qa/rpc-tests/wallet_protectcoinbase.py index fbba2286b..9c7c537cb 100755 --- a/qa/rpc-tests/wallet_protectcoinbase.py +++ b/qa/rpc-tests/wallet_protectcoinbase.py @@ -11,7 +11,6 @@ from test_framework.util import assert_equal, initialize_chain_clean, \ start_nodes, connect_nodes_bi, wait_and_assert_operationid_status import sys -import time import timeit from decimal import Decimal From 5602e1f1a6c1b69f2c87cc1754cf4850a70be525 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Fri, 14 Sep 2018 11:55:33 -0600 Subject: [PATCH 23/31] Allow passing timeout parameter to wait_and_assert_operationid_status --- qa/rpc-tests/mempool_tx_input_limit.py | 2 +- qa/rpc-tests/test_framework/util.py | 4 ++-- qa/rpc-tests/wallet_nullifiers.py | 8 ++++---- qa/rpc-tests/wallet_protectcoinbase.py | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/qa/rpc-tests/mempool_tx_input_limit.py b/qa/rpc-tests/mempool_tx_input_limit.py index f91d9b52d..9a7131cfe 100755 --- a/qa/rpc-tests/mempool_tx_input_limit.py +++ b/qa/rpc-tests/mempool_tx_input_limit.py @@ -54,7 +54,7 @@ class MempoolTxInputLimitTest(BitcoinTestFramework): myopid = self.nodes[0].z_sendmany(node0_taddr, recipients) # Spend should fail due to -mempooltxinputlimit - wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Too many transparent inputs 3 > limit 2") + wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "Too many transparent inputs 3 > limit 2", 120) # Mempool should be empty. assert_equal(set(self.nodes[0].getrawmempool()), set()) diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index 07d56204e..ca5bcee3e 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -383,10 +383,10 @@ def assert_raises(exc, fun, *args, **kwds): raise AssertionError("No exception raised") # Returns txid if operation was a success or None -def wait_and_assert_operationid_status(node, myopid, in_status='success', in_errormsg=None): +def wait_and_assert_operationid_status(node, myopid, in_status='success', in_errormsg=None, timeout=300): print('waiting for async operation {}'.format(myopid)) result = None - for x in xrange(1, 300): # 300 is the timeout + for _ in xrange(1, timeout): results = node.z_getoperationresult([myopid]) if len(results) > 0: result = results[0] diff --git a/qa/rpc-tests/wallet_nullifiers.py b/qa/rpc-tests/wallet_nullifiers.py index 4bd765ba9..9b4e5649c 100755 --- a/qa/rpc-tests/wallet_nullifiers.py +++ b/qa/rpc-tests/wallet_nullifiers.py @@ -25,7 +25,7 @@ class WalletNullifiersTest (BitcoinTestFramework): recipients = [] recipients.append({"address":myzaddr0, "amount":Decimal('10.0')-Decimal('0.0001')}) # utxo amount less fee - wait_and_assert_operationid_status(self.nodes[0], self.nodes[0].z_sendmany(mytaddr, recipients)) + wait_and_assert_operationid_status(self.nodes[0], self.nodes[0].z_sendmany(mytaddr, recipients), timeout=120) self.sync_all() self.nodes[0].generate(1) @@ -52,7 +52,7 @@ class WalletNullifiersTest (BitcoinTestFramework): recipients = [] recipients.append({"address":myzaddr, "amount":7.0}) - wait_and_assert_operationid_status(self.nodes[0], self.nodes[0].z_sendmany(myzaddr0, recipients)) + wait_and_assert_operationid_status(self.nodes[0], self.nodes[0].z_sendmany(myzaddr0, recipients), timeout=120) self.sync_all() self.nodes[0].generate(1) @@ -70,7 +70,7 @@ class WalletNullifiersTest (BitcoinTestFramework): recipients = [] recipients.append({"address":myzaddr3, "amount":2.0}) - wait_and_assert_operationid_status(self.nodes[2], self.nodes[2].z_sendmany(myzaddr, recipients)) + wait_and_assert_operationid_status(self.nodes[2], self.nodes[2].z_sendmany(myzaddr, recipients), timeout=120) self.sync_all() self.nodes[2].generate(1) @@ -97,7 +97,7 @@ class WalletNullifiersTest (BitcoinTestFramework): recipients = [] recipients.append({"address":mytaddr1, "amount":1.0}) - wait_and_assert_operationid_status(self.nodes[1], self.nodes[1].z_sendmany(myzaddr, recipients)) + wait_and_assert_operationid_status(self.nodes[1], self.nodes[1].z_sendmany(myzaddr, recipients), timeout=120) self.sync_all() self.nodes[1].generate(1) diff --git a/qa/rpc-tests/wallet_protectcoinbase.py b/qa/rpc-tests/wallet_protectcoinbase.py index 9c7c537cb..71512840d 100755 --- a/qa/rpc-tests/wallet_protectcoinbase.py +++ b/qa/rpc-tests/wallet_protectcoinbase.py @@ -82,7 +82,7 @@ class WalletProtectCoinbaseTest (BitcoinTestFramework): recipients= [{"address":myzaddr, "amount": Decimal('1')}] myopid = self.nodes[3].z_sendmany(mytaddr, recipients) - wait_and_assert_operationid_status(self.nodes[3], myopid, "failed", "no UTXOs found for taddr from address") + wait_and_assert_operationid_status(self.nodes[3], myopid, "failed", "no UTXOs found for taddr from address", 10) # This send will fail because our wallet does not allow any change when protecting a coinbase utxo, # as it's currently not possible to specify a change address in z_sendmany. @@ -90,7 +90,7 @@ class WalletProtectCoinbaseTest (BitcoinTestFramework): recipients.append({"address":myzaddr, "amount":Decimal('1.23456789')}) myopid = self.nodes[0].z_sendmany(mytaddr, recipients) - error_result = wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "wallet does not allow any change") + error_result = wait_and_assert_operationid_status(self.nodes[0], myopid, "failed", "wallet does not allow any change", 10) # Test that the returned status object contains a params field with the operation's input parameters assert_equal(error_result["method"], "z_sendmany") From 19697025c6e3be176abb056ce5c24ed15f6ed616 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Thu, 13 Sep 2018 16:34:23 -0600 Subject: [PATCH 24/31] Add test for signing raw transactions offline --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/signrawtransaction_offline.py | 57 ++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100755 qa/rpc-tests/signrawtransaction_offline.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 25ecc409a..9953b85ee 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -43,6 +43,7 @@ testScripts=( 'merkle_blocks.py' # 'fundrawtransaction.py' 'signrawtransactions.py' + 'signrawtransaction_offline.py' 'walletbackup.py' 'key_import_export.py' 'nodehandling.py' diff --git a/qa/rpc-tests/signrawtransaction_offline.py b/qa/rpc-tests/signrawtransaction_offline.py new file mode 100755 index 000000000..5eadf2cb3 --- /dev/null +++ b/qa/rpc-tests/signrawtransaction_offline.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python2 + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_true, connect_nodes_bi, \ + initialize_chain_clean, start_node + +class SignOfflineTest (BitcoinTestFramework): + # Setup Methods + def setup_chain(self): + print "Initializing test directory " + self.options.tmpdir + initialize_chain_clean(self.options.tmpdir, 2) + + def setup_network(self): + self.nodes = [ start_node(0, self.options.tmpdir, ["-nuparams=5ba81b19:10"]) ] + self.is_network_split = False + self.sync_all() + + # Tests + def run_test(self): + print "Mining blocks..." + self.nodes[0].generate(101) + + offline_node = start_node(1, self.options.tmpdir, ["-maxconnections=0", "-nuparams=5ba81b19:10"]) + self.nodes.append(offline_node) + + assert_equal(0, len(offline_node.getpeerinfo())) # make sure node 1 has no peers + + privkeys = [self.nodes[0].dumpprivkey(self.nodes[0].getnewaddress())] + taddr = self.nodes[0].getnewaddress() + + tx = self.nodes[0].listunspent()[0] + txid = tx['txid'] + scriptpubkey = tx['scriptPubKey'] + + create_inputs = [{'txid': txid, 'vout': 0}] + sign_inputs = [{'txid': txid, 'vout': 0, 'scriptPubKey': scriptpubkey, 'amount': 10}] + + create_hex = self.nodes[0].createrawtransaction(create_inputs, {taddr: 9.9999}) + print "create:" + print create_hex + + signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys) + print "sign:" + print signed_tx + + # If we return the transaction hash, then we have have not thrown an error (success) + online_tx_hash = self.nodes[0].sendrawtransaction(signed_tx['hex']) + assert_true(len(online_tx_hash) > 0) + + #signed_hex = signed_tx['hex'] + #print "decoded:" + #print self.nodes[0].decoderawtransaction(signed_hex) + print "sent:" + print online_tx_hash + +if __name__ == '__main__': + SignOfflineTest().main() \ No newline at end of file From 40b95273010f9cb57e078b9bcd3330e5829499bc Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Thu, 13 Sep 2018 16:55:18 -0600 Subject: [PATCH 25/31] Incorporate APPROX_RELEASE_HEIGHT when determining what consensus branch to sign with --- qa/rpc-tests/signrawtransaction_offline.py | 10 ---------- src/rpc/rawtransaction.cpp | 8 ++++++-- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/qa/rpc-tests/signrawtransaction_offline.py b/qa/rpc-tests/signrawtransaction_offline.py index 5eadf2cb3..55acf7b4a 100755 --- a/qa/rpc-tests/signrawtransaction_offline.py +++ b/qa/rpc-tests/signrawtransaction_offline.py @@ -36,22 +36,12 @@ class SignOfflineTest (BitcoinTestFramework): sign_inputs = [{'txid': txid, 'vout': 0, 'scriptPubKey': scriptpubkey, 'amount': 10}] create_hex = self.nodes[0].createrawtransaction(create_inputs, {taddr: 9.9999}) - print "create:" - print create_hex signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys) - print "sign:" - print signed_tx # If we return the transaction hash, then we have have not thrown an error (success) online_tx_hash = self.nodes[0].sendrawtransaction(signed_tx['hex']) assert_true(len(online_tx_hash) > 0) - #signed_hex = signed_tx['hex'] - #print "decoded:" - #print self.nodes[0].decoderawtransaction(signed_hex) - print "sent:" - print online_tx_hash - if __name__ == '__main__': SignOfflineTest().main() \ No newline at end of file diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 912cbe3ba..4a3c83610 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -7,6 +7,7 @@ #include "consensus/validation.h" #include "core_io.h" #include "init.h" +#include "deprecation.h" #include "key_io.h" #include "keystore.h" #include "main.h" @@ -872,9 +873,12 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) } bool fHashSingle = ((nHashType & ~SIGHASH_ANYONECANPAY) == SIGHASH_SINGLE); - + // Use the approximate release height if it is greater so offline nodes + // have a better estimation of the current height and will be more likely to + // determine the correct consensus branch ID. + int chainHeight = std::max(chainActive.Height() + 1, APPROX_RELEASE_HEIGHT); // Grab the current consensus branch ID - auto consensusBranchId = CurrentEpochBranchId(chainActive.Height() + 1, Params().GetConsensus()); + auto consensusBranchId = CurrentEpochBranchId(chainHeight, Params().GetConsensus()); // Script verification errors UniValue vErrors(UniValue::VARR); From 36a490677c36833f7552d4cfc204d7167b66f13a Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Mon, 17 Sep 2018 09:49:32 -0600 Subject: [PATCH 26/31] Allow passing branchId when calling signrawtransaction --- src/consensus/upgrades.cpp | 9 +++++++++ src/consensus/upgrades.h | 6 ++++++ src/rpc/rawtransaction.cpp | 13 +++++++++++-- src/test/rpc_tests.cpp | 2 ++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/consensus/upgrades.cpp b/src/consensus/upgrades.cpp index 95bde7925..5785d75ac 100644 --- a/src/consensus/upgrades.cpp +++ b/src/consensus/upgrades.cpp @@ -82,6 +82,15 @@ uint32_t CurrentEpochBranchId(int nHeight, const Consensus::Params& params) { return NetworkUpgradeInfo[CurrentEpoch(nHeight, params)].nBranchId; } +bool IsConsensusBranchId(int branchId) { + for (int idx = Consensus::BASE_SPROUT; idx < Consensus::MAX_NETWORK_UPGRADES; idx++) { + if (branchId == NetworkUpgradeInfo[idx].nBranchId) { + return true; + } + } + return false; +} + bool IsActivationHeight( int nHeight, const Consensus::Params& params, diff --git a/src/consensus/upgrades.h b/src/consensus/upgrades.h index 620dc94c4..638ed862f 100644 --- a/src/consensus/upgrades.h +++ b/src/consensus/upgrades.h @@ -63,6 +63,12 @@ int CurrentEpoch(int nHeight, const Consensus::Params& params); */ uint32_t CurrentEpochBranchId(int nHeight, const Consensus::Params& params); +/** + * Returns true if a given branch id is a valid nBranchId for one of the network + * upgrades contained in NetworkUpgradeInfo. + */ +bool IsConsensusBranchId(int branchId); + /** * Returns true if the given block height is the activation height for the given * upgrade. diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 4a3c83610..ab30b351f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -673,7 +673,7 @@ static void TxInErrorToJSON(const CTxIn& txin, UniValue& vErrorsRet, const std:: UniValue signrawtransaction(const UniValue& params, bool fHelp) { - if (fHelp || params.size() < 1 || params.size() > 4) + if (fHelp || params.size() < 1 || params.size() > 5) throw runtime_error( "signrawtransaction \"hexstring\" ( [{\"txid\":\"id\",\"vout\":n,\"scriptPubKey\":\"hex\",\"redeemScript\":\"hex\"},...] [\"privatekey1\",...] sighashtype )\n" "\nSign inputs for raw transaction (serialized, hex-encoded).\n" @@ -710,6 +710,8 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) " \"ALL|ANYONECANPAY\"\n" " \"NONE|ANYONECANPAY\"\n" " \"SINGLE|ANYONECANPAY\"\n" + "5. \"branchid\" (string, optional) The hex representation of the consensus branch id to sign with." + " This can be used to force signing with consensus rules that are ahead of the node's current height.\n" "\nResult:\n" "{\n" @@ -737,7 +739,7 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) #else LOCK(cs_main); #endif - RPCTypeCheck(params, boost::assign::list_of(UniValue::VSTR)(UniValue::VARR)(UniValue::VARR)(UniValue::VSTR), true); + RPCTypeCheck(params, boost::assign::list_of(UniValue::VSTR)(UniValue::VARR)(UniValue::VARR)(UniValue::VSTR)(UniValue::VSTR), true); vector txData(ParseHexV(params[0], "argument 1")); CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION); @@ -880,6 +882,13 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) // Grab the current consensus branch ID auto consensusBranchId = CurrentEpochBranchId(chainHeight, Params().GetConsensus()); + if (params.size() > 4 && !params[4].isNull()) { + consensusBranchId = ParseHexToUInt32(params[4].get_str()); + if (!IsConsensusBranchId(consensusBranchId)) { + throw runtime_error(params[4].get_str() + " is not a valid consensus branch id"); + } + } + // Script verification errors UniValue vErrors(UniValue::VARR); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 8884d783f..7cb1ef010 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -92,6 +92,8 @@ BOOST_AUTO_TEST_CASE(rpc_rawparams) BOOST_CHECK_NO_THROW(CallRPC(string("signrawtransaction ")+rawtx+" null null NONE|ANYONECANPAY")); BOOST_CHECK_NO_THROW(CallRPC(string("signrawtransaction ")+rawtx+" [] [] NONE|ANYONECANPAY")); BOOST_CHECK_THROW(CallRPC(string("signrawtransaction ")+rawtx+" null null badenum"), runtime_error); + BOOST_CHECK_NO_THROW(CallRPC(string("signrawtransaction ")+rawtx+" [] [] NONE|ANYONECANPAY 5ba81b19")); + BOOST_CHECK_THROW(CallRPC(string("signrawtransaction ")+rawtx+" [] [] ALL NONE|ANYONECANPAY 123abc"), runtime_error); // Only check failure cases for sendrawtransaction, there's no network to send to... BOOST_CHECK_THROW(CallRPC("sendrawtransaction"), runtime_error); From 9ba7479de101e7b9d50040543f5abeb2fbb84dcc Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 17 Sep 2018 09:44:43 -0700 Subject: [PATCH 27/31] Add Sapling fields to JSON RPC output using TxToJSON. --- src/rpc/rawtransaction.cpp | 41 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 912cbe3ba..2aed45097 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -114,6 +114,36 @@ UniValue TxJoinSplitToJSON(const CTransaction& tx) { return vjoinsplit; } +UniValue TxShieldedSpendsToJSON(const CTransaction& tx) { + UniValue vdesc(UniValue::VARR); + for (const SpendDescription& spendDesc : tx.vShieldedSpend) { + UniValue obj(UniValue::VOBJ); + obj.push_back(Pair("cv", spendDesc.cv.GetHex())); + obj.push_back(Pair("anchor", spendDesc.anchor.GetHex())); + obj.push_back(Pair("nullifier", spendDesc.nullifier.GetHex())); + obj.push_back(Pair("rk", spendDesc.rk.GetHex())); + obj.push_back(Pair("proof", HexStr(spendDesc.zkproof.begin(), spendDesc.zkproof.end()))); + obj.push_back(Pair("spendAuthSig", HexStr(spendDesc.spendAuthSig.begin(), spendDesc.spendAuthSig.end()))); + vdesc.push_back(obj); + } + return vdesc; +} + +UniValue TxShieldedOutputsToJSON(const CTransaction& tx) { + UniValue vdesc(UniValue::VARR); + for (const OutputDescription& outputDesc : tx.vShieldedOutput) { + UniValue obj(UniValue::VOBJ); + obj.push_back(Pair("cv", outputDesc.cv.GetHex())); + obj.push_back(Pair("cmu", outputDesc.cm.GetHex())); + obj.push_back(Pair("ephemeralKey", outputDesc.ephemeralKey.GetHex())); + obj.push_back(Pair("encCiphertext", HexStr(outputDesc.encCiphertext.begin(), outputDesc.encCiphertext.end()))); + obj.push_back(Pair("outCiphertext", HexStr(outputDesc.outCiphertext.begin(), outputDesc.outCiphertext.end()))); + obj.push_back(Pair("proof", HexStr(outputDesc.zkproof.begin(), outputDesc.zkproof.end()))); + vdesc.push_back(obj); + } + return vdesc; +} + void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) { entry.push_back(Pair("txid", tx.GetHash().GetHex())); @@ -160,6 +190,17 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) UniValue vjoinsplit = TxJoinSplitToJSON(tx); entry.push_back(Pair("vjoinsplit", vjoinsplit)); + if (tx.fOverwintered && tx.nVersion >= SAPLING_TX_VERSION) { + entry.push_back(Pair("valueBalance", ValueFromAmount(tx.valueBalance))); + UniValue vspenddesc = TxShieldedSpendsToJSON(tx); + entry.push_back(Pair("vShieldedSpend", vspenddesc)); + UniValue voutputdesc = TxShieldedOutputsToJSON(tx); + entry.push_back(Pair("vShieldedOutput", voutputdesc)); + if (!(vspenddesc.empty() && voutputdesc.empty())) { + entry.push_back(Pair("bindingSig", HexStr(tx.bindingSig.begin(), tx.bindingSig.end()))); + } + } + if (!hashBlock.IsNull()) { entry.push_back(Pair("blockhash", hashBlock.GetHex())); BlockMap::iterator mi = mapBlockIndex.find(hashBlock); From 3501519bc83e150bae9ca1a66e63b79e42f2da71 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 17 Sep 2018 10:01:50 -0700 Subject: [PATCH 28/31] Update qa test to check for Sapling related JSON fields. --- qa/rpc-tests/wallet_sapling.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/wallet_sapling.py b/qa/rpc-tests/wallet_sapling.py index 517869820..0ce1c734f 100755 --- a/qa/rpc-tests/wallet_sapling.py +++ b/qa/rpc-tests/wallet_sapling.py @@ -89,7 +89,7 @@ class WalletSaplingTest(BitcoinTestFramework): recipients.append({"address": saplingAddr0, "amount": Decimal('5')}) recipients.append({"address": taddr1, "amount": Decimal('5')}) myopid = self.nodes[1].z_sendmany(saplingAddr1, recipients, 1, 0) - wait_and_assert_operationid_status(self.nodes[1], myopid) + mytxid = wait_and_assert_operationid_status(self.nodes[1], myopid) self.sync_all() self.nodes[2].generate(1) @@ -100,5 +100,26 @@ class WalletSaplingTest(BitcoinTestFramework): assert_equal(self.nodes[1].z_getbalance(saplingAddr1), Decimal('5')) assert_equal(self.nodes[1].z_getbalance(taddr1), Decimal('5')) + # Verify existence of Sapling related JSON fields + resp = self.nodes[0].getrawtransaction(mytxid, 1) + assert_equal(resp['valueBalance'], Decimal('5')) + assert(len(resp['vShieldedSpend']) == 1) + assert(len(resp['vShieldedOutput']) == 2) + assert('bindingSig' in resp) + shieldedSpend = resp['vShieldedSpend'][0] + assert('cv' in shieldedSpend) + assert('anchor' in shieldedSpend) + assert('nullifier' in shieldedSpend) + assert('rk' in shieldedSpend) + assert('proof' in shieldedSpend) + assert('spendAuthSig' in shieldedSpend) + shieldedOutput = resp['vShieldedOutput'][0] + assert('cv' in shieldedOutput) + assert('cmu' in shieldedOutput) + assert('ephemeralKey' in shieldedOutput) + assert('encCiphertext' in shieldedOutput) + assert('outCiphertext' in shieldedOutput) + assert('proof' in shieldedOutput) + if __name__ == '__main__': WalletSaplingTest().main() From bd3c860cb478ceeead2d716caecb9196b2ea0167 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 18 Sep 2018 23:22:50 +0100 Subject: [PATCH 29/31] Use ovk directly in the TransactionBuilder API instead of fvk --- src/gtest/test_transaction_builder.cpp | 12 ++++++------ src/transaction_builder.cpp | 10 +++++----- src/transaction_builder.h | 6 +++--- src/wallet/asyncrpcoperation_sendmany.cpp | 6 +++--- src/wallet/gtest/test_wallet.cpp | 24 +++++++++++------------ 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/gtest/test_transaction_builder.cpp b/src/gtest/test_transaction_builder.cpp index b8fe54466..05a8cb601 100644 --- a/src/gtest/test_transaction_builder.cpp +++ b/src/gtest/test_transaction_builder.cpp @@ -38,7 +38,7 @@ TEST(TransactionBuilder, Invoke) // 0.0005 t-ZEC in, 0.0004 z-ZEC out, 0.0001 t-ZEC fee auto builder1 = TransactionBuilder(consensusParams, 1, &keystore); builder1.AddTransparentInput(COutPoint(), scriptPubKey, 50000); - builder1.AddSaplingOutput(fvk_from, pk, 40000, {}); + builder1.AddSaplingOutput(fvk_from.ovk, pk, 40000, {}); auto maybe_tx1 = builder1.Build(); ASSERT_EQ(static_cast(maybe_tx1), true); auto tx1 = maybe_tx1.get(); @@ -73,7 +73,7 @@ TEST(TransactionBuilder, Invoke) // Check that trying to add a different anchor fails ASSERT_FALSE(builder2.AddSaplingSpend(expsk, note, uint256(), witness)); - builder2.AddSaplingOutput(fvk, pk, 25000, {}); + builder2.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx2 = builder2.Build(); ASSERT_EQ(static_cast(maybe_tx2), true); auto tx2 = maybe_tx2.get(); @@ -153,7 +153,7 @@ TEST(TransactionBuilder, FailsWithNegativeChange) // Fail if there is only a Sapling output // 0.0005 z-ZEC out, 0.0001 t-ZEC fee auto builder = TransactionBuilder(consensusParams, 1); - builder.AddSaplingOutput(fvk, pk, 50000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 50000, {}); EXPECT_FALSE(static_cast(builder.Build())); // Fail if there is only a transparent output @@ -237,7 +237,7 @@ TEST(TransactionBuilder, ChangeOutput) { auto builder = TransactionBuilder(consensusParams, 1, &keystore); builder.AddTransparentInput(COutPoint(), scriptPubKey, 25000); - builder.SendChangeTo(zChangeAddr, fvkOut); + builder.SendChangeTo(zChangeAddr, fvkOut.ovk); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -298,7 +298,7 @@ TEST(TransactionBuilder, SetFee) { auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -315,7 +315,7 @@ TEST(TransactionBuilder, SetFee) { auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); builder.SetFee(20000); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index ed8d8c25b..e137cbdb8 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -47,13 +47,13 @@ bool TransactionBuilder::AddSaplingSpend( } void TransactionBuilder::AddSaplingOutput( - libzcash::SaplingFullViewingKey from, + uint256 ovk, libzcash::SaplingPaymentAddress to, CAmount value, std::array memo) { auto note = libzcash::SaplingNote(to, value); - outputs.emplace_back(from.ovk, note, memo); + outputs.emplace_back(ovk, note, memo); mtx.valueBalance -= value; } @@ -84,9 +84,9 @@ void TransactionBuilder::SetFee(CAmount fee) this->fee = fee; } -void TransactionBuilder::SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, libzcash::SaplingFullViewingKey fvkOut) +void TransactionBuilder::SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, uint256 ovk) { - zChangeAddr = std::make_pair(fvkOut, changeAddr); + zChangeAddr = std::make_pair(ovk, changeAddr); tChangeAddr = boost::none; } @@ -136,7 +136,7 @@ boost::optional TransactionBuilder::Build() auto fvk = spends[0].expsk.full_viewing_key(); auto note = spends[0].note; libzcash::SaplingPaymentAddress changeAddr(note.d, note.pk_d); - AddSaplingOutput(fvk, changeAddr, change, {}); + AddSaplingOutput(fvk.ovk, changeAddr, change, {}); } else { return boost::none; } diff --git a/src/transaction_builder.h b/src/transaction_builder.h index 90f2eff2d..560898381 100644 --- a/src/transaction_builder.h +++ b/src/transaction_builder.h @@ -65,7 +65,7 @@ private: std::vector outputs; std::vector tIns; - boost::optional> zChangeAddr; + boost::optional> zChangeAddr; boost::optional tChangeAddr; public: @@ -83,7 +83,7 @@ public: SaplingWitness witness); void AddSaplingOutput( - libzcash::SaplingFullViewingKey from, + uint256 ovk, libzcash::SaplingPaymentAddress to, CAmount value, std::array memo); @@ -93,7 +93,7 @@ public: bool AddTransparentOutput(CTxDestination& to, CAmount value); - void SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, libzcash::SaplingFullViewingKey fvkOut); + void SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, uint256 ovk); bool SendChangeTo(CTxDestination& changeAddr); diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index ba65b2d75..0984b5e21 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -380,11 +380,11 @@ bool AsyncRPCOperation_sendmany::main_impl() { // Get various necessary keys SaplingExpandedSpendingKey expsk; - SaplingFullViewingKey from; + uint256 ovk; if (isfromzaddr_) { auto sk = boost::get(spendingkey_); expsk = sk.expsk; - from = expsk.full_viewing_key(); + ovk = expsk.full_viewing_key().ovk; } else { // TODO: Set "from" to something! } @@ -450,7 +450,7 @@ bool AsyncRPCOperation_sendmany::main_impl() { auto memo = get_memo_from_hex_string(hexMemo); - builder_.AddSaplingOutput(from, to, value, memo); + builder_.AddSaplingOutput(ovk, to, value, memo); } // Add transparent outputs diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index abc582faa..a67c3b491 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -384,7 +384,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 50000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 50000, {}); builder.SetFee(0); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); @@ -504,7 +504,7 @@ TEST(WalletTests, FindMySaplingNotes) { // Generate transaction auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -644,7 +644,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Generate tx to create output note B auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 35000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 35000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -700,7 +700,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Create transaction to spend note B auto builder2 = TransactionBuilder(consensusParams, 2); ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness)); - builder2.AddSaplingOutput(fvk, pk, 20000, {}); + builder2.AddSaplingOutput(fvk.ovk, pk, 20000, {}); auto maybe_tx2 = builder2.Build(); ASSERT_EQ(static_cast(maybe_tx2), true); auto tx2 = maybe_tx2.get(); @@ -708,7 +708,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Create conflicting transaction which also spends note B auto builder3 = TransactionBuilder(consensusParams, 2); ASSERT_TRUE(builder3.AddSaplingSpend(expsk, note2, anchor, spend_note_witness)); - builder3.AddSaplingOutput(fvk, pk, 19999, {}); + builder3.AddSaplingOutput(fvk.ovk, pk, 19999, {}); auto maybe_tx3 = builder3.Build(); ASSERT_EQ(static_cast(maybe_tx3), true); auto tx3 = maybe_tx3.get(); @@ -809,7 +809,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) { // Generate transaction auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -906,7 +906,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { // Generate transaction auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -1042,7 +1042,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Generate transaction, which sends funds to note B auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -1114,7 +1114,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Create transaction to spend note B auto builder2 = TransactionBuilder(consensusParams, 2); ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness)); - builder2.AddSaplingOutput(fvk, pk, 12500, {}); + builder2.AddSaplingOutput(fvk.ovk, pk, 12500, {}); auto maybe_tx2 = builder2.Build(); ASSERT_EQ(static_cast(maybe_tx2), true); auto tx2 = maybe_tx2.get(); @@ -1744,7 +1744,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { // Generate transaction auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk2, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk2, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -1894,7 +1894,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { // 0.0005 t-ZEC in, 0.0004 z-ZEC out, 0.0001 t-ZEC fee auto builder = TransactionBuilder(consensusParams, 1, &keystore); builder.AddTransparentInput(COutPoint(), scriptPubKey, 50000); - builder.AddSaplingOutput(fvk, pk, 40000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 40000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx1 = maybe_tx.get(); @@ -1951,7 +1951,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { // 0.0004 z-ZEC in, 0.00025 z-ZEC out, 0.0001 t-ZEC fee, 0.00005 z-ZEC change auto builder2 = TransactionBuilder(consensusParams, 2); ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note, anchor, witness)); - builder2.AddSaplingOutput(fvk, pk, 25000, {}); + builder2.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx2 = builder2.Build(); ASSERT_EQ(static_cast(maybe_tx2), true); auto tx2 = maybe_tx2.get(); From bb4b6982e3847d66597b956c52253ba3a6f6fc90 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 19 Sep 2018 00:51:30 +0100 Subject: [PATCH 30/31] Generate an ovk to encrypt outCiphertext for t-addr senders Closes #3506. --- src/test/rpc_wallet_tests.cpp | 102 ++++++++++++++++++++++ src/wallet/asyncrpcoperation_sendmany.cpp | 12 ++- src/zcash/zip32.cpp | 27 ++++++ src/zcash/zip32.h | 3 + 4 files changed, 143 insertions(+), 1 deletion(-) diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index 3bc729f6f..6b132d0a1 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/src/test/rpc_wallet_tests.cpp @@ -1254,6 +1254,108 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals) } +BOOST_AUTO_TEST_CASE(rpc_z_sendmany_taddr_to_sapling) +{ + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + + LOCK(pwalletMain->cs_wallet); + + if (!pwalletMain->HaveHDSeed()) { + pwalletMain->GenerateNewSeed(); + } + + UniValue retValue; + + // add keys manually + auto taddr = pwalletMain->GenerateNewKey().GetID(); + std::string taddr1 = EncodeDestination(taddr); + auto pa = pwalletMain->GenerateNewSaplingZKey(); + std::string zaddr1 = EncodePaymentAddress(pa); + + auto consensusParams = Params().GetConsensus(); + retValue = CallRPC("getblockcount"); + int nextBlockHeight = retValue.get_int() + 1; + + // Add a fake transaction to the wallet + CMutableTransaction mtx = CreateNewContextualCMutableTransaction(consensusParams, nextBlockHeight); + CScript scriptPubKey = CScript() << OP_DUP << OP_HASH160 << ToByteVector(taddr) << OP_EQUALVERIFY << OP_CHECKSIG; + mtx.vout.push_back(CTxOut(5 * COIN, scriptPubKey)); + CWalletTx wtx(pwalletMain, mtx); + pwalletMain->AddToWallet(wtx, true, NULL); + + // Fake-mine the transaction + BOOST_CHECK_EQUAL(0, chainActive.Height()); + CBlock block; + block.hashPrevBlock = chainActive.Tip()->GetBlockHash(); + block.vtx.push_back(wtx); + block.hashMerkleRoot = block.BuildMerkleTree(); + auto blockHash = block.GetHash(); + CBlockIndex fakeIndex {block}; + fakeIndex.nHeight = 1; + mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + chainActive.SetTip(&fakeIndex); + BOOST_CHECK(chainActive.Contains(&fakeIndex)); + BOOST_CHECK_EQUAL(1, chainActive.Height()); + wtx.SetMerkleBranch(block); + pwalletMain->AddToWallet(wtx, true, NULL); + + // Context that z_sendmany requires + auto builder = TransactionBuilder(consensusParams, nextBlockHeight, pwalletMain); + mtx = CreateNewContextualCMutableTransaction(consensusParams, nextBlockHeight); + + std::vector recipients = { SendManyRecipient(zaddr1, 1 * COIN, "ABCD") }; + std::shared_ptr operation( new AsyncRPCOperation_sendmany(builder, mtx, taddr1, {}, recipients, 0) ); + std::shared_ptr ptr = std::dynamic_pointer_cast (operation); + + // Enable test mode so tx is not sent + static_cast(operation.get())->testmode = true; + + // Generate the Sapling shielding transaction + operation->main(); + BOOST_CHECK(operation->isSuccess()); + + // Get the transaction + auto result = operation->getResult(); + BOOST_ASSERT(result.isObject()); + auto hexTx = result["hex"].getValStr(); + CDataStream ss(ParseHex(hexTx), SER_NETWORK, PROTOCOL_VERSION); + CTransaction tx; + ss >> tx; + BOOST_ASSERT(!tx.vShieldedOutput.empty()); + + // We shouldn't be able to decrypt with the empty ovk + BOOST_CHECK(!AttemptSaplingOutDecryption( + tx.vShieldedOutput[0].outCiphertext, + uint256(), + tx.vShieldedOutput[0].cv, + tx.vShieldedOutput[0].cm, + tx.vShieldedOutput[0].ephemeralKey)); + + // We should be able to decrypt the outCiphertext with the ovk + // generated for transparent addresses + HDSeed seed; + BOOST_ASSERT(pwalletMain->GetHDSeed(seed)); + BOOST_CHECK(AttemptSaplingOutDecryption( + tx.vShieldedOutput[0].outCiphertext, + ovkForShieldingFromTaddr(seed), + tx.vShieldedOutput[0].cv, + tx.vShieldedOutput[0].cm, + tx.vShieldedOutput[0].ephemeralKey)); + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); + mapArgs.erase("-developersapling"); + mapArgs.erase("-experimentalfeatures"); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); +} + + /* * This test covers storing encrypted zkeys in the wallet. */ diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 0984b5e21..64b1a5f26 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -386,7 +386,17 @@ bool AsyncRPCOperation_sendmany::main_impl() { expsk = sk.expsk; ovk = expsk.full_viewing_key().ovk; } else { - // TODO: Set "from" to something! + // Sending from a t-address, which we don't have an ovk for. Instead, + // generate a common one from the HD seed. This ensures the data is + // recoverable, while keeping it logically separate from the ZIP 32 + // Sapling key hierarchy, which the user might not be using. + HDSeed seed; + if (!pwalletMain->GetHDSeed(seed)) { + throw JSONRPCError( + RPC_WALLET_ERROR, + "CWallet::GenerateNewSaplingZKey(): HD seed not found"); + } + ovk = ovkForShieldingFromTaddr(seed); } // Set change address if we are using transparent funds diff --git a/src/zcash/zip32.cpp b/src/zcash/zip32.cpp index cdad98aa9..15478843e 100644 --- a/src/zcash/zip32.cpp +++ b/src/zcash/zip32.cpp @@ -8,6 +8,7 @@ #include "random.h" #include "streams.h" #include "version.h" +#include "zcash/prf.h" #include #include @@ -15,6 +16,9 @@ const unsigned char ZCASH_HD_SEED_FP_PERSONAL[crypto_generichash_blake2b_PERSONALBYTES] = {'Z', 'c', 'a', 's', 'h', '_', 'H', 'D', '_', 'S', 'e', 'e', 'd', '_', 'F', 'P'}; +const unsigned char ZCASH_TADDR_OVK_PERSONAL[crypto_generichash_blake2b_PERSONALBYTES] = + {'Z', 'c', 'T', 'a', 'd', 'd', 'r', 'T', 'o', 'S', 'a', 'p', 'l', 'i', 'n', 'g'}; + HDSeed HDSeed::Random(size_t len) { assert(len >= 32); @@ -30,6 +34,29 @@ uint256 HDSeed::Fingerprint() const return h.GetHash(); } +uint256 ovkForShieldingFromTaddr(HDSeed& seed) { + auto rawSeed = seed.RawSeed(); + + // I = BLAKE2b-512("ZcTaddrToSapling", seed) + crypto_generichash_blake2b_state state; + assert(crypto_generichash_blake2b_init_salt_personal( + &state, + NULL, 0, // No key. + 64, + NULL, // No salt. + ZCASH_TADDR_OVK_PERSONAL) == 0); + crypto_generichash_blake2b_update(&state, rawSeed.data(), rawSeed.size()); + auto intermediate = std::array(); + crypto_generichash_blake2b_final(&state, intermediate.data(), 64); + + // I_L = I[0..32] + uint256 intermediate_L; + memcpy(intermediate_L.begin(), intermediate.data(), 32); + + // ovk = truncate_32(PRF^expand(I_L, [0x02])) + return PRF_ovk(intermediate_L); +} + namespace libzcash { boost::optional SaplingExtendedFullViewingKey::Derive(uint32_t i) const diff --git a/src/zcash/zip32.h b/src/zcash/zip32.h index fcd16b4e1..0fc5785bd 100644 --- a/src/zcash/zip32.h +++ b/src/zcash/zip32.h @@ -42,6 +42,9 @@ public: } }; +// This is not part of ZIP 32, but is here because it's linked to the HD seed. +uint256 ovkForShieldingFromTaddr(HDSeed& seed); + namespace libzcash { typedef blob88 diversifier_index_t; From c10249f3ded369a1926fd5dbe2a3d74794a898e6 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Wed, 19 Sep 2018 14:41:02 -0600 Subject: [PATCH 31/31] Remove unused import --- qa/rpc-tests/signrawtransaction_offline.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qa/rpc-tests/signrawtransaction_offline.py b/qa/rpc-tests/signrawtransaction_offline.py index 55acf7b4a..cde9ca166 100755 --- a/qa/rpc-tests/signrawtransaction_offline.py +++ b/qa/rpc-tests/signrawtransaction_offline.py @@ -1,8 +1,7 @@ #!/usr/bin/env python2 from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_true, connect_nodes_bi, \ - initialize_chain_clean, start_node +from test_framework.util import assert_equal, assert_true, initialize_chain_clean, start_node class SignOfflineTest (BitcoinTestFramework): # Setup Methods