Implementation of Overwinter transaction format ZIP 202.

qa/pull-tester/rpc-tests.sh

@@ -17,6 +17,7 @@ testScripts=(
     'wallet_protectcoinbase.py'
     'wallet_shieldcoinbase.py'
     'wallet.py'
+    'wallet_overwintertx.py'
     'wallet_nullifiers.py'
     'wallet_1941.py'
     'listtransactions.py'

qa/rpc-tests/wallet_overwintertx.py

@@ -0,0 +1,110 @@
+#!/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.authproxy import JSONRPCException
+from test_framework.util import assert_equal, initialize_chain_clean, \
+    start_nodes, connect_nodes_bi, sync_blocks, sync_mempools, \
+    wait_and_assert_operationid_status
+
+from decimal import Decimal
+
+class WalletOverwinterTxTest (BitcoinTestFramework):
+
+    def setup_chain(self):
+        print("Initializing test directory "+self.options.tmpdir)
+        initialize_chain_clean(self.options.tmpdir, 4)
+
+    def setup_network(self, split=False):
+        self.nodes = start_nodes(4, self.options.tmpdir, extra_args=[["-nuparams=5ba81b19:200", "-debug=zrpcunsafe", "-txindex"]] * 4 )
+        connect_nodes_bi(self.nodes,0,1)
+        connect_nodes_bi(self.nodes,1,2)
+        connect_nodes_bi(self.nodes,0,2)
+        connect_nodes_bi(self.nodes,0,3)
+        self.is_network_split=False
+        self.sync_all()
+
+    def run_test (self):
+        self.nodes[0].generate(100)
+        self.sync_all()
+        self.nodes[1].generate(98)
+        self.sync_all()
+        # Node 0 has reward from blocks 1 to 98 which are spendable.
+
+        #
+        # Sprout
+        #
+        # Currently at block 198. The next block to be mined 199 is a Sprout block
+        #
+        taddr0 = self.nodes[0].getnewaddress()
+        taddr2 = self.nodes[2].getnewaddress()
+        zaddr2 = self.nodes[2].z_getnewaddress()
+        taddr3 = self.nodes[3].getnewaddress()
+        zaddr3 = self.nodes[3].z_getnewaddress()
+
+        # Send taddr to taddr
+        tsendamount = Decimal('1.23')
+        txid_transparent = self.nodes[0].sendtoaddress(taddr2, tsendamount)
+
+        # Send one coinbase utxo of value 10.0 less a fee of 0.00010000, with no change.
+        zsendamount = Decimal('10.0') - Decimal('0.0001')
+        recipients = []
+        recipients.append({"address":zaddr2, "amount": zsendamount})
+        myopid = self.nodes[0].z_sendmany(taddr0, recipients)
+        txid_shielded = wait_and_assert_operationid_status(self.nodes[0], myopid)
+
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        # Verify balance
+        assert_equal(self.nodes[2].getbalance(), tsendamount)
+        assert_equal(self.nodes[2].z_getbalance(zaddr2), zsendamount)
+
+        # Verify transaction version is 1 or 2 (intended for Sprout)
+        result = self.nodes[0].getrawtransaction(txid_transparent, 1)
+        assert_equal(result["version"], 1)
+        assert_equal(result["overwintered"], False)
+        result = self.nodes[0].getrawtransaction(txid_shielded, 1)
+        assert_equal(result["version"], 2)
+        assert_equal(result["overwintered"], False)
+
+        #
+        # Overwinter
+        #
+        # Currently at block 199. The next block to be mined 200 is an Overwinter block
+        #
+
+        # Send taddr to taddr
+        tsendamount = Decimal('4.56')
+        txid_transparent = self.nodes[0].sendtoaddress(taddr3, tsendamount)
+
+        # Send one coinbase utxo of value 20.0 less a fee of 0.00010000, with no change.
+        zsendamount = Decimal('20.0') - Decimal('0.0001')
+        recipients = []
+        recipients.append({"address":zaddr3, "amount": zsendamount})
+        myopid = self.nodes[0].z_sendmany(taddr0, recipients)
+        txid_shielded = wait_and_assert_operationid_status(self.nodes[0], myopid)
+
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        # Verify balance
+        assert_equal(self.nodes[3].getbalance(), tsendamount)
+        assert_equal(self.nodes[3].z_getbalance(zaddr3), zsendamount)
+
+        # Verify transaction version is 3 (intended for Overwinter)
+        result = self.nodes[0].getrawtransaction(txid_transparent, 1)
+        assert_equal(result["version"], 3)
+        assert_equal(result["overwintered"], True)
+        assert_equal(result["versiongroupid"], "03c48270")
+        result = self.nodes[0].getrawtransaction(txid_shielded, 1)
+        assert_equal(result["version"], 3)
+        assert_equal(result["overwintered"], True)
+        assert_equal(result["versiongroupid"], "03c48270")
+
+if __name__ == '__main__':
+    WalletOverwinterTxTest().main()

src/bitcoin-tx.cpp

@@ -157,7 +157,7 @@ static void RegisterLoad(const string& strInput)
 static void MutateTxVersion(CMutableTransaction& tx, const string& cmdVal)
 {
     int64_t newVersion = atoi64(cmdVal);
-    if (newVersion < CTransaction::MIN_CURRENT_VERSION || newVersion > CTransaction::MAX_CURRENT_VERSION)
+    if (newVersion < CTransaction::SPROUT_MIN_CURRENT_VERSION || newVersion > CTransaction::SPROUT_MAX_CURRENT_VERSION)
         throw runtime_error("Invalid TX version requested");
 
     tx.nVersion = (int) newVersion;

src/chainparams.cpp

@@ -19,6 +19,11 @@
 
 static CBlock CreateGenesisBlock(const char* pszTimestamp, const CScript& genesisOutputScript, uint32_t nTime, const uint256& nNonce, const std::vector<unsigned char>& nSolution, uint32_t nBits, int32_t nVersion, const CAmount& genesisReward)
 {
+    // To create a genesis block for a new chain which is Overwintered:
+    //   txNew.nVersion = 3
+    //   txNew.fOverwintered = true
+    //   txNew.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID
+    //   txNew.nExpiryHeight = <default value>
     CMutableTransaction txNew;
     txNew.nVersion = 1;
     txNew.vin.resize(1);

src/consensus/consensus.h

@@ -9,7 +9,11 @@
 /** The minimum allowed block version (network rule) */
 static const int32_t MIN_BLOCK_VERSION = 4;
 /** The minimum allowed transaction version (network rule) */
-static const int32_t MIN_TX_VERSION = 1;
+static const int32_t SPROUT_MIN_TX_VERSION = 1;
+/** The minimum allowed transaction version (network rule) */
+static const int32_t OVERWINTER_MIN_TX_VERSION = 3;
+/** The maximum allowed transaction version (network rule) */
+static const int32_t OVERWINTER_MAX_TX_VERSION = 3;
 /** The maximum allowed size for a serialized block, in bytes (network rule) */
 static const unsigned int MAX_BLOCK_SIZE = 2000000;
 /** The maximum allowed number of signature check operations in a block (network rule) */

src/gtest/test_checkblock.cpp

@@ -33,6 +33,40 @@ TEST(CheckBlock, VersionTooLow) {
     EXPECT_FALSE(CheckBlock(block, state, verifier, false, false));
 }
 
+
+// Test that a Sprout tx with negative version is still rejected
+// by CheckBlock under Sprout consensus rules.
+TEST(CheckBlock, BlockSproutRejectsBadVersion) {
+    SelectParams(CBaseChainParams::MAIN);
+
+    CMutableTransaction mtx;
+    mtx.vin.resize(1);
+    mtx.vin[0].prevout.SetNull();
+    mtx.vin[0].scriptSig = CScript() << 1 << OP_0;
+    mtx.vout.resize(1);
+    mtx.vout[0].scriptPubKey = CScript() << OP_TRUE;
+    mtx.vout[0].nValue = 0;
+    mtx.vout.push_back(CTxOut(
+        GetBlockSubsidy(1, Params().GetConsensus())/5,
+        Params().GetFoundersRewardScriptAtHeight(1)));
+    mtx.fOverwintered = false;
+    mtx.nVersion = -1;
+    mtx.nVersionGroupId = 0;
+
+    CTransaction tx {mtx};
+    CBlock block;
+    block.vtx.push_back(tx);
+
+    MockCValidationState state;
+    CBlockIndex indexPrev {Params().GenesisBlock()};
+
+    auto verifier = libzcash::ProofVerifier::Strict();
+
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-version-too-low", false)).Times(1);
+    EXPECT_FALSE(CheckBlock(block, state, verifier, false, false));
+}
+
+
 TEST(ContextualCheckBlock, BadCoinbaseHeight) {
     SelectParams(CBaseChainParams::MAIN);
 
@@ -75,3 +109,125 @@ TEST(ContextualCheckBlock, BadCoinbaseHeight) {
     block.vtx[0] = tx4;
     EXPECT_TRUE(ContextualCheckBlock(block, state, &indexPrev));
 }
+
+// Test that a block evaluated under Sprout rules cannot contain Overwinter transactions.
+// This test assumes that mainnet Overwinter activation is at least height 2.
+TEST(ContextualCheckBlock, BlockSproutRulesRejectOverwinterTx) {
+    SelectParams(CBaseChainParams::MAIN);
+
+    CMutableTransaction mtx;
+    mtx.vin.resize(1);
+    mtx.vin[0].prevout.SetNull();
+    mtx.vin[0].scriptSig = CScript() << 1 << OP_0;
+    mtx.vout.resize(1);
+    mtx.vout[0].scriptPubKey = CScript() << OP_TRUE;
+    mtx.vout[0].nValue = 0;
+
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+
+    CTransaction tx {mtx};
+    CBlock block;
+    block.vtx.push_back(tx);
+
+    MockCValidationState state;
+    CBlockIndex indexPrev {Params().GenesisBlock()};
+
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "tx-overwinter-not-active", false)).Times(1);
+    EXPECT_FALSE(ContextualCheckBlock(block, state, &indexPrev));
+}
+
+
+// Test block evaluated under Sprout rules will accept Sprout transactions
+TEST(ContextualCheckBlock, BlockSproutRulesAcceptSproutTx) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+
+    CMutableTransaction mtx;
+    mtx.vin.resize(1);
+    mtx.vin[0].prevout.SetNull();
+    mtx.vin[0].scriptSig = CScript() << 1 << OP_0;
+    mtx.vout.resize(1);
+    mtx.vout[0].scriptPubKey = CScript() << OP_TRUE;
+    mtx.vout[0].nValue = 0;
+    mtx.vout.push_back(CTxOut(
+        GetBlockSubsidy(1, Params().GetConsensus())/5,
+        Params().GetFoundersRewardScriptAtHeight(1)));
+    mtx.fOverwintered = false;
+    mtx.nVersion = 1;
+
+    CTransaction tx {mtx};
+    CBlock block;
+    block.vtx.push_back(tx);
+    MockCValidationState state;
+    CBlockIndex indexPrev {Params().GenesisBlock()};
+
+    EXPECT_TRUE(ContextualCheckBlock(block, state, &indexPrev));
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}
+
+
+// Test block evaluated under Overwinter rules will accept Overwinter transactions
+TEST(ContextualCheckBlock, BlockOverwinterRulesAcceptOverwinterTx) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1);
+
+    CMutableTransaction mtx;
+    mtx.vin.resize(1);
+    mtx.vin[0].prevout.SetNull();
+    mtx.vin[0].scriptSig = CScript() << 1 << OP_0;
+    mtx.vout.resize(1);
+    mtx.vout[0].scriptPubKey = CScript() << OP_TRUE;
+    mtx.vout[0].nValue = 0;
+    mtx.vout.push_back(CTxOut(
+        GetBlockSubsidy(1, Params().GetConsensus())/5,
+        Params().GetFoundersRewardScriptAtHeight(1)));
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+
+    CTransaction tx {mtx};
+    CBlock block;
+    block.vtx.push_back(tx);
+    MockCValidationState state;
+    CBlockIndex indexPrev {Params().GenesisBlock()};
+
+    EXPECT_TRUE(ContextualCheckBlock(block, state, &indexPrev));
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}
+
+
+
+// Test block evaluated under Overwinter rules will reject Sprout transactions
+TEST(ContextualCheckBlock, BlockOverwinterRulesRejectSproutTx) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1);
+
+    CMutableTransaction mtx;
+    mtx.vin.resize(1);
+    mtx.vin[0].prevout.SetNull();
+    mtx.vin[0].scriptSig = CScript() << 1 << OP_0;
+    mtx.vout.resize(1);
+    mtx.vout[0].scriptPubKey = CScript() << OP_TRUE;
+    mtx.vout[0].nValue = 0;
+
+    mtx.nVersion = 2;
+
+    CTransaction tx {mtx};
+    CBlock block;
+    block.vtx.push_back(tx);
+
+    MockCValidationState state;
+    CBlockIndex indexPrev {Params().GenesisBlock()};
+
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "tx-overwinter-active", false)).Times(1);
+    EXPECT_FALSE(ContextualCheckBlock(block, state, &indexPrev));
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}

