Merge bitcoin/bitcoin#33474: [29.x] Backports

2d7ebd2d91 doc: update release notes for 29.x (fanquake)
a8bb76b61f test: add more TRUC reorg coverge (Greg Sanders)
666aec7d49 Mempool: Do not enforce TRUC checks on reorg (Greg Sanders)
6f23ead4a2 fuzz: don't bypass_limits for most mempool harnesses (Greg Sanders)
9d9baafc6f doc: rpc: fix case typo in `finalizepsbt` help (final_scriptwitness) (Sebastian Falbesoner)
22ab141243 rpc: fix getblock(header) returns target for tip (Sjors Provoost)
118abf4c30 test: add block 2016 to mock mainnet (Sjors Provoost)

Pull request description:

  Backports:
  * #33446
  * #33484
  * #33504

ACKs for top commit:
  luke-jr:
    ACK 2d7ebd2d91
  dergoegge:
    ACK 2d7ebd2d91
  marcofleon:
    ACK 2d7ebd2d91

Tree-SHA512: 27b852177d8502d6c703cb0eeb1e4df9d651c9c8add5fbf6ae4eeb4b8aefc145471f38f9794c0ed276bf7ebba1844ecbaf5a84cb5913ca7d0a546f5216ea3b2d

doc/release-notes.md

@@ -1,6 +1,6 @@
-Bitcoin Core version 29.2rc1 is now available from:
+Bitcoin Core version 29.2rc2 is now available from:
 
-  <https://bitcoincore.org/bin/bitcoin-core-29.2/test.rc1/>
+  <https://bitcoincore.org/bin/bitcoin-core-29.2/test.rc2/>
 
 This release includes various bug fixes and performance
 improvements, as well as updated translations.
@@ -43,6 +43,14 @@ Notable changes
 - #33296 net: check for empty header before calling FillBlock
 - #33395 net: do not apply whitelist permissions to onion inbounds
 
+### Mempool
+
+- #33504 mempool: Do not enforce TRUC checks on reorg
+
+### RPC
+
+- #33446 rpc: fix getblock(header) returns target for tip
+
 ### CI
 
 - #32999 ci: Use APT_LLVM_V in msan task
@@ -50,6 +58,10 @@ Notable changes
 - #33258 ci: use LLVM 21
 - #33364 ci: always use tag for LLVM checkout
 
+### Doc
+
+- #33484 doc: rpc: fix case typo in `finalizepsbt` help
+
 ### Misc
 
 - #33310 trace: Workaround GCC bug compiling with old systemtap
@@ -67,6 +79,8 @@ Thanks to everyone who directly contributed to this release:
 - Luke Dashjr
 - MarcoFalke
 - Martin Zumsande
+- Sebastian Falbesoner
+- Sjors Provoost
 - Vasil Dimov
 
 As well as to everyone that helped with translations on

src/rpc/blockchain.cpp

@@ -164,7 +164,7 @@ UniValue blockheaderToJSON(const CBlockIndex& tip, const CBlockIndex& blockindex
     result.pushKV("mediantime", blockindex.GetMedianTimePast());
     result.pushKV("nonce", blockindex.nNonce);
     result.pushKV("bits", strprintf("%08x", blockindex.nBits));
-    result.pushKV("target", GetTarget(tip, pow_limit).GetHex());
+    result.pushKV("target", GetTarget(blockindex, pow_limit).GetHex());
     result.pushKV("difficulty", GetDifficulty(blockindex));
     result.pushKV("chainwork", blockindex.nChainWork.GetHex());
     result.pushKV("nTx", blockindex.nTx);

src/rpc/rawtransaction.cpp

@@ -1494,7 +1494,7 @@ static RPCHelpMan finalizepsbt()
     return RPCHelpMan{"finalizepsbt",
                 "Finalize the inputs of a PSBT. If the transaction is fully signed, it will produce a\n"
                 "network serialized transaction which can be broadcast with sendrawtransaction. Otherwise a PSBT will be\n"
-                "created which has the final_scriptSig and final_scriptWitness fields filled for inputs that are complete.\n"
+                "created which has the final_scriptSig and final_scriptwitness fields filled for inputs that are complete.\n"
                 "Implements the Finalizer and Extractor roles.\n",
                 {
                     {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"},

src/test/fuzz/package_eval.cpp

@@ -324,7 +324,7 @@ FUZZ_TARGET(ephemeral_package_eval, .init = initialize_tx_pool)
                                     return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit, /*client_maxfeerate=*/{}));
 
         const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(),
-                                   /*bypass_limits=*/fuzzed_data_provider.ConsumeBool(), /*test_accept=*/!single_submit));
+                                   /*bypass_limits=*/false, /*test_accept=*/!single_submit));
 
         if (!single_submit && result_package.m_state.GetResult() != PackageValidationResult::PCKG_POLICY) {
             // We don't know anything about the validity since transactions were randomly generated, so

src/test/fuzz/tx_pool.cpp

@@ -295,7 +295,6 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
         std::set<CTransactionRef> added;
         auto txr = std::make_shared<TransactionsDelta>(removed, added);
         node.validation_signals->RegisterSharedValidationInterface(txr);
-        const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
 
         // Make sure ProcessNewPackage on one transaction works.
         // The result is not guaranteed to be the same as what is returned by ATMP.
@@ -310,7 +309,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
                    it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
         }
 
