test: check tx is final when there is no locktime #33030

1. 
   brunoerg commented at 6:41 pm on July 21, 2025: contributor

   According to https://corecheck.dev/mutation/src/consensus/tx_verify.cpp, there is no proper test for the tx.nLockTime == 0 check in the IsFinalTx function, which is understandable, since this check will only be useful for a specific case where the nBlockHeight (block height) is zero. Otherwise, the following check if ((int64_t)tx.nLockTime < ((int64_t)tx.nLockTime < LOCKTIME_THRESHOLD ? (int64_t)nBlockHeight : nBlockTime)) would catch any of it. This PR adds a test case for it.
2. DrahtBot added the label Tests on Jul 21, 2025
3. 
   DrahtBot commented at 6:41 pm on July 21, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33030.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko, darosior, enirox001, achow101

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. darosior approved
5. 
   darosior commented at 6:55 pm on July 21, 2025: member

   Makes sense. utACK 7180b82420c0f303140a93ca635f5ac806bea77d. Could add a comment explaining this to the test as it may not be immediately obvious to a reader.
6. test: IsFinalTx returns true when there is no locktime 065e42976a
7. brunoerg force-pushed on Jul 22, 2025
8. 
   brunoerg commented at 1:04 pm on July 22, 2025: contributor

   Makes sense. utACK 7180b82. Could add a comment explaining this to the test as it may not be immediately obvious to a reader.

   Thank you, I just added a comment for this.

9. 
   maflcko commented at 1:17 pm on July 22, 2025: member

   lgtm ACK 065e42976a70738770af256da810ddc1316a4496
10. DrahtBot requested review from darosior on Jul 22, 2025
11. in src/test/miner_tests.cpp:526 in 065e42976a

    522@@ -523,6 +523,10 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
    523     BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass
    524     BOOST_CHECK(IsFinalTx(CTransaction(tx), m_node.chainman->ActiveChain().Tip()->nHeight + 2, m_node.chainman->ActiveChain().Tip()->GetMedianTimePast())); // Locktime passes on 2nd block
    525 
    526+    // ensure tx is final for a specific case where there is no locktime and block height is zero
    

    ---

    ---

    

    darosior commented at 1:22 pm on July 22, 2025:

    nit, don’t retouch this for me, but my point with the comment wasn’t to describe the line below (anyone can just read block height is 0 and locktime is 0) but to describe why them both being 0 is a special case worth exercising, as you did in OP.
12. darosior approved
13. 
    darosior commented at 1:22 pm on July 22, 2025: member

    utACK 065e42976a70738770af256da810ddc1316a4496
14. 
    enirox001 commented at 3:36 pm on July 22, 2025: contributor

    lgtm

    ACK 065e429: Valuable test case that explicitly demonstrates IsFinalTx behavior when nLockTime is 0

15. 
    achow101 commented at 5:49 pm on July 22, 2025: member

    ACK 065e42976a70738770af256da810ddc1316a4496
16. achow101 merged this on Jul 22, 2025
17. achow101 closed this on Jul 22, 2025

    ---

18. brunoerg deleted the branch on Jul 22, 2025

Contributors
brunoerg DrahtBot darosior maflcko enirox001 achow101

Labels
Tests