src/gtest/test_checktransaction.cpp

@@ -391,3 +391,316 @@ TEST(checktransaction_tests, non_canonical_ed25519_signature) {
     EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false)).Times(1);
     CheckTransactionWithoutProofVerification(tx, state);
 }
+
+TEST(checktransaction_tests, OverwinterConstructors) {
+    CMutableTransaction mtx;
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 20;
+
+    // Check constructor with overwinter fields
+    CTransaction tx(mtx);
+    EXPECT_EQ(tx.nVersion, mtx.nVersion);
+    EXPECT_EQ(tx.fOverwintered, mtx.fOverwintered);
+    EXPECT_EQ(tx.nVersionGroupId, mtx.nVersionGroupId);
+    EXPECT_EQ(tx.nExpiryHeight, mtx.nExpiryHeight);
+
+    // Check constructor of mutable transaction struct
+    CMutableTransaction mtx2(tx);
+    EXPECT_EQ(mtx2.nVersion, mtx.nVersion);
+    EXPECT_EQ(mtx2.fOverwintered, mtx.fOverwintered);
+    EXPECT_EQ(mtx2.nVersionGroupId, mtx.nVersionGroupId);
+    EXPECT_EQ(mtx2.nExpiryHeight, mtx.nExpiryHeight);
+    EXPECT_TRUE(mtx2.GetHash() == mtx.GetHash());
+
+    // Check assignment of overwinter fields
+    CTransaction tx2 = tx;
+    EXPECT_EQ(tx2.nVersion, mtx.nVersion);
+    EXPECT_EQ(tx2.fOverwintered, mtx.fOverwintered);
+    EXPECT_EQ(tx2.nVersionGroupId, mtx.nVersionGroupId);
+    EXPECT_EQ(tx2.nExpiryHeight, mtx.nExpiryHeight);
+    EXPECT_TRUE(tx2 == tx);
+}
+
+TEST(checktransaction_tests, OverwinterSerialization) {
+    CMutableTransaction mtx;
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 99;
+
+    // Check round-trip serialization and deserialization from mtx to tx.
+    {
+        CDataStream ss(SER_DISK, PROTOCOL_VERSION);
+        ss << mtx;
+        CTransaction tx;
+        ss >> tx;
+        EXPECT_EQ(mtx.nVersion, tx.nVersion);
+        EXPECT_EQ(mtx.fOverwintered, tx.fOverwintered);
+        EXPECT_EQ(mtx.nVersionGroupId, tx.nVersionGroupId);
+        EXPECT_EQ(mtx.nExpiryHeight, tx.nExpiryHeight);
+
+        EXPECT_EQ(mtx.GetHash(), CMutableTransaction(tx).GetHash());
+        EXPECT_EQ(tx.GetHash(), CTransaction(mtx).GetHash());
+    }
+
+    // Also check mtx to mtx
+    {
+        CDataStream ss(SER_DISK, PROTOCOL_VERSION);
+        ss << mtx;
+        CMutableTransaction mtx2;
+        ss >> mtx2;
+        EXPECT_EQ(mtx.nVersion, mtx2.nVersion);
+        EXPECT_EQ(mtx.fOverwintered, mtx2.fOverwintered);
+        EXPECT_EQ(mtx.nVersionGroupId, mtx2.nVersionGroupId);
+        EXPECT_EQ(mtx.nExpiryHeight, mtx2.nExpiryHeight);
+
+        EXPECT_EQ(mtx.GetHash(), mtx2.GetHash());
+    }
+
+    // Also check tx to tx
+    {
+        CTransaction tx(mtx);
+        CDataStream ss(SER_DISK, PROTOCOL_VERSION);
+        ss << tx;
+        CTransaction tx2;
+        ss >> tx2;
+        EXPECT_EQ(tx.nVersion, tx2.nVersion);
+        EXPECT_EQ(tx.fOverwintered, tx2.fOverwintered);
+        EXPECT_EQ(tx.nVersionGroupId, tx2.nVersionGroupId);
+        EXPECT_EQ(tx.nExpiryHeight, tx2.nExpiryHeight);
+
+        EXPECT_EQ(mtx.GetHash(), CMutableTransaction(tx).GetHash());
+        EXPECT_EQ(tx.GetHash(), tx2.GetHash());
+    }
+}
+
+TEST(checktransaction_tests, OverwinterDefaultValues) {
+    // Check default values (this will fail when defaults change; test should then be updated)
+    CTransaction tx;
+    EXPECT_EQ(tx.nVersion, 1);
+    EXPECT_EQ(tx.fOverwintered, false);
+    EXPECT_EQ(tx.nVersionGroupId, 0);
+    EXPECT_EQ(tx.nExpiryHeight, 0);
+}
+
+// A valid v3 transaction with no joinsplits
+TEST(checktransaction_tests, OverwinterValidTx) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0);
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+    CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state));
+}
+
+TEST(checktransaction_tests, OverwinterExpiryHeight) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0);
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+
+    {
+        CTransaction tx(mtx);
+        MockCValidationState state;
+        EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state));
+    }
+
+    {
+        mtx.nExpiryHeight = TX_EXPIRY_HEIGHT_THRESHOLD - 1;
+        CTransaction tx(mtx);
+        MockCValidationState state;
+        EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state));
+    }
+
+    {
+        mtx.nExpiryHeight = TX_EXPIRY_HEIGHT_THRESHOLD;
+        CTransaction tx(mtx);
+        MockCValidationState state;
+        EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-expiry-height-too-high", false)).Times(1);
+        CheckTransactionWithoutProofVerification(tx, state);
+    }
+
+    {
+        mtx.nExpiryHeight = std::numeric_limits<uint32_t>::max();
+        CTransaction tx(mtx);
+        MockCValidationState state;
+        EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-expiry-height-too-high", false)).Times(1);
+        CheckTransactionWithoutProofVerification(tx, state);
+    }
+}
+
+
+// Test that a Sprout tx with a negative version number is detected
+// given the new Overwinter logic
+TEST(checktransaction_tests, SproutTxVersionTooLow) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0);
+    mtx.fOverwintered = false;
+    mtx.nVersion = -1;
+
+    CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-version-too-low", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+
+
+// Subclass of CTransaction which doesn't call UpdateHash when constructing
+// from a CMutableTransaction.  This enables us to create a CTransaction
+// with bad values which normally trigger an exception during construction.
+class UNSAFE_CTransaction : public CTransaction {
+    public:
+        UNSAFE_CTransaction(const CMutableTransaction &tx) : CTransaction(tx, true) {}
+};
+
+
+// Test bad Overwinter version number in CheckTransactionWithoutProofVerification
+TEST(checktransaction_tests, OverwinterVersionNumberLow) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0);
+    mtx.fOverwintered = true;
+    mtx.nVersion = OVERWINTER_MIN_TX_VERSION - 1;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+
+    UNSAFE_CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-overwinter-version-too-low", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+// Test bad Overwinter version number in ContextualCheckTransaction
+TEST(checktransaction_tests, OverwinterVersionNumberHigh) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
+
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0);
+    mtx.fOverwintered = true;
+    mtx.nVersion = OVERWINTER_MAX_TX_VERSION + 1;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+
+    UNSAFE_CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-overwinter-version-too-high", false)).Times(1);
+    ContextualCheckTransaction(tx, state, 1, 100);
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}
+
+
+// Test bad Overwinter version group id
+TEST(checktransaction_tests, OverwinterBadVersionGroupId) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0);
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nExpiryHeight = 0;
+    mtx.nVersionGroupId = 0x12345678;
+
+    UNSAFE_CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-tx-version-group-id", false)).Times(1);
+    CheckTransactionWithoutProofVerification(tx, state);
+}
+
+// This tests an Overwinter transaction checked against Sprout
+TEST(checktransaction_tests, OverwinterNotActive) {
+    SelectParams(CBaseChainParams::TESTNET);
+
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+
+    CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "tx-overwinter-not-active", false)).Times(1);
+    ContextualCheckTransaction(tx, state, 1, 100);
+}
+
+// This tests a transaction without the fOverwintered flag set, against the Overwinter consensus rule set.
+TEST(checktransaction_tests, OverwinterFlagNotSet) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
+
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.fOverwintered = false;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+
+    CTransaction tx(mtx);
+    MockCValidationState state;
+    EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "tx-overwinter-flag-not-set", false)).Times(1);
+    ContextualCheckTransaction(tx, state, 1, 100);
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}
+
+
+// Overwinter (NU0) does not allow soft fork to version 4 Overwintered tx.
+TEST(checktransaction_tests, OverwinterInvalidSoftForkVersion) {
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.fOverwintered = true;
+    mtx.nVersion = 4; // This is not allowed
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+
+    CDataStream ss(SER_DISK, PROTOCOL_VERSION);
+    try {
+        ss << mtx;
+        FAIL() << "Expected std::ios_base::failure 'Unknown transaction format'";
+    }
+    catch(std::ios_base::failure & err) {
+        EXPECT_THAT(err.what(), testing::HasSubstr(std::string("Unknown transaction format")));
+    }
+    catch(...) {
+        FAIL() << "Expected std::ios_base::failure 'Unknown transaction format', got some other exception";
+    }
+}
+
+
+// Test CreateNewContextualCMutableTransaction sets default values based on height
+TEST(checktransaction_tests, OverwinteredContextualCreateTx) {
+    SelectParams(CBaseChainParams::REGTEST);
+    const Consensus::Params& consensusParams = Params().GetConsensus();
+    int activationHeight = 5;
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, activationHeight);
+
+    {
+        CMutableTransaction mtx = CreateNewContextualCMutableTransaction(
+            consensusParams, activationHeight - 1);
+
+        EXPECT_EQ(mtx.nVersion, 1);
+        EXPECT_EQ(mtx.fOverwintered, false);
+        EXPECT_EQ(mtx.nVersionGroupId, 0);
+        EXPECT_EQ(mtx.nExpiryHeight, 0);
+    }
+
+    // Overwinter activates
+    {
+        CMutableTransaction mtx = CreateNewContextualCMutableTransaction(
+            consensusParams, activationHeight );
+
+        EXPECT_EQ(mtx.nVersion, 3);
+        EXPECT_EQ(mtx.fOverwintered, true);
+        EXPECT_EQ(mtx.nVersionGroupId, OVERWINTER_VERSION_GROUP_ID);
+        EXPECT_EQ(mtx.nExpiryHeight, 0);
+    }
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}

