From 18f8abb62de00318c69e2a270548e2bdbb554f79 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 21 Sep 2018 11:10:15 -0700 Subject: [PATCH 1/2] Closes #3534. Do not use APPROX_RELEASE_HEIGHT to get consensus branch id when in regtest mode. Co-authored-by: Larry Ruane --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/regtest_signrawtransaction.py | 34 ++++++++++++++++++++++ src/rpc/rawtransaction.cpp | 7 +++-- 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100755 qa/rpc-tests/regtest_signrawtransaction.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 301b822b8..1f25465fa 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -63,6 +63,7 @@ testScripts=( 'rewind_index.py' 'p2p_txexpiry_dos.py' 'p2p_node_bloom.py' + 'regtest_signrawtransaction.py' ); testScriptsExt=( 'getblocktemplate_longpoll.py' diff --git a/qa/rpc-tests/regtest_signrawtransaction.py b/qa/rpc-tests/regtest_signrawtransaction.py new file mode 100755 index 000000000..2e0273677 --- /dev/null +++ b/qa/rpc-tests/regtest_signrawtransaction.py @@ -0,0 +1,34 @@ +#!/usr/bin/env python2 +# Copyright (c) 2018 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 start_nodes, wait_and_assert_operationid_status + +class RegtestSignrawtransactionTest (BitcoinTestFramework): + + def setup_nodes(self): + return start_nodes(4, self.options.tmpdir, [[ + "-nuparams=5ba81b19:200", # Overwinter + "-nuparams=76b809bb:204", # Sapling + ]] * 4) + + def run_test(self): + self.nodes[0].generate(1) + self.sync_all() + taddr = self.nodes[1].getnewaddress() + zaddr1 = self.nodes[1].z_getnewaddress('sprout') + + self.nodes[0].sendtoaddress(taddr, 2.0) + self.nodes[0].generate(1) + self.sync_all() + + # Create and sign Overwinter transaction. + # If the incorrect consensus branch id is selected, there will be a signing error. + opid = self.nodes[1].z_sendmany(taddr, + [{'address': zaddr1, 'amount': 1}]) + wait_and_assert_operationid_status(self.nodes[1], opid) + +if __name__ == '__main__': + RegtestSignrawtransactionTest().main() diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 242403b19..7d3dcc705 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -918,8 +918,11 @@ 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); + // determine the correct consensus branch ID. Regtest mode ignores release height. + int chainHeight = chainActive.Height() + 1; + if (Params().NetworkIDString() != "regtest") { + chainHeight = std::max(chainHeight, APPROX_RELEASE_HEIGHT); + } // Grab the current consensus branch ID auto consensusBranchId = CurrentEpochBranchId(chainHeight, Params().GetConsensus()); From 4c4e1718b123c296a5ebeaddf866dab9082b2531 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 21 Sep 2018 17:16:44 -0700 Subject: [PATCH 2/2] Update qa test as offline regtest nodes need branch id passed in. --- qa/rpc-tests/signrawtransaction_offline.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/signrawtransaction_offline.py b/qa/rpc-tests/signrawtransaction_offline.py index cde9ca166..0d7def43b 100755 --- a/qa/rpc-tests/signrawtransaction_offline.py +++ b/qa/rpc-tests/signrawtransaction_offline.py @@ -2,6 +2,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_true, initialize_chain_clean, start_node +from test_framework.authproxy import JSONRPCException class SignOfflineTest (BitcoinTestFramework): # Setup Methods @@ -36,7 +37,17 @@ class SignOfflineTest (BitcoinTestFramework): create_hex = self.nodes[0].createrawtransaction(create_inputs, {taddr: 9.9999}) - signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys) + # An offline regtest node does not rely on the approx release height of the software + # to determine the consensus rules to be used for signing. + try: + signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys) + self.nodes[0].sendrawtransaction(signed_tx['hex']) + assert(False) + except JSONRPCException: + pass + + # Passing in the consensus branch id resolves the issue for offline regtest nodes. + signed_tx = offline_node.signrawtransaction(create_hex, sign_inputs, privkeys, "ALL", "5ba81b19") # 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'])