-        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
+        const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), /*bypass_limits=*/false, /*test_accept=*/false));
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
         node.validation_signals->SyncWithValidationInterfaceQueue();
         node.validation_signals->UnregisterSharedValidationInterface(txr);
@@ -393,6 +392,9 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
 
     chainstate.SetMempool(&tx_pool);
 
+    // If we ever bypass limits, do not do TRUC invariants checks
+    bool ever_bypassed_limits{false};
+
     LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300)
     {
         const auto mut_tx = ConsumeTransaction(fuzzed_data_provider, txids);
@@ -411,13 +413,17 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
             tx_pool.PrioritiseTransaction(txid.ToUint256(), delta);
         }
 
+        const bool bypass_limits{fuzzed_data_provider.ConsumeBool()};
+        ever_bypassed_limits |= bypass_limits;
+
         const auto tx = MakeTransactionRef(mut_tx);
-        const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
         const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
         if (accepted) {
             txids.push_back(tx->GetHash());
-            CheckMempoolTRUCInvariants(tx_pool);
+            if (!ever_bypassed_limits) {
+                CheckMempoolTRUCInvariants(tx_pool);
+            }
         }
     }
     Finish(fuzzed_data_provider, tx_pool, chainstate);

src/validation.cpp

@@ -1025,26 +1025,28 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     // Even though just checking direct mempool parents for inheritance would be sufficient, we
     // check using the full ancestor set here because it's more convenient to use what we have
     // already calculated.
