diff --git a/qa/rpc-tests/wallet_nullifiers.py b/qa/rpc-tests/wallet_nullifiers.py index 143963e36..ce18f8d51 100755 --- a/qa/rpc-tests/wallet_nullifiers.py +++ b/qa/rpc-tests/wallet_nullifiers.py @@ -21,7 +21,7 @@ class WalletNullifiersTest (BitcoinTestFramework): # send node 0 taddr to zaddr to get out of coinbase mytaddr = self.nodes[0].getnewaddress(); recipients = [] - recipients.append({"address":myzaddr0, "amount":10.0}) + recipients.append({"address":myzaddr0, "amount":Decimal('10.0')-Decimal('0.0001')}) # utxo amount less fee myopid = self.nodes[0].z_sendmany(mytaddr, recipients) opids = [] diff --git a/qa/rpc-tests/wallet_protectcoinbase.py b/qa/rpc-tests/wallet_protectcoinbase.py index f32ca6795..b3d554889 100755 --- a/qa/rpc-tests/wallet_protectcoinbase.py +++ b/qa/rpc-tests/wallet_protectcoinbase.py @@ -60,16 +60,39 @@ class Wallet2Test (BitcoinTestFramework): # coinbase utxos can only be sent to a zaddr. errorString = "" try: - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1) 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 + # Prepare to send taddr->zaddr mytaddr = self.nodes[0].getnewaddress() myzaddr = self.nodes[0].z_getnewaddress() + + # 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":20.0}) + 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: + sleep(1) + else: + status = results[0]["status"] + errorString = results[0]["error"]["message"] + break + assert_equal("failed", status) + assert_equal("wallet does not allow any change" in errorString, True) + + # This send will succeed. We send two coinbase utxos totalling 20.0 less a fee of 0.00010000, with no change. + recipients = [] + recipients.append({"address":myzaddr, "amount": Decimal('20.0') - Decimal('0.0001')}) myopid = self.nodes[0].z_sendmany(mytaddr, recipients) self.wait_for_operationd_success(myopid) self.sync_all() @@ -78,13 +101,13 @@ class Wallet2Test (BitcoinTestFramework): # 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["transparent"]), Decimal('20.0')) + assert_equal(Decimal(resp["private"]), Decimal('19.9999')) assert_equal(Decimal(resp["total"]), Decimal('39.9999')) # convert note to transparent funds recipients = [] - recipients.append({"address":mytaddr, "amount":20.0}) + recipients.append({"address":mytaddr, "amount":Decimal('10.0')}) myopid = self.nodes[0].z_sendmany(myzaddr, recipients) self.wait_for_operationd_success(myopid) self.sync_all() @@ -100,30 +123,30 @@ class Wallet2Test (BitcoinTestFramework): # 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) + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 99999) 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) + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 21) 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 + # Send will succeed because the balance of non-coinbase utxos is 10.0 try: - self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 19.0) + self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 9) except JSONRPCException: assert(False) self.sync_all() - self.nodes[1].generate(10) + self.nodes[1].generate(1) self.sync_all() # check balance - assert_equal(self.nodes[2].getbalance(), Decimal('19')) + assert_equal(self.nodes[2].getbalance(), 9) if __name__ == '__main__': Wallet2Test ().main () diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index eaa245857..354cfc1be 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -301,8 +301,9 @@ bool AsyncRPCOperation_sendmany::main_impl() { * -> zaddrs * * Note: Consensus rule states that coinbase utxos can only be sent to a zaddr. - * Any change over and above the amount specified by the user will be sent - * to the same zaddr the user is sending funds to. + * Local wallet rule does not allow any change when sending coinbase utxos + * since there is currently no way to specify a change address and we don't + * want users accidentally sending excess funds to a recipient. */ if (isfromtaddr_) { add_taddr_outputs_to_tx(); @@ -311,25 +312,13 @@ bool AsyncRPCOperation_sendmany::main_impl() { CAmount fundsSpent = t_outputs_total + minersFee + z_outputs_total; CAmount change = funds - fundsSpent; - // If there is a single zaddr and there are coinbase utxos, change goes to the zaddr. if (change > 0) { - if (isSingleZaddrOutput && selectedUTXOCoinbase) { - std::string address = std::get<0>(zOutputsDeque.front()); - SendManyRecipient smr(address, change, std::string()); - zOutputsDeque.push_back(smr); - - LogPrint("zrpc", "%s: change from coinbase utxo is also sent to the recipient (amount=%s)\n", - getId().substr(0, 10), - FormatMoney(change, false) - ); - - } else if (!isSingleZaddrOutput && selectedUTXOCoinbase) { - // This should not happen and is not allowed - assert(false); + if (selectedUTXOCoinbase) { + assert(isSingleZaddrOutput); + throw JSONRPCError(RPC_WALLET_ERROR, strprintf( + "Change %ld not allowed. When protecting coinbase funds, the wallet does not allow any change as there is currently no way to specify a change address in z_sendmany.", change)); } else { - // If there is a single zaddr and no coinbase utxos, just use a regular output for change. add_taddr_change_output_to_tx(change); - LogPrint("zrpc", "%s: transparent change in transaction output (amount=%s)\n", getId().substr(0, 10), FormatMoney(change, false) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 1bb78d5b1..414a5ce5e 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3176,6 +3176,7 @@ Value z_sendmany(const Array& params, bool fHelp) "z_sendmany \"fromaddress\" [{\"address\":... ,\"amount\":...},...] ( minconf )\n" "\nSend multiple times. Amounts are double-precision floating point numbers." "\nChange from a taddr flows to a new taddr address, while change from zaddr returns to itself." + "\nWhen sending coinbase UTXOs to a zaddr, change is not alllowed. The entire value of the UTXO(s) must be consumed." + HelpRequiringPassphrase() + "\n" "\nArguments:\n" "1. \"fromaddress\" (string, required) The taddr or zaddr to send the funds from.\n"