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

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2025-07-test-isfinaltx-nlocktime-0 changing 1 files +4 −0
  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

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  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
  19. sedited referenced this in commit 02ded863ba on Jul 28, 2025
  20. sedited referenced this in commit 52d7f32bd6 on Jul 28, 2025
  21. alexanderwiederin referenced this in commit 28fe919bf7 on Aug 6, 2025
  22. sedited referenced this in commit b98d982d79 on Aug 7, 2025
  23. alexanderwiederin referenced this in commit 4152176d02 on Aug 8, 2025
  24. alexanderwiederin referenced this in commit 9ef94c31db on Aug 8, 2025
  25. stringintech referenced this in commit 71275a1b5e on Aug 17, 2025
  26. yuvicc referenced this in commit 22f55cf11d on Aug 26, 2025
  27. bug-castercv502 referenced this in commit 4aa5572aea on Sep 28, 2025
  28. stickies-v referenced this in commit a19c56cd7c on Nov 4, 2025
  29. Kino1994 referenced this in commit 06f4c68adb on Jun 28, 2026
  30. BigcoinBGC referenced this in commit 552e7a4a10 on Jun 30, 2026
  31. bitcoin locked this on Jul 30, 2026
Contributors
brunoergDrahtBotdarosiormaflckoenirox001achow101
Labels
Tests
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-08-11 10:51 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me