From 52cfa9c1ee43e90c96e31a712b42926b857b8ee6 Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Wed, 24 Apr 2019 16:20:08 -0600 Subject: [PATCH 1/9] Create method for getting HD seed in RPCs --- src/wallet/asyncrpcoperation_mergetoaddress.cpp | 7 +------ src/wallet/asyncrpcoperation_saplingmigration.cpp | 8 +------- src/wallet/asyncrpcoperation_sendmany.cpp | 7 +------ src/wallet/asyncrpcoperation_shieldcoinbase.cpp | 7 +------ src/wallet/rpcdump.cpp | 3 +-- src/wallet/wallet.cpp | 8 ++++++++ src/wallet/wallet.h | 3 +++ 7 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/wallet/asyncrpcoperation_mergetoaddress.cpp b/src/wallet/asyncrpcoperation_mergetoaddress.cpp index a0b414614..b3870e602 100644 --- a/src/wallet/asyncrpcoperation_mergetoaddress.cpp +++ b/src/wallet/asyncrpcoperation_mergetoaddress.cpp @@ -348,12 +348,7 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() // 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, - "AsyncRPCOperation_sendmany: HD seed not found"); - } + HDSeed seed = pwalletMain->GetHDSeedForRPC(); ovk = ovkForShieldingFromTaddr(seed); } if (!ovk) { diff --git a/src/wallet/asyncrpcoperation_saplingmigration.cpp b/src/wallet/asyncrpcoperation_saplingmigration.cpp index 4076415ea..ab44bf760 100644 --- a/src/wallet/asyncrpcoperation_saplingmigration.cpp +++ b/src/wallet/asyncrpcoperation_saplingmigration.cpp @@ -85,13 +85,7 @@ bool AsyncRPCOperation_saplingmigration::main_impl() { return true; } - HDSeed seed; - if (!pwalletMain->GetHDSeed(seed)) { - throw JSONRPCError( - RPC_WALLET_ERROR, - "AsyncRPCOperation_AsyncRPCOperation_saplingmigration: HD seed not found"); - } - + HDSeed seed = pwalletMain->GetHDSeedForRPC(); libzcash::SaplingPaymentAddress migrationDestAddress = getMigrationDestAddress(seed); auto consensusParams = Params().GetConsensus(); diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index e33440a4d..e30c07483 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -381,12 +381,7 @@ bool AsyncRPCOperation_sendmany::main_impl() { // 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, - "AsyncRPCOperation_sendmany::main_impl(): HD seed not found"); - } + HDSeed seed = pwalletMain->GetHDSeedForRPC(); ovk = ovkForShieldingFromTaddr(seed); } diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp index 4280ac2f1..84811c686 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp @@ -247,12 +247,7 @@ bool ShieldToAddress::operator()(const libzcash::SaplingPaymentAddress &zaddr) c // 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"); - } + HDSeed seed = pwalletMain->GetHDSeedForRPC(); uint256 ovk = ovkForShieldingFromTaddr(seed); // Add transparent inputs diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 600d0de8b..8105f5176 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -506,8 +506,7 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) 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); + HDSeed hdSeed = pwalletMain->GetHDSeedForRPC(); auto rawSeed = hdSeed.RawSeed(); file << strprintf("# HDSeed=%s fingerprint=%s", HexStr(rawSeed.begin(), rawSeed.end()), hdSeed.Fingerprint().GetHex()); file << "\n"; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 93e668923..cf220fcf2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2156,6 +2156,14 @@ bool CWallet::SetCryptedHDSeed(const uint256& seedFp, const std::vectorGetHDSeed(seed)) { + throw JSONRPCError(RPC_WALLET_ERROR, "HD seed not found"); + } + return seed; +} + void CWallet::SetHDChain(const CHDChain& chain, bool memonly) { LOCK(cs_wallet); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7cd808b62..6e4c239d9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1288,6 +1288,9 @@ public: bool SetHDSeed(const HDSeed& seed); bool SetCryptedHDSeed(const uint256& seedFp, const std::vector &vchCryptedSecret); + /* Returns the wallet's HD seed or throw JSONRPCError(...) */ + HDSeed GetHDSeedForRPC() const; + /* Set the HD chain model (chain child index counters) */ void SetHDChain(const CHDChain& chain, bool memonly); const CHDChain& GetHDChain() const { return hdChain; } From 6e82d72852ec81624ad8f833e1fc429786f4dfce Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Fri, 26 Apr 2019 15:31:57 -0600 Subject: [PATCH 2/9] Add rpc to get Sprout to Sapling migration status --- qa/rpc-tests/sprout_sapling_migration.py | 48 ++++++++-- .../asyncrpcoperation_saplingmigration.h | 4 +- src/wallet/rpcwallet.cpp | 96 +++++++++++++++++++ 3 files changed, 140 insertions(+), 8 deletions(-) diff --git a/qa/rpc-tests/sprout_sapling_migration.py b/qa/rpc-tests/sprout_sapling_migration.py index 13dc21890..cdfaa5393 100755 --- a/qa/rpc-tests/sprout_sapling_migration.py +++ b/qa/rpc-tests/sprout_sapling_migration.py @@ -11,12 +11,36 @@ from test_framework.util import assert_equal, assert_true, get_coinbase_address, initialize_chain_clean, start_nodes, wait_and_assert_operationid_status, \ wait_and_assert_operationid_status_result +SAPLING_ADDR = 'zregtestsapling1ssqj3f3majnl270985gqcdqedd9t4nlttjqskccwevj2v20sc25deqspv3masufnwcdy67cydyy' +SAPLING_KEY = 'secret-extended-key-regtest1qv62zt2fqyqqpqrh2qzc08h7gncf4447jh9kvnnnhjg959fkwt7mhw9j8e9at7attx8z6u3953u86vcnsujdc2ckdlcmztjt44x3uxpah5mxtncxd0mqcnz9eq8rghh5m4j44ep5d9702sdvvwawqassulktfegrcp4twxgqdxx4eww3lau0mywuaeztpla2cmvagr5nj98elt45zh6fjznadl6wz52n2uyhdwcm2wlsu8fnxstrk6s4t55t8dy6jkgx5g0cwpchh5qffp8x5' + + +def check_migration_status( + node, + enabled, + non_zero_unmigrated_amount, + non_zero_unfinalized_migrated_amount, + non_zero_finalized_migrated_amount, + finalized_migration_transactions, + len_migration_txids +): + status = node.z_getmigrationstatus() + assert_equal(enabled, status['enabled']) + assert_equal(SAPLING_ADDR, status['destination_address']) + assert_equal(non_zero_unmigrated_amount, Decimal(status['unmigrated_amount']) > Decimal('0.00')) + assert_equal(non_zero_unfinalized_migrated_amount, Decimal(status['unfinalized_migrated_amount']) > Decimal('0')) + assert_equal(non_zero_finalized_migrated_amount, Decimal(status['finalized_migrated_amount']) > Decimal('0')) + assert_equal(finalized_migration_transactions, status['finalized_migration_transactions']) + assert_equal(len_migration_txids, len(status['migration_txids'])) + class SproutSaplingMigration(BitcoinTestFramework): def setup_nodes(self): return start_nodes(4, self.options.tmpdir, [[ '-nuparams=5ba81b19:100', # Overwinter '-nuparams=76b809bb:100', # Sapling + '-migration', + '-migrationdestaddress=' + SAPLING_ADDR ]] * 4) def setup_chain(self): @@ -24,6 +48,10 @@ class SproutSaplingMigration(BitcoinTestFramework): initialize_chain_clean(self.options.tmpdir, 4) def run_test(self): + check_migration_status(self.nodes[0], True, False, False, False, 0, 0) + self.nodes[0].z_setmigration(False) + check_migration_status(self.nodes[0], False, False, False, False, 0, 0) + print "Mining blocks..." self.nodes[0].generate(101) self.sync_all() @@ -31,7 +59,8 @@ class SproutSaplingMigration(BitcoinTestFramework): # Send some ZEC to a Sprout address tAddr = get_coinbase_address(self.nodes[0]) sproutAddr = self.nodes[0].z_getnewaddress('sprout') - saplingAddr = self.nodes[0].z_getnewaddress('sapling') + # Import a previously generated key to test '-migrationdestaddress' + self.nodes[0].z_importkey(SAPLING_KEY) opid = self.nodes[0].z_sendmany(tAddr, [{"address": sproutAddr, "amount": Decimal('10')}], 1, 0) wait_and_assert_operationid_status(self.nodes[0], opid) @@ -39,7 +68,7 @@ class SproutSaplingMigration(BitcoinTestFramework): self.sync_all() assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('10')) - assert_equal(self.nodes[0].z_getbalance(saplingAddr), Decimal('0')) + assert_equal(self.nodes[0].z_getbalance(SAPLING_ADDR), Decimal('0')) # Migrate self.nodes[0].z_setmigration(True) @@ -49,6 +78,7 @@ class SproutSaplingMigration(BitcoinTestFramework): # At 494 we should have no async operations assert_equal(0, len(self.nodes[0].z_getoperationstatus()), "num async operations at 494") + check_migration_status(self.nodes[0], True, True, False, False, 0, 0) self.nodes[0].generate(1) self.sync_all() @@ -74,7 +104,7 @@ class SproutSaplingMigration(BitcoinTestFramework): # At 498 the mempool will be empty and no funds will have moved assert_equal(0, len(self.nodes[0].getrawmempool()), "mempool size at 498") assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('10')) - assert_equal(self.nodes[0].z_getbalance(saplingAddr), Decimal('0')) + assert_equal(self.nodes[0].z_getbalance(SAPLING_ADDR), Decimal('0')) self.nodes[0].generate(1) self.sync_all() @@ -82,20 +112,26 @@ class SproutSaplingMigration(BitcoinTestFramework): # At 499 there will be a transaction in the mempool and the note will be locked assert_equal(1, len(self.nodes[0].getrawmempool()), "mempool size at 499") assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('0')) - assert_equal(self.nodes[0].z_getbalance(saplingAddr), Decimal('0')) - assert_true(self.nodes[0].z_getbalance(saplingAddr, 0) > Decimal('0'), "Unconfirmed sapling") + assert_equal(self.nodes[0].z_getbalance(SAPLING_ADDR), Decimal('0')) + assert_true(self.nodes[0].z_getbalance(SAPLING_ADDR, 0) > Decimal('0'), "Unconfirmed sapling") self.nodes[0].generate(1) self.sync_all() # At 500 funds will have moved sprout_balance = self.nodes[0].z_getbalance(sproutAddr) - sapling_balance = self.nodes[0].z_getbalance(saplingAddr) + sapling_balance = self.nodes[0].z_getbalance(SAPLING_ADDR) print "sprout balance: {}, sapling balance: {}".format(sprout_balance, sapling_balance) assert_true(sprout_balance < Decimal('10'), "Should have less Sprout funds") assert_true(sapling_balance > Decimal('0'), "Should have more Sapling funds") assert_true(sprout_balance + sapling_balance, Decimal('9.9999')) + check_migration_status(self.nodes[0], True, True, True, False, 0, 1) + # At 510 the transactions will be considered 'finalized' + self.nodes[0].generate(10) + self.sync_all() + check_migration_status(self.nodes[0], True, True, False, True, 1, 1) + if __name__ == '__main__': SproutSaplingMigration().main() diff --git a/src/wallet/asyncrpcoperation_saplingmigration.h b/src/wallet/asyncrpcoperation_saplingmigration.h index af26281f6..e077c8eca 100644 --- a/src/wallet/asyncrpcoperation_saplingmigration.h +++ b/src/wallet/asyncrpcoperation_saplingmigration.h @@ -16,6 +16,8 @@ public: AsyncRPCOperation_saplingmigration& operator=(AsyncRPCOperation_saplingmigration const&) = delete; // Copy assign AsyncRPCOperation_saplingmigration& operator=(AsyncRPCOperation_saplingmigration&&) = delete; // Move assign + static libzcash::SaplingPaymentAddress getMigrationDestAddress(const HDSeed& seed); + virtual void main(); virtual UniValue getStatus() const; @@ -28,6 +30,4 @@ private: void setMigrationResult(int numTxCreated); CAmount chooseAmount(const CAmount& availableFunds); - - libzcash::SaplingPaymentAddress getMigrationDestAddress(const HDSeed& seed); }; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 347d65b18..c48ed2c15 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -27,6 +27,7 @@ #include "asyncrpcoperation.h" #include "asyncrpcqueue.h" #include "wallet/asyncrpcoperation_mergetoaddress.h" +#include "wallet/asyncrpcoperation_saplingmigration.h" #include "wallet/asyncrpcoperation_sendmany.h" #include "wallet/asyncrpcoperation_shieldcoinbase.h" @@ -3929,6 +3930,100 @@ UniValue z_setmigration(const UniValue& params, bool fHelp) { return NullUniValue; } +UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { + if (!EnsureWalletIsAvailable(fHelp)) + return NullUniValue; + if (fHelp || params.size() != 0) + throw runtime_error( + "z_getmigrationstatus\n" + "Returns information about the status of the Sprout to Sapling migration.\n" + "In the result a transactions is defined as finalized iff it has ten confirmations.\n" + "Note: It is possible that manually created transactions invloving this wallet\n" + "will be included in the result.\n" + "\nResult:\n" + "{\n" + " \"enabled\": true|false, (boolean) Whether or not migration is enabled\n" + " \"destination_address\": \"zaddr\", (string) The Sapling address which will receive Sprout funds\n" + " \"unmigrated_amount\": nnn.n, (numeric) The total amount of unmigrated " + CURRENCY_UNIT +" \n" + " \"unfinalized_migrated_amount\": nnn.n, (numeric) The total amount of unfinalized " + CURRENCY_UNIT + " \n" + " \"finalized_migrated_amount\": nnn.n, (numeric) The total amount of finalized " + CURRENCY_UNIT + " \n" + " \"finalized_migration_transactions\": nnn, (numeric) The number of migration transactions involving this wallet\n" + " \"time_started\": ttt, (numeric, optional) The block time of the first migration transaction\n" + " \"migration_txids\": [txids] (json array of strings) An array of all migration txids involving this wallet\n" + "}\n" + ); + LOCK2(cs_main, pwalletMain->cs_wallet); + UniValue migrationStatus(UniValue::VOBJ); + migrationStatus.push_back(Pair("enabled", pwalletMain->fSaplingMigrationEnabled)); + // The "destination_address" field MAY be omitted if the "-migrationaddress" + // parameter is not set and no default address has yet been generated. + // Note: The following function may return the default address even if it has not been added to the wallet + auto destinationAddress = AsyncRPCOperation_saplingmigration::getMigrationDestAddress(pwalletMain->GetHDSeedForRPC()); + migrationStatus.push_back(Pair("destination_address", EncodePaymentAddress(destinationAddress))); + // The values of "unmigrated_amount" and "migrated_amount" MUST take into + // account failed transactions, that were not mined within their expiration + // height. + { + std::vector sproutEntries; + std::vector saplingEntries; + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, "", 1); + CAmount unmigratedAmount = 0; + for (const auto& sproutEntry : sproutEntries) { + unmigratedAmount += sproutEntry.plaintext.value(); + } + migrationStatus.push_back(Pair("unmigrated_amount", FormatMoney(unmigratedAmount))); + } + // "migration_txids" is a list of strings representing transaction IDs of all + // known migration transactions involving this wallet, as lowercase hexadecimal + // in RPC byte order. + UniValue migrationTxids(UniValue::VARR); + int currentHeight = chainActive.Height(); + CAmount unfinalizedMigratedAmount = 0; + CAmount finalizedMigratedAmount = 0; + int numFinalizedMigrationTxs = 0; + uint64_t timeStarted = 0; + for (const auto& txPair : pwalletMain->mapWallet) { + CWalletTx tx = txPair.second; + // A given transaction is defined as a migration transaction iff it has: + // * one or more Sprout JoinSplits with nonzero vpub_new field; and + // * no Sapling Spends, and; + // * one or more Sapling Outputs. + if (tx.vjoinsplit.size() > 0 && tx.vShieldedSpend.empty() && tx.vShieldedOutput.size() > 0) { + CAmount migrationAmount = 0; + for (const auto& js : tx.vjoinsplit) { + migrationAmount += js.vpub_new; + } + if (migrationAmount == 0) { + continue; + } + migrationTxids.push_back(txPair.first.ToString()); + CBlockIndex* blockIndex = mapBlockIndex[tx.hashBlock]; + // A transaction is "finalized" iff it has 10 confirmations. + // TODO: subject to change, if the recommended number of confirmations changes. + if (currentHeight >= blockIndex->nHeight + 10) { + finalizedMigratedAmount += migrationAmount; + ++numFinalizedMigrationTxs; + } else { + unfinalizedMigratedAmount += migrationAmount; + } + // The value of "time_started" is the earliest Unix timestamp of any known + // migration transaction involving this wallet; if there is no such transaction, + // then the field is absent. + if (timeStarted == 0 || timeStarted > blockIndex->GetBlockTime()) { + timeStarted = blockIndex->GetBlockTime(); + } + } + } + migrationStatus.push_back(Pair("unfinalized_migrated_amount", FormatMoney(unfinalizedMigratedAmount))); + migrationStatus.push_back(Pair("finalized_migrated_amount", FormatMoney(finalizedMigratedAmount))); + migrationStatus.push_back(Pair("finalized_migration_transactions", numFinalizedMigrationTxs)); + if (timeStarted > 0) { + migrationStatus.push_back(Pair("time_started", timeStarted)); + } + migrationStatus.push_back(Pair("migration_txids", migrationTxids)); + return migrationStatus; +} + /** When estimating the number of coinbase utxos we can shield in a single transaction: 1. Joinsplit description is 1802 bytes. @@ -4683,6 +4778,7 @@ static const CRPCCommand commands[] = { "wallet", "z_mergetoaddress", &z_mergetoaddress, false }, { "wallet", "z_sendmany", &z_sendmany, false }, { "wallet", "z_setmigration", &z_setmigration, false }, + { "wallet", "z_getmigrationstatus", &z_getmigrationstatus, false }, { "wallet", "z_shieldcoinbase", &z_shieldcoinbase, false }, { "wallet", "z_getoperationstatus", &z_getoperationstatus, true }, { "wallet", "z_getoperationresult", &z_getoperationresult, true }, From 5969bd8f553f53d52ee961a750d1e26b3b147c52 Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Mon, 29 Apr 2019 10:39:05 -0600 Subject: [PATCH 3/9] Fix help message --- src/init.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index 653a05fcb..30bdccd51 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -406,7 +406,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageGroup(_("Wallet options:")); strUsage += HelpMessageOpt("-disablewallet", _("Do not load the wallet and disable wallet RPC calls")); strUsage += HelpMessageOpt("-keypool=", strprintf(_("Set key pool size to (default: %u)"), 100)); - strUsage += HelpMessageOpt("-migration=", _("Set to true to enable the Sprout to Sapling migration.")); + strUsage += HelpMessageOpt("-migration", _("Enable the Sprout to Sapling migration")); strUsage += HelpMessageOpt("-migrationdestaddress=", _("Set the Sapling migration address")); if (showDebug) strUsage += HelpMessageOpt("-mintxfee=", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for transaction creation (default: %s)", From 2276133bb36af999d7b80842b320c9687512205b Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Mon, 29 Apr 2019 13:53:29 -0600 Subject: [PATCH 4/9] Test migration using both the parameter and the default Sapling address --- qa/rpc-tests/sprout_sapling_migration.py | 150 ++++++++++++++--------- 1 file changed, 92 insertions(+), 58 deletions(-) diff --git a/qa/rpc-tests/sprout_sapling_migration.py b/qa/rpc-tests/sprout_sapling_migration.py index cdfaa5393..e364a2c87 100755 --- a/qa/rpc-tests/sprout_sapling_migration.py +++ b/qa/rpc-tests/sprout_sapling_migration.py @@ -18,6 +18,7 @@ SAPLING_KEY = 'secret-extended-key-regtest1qv62zt2fqyqqpqrh2qzc08h7gncf4447jh9kv def check_migration_status( node, enabled, + destination_address, non_zero_unmigrated_amount, non_zero_unfinalized_migrated_amount, non_zero_finalized_migrated_amount, @@ -26,7 +27,7 @@ def check_migration_status( ): status = node.z_getmigrationstatus() assert_equal(enabled, status['enabled']) - assert_equal(SAPLING_ADDR, status['destination_address']) + assert_equal(destination_address, status['destination_address']) assert_equal(non_zero_unmigrated_amount, Decimal(status['unmigrated_amount']) > Decimal('0.00')) assert_equal(non_zero_unfinalized_migrated_amount, Decimal(status['unfinalized_migrated_amount']) > Decimal('0')) assert_equal(non_zero_finalized_migrated_amount, Decimal(status['finalized_migrated_amount']) > Decimal('0')) @@ -36,101 +37,134 @@ def check_migration_status( class SproutSaplingMigration(BitcoinTestFramework): def setup_nodes(self): - return start_nodes(4, self.options.tmpdir, [[ + # Activate overwinter/sapling on all nodes + extra_args = [[ '-nuparams=5ba81b19:100', # Overwinter '-nuparams=76b809bb:100', # Sapling + ]] * 4 + # Add migration parameters to nodes[0] + extra_args[0] = extra_args[0] + [ '-migration', '-migrationdestaddress=' + SAPLING_ADDR - ]] * 4) + ] + assert_equal(4, len(extra_args[0])) + assert_equal(2, len(extra_args[1])) + return start_nodes(4, self.options.tmpdir, extra_args) def setup_chain(self): print("Initializing test directory " + self.options.tmpdir) initialize_chain_clean(self.options.tmpdir, 4) - def run_test(self): - check_migration_status(self.nodes[0], True, False, False, False, 0, 0) - self.nodes[0].z_setmigration(False) - check_migration_status(self.nodes[0], False, False, False, False, 0, 0) - - print "Mining blocks..." - self.nodes[0].generate(101) - self.sync_all() - - # Send some ZEC to a Sprout address - tAddr = get_coinbase_address(self.nodes[0]) - sproutAddr = self.nodes[0].z_getnewaddress('sprout') - # Import a previously generated key to test '-migrationdestaddress' - self.nodes[0].z_importkey(SAPLING_KEY) - - opid = self.nodes[0].z_sendmany(tAddr, [{"address": sproutAddr, "amount": Decimal('10')}], 1, 0) - wait_and_assert_operationid_status(self.nodes[0], opid) - self.nodes[0].generate(1) - self.sync_all() - - assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('10')) - assert_equal(self.nodes[0].z_getbalance(SAPLING_ADDR), Decimal('0')) + def run_migration_test(self, node, sproutAddr, saplingAddr, target_height): + # Make sure we are in a good state to run the test + assert_equal(102, node.getblockcount() % 500, "Should be at block 102 % 500") + assert_equal(node.z_getbalance(sproutAddr), Decimal('10')) + assert_equal(node.z_getbalance(saplingAddr), Decimal('0')) + check_migration_status(node, False, saplingAddr, True, False, False, 0, 0) # Migrate - self.nodes[0].z_setmigration(True) - print "Mining to block 494..." - self.nodes[0].generate(392) # 102 -> 494 + node.z_setmigration(True) + print "Mining to block 494 % 500..." + node.generate(392) # 102 % 500 -> 494 % 500 self.sync_all() - # At 494 we should have no async operations - assert_equal(0, len(self.nodes[0].z_getoperationstatus()), "num async operations at 494") - check_migration_status(self.nodes[0], True, True, False, False, 0, 0) + # At 494 % 500 we should have no async operations + assert_equal(0, len(node.z_getoperationstatus()), "num async operations at 494 % 500") + check_migration_status(node, True, saplingAddr, True, False, False, 0, 0) - self.nodes[0].generate(1) + node.generate(1) self.sync_all() - # At 495 we should have an async operation - operationstatus = self.nodes[0].z_getoperationstatus() + # At 495 % 500 we should have an async operation + operationstatus = node.z_getoperationstatus() print "migration operation: {}".format(operationstatus) - assert_equal(1, len(operationstatus), "num async operations at 495") + assert_equal(1, len(operationstatus), "num async operations at 495 % 500") assert_equal('saplingmigration', operationstatus[0]['method']) - assert_equal(500, operationstatus[0]['target_height']) + assert_equal(target_height, operationstatus[0]['target_height']) - result = wait_and_assert_operationid_status_result(self.nodes[0], operationstatus[0]['id']) + result = wait_and_assert_operationid_status_result(node, operationstatus[0]['id']) print "result: {}".format(result) assert_equal('saplingmigration', result['method']) - assert_equal(500, result['target_height']) + assert_equal(target_height, result['target_height']) assert_equal(1, result['result']['num_tx_created']) - assert_equal(0, len(self.nodes[0].getrawmempool()), "mempool size at 495") + assert_equal(0, len(node.getrawmempool()), "mempool size at 495 % 500") - self.nodes[0].generate(3) + node.generate(3) self.sync_all() - # At 498 the mempool will be empty and no funds will have moved - assert_equal(0, len(self.nodes[0].getrawmempool()), "mempool size at 498") - assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('10')) - assert_equal(self.nodes[0].z_getbalance(SAPLING_ADDR), Decimal('0')) + # At 498 % 500 the mempool will be empty and no funds will have moved + assert_equal(0, len(node.getrawmempool()), "mempool size at 498 % 500") + assert_equal(node.z_getbalance(sproutAddr), Decimal('10')) + assert_equal(node.z_getbalance(saplingAddr), Decimal('0')) - self.nodes[0].generate(1) + node.generate(1) self.sync_all() - # At 499 there will be a transaction in the mempool and the note will be locked - assert_equal(1, len(self.nodes[0].getrawmempool()), "mempool size at 499") - assert_equal(self.nodes[0].z_getbalance(sproutAddr), Decimal('0')) - assert_equal(self.nodes[0].z_getbalance(SAPLING_ADDR), Decimal('0')) - assert_true(self.nodes[0].z_getbalance(SAPLING_ADDR, 0) > Decimal('0'), "Unconfirmed sapling") + # At 499 % 500 there will be a transaction in the mempool and the note will be locked + assert_equal(1, len(node.getrawmempool()), "mempool size at 499 % 500") + assert_equal(node.z_getbalance(sproutAddr), Decimal('0')) + assert_equal(node.z_getbalance(saplingAddr), Decimal('0')) + assert_true(node.z_getbalance(saplingAddr, 0) > Decimal('0'), "Unconfirmed sapling balance at 499") - self.nodes[0].generate(1) + node.generate(1) self.sync_all() - # At 500 funds will have moved - sprout_balance = self.nodes[0].z_getbalance(sproutAddr) - sapling_balance = self.nodes[0].z_getbalance(SAPLING_ADDR) + # At 0 % 500 funds will have moved + sprout_balance = node.z_getbalance(sproutAddr) + sapling_balance = node.z_getbalance(saplingAddr) print "sprout balance: {}, sapling balance: {}".format(sprout_balance, sapling_balance) assert_true(sprout_balance < Decimal('10'), "Should have less Sprout funds") assert_true(sapling_balance > Decimal('0'), "Should have more Sapling funds") assert_true(sprout_balance + sapling_balance, Decimal('9.9999')) - check_migration_status(self.nodes[0], True, True, True, False, 0, 1) - # At 510 the transactions will be considered 'finalized' - self.nodes[0].generate(10) + check_migration_status(node, True, saplingAddr, True, True, False, 0, 1) + # At 10 % 500 the transactions will be considered 'finalized' + node.generate(10) self.sync_all() - check_migration_status(self.nodes[0], True, True, False, True, 1, 1) + check_migration_status(node, True, saplingAddr, True, False, True, 1, 1) + + def send_to_sprout_zaddr(self, tAddr, sproutAddr): + # Send some ZEC to a Sprout address + opid = self.nodes[0].z_sendmany(tAddr, [{"address": sproutAddr, "amount": Decimal('10')}], 1, 0) + wait_and_assert_operationid_status(self.nodes[0], opid) + self.nodes[0].generate(1) + self.sync_all() + + def run_test(self): + # Check enabling via '-migration' and disabling via rpc + check_migration_status(self.nodes[0], True, SAPLING_ADDR, False, False, False, 0, 0) + self.nodes[0].z_setmigration(False) + check_migration_status(self.nodes[0], False, SAPLING_ADDR, False, False, False, 0, 0) + + # 1. Test using self.nodes[0] which has the parameter + print "Runing test using '-migrationdestaddress'..." + print "Mining blocks..." + self.nodes[0].generate(101) + self.sync_all() + tAddr = get_coinbase_address(self.nodes[0]) + + # Import a previously generated key to test '-migrationdestaddress' + self.nodes[0].z_importkey(SAPLING_KEY) + sproutAddr0 = self.nodes[0].z_getnewaddress('sprout') + + self.send_to_sprout_zaddr(tAddr, sproutAddr0) + self.run_migration_test(self.nodes[0], sproutAddr0, SAPLING_ADDR, 500) + # Disable migration so only self.nodes[1] has a transaction in the mempool at block 999 + self.nodes[0].z_setmigration(False) + + # 2. Test using self.nodes[1] which will use the default Sapling address + print "Runing test using default Sapling address..." + # Mine more blocks so we start at 102 % 500 + print "Mining blocks..." + self.nodes[1].generate(91) # 511 -> 602 + self.sync_all() + + sproutAddr1 = self.nodes[1].z_getnewaddress('sprout') + saplingAddr1 = self.nodes[1].z_getnewaddress('sapling') + + self.send_to_sprout_zaddr(tAddr, sproutAddr1) + self.run_migration_test(self.nodes[1], sproutAddr1, saplingAddr1, 1000) if __name__ == '__main__': From 108e587c14f2fd3155af506bcc22d16510172437 Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Mon, 29 Apr 2019 14:42:48 -0600 Subject: [PATCH 5/9] Fix typos and update documentation --- qa/rpc-tests/sprout_sapling_migration.py | 6 +++--- src/wallet/rpcwallet.cpp | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/qa/rpc-tests/sprout_sapling_migration.py b/qa/rpc-tests/sprout_sapling_migration.py index e364a2c87..2287f0740 100755 --- a/qa/rpc-tests/sprout_sapling_migration.py +++ b/qa/rpc-tests/sprout_sapling_migration.py @@ -105,7 +105,7 @@ class SproutSaplingMigration(BitcoinTestFramework): assert_equal(1, len(node.getrawmempool()), "mempool size at 499 % 500") assert_equal(node.z_getbalance(sproutAddr), Decimal('0')) assert_equal(node.z_getbalance(saplingAddr), Decimal('0')) - assert_true(node.z_getbalance(saplingAddr, 0) > Decimal('0'), "Unconfirmed sapling balance at 499") + assert_true(node.z_getbalance(saplingAddr, 0) > Decimal('0'), "Unconfirmed sapling balance at 499 % 500") node.generate(1) self.sync_all() @@ -138,7 +138,7 @@ class SproutSaplingMigration(BitcoinTestFramework): check_migration_status(self.nodes[0], False, SAPLING_ADDR, False, False, False, 0, 0) # 1. Test using self.nodes[0] which has the parameter - print "Runing test using '-migrationdestaddress'..." + print "Running test using '-migrationdestaddress'..." print "Mining blocks..." self.nodes[0].generate(101) self.sync_all() @@ -154,7 +154,7 @@ class SproutSaplingMigration(BitcoinTestFramework): self.nodes[0].z_setmigration(False) # 2. Test using self.nodes[1] which will use the default Sapling address - print "Runing test using default Sapling address..." + print "Running test using default Sapling address..." # Mine more blocks so we start at 102 % 500 print "Mining blocks..." self.nodes[1].generate(91) # 511 -> 602 diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c48ed2c15..5fe0a7b1d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3937,13 +3937,14 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { throw runtime_error( "z_getmigrationstatus\n" "Returns information about the status of the Sprout to Sapling migration.\n" - "In the result a transactions is defined as finalized iff it has ten confirmations.\n" - "Note: It is possible that manually created transactions invloving this wallet\n" + "In the result a transactions is defined as finalized if and only if it has\n" + "at least ten confirmations.\n" + "Note: It is possible that manually created transactions involving this wallet\n" "will be included in the result.\n" "\nResult:\n" "{\n" " \"enabled\": true|false, (boolean) Whether or not migration is enabled\n" - " \"destination_address\": \"zaddr\", (string) The Sapling address which will receive Sprout funds\n" + " \"destination_address\": \"zaddr\", (string) The Sapling address that will receive Sprout funds\n" " \"unmigrated_amount\": nnn.n, (numeric) The total amount of unmigrated " + CURRENCY_UNIT +" \n" " \"unfinalized_migrated_amount\": nnn.n, (numeric) The total amount of unfinalized " + CURRENCY_UNIT + " \n" " \"finalized_migrated_amount\": nnn.n, (numeric) The total amount of finalized " + CURRENCY_UNIT + " \n" @@ -3955,7 +3956,7 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { LOCK2(cs_main, pwalletMain->cs_wallet); UniValue migrationStatus(UniValue::VOBJ); migrationStatus.push_back(Pair("enabled", pwalletMain->fSaplingMigrationEnabled)); - // The "destination_address" field MAY be omitted if the "-migrationaddress" + // The "destination_address" field MAY be omitted if the "-migrationdestaddress" // parameter is not set and no default address has yet been generated. // Note: The following function may return the default address even if it has not been added to the wallet auto destinationAddress = AsyncRPCOperation_saplingmigration::getMigrationDestAddress(pwalletMain->GetHDSeedForRPC()); From e9530f40f22d33acee648252a109a2eba0babb56 Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Mon, 29 Apr 2019 17:00:26 -0600 Subject: [PATCH 6/9] use -valueBalance rather than vpub_new to calculate migrated amount --- qa/rpc-tests/sprout_sapling_migration.py | 4 ++++ src/wallet/rpcwallet.cpp | 18 +++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/qa/rpc-tests/sprout_sapling_migration.py b/qa/rpc-tests/sprout_sapling_migration.py index 2287f0740..fd81c5730 100755 --- a/qa/rpc-tests/sprout_sapling_migration.py +++ b/qa/rpc-tests/sprout_sapling_migration.py @@ -123,6 +123,10 @@ class SproutSaplingMigration(BitcoinTestFramework): node.generate(10) self.sync_all() check_migration_status(node, True, saplingAddr, True, False, True, 1, 1) + # Check exact migration status amounts to make sure we account for fee + status = node.z_getmigrationstatus() + assert_equal(sprout_balance, Decimal(status['unmigrated_amount'])) + assert_equal(sapling_balance, Decimal(status['finalized_migrated_amount'])) def send_to_sprout_zaddr(self, tAddr, sproutAddr): # Send some ZEC to a Sprout address diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 5fe0a7b1d..651c1e328 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3989,23 +3989,27 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { // * one or more Sprout JoinSplits with nonzero vpub_new field; and // * no Sapling Spends, and; // * one or more Sapling Outputs. - if (tx.vjoinsplit.size() > 0 && tx.vShieldedSpend.empty() && tx.vShieldedOutput.size() > 0) { - CAmount migrationAmount = 0; + if (tx.vjoinsplit.size() > 0 && tx.vShieldedSpend.empty() && tx.vShieldedOutput.size() > 0 && + tx.vin.empty() && tx.vout.empty()) { + bool nonZeroVPubNew = false; for (const auto& js : tx.vjoinsplit) { - migrationAmount += js.vpub_new; + if (js.vpub_new > 0) { + nonZeroVPubNew = true; + break; + } } - if (migrationAmount == 0) { + if (!nonZeroVPubNew) { continue; } migrationTxids.push_back(txPair.first.ToString()); CBlockIndex* blockIndex = mapBlockIndex[tx.hashBlock]; - // A transaction is "finalized" iff it has 10 confirmations. + // A transaction is "finalized" iff it has at least 10 confirmations. // TODO: subject to change, if the recommended number of confirmations changes. if (currentHeight >= blockIndex->nHeight + 10) { - finalizedMigratedAmount += migrationAmount; + finalizedMigratedAmount -= tx.valueBalance; ++numFinalizedMigrationTxs; } else { - unfinalizedMigratedAmount += migrationAmount; + unfinalizedMigratedAmount -= tx.valueBalance; } // The value of "time_started" is the earliest Unix timestamp of any known // migration transaction involving this wallet; if there is no such transaction, From 345177cfc1085ee49e862760df8c03b8f6206923 Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Tue, 30 Apr 2019 09:52:53 -0600 Subject: [PATCH 7/9] Do not look at vin/vout when determining migration txs and other cleanup --- qa/rpc-tests/sprout_sapling_migration.py | 16 ++++++++-------- src/wallet/rpcwallet.cpp | 5 ++--- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/qa/rpc-tests/sprout_sapling_migration.py b/qa/rpc-tests/sprout_sapling_migration.py index fd81c5730..5f47d7796 100755 --- a/qa/rpc-tests/sprout_sapling_migration.py +++ b/qa/rpc-tests/sprout_sapling_migration.py @@ -64,7 +64,7 @@ class SproutSaplingMigration(BitcoinTestFramework): # Migrate node.z_setmigration(True) - print "Mining to block 494 % 500..." + print("Mining to block 494 % 500...") node.generate(392) # 102 % 500 -> 494 % 500 self.sync_all() @@ -77,13 +77,13 @@ class SproutSaplingMigration(BitcoinTestFramework): # At 495 % 500 we should have an async operation operationstatus = node.z_getoperationstatus() - print "migration operation: {}".format(operationstatus) + print("migration operation: {}".format(operationstatus)) assert_equal(1, len(operationstatus), "num async operations at 495 % 500") assert_equal('saplingmigration', operationstatus[0]['method']) assert_equal(target_height, operationstatus[0]['target_height']) result = wait_and_assert_operationid_status_result(node, operationstatus[0]['id']) - print "result: {}".format(result) + print("result: {}".format(result)) assert_equal('saplingmigration', result['method']) assert_equal(target_height, result['target_height']) assert_equal(1, result['result']['num_tx_created']) @@ -113,7 +113,7 @@ class SproutSaplingMigration(BitcoinTestFramework): # At 0 % 500 funds will have moved sprout_balance = node.z_getbalance(sproutAddr) sapling_balance = node.z_getbalance(saplingAddr) - print "sprout balance: {}, sapling balance: {}".format(sprout_balance, sapling_balance) + print("sprout balance: {}, sapling balance: {}".format(sprout_balance, sapling_balance)) assert_true(sprout_balance < Decimal('10'), "Should have less Sprout funds") assert_true(sapling_balance > Decimal('0'), "Should have more Sapling funds") assert_true(sprout_balance + sapling_balance, Decimal('9.9999')) @@ -142,8 +142,8 @@ class SproutSaplingMigration(BitcoinTestFramework): check_migration_status(self.nodes[0], False, SAPLING_ADDR, False, False, False, 0, 0) # 1. Test using self.nodes[0] which has the parameter - print "Running test using '-migrationdestaddress'..." - print "Mining blocks..." + print("Running test using '-migrationdestaddress'...") + print("Mining blocks...") self.nodes[0].generate(101) self.sync_all() tAddr = get_coinbase_address(self.nodes[0]) @@ -158,9 +158,9 @@ class SproutSaplingMigration(BitcoinTestFramework): self.nodes[0].z_setmigration(False) # 2. Test using self.nodes[1] which will use the default Sapling address - print "Running test using default Sapling address..." + print("Running test using default Sapling address...") # Mine more blocks so we start at 102 % 500 - print "Mining blocks..." + print("Mining blocks...") self.nodes[1].generate(91) # 511 -> 602 self.sync_all() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 651c1e328..f8f008666 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3949,7 +3949,7 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { " \"unfinalized_migrated_amount\": nnn.n, (numeric) The total amount of unfinalized " + CURRENCY_UNIT + " \n" " \"finalized_migrated_amount\": nnn.n, (numeric) The total amount of finalized " + CURRENCY_UNIT + " \n" " \"finalized_migration_transactions\": nnn, (numeric) The number of migration transactions involving this wallet\n" - " \"time_started\": ttt, (numeric, optional) The block time of the first migration transaction\n" + " \"time_started\": ttt, (numeric, optional) The block time of the first migration transaction as a Unix timestamp\n" " \"migration_txids\": [txids] (json array of strings) An array of all migration txids involving this wallet\n" "}\n" ); @@ -3989,8 +3989,7 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { // * one or more Sprout JoinSplits with nonzero vpub_new field; and // * no Sapling Spends, and; // * one or more Sapling Outputs. - if (tx.vjoinsplit.size() > 0 && tx.vShieldedSpend.empty() && tx.vShieldedOutput.size() > 0 && - tx.vin.empty() && tx.vout.empty()) { + if (tx.vjoinsplit.size() > 0 && tx.vShieldedSpend.empty() && tx.vShieldedOutput.size() > 0) { bool nonZeroVPubNew = false; for (const auto& js : tx.vjoinsplit) { if (js.vpub_new > 0) { From e14cf96642de67ebe2043b701e16e64168ca5079 Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Tue, 30 Apr 2019 11:06:08 -0600 Subject: [PATCH 8/9] Calculate the number of confimations in the canonical way --- src/wallet/rpcwallet.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f8f008666..d0e67a1b2 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3978,7 +3978,6 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { // known migration transactions involving this wallet, as lowercase hexadecimal // in RPC byte order. UniValue migrationTxids(UniValue::VARR); - int currentHeight = chainActive.Height(); CAmount unfinalizedMigratedAmount = 0; CAmount finalizedMigratedAmount = 0; int numFinalizedMigrationTxs = 0; @@ -4004,7 +4003,7 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { CBlockIndex* blockIndex = mapBlockIndex[tx.hashBlock]; // A transaction is "finalized" iff it has at least 10 confirmations. // TODO: subject to change, if the recommended number of confirmations changes. - if (currentHeight >= blockIndex->nHeight + 10) { + if (tx.GetDepthInMainChain() >= 10) { finalizedMigratedAmount -= tx.valueBalance; ++numFinalizedMigrationTxs; } else { From 3cad5f454fcf1933640b1d92351837e0858c0d0c Mon Sep 17 00:00:00 2001 From: Eirik0 Date: Tue, 30 Apr 2019 15:47:03 -0600 Subject: [PATCH 9/9] Do not throw an exception if HD Seed is not found when exporting wallet --- src/wallet/rpcdump.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 8105f5176..600d0de8b 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -506,7 +506,8 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) 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->GetHDSeedForRPC(); + HDSeed hdSeed; + pwalletMain->GetHDSeed(hdSeed); auto rawSeed = hdSeed.RawSeed(); file << strprintf("# HDSeed=%s fingerprint=%s", HexStr(rawSeed.begin(), rawSeed.end()), hdSeed.Fingerprint().GetHex()); file << "\n";