src/gtest/test_mempool.cpp

@@ -1,6 +1,7 @@
 #include <gtest/gtest.h>
 #include <gtest/gtest-spi.h>
 
+#include "consensus/upgrades.h"
 #include "consensus/validation.h"
 #include "core_io.h"
 #include "main.h"
@@ -9,6 +10,9 @@
 #include "policy/fees.h"
 #include "util.h"
 
+// Implementation is in test_checktransaction.cpp
+extern CMutableTransaction GetValidTransaction();
+
 // Fake the input of transaction 5295156213414ed77f6e538e7e8ebe14492156906b9fe995b242477818789364
 // - 532639cc6bebed47c1c69ae36dd498c68a012e74ad12729adbd3dbb56f8f3f4a, 0
 class FakeCoinsViewDB : public CCoinsView {
@@ -92,6 +96,8 @@ TEST(Mempool, PriorityStatsDoNotCrash) {
 }
 
 TEST(Mempool, TxInputLimit) {
+    SelectParams(CBaseChainParams::TESTNET);
+
     CTxMemPool pool(::minRelayTxFee);
     bool missingInputs;
 
@@ -136,3 +142,122 @@ TEST(Mempool, TxInputLimit) {
     EXPECT_EQ(state4.GetRejectReason(), "bad-txns-version-too-low");
 }
 
+// Valid overwinter v3 format tx gets rejected because overwinter hasn't activated yet.
+TEST(Mempool, OverwinterNotActiveYet) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+
+    CTxMemPool pool(::minRelayTxFee);
+    bool missingInputs;
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0); // no joinsplits
+    mtx.fOverwintered = true;
+    mtx.nVersion = 3;
+    mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+    mtx.nExpiryHeight = 0;
+    CValidationState state1;
+
+    CTransaction tx1(mtx);
+    EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs));
+    EXPECT_EQ(state1.GetRejectReason(), "tx-overwinter-not-active");
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}
+
+
+// Sprout transaction version 3 when Overwinter is not active:
+// 1. pass CheckTransaction (and CheckTransactionWithoutProofVerification)
+// 2. pass ContextualCheckTransaction
+// 3. fail IsStandardTx
+TEST(Mempool, SproutV3TxFailsAsExpected) {
+    SelectParams(CBaseChainParams::TESTNET);
+
+    CTxMemPool pool(::minRelayTxFee);
+    bool missingInputs;
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0); // no joinsplits
+    mtx.fOverwintered = false;
+    mtx.nVersion = 3;
+    CValidationState state1;
+    CTransaction tx1(mtx);
+
+    EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs));
+    EXPECT_EQ(state1.GetRejectReason(), "version");
+}
+
+
+// Sprout transaction version 3 when Overwinter is always active:
+// 1. pass CheckTransaction (and CheckTransactionWithoutProofVerification)
+// 2. fails ContextualCheckTransaction
+TEST(Mempool, SproutV3TxWhenOverwinterActive) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
+
+    CTxMemPool pool(::minRelayTxFee);
+    bool missingInputs;
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0); // no joinsplits
+    mtx.fOverwintered = false;
+    mtx.nVersion = 3;
+    CValidationState state1;
+    CTransaction tx1(mtx);
+
+    EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs));
+    EXPECT_EQ(state1.GetRejectReason(), "tx-overwinter-flag-not-set");
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}
+
+
+// Sprout transaction with negative version, rejected by the mempool in CheckTransaction
+// under Sprout consensus rules, should still be rejected under Overwinter consensus rules.
+// 1. fails CheckTransaction (specifically CheckTransactionWithoutProofVerification)
+TEST(Mempool, SproutNegativeVersionTxWhenOverwinterActive) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
+
+    CTxMemPool pool(::minRelayTxFee);
+    bool missingInputs;
+    CMutableTransaction mtx = GetValidTransaction();
+    mtx.vjoinsplit.resize(0); // no joinsplits
+    mtx.fOverwintered = false;
+
+    // A Sprout transaction with version -3 is created using Sprout code (as found in Zcashd <= 1.0.14).
+    // First four bytes of transaction, parsed as an uint32_t, has the value: 0xfffffffd
+    // This test simulates an Overwinter node receiving this transaction, but incorrectly deserializing the
+    // transaction due to a (pretend) bug of not detecting the most significant bit, which leads
+    // to not setting fOverwintered and not masking off the most significant bit of the header field.
+    // The resulting Sprout tx with nVersion -3 should be rejected by the Overwinter node's mempool.
+    {
+        mtx.nVersion = -3;
+        EXPECT_EQ(mtx.nVersion, static_cast<int32_t>(0xfffffffd));
+
+        CTransaction tx1(mtx);
+        EXPECT_EQ(tx1.nVersion, -3);
+
+        CValidationState state1;
+        EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs));
+        EXPECT_EQ(state1.GetRejectReason(), "bad-txns-version-too-low");
+    }
+
+    // A Sprout transaction with version -3 created using Overwinter code (as found in Zcashd >= 1.0.15).
+    // First four bytes of transaction, parsed as an uint32_t, has the value: 0x80000003
+    // This test simulates the same pretend bug described above.
+    // The resulting Sprout tx with nVersion -2147483645 should be rejected by the Overwinter node's mempool.
+    {
+        mtx.nVersion = static_cast<int32_t>((1 << 31) | 3);
+        EXPECT_EQ(mtx.nVersion, static_cast<int32_t>(0x80000003));
+
+        CTransaction tx1(mtx);
+        EXPECT_EQ(tx1.nVersion, -2147483645);
+
+        CValidationState state1;
+        EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs));
+        EXPECT_EQ(state1.GetRejectReason(), "bad-txns-version-too-low");
+    }
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}

