Auto merge of #3535 - bitcartel:fix_regtest_signrawtransaction, r=str4d

Do not use APPROX_RELEASE_HEIGHT to get consensus branch id in regtest mode.

Closes #3534

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'

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()

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 @@ def run_test(self):
 
         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'])

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());