From 072099d788553021c5a80956334e32222509a0a5 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 15 Feb 2018 22:19:36 -0800 Subject: [PATCH] Implementation of Overwinter transaction format ZIP 202. --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/wallet_overwintertx.py | 110 ++++++ src/bitcoin-tx.cpp | 2 +- src/chainparams.cpp | 5 + src/consensus/consensus.h | 6 +- src/gtest/test_checkblock.cpp | 156 +++++++++ src/gtest/test_checktransaction.cpp | 313 ++++++++++++++++++ src/gtest/test_mempool.cpp | 125 +++++++ src/main.cpp | 141 +++++++- src/main.h | 9 +- src/miner.cpp | 2 +- src/primitives/transaction.cpp | 50 ++- src/primitives/transaction.h | 96 +++++- src/rpcrawtransaction.cpp | 13 +- src/test/rpc_tests.cpp | 41 +++ src/test/rpc_wallet_tests.cpp | 74 +++-- src/test/sighash_tests.cpp | 2 +- src/utilstrencodings.cpp | 7 + src/utilstrencodings.h | 1 + src/wallet/asyncrpcoperation_sendmany.cpp | 5 +- src/wallet/asyncrpcoperation_sendmany.h | 2 +- .../asyncrpcoperation_shieldcoinbase.cpp | 6 +- src/wallet/asyncrpcoperation_shieldcoinbase.h | 2 +- src/wallet/rpcwallet.cpp | 19 +- src/wallet/wallet.cpp | 3 +- 25 files changed, 1125 insertions(+), 66 deletions(-) create mode 100755 qa/rpc-tests/wallet_overwintertx.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index dfbd78f9a..ec5bd2a48 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/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' diff --git a/qa/rpc-tests/wallet_overwintertx.py b/qa/rpc-tests/wallet_overwintertx.py new file mode 100755 index 000000000..84215eb6b --- /dev/null +++ b/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() diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index c6c7c93ed..9e81a14ec 100644 --- a/src/bitcoin-tx.cpp +++ b/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; diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 22d425991..a0c73dc37 100644 --- a/src/chainparams.cpp +++ b/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& 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 = CMutableTransaction txNew; txNew.nVersion = 1; txNew.vin.resize(1); diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index d700666bc..43fd1e828 100644 --- a/src/consensus/consensus.h +++ b/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) */ diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index edd8a617d..53c0efcc7 100644 --- a/src/gtest/test_checkblock.cpp +++ b/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); +} \ No newline at end of file diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 305977103..704be83ef 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/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::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); +} diff --git a/src/gtest/test_mempool.cpp b/src/gtest/test_mempool.cpp index 14b4d83b1..b9c47b211 100644 --- a/src/gtest/test_mempool.cpp +++ b/src/gtest/test_mempool.cpp @@ -1,6 +1,7 @@ #include #include +#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(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((1 << 31) | 3); + EXPECT_EQ(mtx.nVersion, static_cast(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); +} diff --git a/src/main.cpp b/src/main.cpp index 062507d0e..a17cebe3d 100644 --- a/src/main.cpp +++ b/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; +} diff --git a/src/main.h b/src/main.h index 03b0464eb..150211d15 100644 --- a/src/main.h +++ b/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 *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 diff --git a/src/miner.cpp b/src/miner.cpp index 327228f3f..542cebec8 100644 --- a/src/miner.cpp +++ b/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); diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 4141bea45..b362d3d6e 100644 --- a/src/primitives/transaction.cpp +++ b/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(&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(&fOverwintered) = tx.fOverwintered; *const_cast(&nVersion) = tx.nVersion; + *const_cast(&nVersionGroupId) = tx.nVersionGroupId; *const_cast*>(&vin) = tx.vin; *const_cast*>(&vout) = tx.vout; *const_cast(&nLockTime) = tx.nLockTime; + *const_cast(&nExpiryHeight) = tx.nExpiryHeight; *const_cast*>(&vjoinsplit) = tx.vjoinsplit; *const_cast(&joinSplitPubKey) = tx.joinSplitPubKey; *const_cast(&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++) diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index f9723e399..28347d6a5 100644 --- a/src/primitives/transaction.h +++ b/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 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(MIN_CURRENT_VERSION >= MIN_TX_VERSION, + static_assert(SPROUT_MIN_CURRENT_VERSION >= SPROUT_MIN_TX_VERSION, + "standard rule for tx version should be consistent with network rule"); + + 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 vin; const std::vector vout; const uint32_t nLockTime; + const uint32_t nExpiryHeight; const std::vector vjoinsplit; const uint256 joinSplitPubKey; const joinsplit_sig_t joinSplitSig = {{0}}; @@ -351,11 +376,39 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { - READWRITE(*const_cast(&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(&fOverwintered) = header >> 31; + *const_cast(&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(&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*>(&vin)); READWRITE(*const_cast*>(&vout)); READWRITE(*const_cast(&nLockTime)); + if (isOverwinterV3) { + READWRITE(*const_cast(&nExpiryHeight)); + } if (nVersion >= 2) { READWRITE(*const_cast*>(&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 vin; std::vector vout; uint32_t nLockTime; + uint32_t nExpiryHeight; std::vector vjoinsplit; uint256 joinSplitPubKey; CTransaction::joinsplit_sig_t joinSplitSig = {{0}}; @@ -425,11 +481,39 @@ struct CMutableTransaction template 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) { diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp index bd9b866c2..48a30fbd6 100644 --- a/src/rpcrawtransaction.cpp +++ b/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" diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index c2b29ab67..184b9d54e 100644 --- a/src/test/rpc_tests.cpp +++ b/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() diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index e444eb2a5..3b563de54 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/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 operation(new AsyncRPCOperation_sendmany("",{}, {}, -1)); + std::shared_ptr operation(new AsyncRPCOperation_sendmany(mtx, "",{}, {}, -1)); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "Minconf cannot be negative")); } try { - std::shared_ptr operation(new AsyncRPCOperation_sendmany("",{}, {}, 1)); + std::shared_ptr operation(new AsyncRPCOperation_sendmany(mtx, "",{}, {}, 1)); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "From address parameter missing")); } try { - std::shared_ptr operation( new AsyncRPCOperation_sendmany("tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ", {}, {}, 1) ); + std::shared_ptr operation( new AsyncRPCOperation_sendmany(mtx, "tmRr6yJonqGK23UVhrKuyvTpF8qxQQjKigJ", {}, {}, 1) ); } catch (const UniValue& objError) { BOOST_CHECK( find_error(objError, "No recipients")); } try { std::vector recipients = { SendManyRecipient("dummy",1.0, "") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany("INVALID", recipients, {}, 1) ); + std::shared_ptr 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 recipients = { SendManyRecipient("dummy",1.0, "") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany("zcMuhvq8sEkHALuSU2i4NbNQxshSAYrpCExec45ZjtivYPbuiFPwk6WHy4SvsbeZ4siy1WheuRGjtaJmoD1J8bFqNXhsG6U", recipients, {}, 1) ); + std::shared_ptr 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 recipients = { SendManyRecipient("dummy",1.0, "") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany("ztjiDe569DPNbyTE6TSdJTaSDhoXEHLGvYoUnBU1wfVNU52TEyT6berYtySkd21njAeEoh8fFJUT42kua9r8EnhBaEKqCpP", recipients, {}, 1) ); + std::shared_ptr 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 recipients = { SendManyRecipient(zaddr1,100.0, "DEADBEEF") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany(taddr1, {}, recipients, 1) ); + std::shared_ptr 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 recipients = {SendManyRecipient(taddr1, 100.0, "DEADBEEF")}; - std::shared_ptr operation(new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 0)); + std::shared_ptr 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 recipients = { SendManyRecipient(taddr1,100.0, "DEADBEEF") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 1) ); + std::shared_ptr 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 recipients = { SendManyRecipient(zaddr1,100.0, "DEADBEEF") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 1) ); + std::shared_ptr operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, recipients, {}, 1) ); std::shared_ptr ptr = std::dynamic_pointer_cast (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 recipients = { SendManyRecipient(zaddr1,100.0, "DEADBEEF") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 1) ); + std::shared_ptr operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, recipients, {}, 1) ); std::shared_ptr ptr = std::dynamic_pointer_cast (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 operation( new AsyncRPCOperation_sendmany(zaddr1, recipients, {}, 1) ); + std::shared_ptr operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, recipients, {}, 1) ); std::shared_ptr ptr = std::dynamic_pointer_cast (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 recipients = { SendManyRecipient(zaddr1, 0.0005, "ABCD") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany(zaddr1, {}, recipients, 1) ); + std::shared_ptr operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, {}, recipients, 1) ); std::shared_ptr ptr = std::dynamic_pointer_cast (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 recipients = { SendManyRecipient(zaddr1, 0.0005, "ABCD") }; - std::shared_ptr operation( new AsyncRPCOperation_sendmany(zaddr1, {}, recipients, 1) ); + std::shared_ptr operation( new AsyncRPCOperation_sendmany(mtx, zaddr1, {}, recipients, 1) ); std::shared_ptr ptr = std::dynamic_pointer_cast (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 operation(new AsyncRPCOperation_shieldcoinbase({}, testnetzaddr, -1 )); + std::shared_ptr 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 operation(new AsyncRPCOperation_shieldcoinbase({}, testnetzaddr, 1)); + std::shared_ptr 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 inputs = { ShieldCoinbaseUTXO{uint256(),0,0} }; - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(inputs, mainnetzaddr, 1) ); + std::shared_ptr 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 inputs = { ShieldCoinbaseUTXO{uint256(),0,0}, ShieldCoinbaseUTXO{uint256(),0,0} }; - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(inputs, zaddr) ); + std::shared_ptr 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 inputs = { ShieldCoinbaseUTXO{uint256(),0,0} }; - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(inputs, zaddr) ); + std::shared_ptr 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 inputs = { ShieldCoinbaseUTXO{uint256(),0,100000} }; - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(inputs, zaddr) ); + std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(mtx, inputs, zaddr) ); std::shared_ptr ptr = std::dynamic_pointer_cast (operation); TEST_FRIEND_AsyncRPCOperation_shieldcoinbase proxy(ptr); static_cast(operation.get())->testmode = true; diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index c33eab17e..13d3dd226 100644 --- a/src/test/sighash_tests.cpp +++ b/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()); diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 4648eb0bb..0a5fbb3d2 100644 --- a/src/utilstrencodings.cpp +++ b/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, diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index 173bbefd0..ccdc6a76b 100644 --- a/src/utilstrencodings.h +++ b/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 ParseHex(const char* psz); std::vector ParseHex(const std::string& str); signed char HexDigit(char c); diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 59cd3a8fb..7590e468c 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/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 tOutputs, std::vector 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); diff --git a/src/wallet/asyncrpcoperation_sendmany.h b/src/wallet/asyncrpcoperation_sendmany.h index 69bdbe315..83a976af9 100644 --- a/src/wallet/asyncrpcoperation_sendmany.h +++ b/src/wallet/asyncrpcoperation_sendmany.h @@ -51,7 +51,7 @@ struct WitnessAnchorData { class AsyncRPCOperation_sendmany : public AsyncRPCOperation { public: - AsyncRPCOperation_sendmany(std::string fromAddress, std::vector tOutputs, std::vector zOutputs, int minDepth, CAmount fee = ASYNC_RPC_OPERATION_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue); + AsyncRPCOperation_sendmany(CMutableTransaction contextualTx, std::string fromAddress, std::vector tOutputs, std::vector 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 diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp index a845c6085..e0d4a1861 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp +++ b/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 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); diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.h b/src/wallet/asyncrpcoperation_shieldcoinbase.h index 379aa5bd7..c7faf28e8 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.h +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.h @@ -42,7 +42,7 @@ struct ShieldCoinbaseJSInfo class AsyncRPCOperation_shieldcoinbase : public AsyncRPCOperation { public: - AsyncRPCOperation_shieldcoinbase(std::vector inputs, std::string toAddress, CAmount fee = SHIELD_COINBASE_DEFAULT_MINERS_FEE, UniValue contextInfo = NullUniValue); + AsyncRPCOperation_shieldcoinbase(CMutableTransaction contextualTx, std::vector 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 diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index eb5688f60..0d652cfcb 100644 --- a/src/wallet/rpcwallet.cpp +++ b/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 q = getAsyncRPCQueue(); - std::shared_ptr operation( new AsyncRPCOperation_sendmany(fromaddress, taddrRecipients, zaddrRecipients, nMinDepth, nFee, contextInfo) ); + std::shared_ptr 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 q = getAsyncRPCQueue(); - std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(inputs, destaddress, nFee, contextInfo) ); + std::shared_ptr operation( new AsyncRPCOperation_shieldcoinbase(contextualTx, inputs, destaddress, nFee, contextInfo) ); q->addOperation(operation); AsyncRPCOperationId operationId = operation->getId(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 06dae5e83..b89111458 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2572,7 +2572,8 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt wtxNew.fTimeReceivedIsTxTime = true; wtxNew.BindWallet(this); - CMutableTransaction txNew; + CMutableTransaction txNew = CreateNewContextualCMutableTransaction( + Params().GetConsensus(), chainActive.Height() + 1); // Discourage fee sniping. //