From ec7466b62ffae897351def7e2a7cc493343eb912 Mon Sep 17 00:00:00 2001 From: George Tankersley Date: Tue, 29 May 2018 00:00:00 +0000 Subject: [PATCH 1/7] Refactor ContextualCheckBlock tests (#3187) Initial cleanup. Reduces duplication of code, especially around constructing transactions, resetting the activation heights, and setting up the EXPECT calls for accepting and rejecting tests. Also adds a bunch of comments explaining the test plan and what particular parts of the test are doing. --- src/gtest/test_checkblock.cpp | 434 +++++++++++++++------------------- 1 file changed, 185 insertions(+), 249 deletions(-) diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index 490af7776..3166a152c 100644 --- a/src/gtest/test_checkblock.cpp +++ b/src/gtest/test_checkblock.cpp @@ -67,27 +67,99 @@ TEST(CheckBlock, BlockSproutRejectsBadVersion) { } -TEST(ContextualCheckBlock, BadCoinbaseHeight) { - SelectParams(CBaseChainParams::MAIN); +class ContextualCheckBlockTest : public ::testing::Test { +protected: + virtual void SetUp() { + SelectParams(CBaseChainParams::MAIN); + } + + virtual void TearDown() { + // Revert to test default. No-op on mainnet params. + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + } + + // Returns a valid but empty mutable transaction at block height 1. + CMutableTransaction GetFirstBlockTransaction() { + CMutableTransaction mtx; + + // No inputs. + mtx.vin.resize(1); + mtx.vin[0].prevout.SetNull(); + + // Set height to 1. + mtx.vin[0].scriptSig = CScript() << 1 << OP_0; + + // Give it a single zero-valued, always-valid output. + mtx.vout.resize(1); + mtx.vout[0].scriptPubKey = CScript() << OP_TRUE; + mtx.vout[0].nValue = 0; + + // Give it a Founder's Reward vout for height 1. + mtx.vout.push_back(CTxOut( + GetBlockSubsidy(1, Params().GetConsensus())/5, + Params().GetFoundersRewardScriptAtHeight(1))); + + return mtx; + } + + // Expects a height-1 block containing a given transaction to pass + // ContextualCheckBlock. This is used in accepting (Sprout-Sprout, + // Overwinter-Overwinter, ...) tests. You should not call it without + // calling a SCOPED_TRACE macro first to usefully label any failures. + void ExpectValidBlockFromTx(const CTransaction& tx) { + // Create a block and add the transaction to it. + CBlock block; + block.vtx.push_back(tx); + + // Set the previous block index to the genesis block. + CBlockIndex indexPrev {Params().GenesisBlock()}; + + // We now expect this to be a valid block. + MockCValidationState state; + EXPECT_TRUE(ContextualCheckBlock(block, state, &indexPrev)); + } + + // Expects a height-1 block containing a given transaction to fail + // ContextualCheckBlock. This is used in rejecting (Sprout-Overwinter, + // Overwinter-Sprout, ...) tests. You should not call it without + // calling a SCOPED_TRACE macro first to usefully label any failures. + void ExpectInvalidBlockFromTx(const CTransaction& tx, int level, std::string reason) { + // Create a block and add the transaction to it. + CBlock block; + block.vtx.push_back(tx); + + // Set the previous block index to the genesis block. + CBlockIndex indexPrev {Params().GenesisBlock()}; + + // We now expect this to be an invalid block, for the given reason. + MockCValidationState state; + EXPECT_CALL(state, DoS(level, false, REJECT_INVALID, reason, false)).Times(1); + EXPECT_FALSE(ContextualCheckBlock(block, state, &indexPrev)); + } + +}; + + +TEST_F(ContextualCheckBlockTest, BadCoinbaseHeight) { + // Put a zero-height transaction in a block + CMutableTransaction mtx = GetFirstBlockTransaction(); + mtx.vin[0].scriptSig = CScript() << 0 << OP_0; + mtx.vout.pop_back(); // remove the FR output - // Create a block with no height in scriptSig - CMutableTransaction mtx; - mtx.vin.resize(1); - mtx.vin[0].prevout.SetNull(); - mtx.vin[0].scriptSig = CScript() << OP_0; - mtx.vout.resize(1); - mtx.vout[0].scriptPubKey = CScript() << OP_TRUE; - mtx.vout[0].nValue = 0; - CTransaction tx {mtx}; CBlock block; - block.vtx.push_back(tx); + block.vtx.push_back(mtx); // Treating block as genesis should pass MockCValidationState state; EXPECT_TRUE(ContextualCheckBlock(block, state, NULL)); + // Give the transaction a Founder's Reward vout + mtx.vout.push_back(CTxOut( + GetBlockSubsidy(1, Params().GetConsensus())/5, + Params().GetFoundersRewardScriptAtHeight(1))); + // Treating block as non-genesis should fail - mtx.vout.push_back(CTxOut(GetBlockSubsidy(1, Params().GetConsensus())/5, Params().GetFoundersRewardScriptAtHeight(1))); CTransaction tx2 {mtx}; block.vtx[0] = tx2; CBlock prev; @@ -110,292 +182,156 @@ TEST(ContextualCheckBlock, BadCoinbaseHeight) { EXPECT_TRUE(ContextualCheckBlock(block, state, &indexPrev)); } +// TEST PLAN: first, check that each ruleset accepts its own transaction type. +// Currently (May 2018) this means we'll test Sprout-Sprout, +// Overwinter-Overwinter, and Sapling-Sapling. -// Test that a block evaluated under Sprout rules cannot contain Sapling transactions. +// Test block evaluated under Sprout rules will accept Sprout transactions. // This test assumes that mainnet Overwinter activation is at least height 2. -TEST(ContextualCheckBlock, BlockSproutRulesRejectSaplingTx) { - SelectParams(CBaseChainParams::MAIN); +TEST_F(ContextualCheckBlockTest, BlockSproutRulesAcceptSproutTx) { + CMutableTransaction mtx = GetFirstBlockTransaction(); - 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 = SAPLING_TX_VERSION; - mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID; - - CTransaction tx {mtx}; - CBlock block; - block.vtx.push_back(tx); - - MockCValidationState state; - CBlockIndex indexPrev {Params().GenesisBlock()}; - - EXPECT_CALL(state, DoS(0, false, REJECT_INVALID, "tx-overwinter-not-active", false)).Times(1); - EXPECT_FALSE(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 = OVERWINTER_TX_VERSION; - 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(0, 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))); + // Make it a Sprout transaction w/o JoinSplits 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); + SCOPED_TRACE("BlockSproutRulesAcceptSproutTx"); + ExpectValidBlockFromTx(CTransaction(mtx)); } -// Test that a block evaluated under Overwinter rules cannot contain Sapling transactions. -TEST(ContextualCheckBlock, BlockOverwinterRulesRejectSaplingTx) { +// Test block evaluated under Overwinter rules will accept Overwinter transactions. +TEST_F(ContextualCheckBlockTest, 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))); + CMutableTransaction mtx = GetFirstBlockTransaction(); - mtx.fOverwintered = true; - mtx.nVersion = SAPLING_TX_VERSION; - mtx.nVersionGroupId = SAPLING_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, "bad-overwinter-tx-version-group-id", false)).Times(1); - EXPECT_FALSE(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))); + // Make it an Overwinter transaction mtx.fOverwintered = true; mtx.nVersion = OVERWINTER_TX_VERSION; 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); + SCOPED_TRACE("BlockOverwinterRulesAcceptOverwinterTx"); + ExpectValidBlockFromTx(CTransaction(mtx)); } // Test that a block evaluated under Sapling rules can contain Sapling transactions. -TEST(ContextualCheckBlock, BlockSaplingRulesAcceptSaplingTx) { +TEST_F(ContextualCheckBlockTest, BlockSaplingRulesAcceptSaplingTx) { SelectParams(CBaseChainParams::REGTEST); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 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))); + CMutableTransaction mtx = GetFirstBlockTransaction(); + // Make it a Sapling transaction mtx.fOverwintered = true; mtx.nVersion = SAPLING_TX_VERSION; mtx.nVersionGroupId = SAPLING_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); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + SCOPED_TRACE("BlockSaplingRulesAcceptSaplingTx"); + ExpectValidBlockFromTx(CTransaction(mtx)); } +// TEST PLAN: next, check that each ruleset will not accept other transaction types. +// Currently (May 2018) this means we'll test Sprout-Overwinter, +// Sprout-Sapling, Overwinter-Sprout, Overwinter-Sapling, Sapling-Sprout, and +// Sapling-Overwinter. -// Test block evaluated under Sapling rules cannot contain Overwinter transactions -TEST(ContextualCheckBlock, BlockSaplingRulesRejectOverwinterTx) { - SelectParams(CBaseChainParams::REGTEST); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 1); +// 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_F(ContextualCheckBlockTest, BlockSproutRulesRejectOverwinterTx) { + CMutableTransaction mtx = GetFirstBlockTransaction(); - 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))); + // Make it an Overwinter transaction mtx.fOverwintered = true; mtx.nVersion = OVERWINTER_TX_VERSION; mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID; - CTransaction tx {mtx}; - CBlock block; - block.vtx.push_back(tx); - MockCValidationState state; - CBlockIndex indexPrev {Params().GenesisBlock()}; + SCOPED_TRACE("BlockSproutRulesRejectOverwinterTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 0, "tx-overwinter-not-active"); +}; - EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-sapling-tx-version-group-id", false)).Times(1); - EXPECT_FALSE(ContextualCheckBlock(block, state, &indexPrev)); - // Revert to default - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); +// Test that a block evaluated under Sprout rules cannot contain Sapling +// transactions. This test assumes that mainnet Overwinter activation is at +// least height 2. +TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectSaplingTx) { + CMutableTransaction mtx = GetFirstBlockTransaction(); + + // Make it a Sapling transaction + mtx.fOverwintered = true; + mtx.nVersion = SAPLING_TX_VERSION; + mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID; + + SCOPED_TRACE("BlockSproutRulesRejectSaplingTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 0, "tx-overwinter-not-active"); } +// Test block evaluated under Overwinter rules will reject Sprout transactions +TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectSproutTx) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); + + CMutableTransaction mtx = GetFirstBlockTransaction(); + + // Set the version to Sprout+JoinSplit (but nJoinSplit will be 0). + mtx.nVersion = 2; + + SCOPED_TRACE("BlockOverwinterRulesRejectSproutTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "tx-overwinter-active"); +} + + +// Test that a block evaluated under Overwinter rules cannot contain Sapling transactions. +TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectSaplingTx) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); + + CMutableTransaction mtx = GetFirstBlockTransaction(); + + // Make it a Sapling transaction + mtx.fOverwintered = true; + mtx.nVersion = SAPLING_TX_VERSION; + mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID; + + SCOPED_TRACE("BlockOverwinterRulesRejectSaplingTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "bad-overwinter-tx-version-group-id"); +} + // Test block evaluated under Sapling rules cannot contain Sprout transactions -TEST(ContextualCheckBlock, BlockSaplingRulesRejectSproutTx) { +TEST_F(ContextualCheckBlockTest, BlockSaplingRulesRejectSproutTx) { SelectParams(CBaseChainParams::REGTEST); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 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; + CMutableTransaction mtx = GetFirstBlockTransaction(); + // Set the version to Sprout+JoinSplit (but nJoinSplit will be 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); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + SCOPED_TRACE("BlockSaplingRulesRejectSproutTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "tx-overwinter-active"); +} + + +// Test block evaluated under Sapling rules cannot contain Overwinter transactions +TEST_F(ContextualCheckBlockTest, BlockSaplingRulesRejectOverwinterTx) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 1); + + CMutableTransaction mtx = GetFirstBlockTransaction(); + + // Make it an Overwinter transaction + mtx.fOverwintered = true; + mtx.nVersion = OVERWINTER_TX_VERSION; + mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID; + + SCOPED_TRACE("BlockSaplingRulesRejectOverwinterTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "bad-sapling-tx-version-group-id"); } From e601446adc631f76d95a7ad8b7446542cbdf3fda Mon Sep 17 00:00:00 2001 From: George Tankersley Date: Tue, 29 May 2018 00:00:00 +0000 Subject: [PATCH 2/7] Refactor ContextualCheckBlock tests Combines some of the needlessly separate tests. Each formerly separate test is tagged with a SCOPED_TRACE to make sure logs are still useful. --- src/gtest/test_checkblock.cpp | 46 ++++++++--------------------------- 1 file changed, 10 insertions(+), 36 deletions(-) diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index 3166a152c..e709bbb6d 100644 --- a/src/gtest/test_checkblock.cpp +++ b/src/gtest/test_checkblock.cpp @@ -234,15 +234,15 @@ TEST_F(ContextualCheckBlockTest, BlockSaplingRulesAcceptSaplingTx) { ExpectValidBlockFromTx(CTransaction(mtx)); } -// TEST PLAN: next, check that each ruleset will not accept other transaction types. -// Currently (May 2018) this means we'll test Sprout-Overwinter, +// TEST PLAN: next, check that each ruleset will not accept other transaction +// types. Currently (May 2018) this means we'll test Sprout-Overwinter, // Sprout-Sapling, Overwinter-Sprout, Overwinter-Sapling, Sapling-Sprout, and // Sapling-Overwinter. -// Test that a block evaluated under Sprout rules cannot contain Overwinter +// Test that a block evaluated under Sprout rules cannot contain non-Sprout // transactions. This test assumes that mainnet Overwinter activation is at // least height 2. -TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectOverwinterTx) { +TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectOtherTx) { CMutableTransaction mtx = GetFirstBlockTransaction(); // Make it an Overwinter transaction @@ -252,14 +252,6 @@ TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectOverwinterTx) { SCOPED_TRACE("BlockSproutRulesRejectOverwinterTx"); ExpectInvalidBlockFromTx(CTransaction(mtx), 0, "tx-overwinter-not-active"); -}; - - -// Test that a block evaluated under Sprout rules cannot contain Sapling -// transactions. This test assumes that mainnet Overwinter activation is at -// least height 2. -TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectSaplingTx) { - CMutableTransaction mtx = GetFirstBlockTransaction(); // Make it a Sapling transaction mtx.fOverwintered = true; @@ -268,11 +260,12 @@ TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectSaplingTx) { SCOPED_TRACE("BlockSproutRulesRejectSaplingTx"); ExpectInvalidBlockFromTx(CTransaction(mtx), 0, "tx-overwinter-not-active"); -} +}; -// Test block evaluated under Overwinter rules will reject Sprout transactions -TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectSproutTx) { +// Test block evaluated under Overwinter rules cannot contain non-Overwinter +// transactions. +TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectOtherTx) { SelectParams(CBaseChainParams::REGTEST); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); @@ -283,15 +276,6 @@ TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectSproutTx) { SCOPED_TRACE("BlockOverwinterRulesRejectSproutTx"); ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "tx-overwinter-active"); -} - - -// Test that a block evaluated under Overwinter rules cannot contain Sapling transactions. -TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectSaplingTx) { - SelectParams(CBaseChainParams::REGTEST); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); - - CMutableTransaction mtx = GetFirstBlockTransaction(); // Make it a Sapling transaction mtx.fOverwintered = true; @@ -303,8 +287,8 @@ TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectSaplingTx) { } -// Test block evaluated under Sapling rules cannot contain Sprout transactions -TEST_F(ContextualCheckBlockTest, BlockSaplingRulesRejectSproutTx) { +// Test block evaluated under Sapling rules cannot contain non-Sapling transactions. +TEST_F(ContextualCheckBlockTest, BlockSaplingRulesRejectOtherTx) { SelectParams(CBaseChainParams::REGTEST); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 1); @@ -316,16 +300,6 @@ TEST_F(ContextualCheckBlockTest, BlockSaplingRulesRejectSproutTx) { SCOPED_TRACE("BlockSaplingRulesRejectSproutTx"); ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "tx-overwinter-active"); -} - - -// Test block evaluated under Sapling rules cannot contain Overwinter transactions -TEST_F(ContextualCheckBlockTest, BlockSaplingRulesRejectOverwinterTx) { - SelectParams(CBaseChainParams::REGTEST); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 1); - - CMutableTransaction mtx = GetFirstBlockTransaction(); // Make it an Overwinter transaction mtx.fOverwintered = true; From d70d103ee031c9d7a6722a5f0dba939356dd3d9a Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 30 Aug 2018 14:41:50 +0100 Subject: [PATCH 3/7] Ensure SCOPED_TRACE falls out of scope when necessary --- src/gtest/test_checkblock.cpp | 36 +++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index e709bbb6d..829e07e7c 100644 --- a/src/gtest/test_checkblock.cpp +++ b/src/gtest/test_checkblock.cpp @@ -250,16 +250,20 @@ TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectOtherTx) { mtx.nVersion = OVERWINTER_TX_VERSION; mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID; - SCOPED_TRACE("BlockSproutRulesRejectOverwinterTx"); - ExpectInvalidBlockFromTx(CTransaction(mtx), 0, "tx-overwinter-not-active"); + { + SCOPED_TRACE("BlockSproutRulesRejectOverwinterTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 0, "tx-overwinter-not-active"); + } // Make it a Sapling transaction mtx.fOverwintered = true; mtx.nVersion = SAPLING_TX_VERSION; mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID; - SCOPED_TRACE("BlockSproutRulesRejectSaplingTx"); - ExpectInvalidBlockFromTx(CTransaction(mtx), 0, "tx-overwinter-not-active"); + { + SCOPED_TRACE("BlockSproutRulesRejectSaplingTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 0, "tx-overwinter-not-active"); + } }; @@ -274,16 +278,20 @@ TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectOtherTx) { // Set the version to Sprout+JoinSplit (but nJoinSplit will be 0). mtx.nVersion = 2; - SCOPED_TRACE("BlockOverwinterRulesRejectSproutTx"); - ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "tx-overwinter-active"); + { + SCOPED_TRACE("BlockOverwinterRulesRejectSproutTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "tx-overwinter-active"); + } // Make it a Sapling transaction mtx.fOverwintered = true; mtx.nVersion = SAPLING_TX_VERSION; mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID; - SCOPED_TRACE("BlockOverwinterRulesRejectSaplingTx"); - ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "bad-overwinter-tx-version-group-id"); + { + SCOPED_TRACE("BlockOverwinterRulesRejectSaplingTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "bad-overwinter-tx-version-group-id"); + } } @@ -298,14 +306,18 @@ TEST_F(ContextualCheckBlockTest, BlockSaplingRulesRejectOtherTx) { // Set the version to Sprout+JoinSplit (but nJoinSplit will be 0). mtx.nVersion = 2; - SCOPED_TRACE("BlockSaplingRulesRejectSproutTx"); - ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "tx-overwinter-active"); + { + SCOPED_TRACE("BlockSaplingRulesRejectSproutTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "tx-overwinter-active"); + } // Make it an Overwinter transaction mtx.fOverwintered = true; mtx.nVersion = OVERWINTER_TX_VERSION; mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID; - SCOPED_TRACE("BlockSaplingRulesRejectOverwinterTx"); - ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "bad-sapling-tx-version-group-id"); + { + SCOPED_TRACE("BlockSaplingRulesRejectOverwinterTx"); + ExpectInvalidBlockFromTx(CTransaction(mtx), 100, "bad-sapling-tx-version-group-id"); + } } From fb22b3bbd8409e44e72f48c297576020b5884b93 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 30 Aug 2018 14:44:50 +0100 Subject: [PATCH 4/7] Revert NU activation heights in reverse order Ensures that global state remains consistent. --- src/gtest/test_checkblock.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index 829e07e7c..726dac324 100644 --- a/src/gtest/test_checkblock.cpp +++ b/src/gtest/test_checkblock.cpp @@ -75,8 +75,8 @@ protected: virtual void TearDown() { // Revert to test default. No-op on mainnet params. - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } // Returns a valid but empty mutable transaction at block height 1. From d7bcbfaee4ae6850f675edc79d15eb79af388864 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 30 Aug 2018 14:47:46 +0100 Subject: [PATCH 5/7] Fix test after refactor to check bacd-cb-height rule on a genesis block --- src/gtest/test_checkblock.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index 726dac324..e2b67cbe7 100644 --- a/src/gtest/test_checkblock.cpp +++ b/src/gtest/test_checkblock.cpp @@ -142,9 +142,9 @@ protected: TEST_F(ContextualCheckBlockTest, BadCoinbaseHeight) { - // Put a zero-height transaction in a block + // Put a transaction in a block with no height in scriptSig CMutableTransaction mtx = GetFirstBlockTransaction(); - mtx.vin[0].scriptSig = CScript() << 0 << OP_0; + mtx.vin[0].scriptSig = CScript() << OP_0; mtx.vout.pop_back(); // remove the FR output CBlock block; From 2962a72e352c3041fe6c66aaa52740dc11c15cfc Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 30 Aug 2018 14:58:19 +0100 Subject: [PATCH 6/7] Rename GetFirstBlockTransaction() to GetFirstBlockCoinbaseTx() --- src/gtest/test_checkblock.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index e2b67cbe7..460135b06 100644 --- a/src/gtest/test_checkblock.cpp +++ b/src/gtest/test_checkblock.cpp @@ -80,7 +80,7 @@ protected: } // Returns a valid but empty mutable transaction at block height 1. - CMutableTransaction GetFirstBlockTransaction() { + CMutableTransaction GetFirstBlockCoinbaseTx() { CMutableTransaction mtx; // No inputs. @@ -143,7 +143,7 @@ protected: TEST_F(ContextualCheckBlockTest, BadCoinbaseHeight) { // Put a transaction in a block with no height in scriptSig - CMutableTransaction mtx = GetFirstBlockTransaction(); + CMutableTransaction mtx = GetFirstBlockCoinbaseTx(); mtx.vin[0].scriptSig = CScript() << OP_0; mtx.vout.pop_back(); // remove the FR output @@ -189,7 +189,7 @@ TEST_F(ContextualCheckBlockTest, BadCoinbaseHeight) { // Test block evaluated under Sprout rules will accept Sprout transactions. // This test assumes that mainnet Overwinter activation is at least height 2. TEST_F(ContextualCheckBlockTest, BlockSproutRulesAcceptSproutTx) { - CMutableTransaction mtx = GetFirstBlockTransaction(); + CMutableTransaction mtx = GetFirstBlockCoinbaseTx(); // Make it a Sprout transaction w/o JoinSplits mtx.fOverwintered = false; @@ -205,7 +205,7 @@ TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesAcceptOverwinterTx) { SelectParams(CBaseChainParams::REGTEST); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); - CMutableTransaction mtx = GetFirstBlockTransaction(); + CMutableTransaction mtx = GetFirstBlockCoinbaseTx(); // Make it an Overwinter transaction mtx.fOverwintered = true; @@ -223,7 +223,7 @@ TEST_F(ContextualCheckBlockTest, BlockSaplingRulesAcceptSaplingTx) { UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 1); - CMutableTransaction mtx = GetFirstBlockTransaction(); + CMutableTransaction mtx = GetFirstBlockCoinbaseTx(); // Make it a Sapling transaction mtx.fOverwintered = true; @@ -243,7 +243,7 @@ TEST_F(ContextualCheckBlockTest, BlockSaplingRulesAcceptSaplingTx) { // transactions. This test assumes that mainnet Overwinter activation is at // least height 2. TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectOtherTx) { - CMutableTransaction mtx = GetFirstBlockTransaction(); + CMutableTransaction mtx = GetFirstBlockCoinbaseTx(); // Make it an Overwinter transaction mtx.fOverwintered = true; @@ -273,7 +273,7 @@ TEST_F(ContextualCheckBlockTest, BlockOverwinterRulesRejectOtherTx) { SelectParams(CBaseChainParams::REGTEST); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); - CMutableTransaction mtx = GetFirstBlockTransaction(); + CMutableTransaction mtx = GetFirstBlockCoinbaseTx(); // Set the version to Sprout+JoinSplit (but nJoinSplit will be 0). mtx.nVersion = 2; @@ -301,7 +301,7 @@ TEST_F(ContextualCheckBlockTest, BlockSaplingRulesRejectOtherTx) { UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 1); - CMutableTransaction mtx = GetFirstBlockTransaction(); + CMutableTransaction mtx = GetFirstBlockCoinbaseTx(); // Set the version to Sprout+JoinSplit (but nJoinSplit will be 0). mtx.nVersion = 2; From 17b6a9d3766cd8ad0179f5a870999a8a7027c108 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 11 Sep 2018 14:49:47 -0700 Subject: [PATCH 7/7] Update comment for test ContextualCheckBlockTest.BlockSproutRulesRejectOtherTx --- src/gtest/test_checkblock.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gtest/test_checkblock.cpp b/src/gtest/test_checkblock.cpp index 460135b06..57fdb16f7 100644 --- a/src/gtest/test_checkblock.cpp +++ b/src/gtest/test_checkblock.cpp @@ -240,8 +240,8 @@ TEST_F(ContextualCheckBlockTest, BlockSaplingRulesAcceptSaplingTx) { // Sapling-Overwinter. // Test that a block evaluated under Sprout rules cannot contain non-Sprout -// transactions. This test assumes that mainnet Overwinter activation is at -// least height 2. +// transactions which require Overwinter to be active. This test assumes that +// mainnet Overwinter activation is at least height 2. TEST_F(ContextualCheckBlockTest, BlockSproutRulesRejectOtherTx) { CMutableTransaction mtx = GetFirstBlockCoinbaseTx();