From 76277ad8af05195aa27d731d568ed06a4366a1f0 Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Thu, 14 Jun 2018 03:50:52 -0700 Subject: [PATCH 1/3] Take expiryheight as param to createrawtransaction --- src/rpc/client.cpp | 2 +- src/rpc/rawtransaction.cpp | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 3fc68ee6f..5288d977e 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -77,6 +77,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "createrawtransaction", 0 }, { "createrawtransaction", 1 }, { "createrawtransaction", 2 }, + { "createrawtransaction", 3 }, { "signrawtransaction", 1 }, { "signrawtransaction", 2 }, { "sendrawtransaction", 1 }, @@ -189,4 +190,3 @@ UniValue RPCConvertValues(const std::string &strMethod, const std::vector 3) + if (fHelp || params.size() < 2 || params.size() > 4) throw runtime_error( "createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] {\"address\":amount,...} ( locktime )\n" "\nCreate a transaction spending the given inputs and sending to the given addresses.\n" @@ -434,6 +434,7 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) " ,...\n" " }\n" "3. locktime (numeric, optional, default=0) Raw locktime. Non-0 value also locktime-activates inputs\n" + "4. expiryheight (numeric, optional, default=0) Expiry height of transaction\n" "\nResult:\n" "\"transaction\" (string) hex string of the transaction\n" @@ -443,7 +444,7 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) ); LOCK(cs_main); - RPCTypeCheck(params, boost::assign::list_of(UniValue::VARR)(UniValue::VOBJ)(UniValue::VNUM), true); + RPCTypeCheck(params, boost::assign::list_of(UniValue::VARR)(UniValue::VOBJ)(UniValue::VNUM)(UniValue::VNUM), true); if (params[0].isNull() || params[1].isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, arguments 1 and 2 must be non-null"); @@ -453,12 +454,6 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) int nextBlockHeight = chainActive.Height() + 1; CMutableTransaction rawTx = CreateNewContextualCMutableTransaction( Params().GetConsensus(), nextBlockHeight); - - if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { - if (rawTx.nExpiryHeight >= TX_EXPIRY_HEIGHT_THRESHOLD){ - throw JSONRPCError(RPC_INVALID_PARAMETER, "nExpiryHeight must be less than TX_EXPIRY_HEIGHT_THRESHOLD."); - } - } if (params.size() > 2 && !params[2].isNull()) { int64_t nLockTime = params[2].get_int64(); @@ -466,6 +461,16 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, locktime out of range"); rawTx.nLockTime = nLockTime; } + + if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { + if (params.size() > 3 && !params[3].isNull()) { + int64_t nExpiryHeight = params[3].get_int64(); + if (nExpiryHeight < 0 || nExpiryHeight >= TX_EXPIRY_HEIGHT_THRESHOLD) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, nExpiryHeight must be less than TX_EXPIRY_HEIGHT_THRESHOLD."); + } + rawTx.nExpiryHeight = nExpiryHeight; + } + } for (size_t idx = 0; idx < inputs.size(); idx++) { const UniValue& input = inputs[idx]; From 87d55e1322d601dbe64d0eec49fda8815ad6d519 Mon Sep 17 00:00:00 2001 From: Simon Date: Sun, 29 Jul 2018 20:57:33 -0700 Subject: [PATCH 2/3] Clean up help messages for RPC createrawtransaction. Also return error if expiryheight used when Overwinter not active. --- src/rpc/rawtransaction.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index b54638a09..912cbe3ba 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -412,7 +412,7 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) { if (fHelp || params.size() < 2 || params.size() > 4) throw runtime_error( - "createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] {\"address\":amount,...} ( locktime )\n" + "createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] {\"address\":amount,...} ( locktime ) ( expiryheight )\n" "\nCreate a transaction spending the given inputs and sending to the given addresses.\n" "Returns hex-encoded raw transaction.\n" "Note that the transaction's inputs are not signed, and\n" @@ -422,7 +422,7 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) "1. \"transactions\" (string, required) A json array of json objects\n" " [\n" " {\n" - " \"txid\":\"id\", (string, required) The transaction id\n" + " \"txid\":\"id\", (string, required) The transaction id\n" " \"vout\":n (numeric, required) The output number\n" " \"sequence\":n (numeric, optional) The sequence number\n" " }\n" @@ -433,8 +433,8 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) " \"address\": x.xxx (numeric, required) The key is the Zcash address, the value is the " + CURRENCY_UNIT + " amount\n" " ,...\n" " }\n" - "3. locktime (numeric, optional, default=0) Raw locktime. Non-0 value also locktime-activates inputs\n" - "4. expiryheight (numeric, optional, default=0) Expiry height of transaction\n" + "3. locktime (numeric, optional, default=0) Raw locktime. Non-0 value also locktime-activates inputs\n" + "4. expiryheight (numeric, optional, default=" + strprintf("%d", DEFAULT_TX_EXPIRY_DELTA) + ") Expiry height of transaction (if Overwinter is active)\n" "\nResult:\n" "\"transaction\" (string) hex string of the transaction\n" @@ -462,13 +462,15 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp) rawTx.nLockTime = nLockTime; } - if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { - if (params.size() > 3 && !params[3].isNull()) { + if (params.size() > 3 && !params[3].isNull()) { + if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { int64_t nExpiryHeight = params[3].get_int64(); if (nExpiryHeight < 0 || nExpiryHeight >= TX_EXPIRY_HEIGHT_THRESHOLD) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, nExpiryHeight must be less than TX_EXPIRY_HEIGHT_THRESHOLD."); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, expiryheight must be nonnegative and less than %d.", TX_EXPIRY_HEIGHT_THRESHOLD)); } rawTx.nExpiryHeight = nExpiryHeight; + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expiryheight can only be used if Overwinter is active when the transaction is mined"); } } From f01c11bd0f53d11d9123e9cbae7bb0e149c3fce8 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 30 Jul 2018 10:24:10 -0700 Subject: [PATCH 3/3] Add tests for expiryheight parameter of RPC createrawtransaction. --- qa/rpc-tests/wallet_overwintertx.py | 26 ++++++++++++++++++++++++++ src/test/rpc_tests.cpp | 2 ++ 2 files changed, 28 insertions(+) diff --git a/qa/rpc-tests/wallet_overwintertx.py b/qa/rpc-tests/wallet_overwintertx.py index 61932fece..d77a114db 100755 --- a/qa/rpc-tests/wallet_overwintertx.py +++ b/qa/rpc-tests/wallet_overwintertx.py @@ -6,6 +6,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, initialize_chain_clean, \ start_nodes, connect_nodes_bi, wait_and_assert_operationid_status +from test_framework.authproxy import JSONRPCException from decimal import Decimal @@ -46,6 +47,13 @@ class WalletOverwinterTxTest (BitcoinTestFramework): assert_equal(bci['consensus']['nextblock'], '00000000') assert_equal(bci['upgrades']['5ba81b19']['status'], 'pending') + # Cannot use the expiryheight parameter of createrawtransaction if Overwinter is not active in the next block + try: + self.nodes[0].createrawtransaction([], {}, 0, 99) + except JSONRPCException,e: + errorString = e.error['message'] + assert_equal("Invalid parameter, expiryheight can only be used if Overwinter is active when the transaction is mined" in errorString, True) + # Node 0 sends transparent funds to Node 2 tsendamount = Decimal('1.0') txid_transparent = self.nodes[0].sendtoaddress(taddr2, tsendamount) @@ -92,6 +100,24 @@ class WalletOverwinterTxTest (BitcoinTestFramework): assert_equal(bci['consensus']['nextblock'], '5ba81b19') assert_equal(bci['upgrades']['5ba81b19']['status'], 'pending') + # Test using expiryheight parameter of createrawtransaction when Overwinter is active in the next block + errorString = "" + try: + self.nodes[0].createrawtransaction([], {}, 0, 499999999) + except JSONRPCException,e: + errorString = e.error['message'] + assert_equal("", errorString) + try: + self.nodes[0].createrawtransaction([], {}, 0, -1) + except JSONRPCException,e: + errorString = e.error['message'] + assert_equal("Invalid parameter, expiryheight must be nonnegative and less than 500000000" in errorString, True) + try: + self.nodes[0].createrawtransaction([], {}, 0, 500000000) + except JSONRPCException,e: + errorString = e.error['message'] + assert_equal("Invalid parameter, expiryheight must be nonnegative and less than 500000000" in errorString, True) + # Node 0 sends transparent funds to Node 3 tsendamount = Decimal('1.0') txid_transparent = self.nodes[0].sendtoaddress(taddr3, tsendamount) diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index a05006f4b..8884d783f 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -73,6 +73,8 @@ BOOST_AUTO_TEST_CASE(rpc_rawparams) BOOST_CHECK_THROW(CallRPC("createrawtransaction {} {}"), runtime_error); BOOST_CHECK_NO_THROW(CallRPC("createrawtransaction [] {}")); BOOST_CHECK_THROW(CallRPC("createrawtransaction [] {} extra"), runtime_error); + BOOST_CHECK_NO_THROW(CallRPC("createrawtransaction [] {} 0")); + BOOST_CHECK_THROW(CallRPC("createrawtransaction [] {} 0 0"), runtime_error); // Overwinter is not active BOOST_CHECK_THROW(CallRPC("decoderawtransaction"), runtime_error); BOOST_CHECK_THROW(CallRPC("decoderawtransaction null"), runtime_error);