Merge #21592: test: Remove option to make TestChain100Setup non-deterministic

fa6183d test: Remove option to make TestChain100Setup non-deterministic (MarcoFalke)
fa732bc test: Use compressed keys in TestChain100Setup (MarcoFalke)

Pull request description:

  Seems odd to have an option for non-deterministic tests
  when the goal should be for all tests to be deterministic.

ACKs for top commit:
  jamesob:
    ACK bitcoin/bitcoin@fa6183d
  practicalswift:
    cr ACK fa6183d: patch looks deterministic!

Tree-SHA512: 6897a9f36e0dfb7d63b25dd6984414b3ee8a62458ad232cb21ed5077184fdb0bc626996e4ac84ef0bdd452b9f17c54aac75a71575b8e723b84cac07c9f9d5611

Author: MarcoFalke

src/chainparams.cpp

@@ -441,7 +441,7 @@ class CRegTestParams : public CChainParams {
         m_assumeutxo_data = MapAssumeutxo{
             {
                 110,
-                {uint256S("0x76fd7334ac7c1baf57ddc0c626f073a655a35d98a4258cd1382c8cc2b8392e10"), 110},
+                {uint256S("0x1ebbf5850204c0bdb15bf030f47c7fe91d45c44c712697e4509ba67adb01c618"), 110},
             },
             {
                 210,

src/test/util/setup_common.cpp

@@ -202,43 +202,31 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
     }
 }
 
-TestChain100Setup::TestChain100Setup(bool deterministic)
+TestChain100Setup::TestChain100Setup()
 {
-    m_deterministic = deterministic;
-
-    if (m_deterministic) {
-        SetMockTime(1598887952);
-        constexpr std::array<unsigned char, 32> vchKey = {
-            {
-                0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1
-            }
-        };
-        coinbaseKey.Set(vchKey.begin(), vchKey.end(), false);
-    } else {
-        coinbaseKey.MakeNewKey(true);
-    }
+    SetMockTime(1598887952);
+    constexpr std::array<unsigned char, 32> vchKey = {
+        {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}};
+    coinbaseKey.Set(vchKey.begin(), vchKey.end(), true);
 
     // Generate a 100-block chain:
     this->mineBlocks(COINBASE_MATURITY);
 
-    if (m_deterministic) {
+    {
         LOCK(::cs_main);
         assert(
             m_node.chainman->ActiveChain().Tip()->GetBlockHash().ToString() ==
-            "49c95db1e470fed04496d801c9d8fbb78155d2c7f855232c918823d2c17d0cf6");
+            "571d80a9967ae599cec0448b0b0ba1cfb606f584d8069bd7166b86854ba7a191");
     }
 }
 
 void TestChain100Setup::mineBlocks(int num_blocks)
 {
     CScript scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
-    for (int i = 0; i < num_blocks; i++)
-    {
+    for (int i = 0; i < num_blocks; i++) {
         std::vector<CMutableTransaction> noTxns;
         CBlock b = CreateAndProcessBlock(noTxns, scriptPubKey);
-        if (m_deterministic) {
-            SetMockTime(GetTime() + 1);
-        }
+        SetMockTime(GetTime() + 1);
         m_coinbase_txns.push_back(b.vtx[0]);
     }
 }
@@ -315,9 +303,7 @@ CMutableTransaction TestChain100Setup::CreateValidMempoolTransaction(CTransactio
 TestChain100Setup::~TestChain100Setup()
 {
     gArgs.ForceSetArg("-segwitheight", "0");
-    if (m_deterministic) {
-        SetMockTime(0);
-    }
+    SetMockTime(0);
 }
 
 CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) const

src/test/util/setup_common.h

@@ -112,7 +112,7 @@ class CScript;
  * Testing fixture that pre-creates a 100-block REGTEST-mode block chain
  */
 struct TestChain100Setup : public RegTestingSetup {
-    TestChain100Setup(bool deterministic = false);
+    TestChain100Setup();
 
     /**
      * Create a new block with just given transactions, coinbase paying to
@@ -143,16 +143,10 @@ struct TestChain100Setup : public RegTestingSetup {
 
     ~TestChain100Setup();
 
-    bool m_deterministic;
     std::vector<CTransactionRef> m_coinbase_txns; // For convenience, coinbase transactions
     CKey coinbaseKey; // private/public key needed to spend coinbase transactions
 };
 
-
-struct TestChain100DeterministicSetup : public TestChain100Setup {
-    TestChain100DeterministicSetup() : TestChain100Setup(true) { }
-};
-
 /**
  * Make a test setup that has disk access to the debug.log file disabled. Can
  * be used in "hot loops", for example fuzzing or benchmarking.

src/test/validation_chainstatemanager_tests.cpp

@@ -200,7 +200,7 @@ CreateAndActivateUTXOSnapshot(NodeContext& node, const fs::path root, F malleati
 }
 
 //! Test basic snapshot activation.
-BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100DeterministicSetup)
+BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup)
 {
     ChainstateManager& chainman = *Assert(m_node.chainman);
 

src/test/validation_tests.cpp

@@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(test_assumeutxo)
     }
 
     const auto out110 = *ExpectedAssumeutxo(110, *params);
-    BOOST_CHECK_EQUAL(out110.hash_serialized, uint256S("76fd7334ac7c1baf57ddc0c626f073a655a35d98a4258cd1382c8cc2b8392e10"));
+    BOOST_CHECK_EQUAL(out110.hash_serialized, uint256S("1ebbf5850204c0bdb15bf030f47c7fe91d45c44c712697e4509ba67adb01c618"));
     BOOST_CHECK_EQUAL(out110.nChainTx, (unsigned int)110);
 
     const auto out210 = *ExpectedAssumeutxo(210, *params);