src/main.cpp

@@ -641,16 +641,22 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRE
 }
 
 
-
-
-
-
-
-bool IsStandardTx(const CTransaction& tx, string& reason)
+bool IsStandardTx(const CTransaction& tx, string& reason, const int nHeight)
 {
-    if (tx.nVersion > CTransaction::MAX_CURRENT_VERSION || tx.nVersion < CTransaction::MIN_CURRENT_VERSION) {
-        reason = "version";
-        return false;
+    bool isOverwinter = NetworkUpgradeActive(nHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER);
+
+    if (isOverwinter) {
+        // Overwinter standard rules apply
+        if (tx.nVersion > CTransaction::OVERWINTER_MAX_CURRENT_VERSION || tx.nVersion < CTransaction::OVERWINTER_MIN_CURRENT_VERSION) {
+            reason = "overwinter-version";
+            return false;
+        }
+    } else {
+        // Sprout standard rules apply
+        if (tx.nVersion > CTransaction::SPROUT_MAX_CURRENT_VERSION || tx.nVersion < CTransaction::SPROUT_MIN_CURRENT_VERSION) {
+            reason = "version";
+            return false;
+        }
     }
 
     BOOST_FOREACH(const CTxIn& txin, tx.vin)
@@ -840,6 +846,50 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in
     return nSigOps;
 }
 
+/**
+ * Check a transaction contextually against a set of consensus rules valid at a given block height.
+ * 
+ * Notes:
+ * 1. AcceptToMemoryPool calls CheckTransaction and this function.
+ * 2. ProcessNewBlock calls AcceptBlock, which calls CheckBlock (which calls CheckTransaction)
+ *    and ContextualCheckBlock (which calls this function).
+ */
+bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, const int nHeight, const int dosLevel)
+{
+    bool isOverwinter = NetworkUpgradeActive(nHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER);
+    bool isSprout = !isOverwinter;
+
+    // If Sprout rules apply, reject transactions which are intended for Overwinter and beyond
+    if (isSprout && tx.fOverwintered) {
+        return state.DoS(dosLevel, error("ContextualCheckTransaction(): overwinter is not active yet"),
+                         REJECT_INVALID, "tx-overwinter-not-active");
+    }
+
+    // If Overwinter rules apply:
+    if (isOverwinter) {
+        // Reject transactions with valid version but missing overwinter flag
+        if (tx.nVersion >= OVERWINTER_MIN_TX_VERSION && !tx.fOverwintered) {
+            return state.DoS(dosLevel, error("ContextualCheckTransaction(): overwinter flag must be set"),
+                            REJECT_INVALID, "tx-overwinter-flag-not-set");
+        }
+
+        // Reject transactions with invalid version
+        if (tx.fOverwintered && tx.nVersion > OVERWINTER_MAX_TX_VERSION ) {
+            return state.DoS(100, error("CheckTransaction(): overwinter version too high"),
+                REJECT_INVALID, "bad-tx-overwinter-version-too-high");
+        }
+
+        // Reject transactions intended for Sprout
+        if (!tx.fOverwintered) {
+            return state.DoS(dosLevel, error("ContextualCheckTransaction: overwinter is active"),
+                            REJECT_INVALID, "tx-overwinter-active");
+        }
+    }
+
+    return true;
+}
+
+
 bool CheckTransaction(const CTransaction& tx, CValidationState &state,
                       libzcash::ProofVerifier& verifier)
 {
@@ -866,11 +916,46 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
 {
     // Basic checks that don't depend on any context
 
-    // Check transaction version
-    if (tx.nVersion < MIN_TX_VERSION) {
+    /**
+     * Previously:
+     * 1. The consensus rule below was:
+     *        if (tx.nVersion < SPROUT_MIN_TX_VERSION) { ... }
+     *    which checked if tx.nVersion fell within the range:
+     *        INT32_MIN <= tx.nVersion < SPROUT_MIN_TX_VERSION
+     * 2. The parser allowed tx.nVersion to be negative
+     *
+     * Now:
+     * 1. The consensus rule checks to see if tx.Version falls within the range:
+     *        0 <= tx.nVersion < SPROUT_MIN_TX_VERSION
+     * 2. The previous consensus rule checked for negative values within the range:
+     *        INT32_MIN <= tx.nVersion < 0
+     *    This is unnecessary for Overwinter transactions since the parser now
+     *    interprets the sign bit as fOverwintered, so tx.nVersion is always >=0,
+     *    and when Overwinter is not active ContextualCheckTransaction rejects
+     *    transactions with fOverwintered set.  When fOverwintered is set,
+     *    this function and ContextualCheckTransaction will together check to
+     *    ensure tx.nVersion avoids the following ranges:
+     *        0 <= tx.nVersion < OVERWINTER_MIN_TX_VERSION
+     *        OVERWINTER_MAX_TX_VERSION < tx.nVersion <= INT32_MAX
+     */
+    if (!tx.fOverwintered && tx.nVersion < SPROUT_MIN_TX_VERSION) {
         return state.DoS(100, error("CheckTransaction(): version too low"),
                          REJECT_INVALID, "bad-txns-version-too-low");
     }
+    else if (tx.fOverwintered) {
+        if (tx.nVersion < OVERWINTER_MIN_TX_VERSION) {
+            return state.DoS(100, error("CheckTransaction(): overwinter version too low"),
+                REJECT_INVALID, "bad-tx-overwinter-version-too-low");
+        }
+        if (tx.nVersionGroupId != OVERWINTER_VERSION_GROUP_ID) {
+            return state.DoS(100, error("CheckTransaction(): unknown tx version group id"),
+                    REJECT_INVALID, "bad-tx-version-group-id");
+        }
+        if (tx.nExpiryHeight >= TX_EXPIRY_HEIGHT_THRESHOLD) {
+            return state.DoS(100, error("CheckTransaction(): expiry height is too high"),
+                            REJECT_INVALID, "bad-tx-expiry-height-too-high");
+        }
+    }
 
     // Transactions can contain empty `vin` and `vout` so long as
     // `vjoinsplit` is non-empty.
@@ -1078,6 +1163,13 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
     if (!CheckTransaction(tx, state, verifier))
         return error("AcceptToMemoryPool: CheckTransaction failed");
 
+    // DoS level set to 10 to be more forgiving.
+    // Check transaction contextually against the set of consensus rules which apply in the next block to be mined.
+    int nextBlockHeight = chainActive.Height() + 1;
+    if (!ContextualCheckTransaction(tx, state, nextBlockHeight, 10)) {
+        return error("AcceptToMemoryPool: ContextualCheckTransaction failed");
+    }
+
     // Coinbase is only valid in a block, not as a loose transaction
     if (tx.IsCoinBase())
         return state.DoS(100, error("AcceptToMemoryPool: coinbase as individual tx"),
@@ -1085,7 +1177,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
 
     // Rather not work on nonstandard transactions (unless -testnet/-regtest)
     string reason;
-    if (Params().RequireStandard() && !IsStandardTx(tx, reason))
+    if (Params().RequireStandard() && !IsStandardTx(tx, reason, nextBlockHeight))
         return state.DoS(0,
                          error("AcceptToMemoryPool: nonstandard transaction: %s", reason),
                          REJECT_NONSTANDARD, reason);
@@ -3133,6 +3225,12 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIn
 
     // Check that all transactions are finalized
     BOOST_FOREACH(const CTransaction& tx, block.vtx) {
+
+        // Check transaction contextually against consensus rules at block height
+        if (!ContextualCheckTransaction(tx, state, nHeight, 100)) {
+            return false; // Failure reason has been set in validation state object
+        }
+
         int nLockTimeFlags = 0;
         int64_t nLockTimeCutoff = (nLockTimeFlags & LOCKTIME_MEDIAN_TIME_PAST)
                                 ? pindexPrev->GetMedianTimePast()
@@ -5735,3 +5833,22 @@ public:
         mapOrphanTransactionsByPrev.clear();
     }
 } instance_of_cmaincleanup;
+
+
+// Set default values of new CMutableTransaction based on consensus rules at given height.
+CMutableTransaction CreateNewContextualCMutableTransaction(const Consensus::Params& consensusParams, int nHeight)
+{
+    CMutableTransaction mtx;
+
+    bool isOverwintered = NetworkUpgradeActive(nHeight, consensusParams, Consensus::UPGRADE_OVERWINTER);
+    if (isOverwintered) {
+        mtx.fOverwintered = true;
+        mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
+        mtx.nVersion = 3;
+        // Expiry height is not set. Only fields required for a parser to treat as a valid Overwinter V3 tx.
+
+        // TODO: In future, when moving from Overwinter to Sapling, it will be useful
+        // to set the expiry height to: min(activation_height - 1, default_expiry_height)
+    }
+    return mtx;
+}

src/main.h

@@ -15,6 +15,7 @@
 #include "chainparams.h"
 #include "coins.h"
 #include "consensus/consensus.h"
+#include "consensus/upgrades.h"
 #include "net.h"
 #include "primitives/block.h"
 #include "primitives/transaction.h"
@@ -332,6 +333,9 @@ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, cons
                            unsigned int flags, bool cacheStore, const Consensus::Params& consensusParams,
                            std::vector<CScriptCheck> *pvChecks = NULL);
 
+/** Check a transaction contextually against a set of consensus rules */
+bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, int nHeight, int dosLevel);
+
 /** Apply the effects of this transaction on the UTXO set represented by view */
 void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCache &inputs, int nHeight);
 