-    if (const auto err{SingleTRUCChecks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
+    if (!args.m_bypass_limits) {
-        // Single transaction contexts only.
+        if (const auto err{SingleTRUCChecks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
-        if (args.m_allow_sibling_eviction && err->second != nullptr) {
+            // Single transaction contexts only.
-            // We should only be considering where replacement is considered valid as well.
+            if (args.m_allow_sibling_eviction && err->second != nullptr) {
-            Assume(args.m_allow_replacement);
+                // We should only be considering where replacement is considered valid as well.
+                Assume(args.m_allow_replacement);
 
-            // Potential sibling eviction. Add the sibling to our list of mempool conflicts to be
+                // Potential sibling eviction. Add the sibling to our list of mempool conflicts to be
-            // included in RBF checks.
+                // included in RBF checks.
-            ws.m_conflicts.insert(err->second->GetHash());
+                ws.m_conflicts.insert(err->second->GetHash());
-            // Adding the sibling to m_iters_conflicting here means that it doesn't count towards
+                // Adding the sibling to m_iters_conflicting here means that it doesn't count towards
-            // RBF Carve Out above. This is correct, since removing to-be-replaced transactions from
+                // RBF Carve Out above. This is correct, since removing to-be-replaced transactions from
-            // the descendant count is done separately in SingleTRUCChecks for TRUC transactions.
+                // the descendant count is done separately in SingleTRUCChecks for TRUC transactions.
-            ws.m_iters_conflicting.insert(m_pool.GetIter(err->second->GetHash()).value());
+                ws.m_iters_conflicting.insert(m_pool.GetIter(err->second->GetHash()).value());
-            ws.m_sibling_eviction = true;
+                ws.m_sibling_eviction = true;
-            // The sibling will be treated as part of the to-be-replaced set in ReplacementChecks.
+                // The sibling will be treated as part of the to-be-replaced set in ReplacementChecks.
-            // Note that we are not checking whether it opts in to replaceability via BIP125 or TRUC
+                // Note that we are not checking whether it opts in to replaceability via BIP125 or TRUC
-            // (which is normally done in PreChecks). However, the only way a TRUC transaction can
+                // (which is normally done in PreChecks). However, the only way a TRUC transaction can
-            // have a non-TRUC and non-BIP125 descendant is due to a reorg.
+                // have a non-TRUC and non-BIP125 descendant is due to a reorg.
-        } else {
+            } else {
-            return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "TRUC-violation", err->first);
+                return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "TRUC-violation", err->first);
+            }
         }
     }
 

test/functional/data/README.md

@@ -11,9 +11,10 @@ The alternate mainnet chain was generated as follows:
 - restart node with a faketime 2 minutes later
 
 ```sh
-for i in {1..2015}
+for i in {1..2016}
 do
- faketime "`date -d @"$(( 1231006505 + $i * 120 ))"  +'%Y-%m-%d %H:%M:%S'`" \
+ t=$(( 1231006505 + $i * 120 ))
+ faketime "`date -d @$t  +'%Y-%m-%d %H:%M:%S'`" \
  bitcoind -connect=0 -nocheckpoints -stopatheight=$i
 done
 ```
@@ -21,7 +22,9 @@ done
 The CPU miner is kept running as follows:
 
 ```sh
-./minerd --coinbase-addr 1NQpH6Nf8QtR2HphLRcvuVqfhXBXsiWn8r --no-stratum --algo sha256d --no-longpoll --scantime 3 --retry-pause 1
+./minerd -u ... -p ... -o http://127.0.0.1:8332 --no-stratum \
+        --coinbase-addr 1NQpH6Nf8QtR2HphLRcvuVqfhXBXsiWn8r \
+        --algo sha256d --no-longpoll --scantime 3 --retry-pause 1
 ```
 
 The payout address is derived from first BIP32 test vector master key:
@@ -40,3 +43,8 @@ The timestamp was not kept constant because at difficulty 1 it's not sufficient
 to only grind the nonce. Grinding the extra_nonce or version field instead
 would have required additional (stratum) software. It would also make it more
 complicated to reconstruct the blocks in this test.
+
+The `getblocktemplate` RPC code needs to be patched to ignore not being connected
+to any peers, and to ignore the IBD status check.
+
+On macOS use `faketime "@$t"` instead.

test/functional/data/mainnet_alt.json

@@ -2014,7 +2014,8 @@
       1231247971,
       1231248071,
       1231248198,
-      1231248322
+      1231248322,
+      1231248621
     ],
     "nonces": [
       2345621585,
@@ -4031,6 +4032,7 @@
       3658502865,
       2519048297,
       1915965760,
-      1183846025
+      1183846025,
+      2713372123
     ]
 }

test/functional/mempool_truc.py

@@ -164,23 +164,36 @@ class MempoolTRUC(BitcoinTestFramework):
     def test_truc_reorg(self):
         node = self.nodes[0]
         self.log.info("Test that, during a reorg, TRUC rules are not enforced")
-        tx_v2_block = self.wallet.send_self_transfer(from_node=node, version=2)
+        self.check_mempool([])
-        tx_v3_block = self.wallet.send_self_transfer(from_node=node, version=3)
+
-        tx_v3_block2 = self.wallet.send_self_transfer(from_node=node, version=3)
+        # Testing 2<-3 versions allowed
-        self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"]])
+        tx_v2_block = self.wallet.create_self_transfer(version=2)
+
+        # Testing 3<-2 versions allowed
+        tx_v3_block = self.wallet.create_self_transfer(version=3)
+
+        # Testing overly-large child size
+        tx_v3_block2 = self.wallet.create_self_transfer(version=3)
+
+        # Also create a linear chain of 3 TRUC transactions that will be directly mined, followed by one v2 in-mempool after block is made
+        tx_chain_1 = self.wallet.create_self_transfer(version=3)
+        tx_chain_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_chain_1["new_utxo"], version=3)
+        tx_chain_3 = self.wallet.create_self_transfer(utxo_to_spend=tx_chain_2["new_utxo"], version=3)
+
+        tx_to_mine = [tx_v3_block["hex"], tx_v2_block["hex"], tx_v3_block2["hex"], tx_chain_1["hex"], tx_chain_2["hex"], tx_chain_3["hex"]]
+        block = self.generateblock(node, output="raw(42)", transactions=tx_to_mine)
 
-        block = self.generate(node, 1)
         self.check_mempool([])
         tx_v2_from_v3 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block["new_utxo"], version=2)
         tx_v3_from_v2 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v2_block["new_utxo"], version=3)
         tx_v3_child_large = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block2["new_utxo"], target_vsize=1250, version=3)
         assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], TRUC_CHILD_MAX_VSIZE)
