diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index e75999199..0d70968eb 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -11,6 +11,7 @@ export BITCOIND=${REAL_BITCOIND} #Run the tests testScripts=( + 'wallet_protectcoinbase.py' 'wallet.py' 'listtransactions.py' 'mempool_resurrect_test.py' diff --git a/qa/rpc-tests/wallet_protectcoinbase.py b/qa/rpc-tests/wallet_protectcoinbase.py new file mode 100755 index 000000000..78550e4c1 --- /dev/null +++ b/qa/rpc-tests/wallet_protectcoinbase.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python2 +# Copyright (c) 2016 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 * +from time import * + +class Wallet2Test (BitcoinTestFramework): + + def setup_chain(self): + print("Initializing test directory "+self.options.tmpdir) + initialize_chain_clean(self.options.tmpdir, 4) + + # Start nodes with -regtestprotectcoinbase to set fCoinbaseMustBeProtected to true. + def setup_network(self, split=False): + self.nodes = start_nodes(3, self.options.tmpdir, extra_args=[['-regtestprotectcoinbase']] * 3 ) + connect_nodes_bi(self.nodes,0,1) + connect_nodes_bi(self.nodes,1,2) + connect_nodes_bi(self.nodes,0,2) + self.is_network_split=False + self.sync_all() + + def wait_for_operationd_success(self, myopid): + print('waiting for async operation {}'.format(myopid)) + 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: + sleep(1) + else: + status = results[0]["status"] + break + print('...returned status: {}'.format(status)) + assert_equal("success", status) + + def run_test (self): + print "Mining blocks..." + + self.nodes[0].generate(4) + + walletinfo = self.nodes[0].getwalletinfo() + assert_equal(walletinfo['immature_balance'], 40) + assert_equal(walletinfo['balance'], 0) + + self.sync_all() + self.nodes[1].generate(101) + self.sync_all() + + assert_equal(self.nodes[0].getbalance(), 40) + assert_equal(self.nodes[1].getbalance(), 10) + assert_equal(self.nodes[2].getbalance(), 0) + + # Send will fail because we are enforcing the consensus rule that + # coinbase utxos can only be sent to a zaddr. + errorString = "" + try: + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) + except JSONRPCException,e: + errorString = e.error['message'] + assert_equal("Coinbase funds can only be sent to a zaddr" in errorString, True) + + # send node 0 taddr to node 0 zaddr + mytaddr = self.nodes[0].getnewaddress() + myzaddr = self.nodes[0].z_getnewaddress() + recipients = [] + recipients.append({"address":myzaddr, "amount":20.0}) + myopid = self.nodes[0].z_sendmany(mytaddr, recipients) + self.wait_for_operationd_success(myopid) + self.nodes[1].generate(1) + self.sync_all() + + # check balances (the z_sendmany consumes 3 coinbase utxos) + resp = self.nodes[0].z_gettotalbalance() + assert_equal(Decimal(resp["transparent"]), Decimal('10.0')) + assert_equal(Decimal(resp["private"]), Decimal('29.9999')) + assert_equal(Decimal(resp["total"]), Decimal('39.9999')) + + # convert note to transparent funds + recipients = [] + recipients.append({"address":mytaddr, "amount":20.0}) + myopid = self.nodes[0].z_sendmany(myzaddr, recipients) + self.wait_for_operationd_success(myopid) + self.nodes[1].generate(1) + self.sync_all() + + # check balances + resp = self.nodes[0].z_gettotalbalance() + assert_equal(Decimal(resp["transparent"]), Decimal('30.0')) + assert_equal(Decimal(resp["private"]), Decimal('9.9998')) + assert_equal(Decimal(resp["total"]), Decimal('39.9998')) + + # Send will fail because send amount is too big, even when including coinbase utxos + errorString = "" + try: + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 99999.0) + except JSONRPCException,e: + errorString = e.error['message'] + assert_equal("Insufficient funds" in errorString, True) + + # Send will fail because of insufficient funds unless sender uses coinbase utxos + try: + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 21.0) + except JSONRPCException,e: + errorString = e.error['message'] + assert_equal("Insufficient funds, coinbase funds can only be spent after they have been sent to a zaddr" in errorString, True) + + # Send will succeed because the balance of non-coinbase utxos is 20.0 + try: + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 19.0) + except JSONRPCException: + assert(False) + + self.nodes[1].generate(10) + self.sync_all() + + # check balance + assert_equal(self.nodes[2].getbalance(), Decimal('19')) + +if __name__ == '__main__': + Wallet2Test ().main () diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 8446993ce..523bd4e04 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -336,6 +336,11 @@ CChainParams &Params(CBaseChainParams::Network network) { void SelectParams(CBaseChainParams::Network network) { SelectBaseParams(network); pCurrentParams = &Params(network); + + // Some python qa rpc tests need to enforce the coinbase consensus rule + if (network == CBaseChainParams::REGTEST && mapArgs.count("-regtestprotectcoinbase")) { + regTestParams.SetRegTestCoinbaseMustBeProtected(); + } } bool SelectParamsFromCommandLine() diff --git a/src/chainparams.h b/src/chainparams.h index ac9d099ed..c60baf6e3 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -83,6 +83,8 @@ public: std::string GetFoundersRewardAddressAtIndex(int i) const; /** #1398 to return a fixed founders reward script for miner_tests */ bool fMinerTestModeForFoundersRewardScript = false; + /** Enforce coinbase consensus rule in regtest mode */ + void SetRegTestCoinbaseMustBeProtected() { consensus.fCoinbaseMustBeProtected = true; } protected: CChainParams() {} diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 0f6638387..a374aee62 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -715,7 +715,7 @@ bool AsyncRPCOperation_sendmany::find_utxos(bool fAcceptCoinbase=false) { LOCK2(cs_main, pwalletMain->cs_wallet); - pwalletMain->AvailableCoins(vecOutputs, false, NULL, true); + pwalletMain->AvailableCoins(vecOutputs, false, NULL, true, fAcceptCoinbase); BOOST_FOREACH(const COutput& out, vecOutputs) { if (out.nDepth < mindepth_) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 87deb0241..e8cb3493e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2050,7 +2050,7 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const /** * populate vCoins with vector of available COutputs. */ -void CWallet::AvailableCoins(vector& vCoins, bool fOnlyConfirmed, const CCoinControl *coinControl, bool fIncludeZeroValue) const +void CWallet::AvailableCoins(vector& vCoins, bool fOnlyConfirmed, const CCoinControl *coinControl, bool fIncludeZeroValue, bool fIncludeCoinBase) const { vCoins.clear(); @@ -2067,6 +2067,9 @@ void CWallet::AvailableCoins(vector& vCoins, bool fOnlyConfirmed, const if (fOnlyConfirmed && !pcoin->IsTrusted()) continue; + if (pcoin->IsCoinBase() && !fIncludeCoinBase) + continue; + if (pcoin->IsCoinBase() && pcoin->GetBlocksToMaturity() > 0) continue; @@ -2232,10 +2235,38 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int return true; } -bool CWallet::SelectCoins(const CAmount& nTargetValue, set >& setCoinsRet, CAmount& nValueRet, const CCoinControl* coinControl) const +bool CWallet::SelectCoins(const CAmount& nTargetValue, set >& setCoinsRet, CAmount& nValueRet, bool& fOnlyCoinbaseCoinsRet, bool& fNeedCoinbaseCoinsRet, const CCoinControl* coinControl) const { - vector vCoins; - AvailableCoins(vCoins, true, coinControl); + // Output parameter fOnlyCoinbaseCoinsRet is set to true when the only available coins are coinbase utxos. + vector vCoinsNoCoinbase, vCoinsWithCoinbase; + AvailableCoins(vCoinsNoCoinbase, true, coinControl, false, false); + AvailableCoins(vCoinsWithCoinbase, true, coinControl, false, true); + fOnlyCoinbaseCoinsRet = vCoinsNoCoinbase.size() == 0 && vCoinsWithCoinbase.size() > 0; + + // If coinbase utxos can only be sent to zaddrs, exclude any coinbase utxos from coin selection. + bool fProtectCoinbase = Params().GetConsensus().fCoinbaseMustBeProtected; + vector vCoins = (fProtectCoinbase) ? vCoinsNoCoinbase : vCoinsWithCoinbase; + + // Output parameter fNeedCoinbaseCoinsRet is set to true if coinbase utxos need to be spent to meet target amount + if (fProtectCoinbase && vCoinsWithCoinbase.size() > vCoinsNoCoinbase.size()) { + CAmount value = 0; + for (const COutput& out : vCoinsNoCoinbase) { + if (!out.fSpendable) { + continue; + } + value += out.tx->vout[out.i].nValue; + } + if (value <= nTargetValue) { + CAmount valueWithCoinbase = 0; + for (const COutput& out : vCoinsWithCoinbase) { + if (!out.fSpendable) { + continue; + } + valueWithCoinbase += out.tx->vout[out.i].nValue; + } + fNeedCoinbaseCoinsRet = (valueWithCoinbase >= nTargetValue); + } + } // coin control -> return all selected outputs (we want all selected to go into the transaction for sure) if (coinControl && coinControl->HasSelected()) @@ -2355,9 +2386,17 @@ bool CWallet::CreateTransaction(const vector& vecSend, // Choose coins to use set > setCoins; CAmount nValueIn = 0; - if (!SelectCoins(nTotalValue, setCoins, nValueIn, coinControl)) + bool fOnlyCoinbaseCoins = false; + bool fNeedCoinbaseCoins = false; + if (!SelectCoins(nTotalValue, setCoins, nValueIn, fOnlyCoinbaseCoins, fNeedCoinbaseCoins, coinControl)) { - strFailReason = _("Insufficient funds"); + if (fOnlyCoinbaseCoins && Params().GetConsensus().fCoinbaseMustBeProtected) { + strFailReason = _("Coinbase funds can only be sent to a zaddr"); + } else if (fNeedCoinbaseCoins) { + strFailReason = _("Insufficient funds, coinbase funds can only be spent after they have been sent to a zaddr"); + } else { + strFailReason = _("Insufficient funds"); + } return false; } BOOST_FOREACH(PAIRTYPE(const CWalletTx*, unsigned int) pcoin, setCoins) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c3f1fd63f..376daba99 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -556,7 +556,7 @@ public: class CWallet : public CCryptoKeyStore, public CValidationInterface { private: - bool SelectCoins(const CAmount& nTargetValue, std::set >& setCoinsRet, CAmount& nValueRet, const CCoinControl *coinControl = NULL) const; + bool SelectCoins(const CAmount& nTargetValue, std::set >& setCoinsRet, CAmount& nValueRet, bool& fOnlyCoinbaseCoinsRet, bool& fNeedCoinbaseCoinsRet, const CCoinControl *coinControl = NULL) const; CWalletDB *pwalletdbEncryption; @@ -689,7 +689,7 @@ public: //! check whether we are allowed to upgrade (or already support) to the named feature bool CanSupportFeature(enum WalletFeature wf) { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; } - void AvailableCoins(std::vector& vCoins, bool fOnlyConfirmed=true, const CCoinControl *coinControl = NULL, bool fIncludeZeroValue=false) const; + void AvailableCoins(std::vector& vCoins, bool fOnlyConfirmed=true, const CCoinControl *coinControl = NULL, bool fIncludeZeroValue=false, bool fIncludeCoinBase=true) const; bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, std::vector vCoins, std::set >& setCoinsRet, CAmount& nValueRet) const; bool IsSpent(const uint256& hash, unsigned int n) const;