@@ -342,7 +346,7 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
 /** Check for standard transaction types
  * @return True if all outputs (scriptPubKeys) use only standard transaction forms
  */
-bool IsStandardTx(const CTransaction& tx, std::string& reason);
+bool IsStandardTx(const CTransaction& tx, std::string& reason, int nHeight = 0);
 
 /**
  * Check if transaction is final and can be included in a block with the
@@ -534,4 +538,7 @@ namespace Consensus {
 bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, const Consensus::Params& consensusParams);
 }
 
+/** Return a CMutableTransaction with contextual default values based on set of consensus rules at height */
+CMutableTransaction CreateNewContextualCMutableTransaction(const Consensus::Params& consensusParams, int nHeight);
+
 #endif // BITCOIN_MAIN_H

src/miner.cpp

@@ -336,7 +336,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
         LogPrintf("CreateNewBlock(): total size %u\n", nBlockSize);
 
         // Create coinbase tx
-        CMutableTransaction txNew;
+        CMutableTransaction txNew = CreateNewContextualCMutableTransaction(chainparams.GetConsensus(), nHeight);
         txNew.vin.resize(1);
         txNew.vin[0].prevout.SetNull();
         txNew.vout.resize(1);

src/primitives/transaction.cpp

@@ -146,8 +146,9 @@ std::string CTxOut::ToString() const
     return strprintf("CTxOut(nValue=%d.%08d, scriptPubKey=%s)", nValue / COIN, nValue % COIN, scriptPubKey.ToString().substr(0,30));
 }
 
-CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::MIN_CURRENT_VERSION), nLockTime(0) {}
-CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime),
+CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::SPROUT_MIN_CURRENT_VERSION), fOverwintered(false), nVersionGroupId(0), nExpiryHeight(0), nLockTime(0) {}
+CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), fOverwintered(tx.fOverwintered), nVersionGroupId(tx.nVersionGroupId), nExpiryHeight(tx.nExpiryHeight),
+                                                                   vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime),
                                                                    vjoinsplit(tx.vjoinsplit), joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig)
 {
     
@@ -163,19 +164,34 @@ void CTransaction::UpdateHash() const
     *const_cast<uint256*>(&hash) = SerializeHash(*this);
 }
 
-CTransaction::CTransaction() : nVersion(CTransaction::MIN_CURRENT_VERSION), vin(), vout(), nLockTime(0), vjoinsplit(), joinSplitPubKey(), joinSplitSig() { }
+CTransaction::CTransaction() : nVersion(CTransaction::SPROUT_MIN_CURRENT_VERSION), fOverwintered(false), nVersionGroupId(0), nExpiryHeight(0), vin(), vout(), nLockTime(0), vjoinsplit(), joinSplitPubKey(), joinSplitSig() { }
 
-CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), vjoinsplit(tx.vjoinsplit),
-                                                            joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig)
+CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), fOverwintered(tx.fOverwintered), nVersionGroupId(tx.nVersionGroupId), nExpiryHeight(tx.nExpiryHeight),
+                                                            vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime),
+                                                            vjoinsplit(tx.vjoinsplit), joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig)
 {
     UpdateHash();
 }
 
+// Protected constructor which only derived classes can call.
+// For developer testing only.
+CTransaction::CTransaction(
+    const CMutableTransaction &tx,
+    bool evilDeveloperFlag) : nVersion(tx.nVersion), fOverwintered(tx.fOverwintered), nVersionGroupId(tx.nVersionGroupId), nExpiryHeight(tx.nExpiryHeight),
+                              vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime),
+                              vjoinsplit(tx.vjoinsplit), joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig)
+{
+    assert(evilDeveloperFlag);
+}
+
 CTransaction& CTransaction::operator=(const CTransaction &tx) {
+    *const_cast<bool*>(&fOverwintered) = tx.fOverwintered;
     *const_cast<int*>(&nVersion) = tx.nVersion;
+    *const_cast<uint32_t*>(&nVersionGroupId) = tx.nVersionGroupId;
     *const_cast<std::vector<CTxIn>*>(&vin) = tx.vin;
     *const_cast<std::vector<CTxOut>*>(&vout) = tx.vout;
     *const_cast<unsigned int*>(&nLockTime) = tx.nLockTime;
+    *const_cast<uint32_t*>(&nExpiryHeight) = tx.nExpiryHeight;
     *const_cast<std::vector<JSDescription>*>(&vjoinsplit) = tx.vjoinsplit;
     *const_cast<uint256*>(&joinSplitPubKey) = tx.joinSplitPubKey;
     *const_cast<joinsplit_sig_t*>(&joinSplitSig) = tx.joinSplitSig;
@@ -248,12 +264,24 @@ unsigned int CTransaction::CalculateModifiedSize(unsigned int nTxSize) const
 std::string CTransaction::ToString() const
 {
     std::string str;
-    str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%u, vout.size=%u, nLockTime=%u)\n",
-        GetHash().ToString().substr(0,10),
-        nVersion,
-        vin.size(),
-        vout.size(),
-        nLockTime);
+    if (!fOverwintered) {
+        str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%u, vout.size=%u, nLockTime=%u)\n",
+            GetHash().ToString().substr(0,10),
+            nVersion,
+            vin.size(),
+            vout.size(),
+            nLockTime);
+    } else if (nVersion >= 3) {
+        str += strprintf("CTransaction(hash=%s, ver=%d, fOverwintered=%d, nVersionGroupId=%08x, vin.size=%u, vout.size=%u, nLockTime=%u, nExpiryHeight=%u)\n",
+            GetHash().ToString().substr(0,10),
+            nVersion,
+            fOverwintered,
+            nVersionGroupId,
+            vin.size(),
+            vout.size(),
+            nLockTime,
+            nExpiryHeight);
+    }
     for (unsigned int i = 0; i < vin.size(); i++)
         str += "    " + vin[i].ToString() + "\n";
     for (unsigned int i = 0; i < vout.size(); i++)

src/primitives/transaction.h

@@ -304,6 +304,13 @@ public:
     std::string ToString() const;
 };
 
+// The maximum value which is valid for expiry height, used by CTransaction and CMutableTransaction
+static constexpr uint32_t TX_EXPIRY_HEIGHT_THRESHOLD = 500000000;
+
+// Overwinter version group id
+static constexpr uint32_t OVERWINTER_VERSION_GROUP_ID = 0x03C48270;
+static_assert(OVERWINTER_VERSION_GROUP_ID != 0, "version group id must be non-zero as specified in ZIP 202");
+
 struct CMutableTransaction;
 
 /** The basic transaction that is broadcasted on the network and contained in
@@ -316,14 +323,29 @@ private:
     const uint256 hash;
     void UpdateHash() const;
 
+protected:
+    /** Developer testing only.  Set evilDeveloperFlag to true.
+     * Convert a CMutableTransaction into a CTransaction without invoking UpdateHash()
+     */
+    CTransaction(const CMutableTransaction &tx, bool evilDeveloperFlag);
+
 public:
     typedef boost::array<unsigned char, 64> joinsplit_sig_t;
 
-    // Transactions that include a list of JoinSplits are version 2.
-    static const int32_t MIN_CURRENT_VERSION = 1;
-    static const int32_t MAX_CURRENT_VERSION = 2;
+    // Transactions that include a list of JoinSplits are >= version 2.
+    static const int32_t SPROUT_MIN_CURRENT_VERSION = 1;
+    static const int32_t SPROUT_MAX_CURRENT_VERSION = 2;
+    static const int32_t OVERWINTER_MIN_CURRENT_VERSION = 3;
+    static const int32_t OVERWINTER_MAX_CURRENT_VERSION = 3;
+
+    static_assert(SPROUT_MIN_CURRENT_VERSION >= SPROUT_MIN_TX_VERSION,
+                  "standard rule for tx version should be consistent with network rule");
 
