From 9384e74c50f8b1b49e283b84ab8132d1251d3dc1 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 10 Nov 2016 16:57:36 -0800 Subject: [PATCH] Closes #1833. Format currency amounts in z_sendmany error message. Improve coverage of possible error states from z_sendmany. Refactor qa test for z_sendmany operations. --- qa/rpc-tests/wallet_protectcoinbase.py | 29 +++++++++++++++++------ src/wallet/asyncrpcoperation_sendmany.cpp | 12 +++++++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/qa/rpc-tests/wallet_protectcoinbase.py b/qa/rpc-tests/wallet_protectcoinbase.py index eabe26bf4..e05ccb529 100755 --- a/qa/rpc-tests/wallet_protectcoinbase.py +++ b/qa/rpc-tests/wallet_protectcoinbase.py @@ -8,7 +8,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * from time import * -class Wallet2Test (BitcoinTestFramework): +class WalletProtectCoinbaseTest (BitcoinTestFramework): def setup_chain(self): print("Initializing test directory "+self.options.tmpdir) @@ -23,21 +23,28 @@ class Wallet2Test (BitcoinTestFramework): self.is_network_split=False self.sync_all() - def wait_for_operationd_success(self, myopid): + def wait_and_assert_operationid_status(self, myopid, in_status='success', in_errormsg=None): print('waiting for async operation {}'.format(myopid)) opids = [] opids.append(myopid) timeout = 120 status = None + errormsg = 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"] + if status == "failed": + errormsg = results[0]['error']['message'] break print('...returned status: {}'.format(status)) - assert_equal("success", status) + assert_equal(in_status, status) + if errormsg is not None: + assert(in_errormsg is not None) + assert_equal(in_errormsg in errormsg, True) + print('...returned error: {}'.format(errormsg)) def run_test (self): print "Mining blocks..." @@ -94,7 +101,7 @@ class Wallet2Test (BitcoinTestFramework): 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.wait_and_assert_operationid_status(myopid) self.sync_all() self.nodes[1].generate(1) self.sync_all() @@ -109,7 +116,7 @@ class Wallet2Test (BitcoinTestFramework): recipients = [] recipients.append({"address":mytaddr, "amount":Decimal('10.0')}) myopid = self.nodes[0].z_sendmany(myzaddr, recipients) - self.wait_for_operationd_success(myopid) + self.wait_and_assert_operationid_status(myopid) self.sync_all() self.nodes[1].generate(1) self.sync_all() @@ -128,6 +135,14 @@ class Wallet2Test (BitcoinTestFramework): errorString = e.error['message'] assert_equal("Insufficient funds" in errorString, True) + # z_sendmany will fail because of insufficient funds + recipients = [] + recipients.append({"address":self.nodes[1].getnewaddress(), "amount":Decimal('10000.0')}) + myopid = self.nodes[0].z_sendmany(mytaddr, recipients) + self.wait_and_assert_operationid_status(myopid, "failed", "Insufficient transparent funds, have 10.00, need 10000.0001") + myopid = self.nodes[0].z_sendmany(myzaddr, recipients) + self.wait_and_assert_operationid_status(myopid, "failed", "Insufficient protected funds, have 9.9998, need 10000.0001") + # Send will fail because of insufficient funds unless sender uses coinbase utxos try: self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 21) @@ -156,7 +171,7 @@ class Wallet2Test (BitcoinTestFramework): newzaddr = self.nodes[2].z_getnewaddress() recipients.append({"address":newzaddr, "amount":amount_per_recipient}) myopid = self.nodes[0].z_sendmany(myzaddr, recipients) - self.wait_for_operationd_success(myopid) + self.wait_and_assert_operationid_status(myopid) self.sync_all() self.nodes[1].generate(1) self.sync_all() @@ -166,4 +181,4 @@ class Wallet2Test (BitcoinTestFramework): assert_equal(Decimal(resp["private"]), num_recipients * amount_per_recipient) if __name__ == '__main__': - Wallet2Test ().main () + WalletProtectCoinbaseTest().main() diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 7f9549b74..f0facdf6e 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -200,11 +200,15 @@ bool AsyncRPCOperation_sendmany::main_impl() { assert(!isfromzaddr_ || t_inputs_total == 0); if (isfromtaddr_ && (t_inputs_total < targetAmount)) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strprintf("Insufficient transparent funds, have %ld, need %ld", t_inputs_total, targetAmount)); + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, + strprintf("Insufficient transparent funds, have %s, need %s", + FormatMoney(t_inputs_total), FormatMoney(targetAmount))); } if (isfromzaddr_ && (z_inputs_total < targetAmount)) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strprintf("Insufficient protected funds, have %ld, need %ld", z_inputs_total, targetAmount)); + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, + strprintf("Insufficient protected funds, have %s, need %s", + FormatMoney(z_inputs_total), FormatMoney(targetAmount))); } // If from address is a taddr, select UTXOs to spend @@ -319,7 +323,9 @@ bool AsyncRPCOperation_sendmany::main_impl() { 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)); + "Change %s 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.", FormatMoney(change))); } else { add_taddr_change_output_to_tx(change); LogPrint("zrpc", "%s: transparent change in transaction output (amount=%s)\n",