-        self.check_mempool([tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]])
+        tx_chain_4 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_chain_3["new_utxo"], version=2)
-        node.invalidateblock(block[0])
+        self.check_mempool([tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"], tx_chain_4["txid"]])
-        self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"], tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]])
-        # This is needed because generate() will create the exact same block again.
-        node.reconsiderblock(block[0])
 
+        # Reorg should have all block transactions re-accepted, ignoring TRUC enforcement
+        node.invalidateblock(block["hash"])
+        self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"], tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"], tx_chain_1["txid"], tx_chain_2["txid"], tx_chain_3["txid"], tx_chain_4["txid"]])
 
     @cleanup(extra_args=["-limitdescendantsize=10", "-datacarriersize=40000"])
     def test_nondefault_package_limits(self):

test/functional/mining_mainnet.py

@@ -54,15 +54,15 @@ class MiningMainnetTest(BitcoinTestFramework):
 
         self.add_wallet_options(parser)
 
-    def mine(self, height, prev_hash, blocks, node, fees=0):
+    def mine(self, height, prev_hash, blocks, node):
         self.log.debug(f"height={height}")
         block = CBlock()
         block.nVersion = 0x20000000
         block.hashPrevBlock = int(prev_hash, 16)
         block.nTime = blocks['timestamps'][height - 1]
-        block.nBits = DIFF_1_N_BITS
+        block.nBits = DIFF_1_N_BITS if height < 2016 else DIFF_4_N_BITS
         block.nNonce = blocks['nonces'][height - 1]
-        block.vtx = [create_coinbase(height=height, script_pubkey=bytes.fromhex(COINBASE_SCRIPT_PUBKEY), retarget_period=2016)]
+        block.vtx = [create_coinbase(height=height, script_pubkey=bytes.fromhex(COINBASE_SCRIPT_PUBKEY), halving_period=210000)]
         block.hashMerkleRoot = block.calc_merkle_root()
         block.rehash()
         block_hex = block.serialize(with_witness=False).hex()
@@ -81,12 +81,15 @@ class MiningMainnetTest(BitcoinTestFramework):
         self.log.info("Load alternative mainnet blocks")
         path = os.path.join(os.path.dirname(os.path.realpath(__file__)), self.options.datafile)
         prev_hash = node.getbestblockhash()
+        blocks = None
         with open(path, encoding='utf-8') as f:
             blocks = json.load(f)
             n_blocks = len(blocks['timestamps'])
-            assert_equal(n_blocks, 2015)
+            assert_equal(n_blocks, 2016)
-            for i in range(2015):
+
-                prev_hash = self.mine(i + 1, prev_hash, blocks, node)
+        # Mine up to the last block of the first retarget period
+        for i in range(2015):
+            prev_hash = self.mine(i + 1, prev_hash, blocks, node)
 
         assert_equal(node.getblockcount(), 2015)
 
@@ -101,5 +104,21 @@ class MiningMainnetTest(BitcoinTestFramework):
         assert_equal(mining_info['next']['bits'], nbits_str(DIFF_4_N_BITS))
         assert_equal(mining_info['next']['target'], target_str(DIFF_4_TARGET))
 
+        # Mine first block of the second retarget period
+        height = 2016
+        prev_hash = self.mine(height, prev_hash, blocks, node)
+        assert_equal(node.getblockcount(), height)
+
+        mining_info = node.getmininginfo()
+        assert_equal(mining_info['difficulty'], 4)
+
+        self.log.info("getblock RPC should show historical target")
+        block_info = node.getblock(node.getblockhash(1))
+
+        assert_equal(block_info['difficulty'], 1)
+        assert_equal(block_info['bits'], nbits_str(DIFF_1_N_BITS))
+        assert_equal(block_info['target'], target_str(DIFF_1_TARGET))
+
+
 if __name__ == '__main__':
     MiningMainnetTest(__file__).main()

test/functional/test_framework/blocktools.py

@@ -143,7 +143,7 @@ def script_BIP34_coinbase_height(height):
     return CScript([CScriptNum(height)])
 
 
-def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_script=None, fees=0, nValue=50, retarget_period=REGTEST_RETARGET_PERIOD):
+def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_script=None, fees=0, nValue=50, halving_period=REGTEST_RETARGET_PERIOD):
     """Create a coinbase transaction.
 
     If pubkey is passed in, the coinbase output will be a P2PK output;
@@ -156,7 +156,7 @@ def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_scr
     coinbaseoutput = CTxOut()
     coinbaseoutput.nValue = nValue * COIN
     if nValue == 50:
-        halvings = int(height / retarget_period)
+        halvings = int(height / halving_period)
         coinbaseoutput.nValue >>= halvings
         coinbaseoutput.nValue += fees
     if pubkey is not None: