From 19697025c6e3be176abb056ce5c24ed15f6ed616 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Thu, 13 Sep 2018 16:34:23 -0600 Subject: [PATCH 1/4] Add test for signing raw transactions offline --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/signrawtransaction_offline.py | 57 ++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100755 qa/rpc-tests/signrawtransaction_offline.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 25ecc409a7e..9953b85ee2e 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -43,6 +43,7 @@ testScripts=( 'merkle_blocks.py' # 'fundrawtransaction.py' 'signrawtransactions.py' + 'signrawtransaction_offline.py' 'walletbackup.py' 'key_import_export.py' 'nodehandling.py' diff --git a/qa/rpc-tests/signrawtransaction_offline.py b/qa/rpc-tests/signrawtransaction_offline.py new file mode 100755 index 00000000000..5eadf2cb3cf --- /dev/null +++ b/qa/rpc-tests/signrawtransaction_offline.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python2 + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_true, connect_nodes_bi, \ + initialize_chain_clean, start_node + +class SignOfflineTest (BitcoinTestFramework): + # Setup Methods + def setup_chain(self): + print "Initializing test directory " + self.options.tmpdir + initialize_chain_clean(self.options.tmpdir, 2) + + def setup_network(self): + self.nodes = [ start_node(0, self.options.tmpdir, ["-nuparams=5ba81b19:10"]) ] + self.is_network_split = False + self.sync_all() + + # Tests + def run_test(self): + print "Mining blocks..." + self.nodes[0].generate(101) + + offline_node = start_node(1, self.options.tmpdir, ["-maxconnections=0", "-nuparams=5ba81b19:10"]) + self.nodes.append(offline_node) + + assert_equal(0, len(offline_node.getpeerinfo())) # make sure node 1 has no peers + + privkeys = [self.nodes[0].dumpprivkey(self.nodes[0].getnewaddress())] + taddr = self.nodes[0].getnewaddress() + + tx = self.nodes[0].listunspent()[0] + txid = tx['txid'] + scriptpubkey = tx['scriptPubKey'] + + create_inputs = [{'txid': txid, 'vout': 0}] + sign_inputs = [{'txid': txid, 'vout': 0, 'scriptPubKey': scriptpubkey, 'amount': 10}] + + create_hex = self.nodes[0].createrawtransaction(create_inputs, {taddr: 9.9999}) + print "create:" + print create_hex + + signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys) + print "sign:" + print signed_tx + + # If we return the transaction hash, then we have have not thrown an error (success) + online_tx_hash = self.nodes[0].sendrawtransaction(signed_tx['hex']) + assert_true(len(online_tx_hash) > 0) + + #signed_hex = signed_tx['hex'] + #print "decoded:" + #print self.nodes[0].decoderawtransaction(signed_hex) + print "sent:" + print online_tx_hash + +if __name__ == '__main__': + SignOfflineTest().main() \ No newline at end of file From 40b95273010f9cb57e078b9bcd3330e5829499bc Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Thu, 13 Sep 2018 16:55:18 -0600 Subject: [PATCH 2/4] Incorporate APPROX_RELEASE_HEIGHT when determining what consensus branch to sign with --- qa/rpc-tests/signrawtransaction_offline.py | 10 ---------- src/rpc/rawtransaction.cpp | 8 ++++++-- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/qa/rpc-tests/signrawtransaction_offline.py b/qa/rpc-tests/signrawtransaction_offline.py index 5eadf2cb3cf..55acf7b4ad2 100755 --- a/qa/rpc-tests/signrawtransaction_offline.py +++ b/qa/rpc-tests/signrawtransaction_offline.py @@ -36,22 +36,12 @@ def run_test(self): sign_inputs = [{'txid': txid, 'vout': 0, 'scriptPubKey': scriptpubkey, 'amount': 10}] create_hex = self.nodes[0].createrawtransaction(create_inputs, {taddr: 9.9999}) - print "create:" - print create_hex signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys) - print "sign:" - print signed_tx # If we return the transaction hash, then we have have not thrown an error (success) online_tx_hash = self.nodes[0].sendrawtransaction(signed_tx['hex']) assert_true(len(online_tx_hash) > 0) - #signed_hex = signed_tx['hex'] - #print "decoded:" - #print self.nodes[0].decoderawtransaction(signed_hex) - print "sent:" - print online_tx_hash - if __name__ == '__main__': SignOfflineTest().main() \ No newline at end of file diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 912cbe3baae..4a3c836101c 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -7,6 +7,7 @@ #include "consensus/validation.h" #include "core_io.h" #include "init.h" +#include "deprecation.h" #include "key_io.h" #include "keystore.h" #include "main.h" @@ -872,9 +873,12 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) } bool fHashSingle = ((nHashType & ~SIGHASH_ANYONECANPAY) == SIGHASH_SINGLE); - + // Use the approximate release height if it is greater so offline nodes + // have a better estimation of the current height and will be more likely to + // determine the correct consensus branch ID. + int chainHeight = std::max(chainActive.Height() + 1, APPROX_RELEASE_HEIGHT); // Grab the current consensus branch ID - auto consensusBranchId = CurrentEpochBranchId(chainActive.Height() + 1, Params().GetConsensus()); + auto consensusBranchId = CurrentEpochBranchId(chainHeight, Params().GetConsensus()); // Script verification errors UniValue vErrors(UniValue::VARR); From 36a490677c36833f7552d4cfc204d7167b66f13a Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Mon, 17 Sep 2018 09:49:32 -0600 Subject: [PATCH 3/4] Allow passing branchId when calling signrawtransaction --- src/consensus/upgrades.cpp | 9 +++++++++ src/consensus/upgrades.h | 6 ++++++ src/rpc/rawtransaction.cpp | 13 +++++++++++-- src/test/rpc_tests.cpp | 2 ++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/consensus/upgrades.cpp b/src/consensus/upgrades.cpp index 95bde792577..5785d75acec 100644 --- a/src/consensus/upgrades.cpp +++ b/src/consensus/upgrades.cpp @@ -82,6 +82,15 @@ uint32_t CurrentEpochBranchId(int nHeight, const Consensus::Params& params) { return NetworkUpgradeInfo[CurrentEpoch(nHeight, params)].nBranchId; } +bool IsConsensusBranchId(int branchId) { + for (int idx = Consensus::BASE_SPROUT; idx < Consensus::MAX_NETWORK_UPGRADES; idx++) { + if (branchId == NetworkUpgradeInfo[idx].nBranchId) { + return true; + } + } + return false; +} + bool IsActivationHeight( int nHeight, const Consensus::Params& params, diff --git a/src/consensus/upgrades.h b/src/consensus/upgrades.h index 620dc94c4b0..638ed862f27 100644 --- a/src/consensus/upgrades.h +++ b/src/consensus/upgrades.h @@ -63,6 +63,12 @@ int CurrentEpoch(int nHeight, const Consensus::Params& params); */ uint32_t CurrentEpochBranchId(int nHeight, const Consensus::Params& params); +/** + * Returns true if a given branch id is a valid nBranchId for one of the network + * upgrades contained in NetworkUpgradeInfo. + */ +bool IsConsensusBranchId(int branchId); + /** * Returns true if the given block height is the activation height for the given * upgrade. diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 4a3c836101c..ab30b351f48 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -673,7 +673,7 @@ static void TxInErrorToJSON(const CTxIn& txin, UniValue& vErrorsRet, const std:: UniValue signrawtransaction(const UniValue& params, bool fHelp) { - if (fHelp || params.size() < 1 || params.size() > 4) + if (fHelp || params.size() < 1 || params.size() > 5) throw runtime_error( "signrawtransaction \"hexstring\" ( [{\"txid\":\"id\",\"vout\":n,\"scriptPubKey\":\"hex\",\"redeemScript\":\"hex\"},...] [\"privatekey1\",...] sighashtype )\n" "\nSign inputs for raw transaction (serialized, hex-encoded).\n" @@ -710,6 +710,8 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) " \"ALL|ANYONECANPAY\"\n" " \"NONE|ANYONECANPAY\"\n" " \"SINGLE|ANYONECANPAY\"\n" + "5. \"branchid\" (string, optional) The hex representation of the consensus branch id to sign with." + " This can be used to force signing with consensus rules that are ahead of the node's current height.\n" "\nResult:\n" "{\n" @@ -737,7 +739,7 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) #else LOCK(cs_main); #endif - RPCTypeCheck(params, boost::assign::list_of(UniValue::VSTR)(UniValue::VARR)(UniValue::VARR)(UniValue::VSTR), true); + RPCTypeCheck(params, boost::assign::list_of(UniValue::VSTR)(UniValue::VARR)(UniValue::VARR)(UniValue::VSTR)(UniValue::VSTR), true); vector txData(ParseHexV(params[0], "argument 1")); CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION); @@ -880,6 +882,13 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) // Grab the current consensus branch ID auto consensusBranchId = CurrentEpochBranchId(chainHeight, Params().GetConsensus()); + if (params.size() > 4 && !params[4].isNull()) { + consensusBranchId = ParseHexToUInt32(params[4].get_str()); + if (!IsConsensusBranchId(consensusBranchId)) { + throw runtime_error(params[4].get_str() + " is not a valid consensus branch id"); + } + } + // Script verification errors UniValue vErrors(UniValue::VARR); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 8884d783fdd..7cb1ef010e9 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -92,6 +92,8 @@ BOOST_AUTO_TEST_CASE(rpc_rawparams) BOOST_CHECK_NO_THROW(CallRPC(string("signrawtransaction ")+rawtx+" null null NONE|ANYONECANPAY")); BOOST_CHECK_NO_THROW(CallRPC(string("signrawtransaction ")+rawtx+" [] [] NONE|ANYONECANPAY")); BOOST_CHECK_THROW(CallRPC(string("signrawtransaction ")+rawtx+" null null badenum"), runtime_error); + BOOST_CHECK_NO_THROW(CallRPC(string("signrawtransaction ")+rawtx+" [] [] NONE|ANYONECANPAY 5ba81b19")); + BOOST_CHECK_THROW(CallRPC(string("signrawtransaction ")+rawtx+" [] [] ALL NONE|ANYONECANPAY 123abc"), runtime_error); // Only check failure cases for sendrawtransaction, there's no network to send to... BOOST_CHECK_THROW(CallRPC("sendrawtransaction"), runtime_error); From c10249f3ded369a1926fd5dbe2a3d74794a898e6 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Wed, 19 Sep 2018 14:41:02 -0600 Subject: [PATCH 4/4] Remove unused import --- qa/rpc-tests/signrawtransaction_offline.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qa/rpc-tests/signrawtransaction_offline.py b/qa/rpc-tests/signrawtransaction_offline.py index 55acf7b4ad2..cde9ca16665 100755 --- a/qa/rpc-tests/signrawtransaction_offline.py +++ b/qa/rpc-tests/signrawtransaction_offline.py @@ -1,8 +1,7 @@ #!/usr/bin/env python2 from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_true, connect_nodes_bi, \ - initialize_chain_clean, start_node +from test_framework.util import assert_equal, assert_true, initialize_chain_clean, start_node class SignOfflineTest (BitcoinTestFramework): # Setup Methods