-    static_assert(MIN_CURRENT_VERSION >= MIN_TX_VERSION,
+    static_assert(OVERWINTER_MIN_CURRENT_VERSION >= OVERWINTER_MIN_TX_VERSION,
+                  "standard rule for tx version should be consistent with network rule");
+
+    static_assert( (OVERWINTER_MAX_CURRENT_VERSION <= OVERWINTER_MAX_TX_VERSION &&
+                    OVERWINTER_MAX_CURRENT_VERSION >= OVERWINTER_MIN_CURRENT_VERSION),
                   "standard rule for tx version should be consistent with network rule");
 
     // The local variables are made const to prevent unintended modification
@@ -331,10 +353,13 @@ public:
     // actually immutable; deserialization and assignment are implemented,
     // and bypass the constness. This is safe, as they update the entire
     // structure, including the hash.
+    const bool fOverwintered;
     const int32_t nVersion;
+    const uint32_t nVersionGroupId;
     const std::vector<CTxIn> vin;
     const std::vector<CTxOut> vout;
     const uint32_t nLockTime;
+    const uint32_t nExpiryHeight;
     const std::vector<JSDescription> vjoinsplit;
     const uint256 joinSplitPubKey;
     const joinsplit_sig_t joinSplitSig = {{0}};
@@ -351,11 +376,39 @@ public:
 
     template <typename Stream, typename Operation>
     inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
-        READWRITE(*const_cast<int32_t*>(&this->nVersion));
+        if (ser_action.ForRead()) {
+            // When deserializing, unpack the 4 byte header to extract fOverwintered and nVersion.
+            uint32_t header;
+            READWRITE(header);
+            *const_cast<bool*>(&fOverwintered) = header >> 31;
+            *const_cast<int32_t*>(&this->nVersion) = header & 0x7FFFFFFF;
+        } else {
+            // When serializing v1 and v2, the 4 byte header is nVersion
+            uint32_t header = this->nVersion;
+            // When serializing Overwintered tx, the 4 byte header is the combination of fOverwintered and nVersion
+            if (fOverwintered) {
+                header |= 1 << 31;
+            }
+            READWRITE(header);
+        }
         nVersion = this->nVersion;
+        if (fOverwintered) {
+            READWRITE(*const_cast<uint32_t*>(&this->nVersionGroupId));
+        }
+
+        bool isOverwinterV3 = fOverwintered &&
+                              nVersionGroupId == OVERWINTER_VERSION_GROUP_ID &&
+                              nVersion == 3;
+        if (fOverwintered && !isOverwinterV3) {
+            throw std::ios_base::failure("Unknown transaction format");
+        }
+
         READWRITE(*const_cast<std::vector<CTxIn>*>(&vin));
         READWRITE(*const_cast<std::vector<CTxOut>*>(&vout));
         READWRITE(*const_cast<uint32_t*>(&nLockTime));
+        if (isOverwinterV3) {
+            READWRITE(*const_cast<uint32_t*>(&nExpiryHeight));
+        }
         if (nVersion >= 2) {
             READWRITE(*const_cast<std::vector<JSDescription>*>(&vjoinsplit));
             if (vjoinsplit.size() > 0) {
@@ -410,10 +463,13 @@ public:
 /** A mutable version of CTransaction. */
 struct CMutableTransaction
 {
+    bool fOverwintered;
     int32_t nVersion;
+    uint32_t nVersionGroupId;
     std::vector<CTxIn> vin;
     std::vector<CTxOut> vout;
     uint32_t nLockTime;
+    uint32_t nExpiryHeight;
     std::vector<JSDescription> vjoinsplit;
     uint256 joinSplitPubKey;
     CTransaction::joinsplit_sig_t joinSplitSig = {{0}};
@@ -425,11 +481,39 @@ struct CMutableTransaction
 
     template <typename Stream, typename Operation>
     inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) {
-        READWRITE(this->nVersion);
+        if (ser_action.ForRead()) {
+            // When deserializing, unpack the 4 byte header to extract fOverwintered and nVersion.
+            uint32_t header;
+            READWRITE(header);
+            fOverwintered = header >> 31;
+            this->nVersion = header & 0x7FFFFFFF;
+        } else {
+            // When serializing v1 and v2, the 4 byte header is nVersion
+            uint32_t header = this->nVersion;
+            // When serializing Overwintered tx, the 4 byte header is the combination of fOverwintered and nVersion
+            if (fOverwintered) {
+                header |= 1 << 31;
+            }
+            READWRITE(header);
+        }
         nVersion = this->nVersion;
+        if (fOverwintered) {
+            READWRITE(nVersionGroupId);
+        }
+
+        bool isOverwinterV3 = fOverwintered &&
+                              nVersionGroupId == OVERWINTER_VERSION_GROUP_ID &&
+                              nVersion == 3;
+        if (fOverwintered && !isOverwinterV3) {
+            throw std::ios_base::failure("Unknown transaction format");
+        }
+
         READWRITE(vin);
         READWRITE(vout);
         READWRITE(nLockTime);
+        if (isOverwinterV3) {
+            READWRITE(nExpiryHeight);
+        }
         if (nVersion >= 2) {
             READWRITE(vjoinsplit);
             if (vjoinsplit.size() > 0) {

src/rpcrawtransaction.cpp

@@ -113,8 +113,15 @@ UniValue TxJoinSplitToJSON(const CTransaction& tx) {
 void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry)
 {
     entry.push_back(Pair("txid", tx.GetHash().GetHex()));
+    entry.push_back(Pair("overwintered", tx.fOverwintered));
     entry.push_back(Pair("version", tx.nVersion));
+    if (tx.fOverwintered) {
+        entry.push_back(Pair("versiongroupid", HexInt(tx.nVersionGroupId)));
+    }
     entry.push_back(Pair("locktime", (int64_t)tx.nLockTime));
+    if (tx.fOverwintered) {
+        entry.push_back(Pair("expiryheight", (int64_t)tx.nExpiryHeight));
+    }
     UniValue vin(UniValue::VARR);
     BOOST_FOREACH(const CTxIn& txin, tx.vin) {
         UniValue in(UniValue::VOBJ);
@@ -435,7 +442,8 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp)
     UniValue inputs = params[0].get_array();
     UniValue sendTo = params[1].get_obj();
 
-    CMutableTransaction rawTx;
+    CMutableTransaction rawTx = CreateNewContextualCMutableTransaction(
+        Params().GetConsensus(), chainActive.Height() + 1);
 
     for (size_t idx = 0; idx < inputs.size(); idx++) {
         const UniValue& input = inputs[idx];
@@ -488,8 +496,11 @@ UniValue decoderawtransaction(const UniValue& params, bool fHelp)
             "\nResult:\n"
             "{\n"
             "  \"txid\" : \"id\",        (string) The transaction id\n"
+            "  \"overwintered\" : bool   (boolean) The Overwintered flag\n" 
             "  \"version\" : n,          (numeric) The version\n"
+            "  \"versiongroupid\": \"hex\"   (string, optional) The version group id (Overwintered txs)\n"
             "  \"locktime\" : ttt,       (numeric) The lock time\n"
+            "  \"expiryheight\" : n,     (numeric, optional) Last valid block height for mining transaction (Overwintered txs)\n"
             "  \"vin\" : [               (array of json objects)\n"
             "     {\n"
             "       \"txid\": \"id\",    (string) The transaction id\n"

src/test/rpc_tests.cpp

@@ -7,6 +7,7 @@
 
 #include "base58.h"
 #include "netbase.h"
+#include "utilstrencodings.h"
 
 #include "test/test_bitcoin.h"
 
@@ -295,4 +296,44 @@ BOOST_AUTO_TEST_CASE(rpc_ban)
     BOOST_CHECK_EQUAL(adr.get_str(), "2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff");
 }
 
+
+BOOST_AUTO_TEST_CASE(rpc_raw_create_overwinter_v3)
+{
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
+
+    // Sample regtest address:
+    // public: tmHU5HLMu3yS8eoNvbrU1NWeJaGf6jxehru
+    // private: cW1G4SxEm5rui2RQtBcSUZrERTVYPtyZXKbSi5MCwBqzbn5kqwbN
+
+    UniValue r;
+    std::string prevout =
+      "[{\"txid\":\"b4cc287e58f87cdae59417329f710f3ecd75a4ee1d2872b7248f50977c8493f3\","
+      "\"vout\":1}]";
+    r = CallRPC(string("createrawtransaction ") + prevout + " " +
+      "{\"tmHU5HLMu3yS8eoNvbrU1NWeJaGf6jxehru\":11}");
+    std::string rawhex = r.get_str();
+    BOOST_CHECK_NO_THROW(r = CallRPC(string("decoderawtransaction ") + rawhex));
+    BOOST_CHECK_EQUAL(find_value(r.get_obj(), "overwintered").get_bool(), true);
+    BOOST_CHECK_EQUAL(find_value(r.get_obj(), "version").get_int(), 3);
+    BOOST_CHECK_EQUAL(find_value(r.get_obj(), "expiryheight").get_int(), 0);
+    BOOST_CHECK_EQUAL(
+        ParseHexToUInt32(find_value(r.get_obj(), "versiongroupid").get_str()),
+        OVERWINTER_VERSION_GROUP_ID);
+
+    // Sanity check we can deserialize the raw hex
+    // 030000807082c40301f393847c97508f24b772281deea475cd3e0f719f321794e5da7cf8587e28ccb40100000000ffffffff0100ab9041000000001976a914550dc92d3ff8d1f0cb6499fddf2fe43b745330cd88ac000000000000000000
+    CDataStream ss(ParseHex(rawhex), SER_DISK, PROTOCOL_VERSION);
+    CTransaction tx;
+    ss >> tx;
+    CDataStream ss2(ParseHex(rawhex), SER_DISK, PROTOCOL_VERSION);
+    CMutableTransaction mtx;
+    ss2 >> mtx;
+    BOOST_CHECK_EQUAL(tx.GetHash().GetHex(), CTransaction(mtx).GetHash().GetHex());
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/rpc_wallet_tests.cpp

@@ -908,28 +908,36 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_parameters)
     BOOST_CHECK_THROW(CallRPC(string("z_sendmany tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ ")
             + "[{\"address\":\"" + zaddr1 + "\", \"amount\":123.456}]"), runtime_error);
 
+    // Mutable tx containing contextual information we need to build tx
+    UniValue retValue = CallRPC("getblockcount");
+    int nHeight = retValue.get_int();
+    CMutableTransaction mtx = CreateNewContextualCMutableTransaction(Params().GetConsensus(), nHeight + 1);
+    if (mtx.nVersion == 1) {
+        mtx.nVersion = 2;
+    }
+
     // Test constructor of AsyncRPCOperation_sendmany
     try {
-        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany("",{}, {}, -1));
+        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(mtx, "",{}, {}, -1));
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "Minconf cannot be negative"));
     }
 
     try {
-        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany("",{}, {}, 1));
+        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(mtx, "",{}, {}, 1));
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "From address parameter missing"));
     }
 
     try {
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany("tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ", {}, {}, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, "tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ", {}, {}, 1) );
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "No recipients"));
     }
 
     try {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient("dummy",1.0, "") };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany("INVALID", recipients, {}, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, "INVALID", recipients, {}, 1) );
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "payment address is invalid"));
     }
@@ -937,7 +945,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_parameters)
     // Testnet payment addresses begin with 'zt'.  This test detects an incorrect prefix.
     try {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient("dummy",1.0, "") };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany("zcMuhvq8sEkHALuSU2i4NbNQxshSAYrpCExec45ZjtivYPbuiFPwk6WHy4SvsbeZ4siy1WheuRGjtaJmoD1J8bFqNXhsG6U", recipients, {}, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, "zcMuhvq8sEkHALuSU2i4NbNQxshSAYrpCExec45ZjtivYPbuiFPwk6WHy4SvsbeZ4siy1WheuRGjtaJmoD1J8bFqNXhsG6U", recipients, {}, 1) );
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "payment address is for wrong network type"));
     }
@@ -946,7 +954,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_parameters)
     // invokes a method on pwalletMain, which is undefined in the google test environment.
     try {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient("dummy",1.0, "") };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany("ztjiDe569DPNbyTE6TSdJTaSDhoXEHLGvYoUnBU1wfVNU52TEyT6berYtySkd21njAeEoh8fFJUT42kua9r8EnhBaEKqCpP", recipients, {}, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, "ztjiDe569DPNbyTE6TSdJTaSDhoXEHLGvYoUnBU1wfVNU52TEyT6berYtySkd21njAeEoh8fFJUT42kua9r8EnhBaEKqCpP", recipients, {}, 1) );
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "no spending key found for zaddr"));
     }
@@ -962,6 +970,14 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
 
     UniValue retValue;
 
+    // Mutable tx containing contextual information we need to build tx
+    retValue = CallRPC("getblockcount");
+    int nHeight = retValue.get_int();
+    CMutableTransaction mtx = CreateNewContextualCMutableTransaction(Params().GetConsensus(), nHeight + 1);
+    if (mtx.nVersion == 1) {
+        mtx.nVersion = 2;
+    }
+
     // add keys manually
     BOOST_CHECK_NO_THROW(retValue = CallRPC("getnewaddress"));
     std::string taddr1 = retValue.get_str();
@@ -971,7 +987,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
     // there are no utxos to spend
     {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1,100.0, "DEADBEEF") };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(taddr1, {}, recipients, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, taddr1, {}, recipients, 1) );
         operation->main();
         BOOST_CHECK(operation->isFailed());
         std::string msg = operation->getErrorMessage();
@@ -982,7 +998,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
     {
         try {
             std::vector<SendManyRecipient> recipients = {SendManyRecipient(taddr1, 100.0, "DEADBEEF")};
-            std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 0));
+            std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(mtx, zaddr1, recipients, {}, 0));
             BOOST_CHECK(false); // Fail test if an exception is not thrown
         } catch (const UniValue& objError) {
             BOOST_CHECK(find_error(objError, "Minconf cannot be zero when sending from zaddr"));
@@ -993,7 +1009,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
     // there are no unspent notes to spend
     {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(taddr1,100.0, "DEADBEEF") };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, recipients, {}, 1) );
         operation->main();
         BOOST_CHECK(operation->isFailed());
         std::string msg = operation->getErrorMessage();
@@ -1003,7 +1019,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
     // get_memo_from_hex_string())
     {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1,100.0, "DEADBEEF") };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, recipients, {}, 1) );
         std::shared_ptr<AsyncRPCOperation_sendmany> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_sendmany> (operation);
         TEST_FRIEND_AsyncRPCOperation_sendmany proxy(ptr);
 
@@ -1054,7 +1070,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
     // add_taddr_change_output_to_tx() will append a vout to a raw transaction
     {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1,100.0, "DEADBEEF") };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, recipients, {}, 1) );
         std::shared_ptr<AsyncRPCOperation_sendmany> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_sendmany> (operation);
         TEST_FRIEND_AsyncRPCOperation_sendmany proxy(ptr);
 
@@ -1083,7 +1099,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
             SendManyRecipient("tmUSbHz3vxnwLvRyNDXbwkZxjVyDodMJEhh",CAmount(4.56), ""),
             SendManyRecipient("tmYZAXYPCP56Xa5JQWWPZuK7o7bfUQW6kkd",CAmount(7.89), ""),
         };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, recipients, {}, 1) );
         std::shared_ptr<AsyncRPCOperation_sendmany> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_sendmany> (operation);
         TEST_FRIEND_AsyncRPCOperation_sendmany proxy(ptr);
 
@@ -1106,7 +1122,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
         // we have the spending key for the dummy recipient zaddr1
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1, 0.0005, "ABCD") };
 
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(zaddr1, {}, recipients, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, {}, recipients, 1) );
         std::shared_ptr<AsyncRPCOperation_sendmany> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_sendmany> (operation);
         TEST_FRIEND_AsyncRPCOperation_sendmany proxy(ptr);
 
@@ -1131,7 +1147,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
         // Dummy input so the operation object can be instantiated.
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1, 0.0005, "ABCD") };
 
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(zaddr1, {}, recipients, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, {}, recipients, 1) );
         std::shared_ptr<AsyncRPCOperation_sendmany> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_sendmany> (operation);
         TEST_FRIEND_AsyncRPCOperation_sendmany proxy(ptr);
 
@@ -1286,18 +1302,26 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters)
     "100 -1"
     ), runtime_error);
 
+    // Mutable tx containing contextual information we need to build tx
+    UniValue retValue = CallRPC("getblockcount");
+    int nHeight = retValue.get_int();
+    CMutableTransaction mtx = CreateNewContextualCMutableTransaction(Params().GetConsensus(), nHeight + 1);
+    if (mtx.nVersion == 1) {
+        mtx.nVersion = 2;
+    }
+
     // Test constructor of AsyncRPCOperation_sendmany
     std::string testnetzaddr = "ztjiDe569DPNbyTE6TSdJTaSDhoXEHLGvYoUnBU1wfVNU52TEyT6berYtySkd21njAeEoh8fFJUT42kua9r8EnhBaEKqCpP";
     std::string mainnetzaddr = "zcMuhvq8sEkHALuSU2i4NbNQxshSAYrpCExec45ZjtivYPbuiFPwk6WHy4SvsbeZ4siy1WheuRGjtaJmoD1J8bFqNXhsG6U";
 
     try {
-        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_shieldcoinbase({}, testnetzaddr, -1 ));
+        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_shieldcoinbase(mtx, {}, testnetzaddr, -1 ));
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "Fee is out of range"));
     }
 
     try {
-        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_shieldcoinbase({}, testnetzaddr, 1));
+        std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_shieldcoinbase(mtx, {}, testnetzaddr, 1));
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "Empty inputs"));
     }
@@ -1305,7 +1329,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters)
     // Testnet payment addresses begin with 'zt'.  This test detects an incorrect prefix.
     try {
         std::vector<ShieldCoinbaseUTXO> inputs = { ShieldCoinbaseUTXO{uint256(),0,0} };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(inputs, mainnetzaddr, 1) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, mainnetzaddr, 1) );
     } catch (const UniValue& objError) {
         BOOST_CHECK( find_error(objError, "payment address is for wrong network type"));
     }
@@ -1320,8 +1344,14 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals)
 
     LOCK(pwalletMain->cs_wallet);
 
-    UniValue retValue;
-
+    // Mutable tx containing contextual information we need to build tx
+    UniValue retValue = CallRPC("getblockcount");
+    int nHeight = retValue.get_int();
+    CMutableTransaction mtx = CreateNewContextualCMutableTransaction(Params().GetConsensus(), nHeight + 1);
+    if (mtx.nVersion == 1) {
+        mtx.nVersion = 2;
+    }
+    
     // Test that option -mempooltxinputlimit is respected.
     mapArgs["-mempooltxinputlimit"] = "1";
 
@@ -1332,7 +1362,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals)
     // Supply 2 inputs when mempool limit is 1
     {
         std::vector<ShieldCoinbaseUTXO> inputs = { ShieldCoinbaseUTXO{uint256(),0,0}, ShieldCoinbaseUTXO{uint256(),0,0} };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(inputs, zaddr) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) );
         operation->main();
         BOOST_CHECK(operation->isFailed());
         std::string msg = operation->getErrorMessage();
@@ -1342,7 +1372,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals)
     // Insufficient funds
     {
         std::vector<ShieldCoinbaseUTXO> inputs = { ShieldCoinbaseUTXO{uint256(),0,0} };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(inputs, zaddr) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) );
         operation->main();
         BOOST_CHECK(operation->isFailed());
         std::string msg = operation->getErrorMessage();
@@ -1353,7 +1383,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals)
     {
         // Dummy input so the operation object can be instantiated.
         std::vector<ShieldCoinbaseUTXO> inputs = { ShieldCoinbaseUTXO{uint256(),0,100000} };
-        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(inputs, zaddr) );
+        std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) );
         std::shared_ptr<AsyncRPCOperation_shieldcoinbase> ptr = std::dynamic_pointer_cast<AsyncRPCOperation_shieldcoinbase> (operation);
         TEST_FRIEND_AsyncRPCOperation_shieldcoinbase proxy(ptr);
         static_cast<AsyncRPCOperation_shieldcoinbase *>(operation.get())->testmode = true;

src/test/sighash_tests.cpp

@@ -234,7 +234,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data)
           stream >> tx;
 
           CValidationState state;
-          if (tx.nVersion < MIN_TX_VERSION) {
+          if (tx.nVersion < SPROUT_MIN_TX_VERSION) {
               // Transaction must be invalid
               BOOST_CHECK_MESSAGE(!CheckTransactionWithoutProofVerification(tx, state), strTest);
               BOOST_CHECK(!state.IsValid());

src/utilstrencodings.cpp

@@ -54,6 +54,13 @@ std::string HexInt(uint32_t val)
     return ss.str();
 }
 
+uint32_t ParseHexToUInt32(const std::string& str) {
+    std::istringstream converter(str);
+    uint32_t value;
+    converter >> std::hex >> value;
+    return value;
+}
+
 const signed char p_util_hexdigit[256] =
 { -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
   -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,

src/utilstrencodings.h

@@ -25,6 +25,7 @@
 std::string SanitizeFilename(const std::string& str);
 std::string SanitizeString(const std::string& str);
 std::string HexInt(uint32_t val);
+uint32_t ParseHexToUInt32(const std::string& str);
 std::vector<unsigned char> ParseHex(const char* psz);
 std::vector<unsigned char> ParseHex(const std::string& str);
 signed char HexDigit(char c);

src/wallet/asyncrpcoperation_sendmany.cpp

@@ -50,14 +50,16 @@ int find_output(UniValue obj, int n) {
 }
 
 AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
+        CMutableTransaction contextualTx,
         std::string fromAddress,
         std::vector<SendManyRecipient> tOutputs,
         std::vector<SendManyRecipient> zOutputs,
         int minDepth,
         CAmount fee,
         UniValue contextInfo) :
-        fromaddress_(fromAddress), t_outputs_(tOutputs), z_outputs_(zOutputs), mindepth_(minDepth), fee_(fee), contextinfo_(contextInfo)
+        tx_(contextualTx), fromaddress_(fromAddress), t_outputs_(tOutputs), z_outputs_(zOutputs), mindepth_(minDepth), fee_(fee), contextinfo_(contextInfo)
 {
+    assert(contextualTx.nVersion >= 2);  // transaction format version must support vjoinsplit
     assert(fee_ >= 0);
 
     if (minDepth < 0) {
@@ -370,7 +372,6 @@ bool AsyncRPCOperation_sendmany::main_impl() {
     
     // Prepare raw transaction to handle JoinSplits
     CMutableTransaction mtx(tx_);
-    mtx.nVersion = 2;
     crypto_sign_keypair(joinSplitPubKey_.begin(), joinSplitPrivKey_);
     mtx.joinSplitPubKey = joinSplitPubKey_;
     tx_ = CTransaction(mtx);

src/wallet/asyncrpcoperation_sendmany.h

@@ -51,7 +51,7 @@ struct WitnessAnchorData {
 
 class AsyncRPCOperation_sendmany : public AsyncRPCOperation {
 public:
-    AsyncRPCOperation_sendmany(std::string fromAddress, std::vector<SendManyRecipient> tOutputs, std::vector<SendManyRecipient> zOutputs, int minDepth, CAmount fee = ASYNC_RPC_OPERATION_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue);
+    AsyncRPCOperation_sendmany(CMutableTransaction contextualTx, std::string fromAddress, std::vector<SendManyRecipient> tOutputs, std::vector<SendManyRecipient> zOutputs, int minDepth, CAmount fee = ASYNC_RPC_OPERATION_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue);
     virtual ~AsyncRPCOperation_sendmany();
     
     // We don't want to be copied or moved around

src/wallet/asyncrpcoperation_shieldcoinbase.cpp

@@ -52,12 +52,15 @@ static int find_output(UniValue obj, int n) {
 }
 
 AsyncRPCOperation_shieldcoinbase::AsyncRPCOperation_shieldcoinbase(
+        CMutableTransaction contextualTx,
         std::vector<ShieldCoinbaseUTXO> inputs,
         std::string toAddress,
         CAmount fee,
         UniValue contextInfo) :
-        inputs_(inputs), fee_(fee), contextinfo_(contextInfo)
+        tx_(contextualTx), inputs_(inputs), fee_(fee), contextinfo_(contextInfo)
 {
+    assert(contextualTx.nVersion >= 2);  // transaction format version must support vjoinsplit
+
     if (fee < 0 || fee > MAX_MONEY) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Fee is out of range");
     }
@@ -213,7 +216,6 @@ bool AsyncRPCOperation_shieldcoinbase::main_impl() {
 
     // Prepare raw transaction to handle JoinSplits
     CMutableTransaction mtx(tx_);
-    mtx.nVersion = 2;
     crypto_sign_keypair(joinSplitPubKey_.begin(), joinSplitPrivKey_);
     mtx.joinSplitPubKey = joinSplitPubKey_;
     tx_ = CTransaction(mtx);

src/wallet/asyncrpcoperation_shieldcoinbase.h

@@ -42,7 +42,7 @@ struct ShieldCoinbaseJSInfo
 
 class AsyncRPCOperation_shieldcoinbase : public AsyncRPCOperation {
 public:
-    AsyncRPCOperation_shieldcoinbase(std::vector<ShieldCoinbaseUTXO> inputs, std::string toAddress, CAmount fee = SHIELD_COINBASE_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue);
+    AsyncRPCOperation_shieldcoinbase(CMutableTransaction contextualTx, std::vector<ShieldCoinbaseUTXO> inputs, std::string toAddress, CAmount fee = SHIELD_COINBASE_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue);
     virtual ~AsyncRPCOperation_shieldcoinbase();
 
     // We don't want to be copied or moved around

src/wallet/rpcwallet.cpp

@@ -3525,9 +3525,16 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
     o.push_back(Pair("fee", std::stod(FormatMoney(nFee))));
     UniValue contextInfo = o;
 
+    // Contextual transaction we will build on
+    CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction(
+        Params().GetConsensus(), chainActive.Height() + 1);
+    if (contextualTx.nVersion == 1) {
+        contextualTx.nVersion = 2; // Tx format should support vjoinsplits 
+    }
+
     // Create operation and add to global queue
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
-    std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(fromaddress, taddrRecipients, zaddrRecipients, nMinDepth, nFee, contextInfo) );
+    std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(contextualTx, fromaddress, taddrRecipients, zaddrRecipients, nMinDepth, nFee, contextInfo) );
     q->addOperation(operation);
     AsyncRPCOperationId operationId = operation->getId();
     return operationId;
@@ -3709,9 +3716,17 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
     contextInfo.push_back(Pair("toaddress", params[1]));
     contextInfo.push_back(Pair("fee", ValueFromAmount(nFee)));
 
+    // Contextual transaction we will build on
+    CMutableTransaction contextualTx = CreateNewContextualCMutableTransaction(
+        Params().GetConsensus(),
+        chainActive.Height() + 1);
+    if (contextualTx.nVersion == 1) {
+        contextualTx.nVersion = 2; // Tx format should support vjoinsplits 
+    }
+
     // Create operation and add to global queue
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
-    std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(inputs, destaddress, nFee, contextInfo) );
+    std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_shieldcoinbase(contextualTx, inputs, destaddress, nFee, contextInfo) );
     q->addOperation(operation);
     AsyncRPCOperationId operationId = operation->getId();
 

src/wallet/wallet.cpp

@@ -2572,7 +2572,8 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
 
     wtxNew.fTimeReceivedIsTxTime = true;
     wtxNew.BindWallet(this);
-    CMutableTransaction txNew;
+    CMutableTransaction txNew = CreateNewContextualCMutableTransaction(
+        Params().GetConsensus(), chainActive.Height() + 1);
 
     // Discourage fee sniping.
     //