tests: Add unix timestamp tests for OP_CLTV and max mediantimepast tests #32229

pull Christewart wants to merge 1 commits into bitcoin:master from Christewart:2025-04-06-locktime-tests changing 2 files +88 −7
  1. Christewart commented at 4:44 PM on April 7, 2025: contributor

    This PR adds a functional (p2p/block-validation) test for OP_CHECKLOCKTIMEVERIFY (BIP65) using unix-timestamp locktimes in test/functional/feature_cltv.py. It also adds tests around the maximum possible median-time-past (MTP) value, UINT32_MAX, and how that interacts with transaction finality.

    The unix-timestamp portion of BIP65 already has unit-level coverage in transaction_tests.cpp (via tx_valid.json/tx_invalid.json), but those tests call VerifyScript() directly and never exercise median-time-past (MTP) or block connection. MTP-vs-nLockTime behavior itself is covered separately in miner_tests.cpp (lines 525-534) and feature_csv_activation.py (lines 340-355), but neither exercises OP_CLTV specifically. This test closes that gap: it builds and submits full blocks over p2p and checks that a block containing an OP_CLTV-locked output is only accepted once BIP113 MTP actually satisfies the requested locktime.

    This PR also adds a BIP113 test to feature_csv_activation.py showing that MTP can never advance past UINT32_MAX - 1, so an output with a plain nLockTime of UINT32_MAX can never be confirmed. This is a general BIP113 property independent of OP_CLTV.

    Thanks to @mabu44 and @sedited for the review so far.

  2. DrahtBot commented at 4:44 PM on April 7, 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/32229.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK sedited

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  3. DrahtBot added the label Tests on Apr 7, 2025
  4. Christewart force-pushed on Apr 7, 2025
  5. DrahtBot added the label CI failed on Apr 7, 2025
  6. Christewart force-pushed on Apr 8, 2025
  7. DrahtBot removed the label CI failed on Apr 8, 2025
  8. in test/functional/feature_cltv.py:None in 69b6bb9438 outdated 
     205 | +        tip = block.sha256
 206 | +
 207 | +        current_time = int(time.time())
 208 | +        locktime = current_time + (60 * 60) # 1 hour
 209 | +
 210 | +        self.log.info("Test that one block mined with correct wall clock time does not satisfy meadian-time-past")
    mabu44 commented at 8:21 AM on April 8, 2025:

    nit: typo "meadian"

  9. in test/functional/feature_cltv.py:None in 69b6bb9438 outdated 
     225 | +            peer.send_and_ping(msg_block(b))
 226 | +            tip = b.sha256
 227 | +
 228 | +        # now our median-past-time should allow for a transaction with
 229 | +        # a locktime of +1 hour to be confirmed
 230 | +        spendtx = wallet.create_self_transfer()['tx']
    mabu44 commented at 8:27 AM on April 8, 2025:

    Do we need to create a new time-locked transaction? Cannot we use the one that is already in spendtx?

    Christewart commented at 4:26 PM on April 8, 2025:

    I believe so because the previous one was mined in a valid block here: https://github.com/Christewart/bitcoin/blob/69b6bb9438095f84ce90aecfc4e76b0ff21cc2b8/test/functional/feature_cltv.py#L213

    mabu44 commented at 11:44 AM on April 10, 2025:

    This tx was mined in line 195, not 213. I would move the creation of the transaction from line 230 to line 209. Otherwise, the check at line 218 could succeed not for the timelock (that is what we expect) but because of the duplicated transaction.

  10. in test/functional/feature_cltv.py:None in 69b6bb9438 outdated 
     242 | +        UINT32_MAX = 2**32 - 1
 243 | +        self.nodes[0].setmocktime(UINT32_MAX)
 244 | +        # need to re-add connection as setting mocktime times out the p2p connection
 245 | +        peer = self.nodes[0].add_p2p_connection(P2PInterface())
 246 | +        tip = block2.sha256
 247 | +        # 6 blocks are allowed to have UINT32_MAX block time
    mabu44 commented at 8:43 AM on April 8, 2025:

    Here you are testing what will happen in the year 2106 if no changes are deployed in the meantime. Right?

  11. mabu44 commented at 12:01 PM on April 8, 2025: none

    The Unix-timestamp portion of BIP65 is currently tested in the c++ unit tests in "transactions_tests.cpp". Test data includes transactions that have OP_CLTV with Unix timestamps (the files are /src/test/data/tx_valid.json and tx_invalid.json). Does your test cover scenarios that are not covered in the existing tests?

  12. Christewart commented at 4:23 PM on April 8, 2025: contributor

    Hi @mabu44 . Thanks for taking a look at this.

    Do your test cover scenarios that are not covered in the existing tests?

    Yes I believe so. I took a look at the values in tx_valid.json and tx_invalid.json. Here is a table

    OP_CLTV parameter Transaction locktime field Valid
    0 0 true
    499999999 499999999 true
    0 499999999 true
    500000000 500000000 true
    4294967295 4294967295 true
    500000000 4294967295 true
    0 0 true
    -11 500000000 true
    -19 4294967294 true
    0 0 true
    1 0 false
    499999999 499999998 false
    500000001 500000000 false
    4294967295 4294967294 false
    -1 0 false
    -1 500000000 false
    0 0 false
    0 0 false
    0 500000000 false
    499999999 500000000 false
    500000000 0 false
    500000000 499999999 false
    4294967296 4294967295 false
    2147483648 2147483647 false
    0 0 false

    This test also directly tests the median-time-past logic. This is not tested in tx_{valid,invalid}.json as we just directly call VerifyScript() for individual transactions in the test files.

    Interestingly, i think there may be one test case overlap with tx_valid.json. While this transaction could be valid with relay policy I don't believe a transaction with these locktimes would be allowed to be included in a block? This is because median-time-past logic for a block with this transaction could be not be satisfied as per my last test case here.

    | 4294967295 | 4294967295 | true |

    Some more eyes on this could be good as I may be making a mistake.

  13. Christewart force-pushed on Apr 8, 2025
  14. in test/functional/feature_cltv.py:None in 01d2068fb6 outdated 
     227 | +
 228 | +        # now our median-past-time should allow for a transaction with
 229 | +        # a locktime of +1 hour to be confirmed
 230 | +        spendtx = wallet.create_self_transfer()['tx']
 231 | +        cltv_validate(spendtx,locktime)
 232 | +        block2.vtx.append(spendtx)
    mabu44 commented at 11:46 AM on April 10, 2025:

    This line can be removed

  15. in test/functional/feature_cltv.py:None in 01d2068fb6 outdated 
     242 | +        UINT32_MAX = 2**32 - 1
 243 | +        self.nodes[0].setmocktime(UINT32_MAX)
 244 | +        # need to re-add connection as setting mocktime times out the p2p connection
 245 | +        peer = self.nodes[0].add_p2p_connection(P2PInterface())
 246 | +        tip = block2.sha256
 247 | +        # 6 blocks are allowed to have UINT32_MAX block time
    mabu44 commented at 11:47 AM on April 10, 2025:

    This comment was not updated to reflect what the code is doing (same for the comments at line 256 and 258).

  16. mabu44 commented at 12:03 PM on April 10, 2025: none

    So this is an integration test of:

    with some more edge cases relative to the year 2106. Right?

    Interestingly, i think there may be one test case overlap with tx_valid.json. While this transaction could be valid with relay policy I don't believe a transaction with these locktimes would be allowed to be included in a block? This is because median-time-past logic for a block with this transaction could be not be satisfied as per my last test case here.

    | 4294967295 | 4294967295 | true |

    I think that such a transaction will become relay-valid in 2106, but, unless consensus changes, it will never be mined because no blocks will be produced to mine it.

  17. Christewart commented at 5:11 PM on April 10, 2025: contributor

    Thank you again for the careful review. I've integrated your code review in 024c3bf

    So this is an integration test of:

    * _Transaction validity based on input script and transaction locktime_: currently tested in [transactions_tests.cpp](https://github.com/bitcoin/bitcoin/blob/master/src/test/transaction_tests.cpp);

* _Transaction validity based on median block time_: currently tested in [miner_tests.cpp (lines 525-534)](https://github.com/bitcoin/bitcoin/blob/b8cefeb221490868d62b7a0695f8b5ea392d3654/src/test/miner_tests.cpp#L525-L534) and [feature_csv_activation.py (lines 340-355)](https://github.com/bitcoin/bitcoin/blob/b8cefeb221490868d62b7a0695f8b5ea392d3654/test/functional/feature_csv_activation.py#L340-L355)

    with some more edge cases relative to the year 2106. Right?

    Yes.

    Interestingly, i think there may be one test case overlap with tx_valid.json. While this transaction could be valid with relay policy I don't believe a transaction with these locktimes would be allowed to be included in a block? This is because median-time-past logic for a block with this transaction could be not be satisfied as per my last test case here.

    | 4294967295 | 4294967295 | true |

    I think that such a transaction will become relay-valid in 2106, but, unless consensus changes, it will never be mined because no blocks will be produced to mine it.

    That is my understanding as well :+1:

  18. DrahtBot added the label CI failed on Apr 29, 2025
  19. DrahtBot removed the label CI failed on May 2, 2025
  20. DrahtBot added the label CI failed on Jun 14, 2025
  21. DrahtBot removed the label CI failed on Jun 16, 2025
  22. DrahtBot added the label CI failed on Jul 23, 2025
  23. in test/functional/feature_cltv.py:None in 024c3bf3c7 outdated 
     200 | @@ -199,6 +201,65 @@ def run_test(self):
 201 |          self.test_cltv_info(is_active=True)  # Active as of current tip
 202 |          assert_equal(int(self.nodes[0].getbestblockhash(), 16), block.sha256)
 203 |  
 204 | +        # test wall clock time
 205 | +        tip = block.sha256
    DrahtBot commented at 8:45 AM on July 23, 2025: 
                                   AttributeError: 'CBlock' object has no attribute 'sha256'
  24. Christewart force-pushed on Aug 2, 2025
  25. Christewart requested review from mabu44 on Aug 2, 2025
  26. DrahtBot removed the label CI failed on Aug 2, 2025
  27. fanquake commented at 11:02 AM on December 3, 2025: member

    cc @petertodd

  28. DrahtBot commented at 2:06 AM on January 29, 2026: contributor

    <!--8ac04cdde196e94527acabf64b896448-->

    There hasn't been much activity lately. What is the status here?

    Finding reviewers may take time. However, if the patch is no longer relevant, please close this pull request. If the author lost interest or time to work on this, please close it and mark it with one of the labels 'Up for grabs' or 'Insufficient Review', so that it can be picked up in the future.

  29. sedited commented at 1:13 PM on March 3, 2026: contributor

    If I understand this change correctly, this adds test cases for the finality of a block's transactions and how they might interplay with CLTV. Can you re-formulate the PR description to make it clear what this actually tests? Contrary to your current description, it looks like we are already testing unix timestamp values in the locktime extensively. Are the finality tests better suited for feature_csv_activation?

  30. fanquake commented at 2:56 PM on April 30, 2026: member

    @Christewart any chance you want to follow up to the above?

  31. bitcoin deleted a comment on Apr 30, 2026
  32. Christewart commented at 6:42 PM on April 30, 2026: contributor

    Hi @sedited

    it looks like we are already testing unix timestamp values in the locktime extensively.

    feature_csv_activation.py tests relative lock times and the OP_CHECKSEQUENCEVERIFY opcode. This test case is testing OP_CHECKLOCKTIMEVERIFY and absolute wall clock locktimes ("unix timestamps"). Different fields on the transaction are used for verification (sequence vs locktime).

    Are the finality tests better suited for feature_csv_activation

    In my opinion no so since this is testing absolute wall clock locktimes via a different opcode - OP_CLTV.

    The real interesting part to me is the setting the unix timestamp of the node to UINT32_MAX, and then attempting to spend OP_CLTV locked utxos when we have reached the UINT32_MAX timestamp in the locktime field.

    Since we cannot satisfy the MTP requirement of BIP113 (i.e. we cannot create a block with timestamp UINT32_MAX+1 so we are following BIP113 and monotonically increase the timestamp on the block) - the utxo can never be spent.

    From a quick glance through feature_csv_activation.py I don't see any references to 2**32 or UINT32_MAX - so I doubt this edge case is tested there - although if you could link to an exact line that it was tested on that would be helpful.

    In general, I think @mabu44 's comment was good:

    #32229#pullrequestreview-2756380784

    EDIT:

    Looking closer at feature_csv_activation.py it does look like it tests BIP113 logic (its unfortunate the file is named after the opcode and is testing more than the opcode). The logic testing UINT32_MAX could be moved over to that file, however the OP_CLTV test logic should stay here and I still believe there is no integration tests for OP_CLTV and unix timestamps - please link if you find them

  33. sedited commented at 11:18 AM on May 22, 2026: contributor

    The logic testing UINT32_MAX could be moved over to that file,

    Yes, sorry for not being clearer on this before, but that is what I was suggesting.

    however the OP_CLTV test logic should stay here and I still believe there is no integration tests for OP_CLTV and unix timestamps - please link if you find them.

    Thanks for linking mabu44's comment again, I was referring to the same tests too. Maybe mention them in the PR descritpion?

    The real interesting part to me is the setting the unix timestamp of the node to UINT32_MAX, and then attempting to spend OP_CLTV locked utxos when we have reached the UINT32_MAX timestamp in the locktime field.

    I guess once that MTP is reached, we'll have to change some part of the calculation anyway, but I'm fine with having a test for documenting this.

    Concept ACK

  34. test: add functional coverage for OP_CLTV unix-timestamp locktimes and BIP113 interaction
    Add a functional (p2p/block-validation) test for OP_CHECKLOCKTIMEVERIFY
using unix-timestamp locktimes, including its interplay with BIP113
median-time-past (MTP) rules: a block containing an OP_CLTV-locked
output is only accepted once MTP satisfies the requested locktime.

Also add a BIP113 test to feature_csv_activation.py showing that MTP
can never advance past UINT32_MAX - 1, so an output with a plain
nLockTime of UINT32_MAX can never be confirmed. This is a general
BIP113 property independent of OP_CLTV, so it lives alongside the
other BIP113 tests rather than in feature_cltv.py.
    533d0c6083
  35. Christewart force-pushed on Jul 5, 2026
  36. Christewart commented at 7:17 PM on July 5, 2026: contributor

    Hi @sedited , I pushed a new commit: 533d0c608312beaeb47bb5373169a17f9c756814

    Here are the things that i've changed since you've last reviewed

    • Rewrote the PR description to clarify what this actually tests and to credit the existing coverage you and @mabu44 pointed to (see above).
    • Split the UINT32_MAX / median-time-past edge case out of feature_cltv.py and into feature_csv_activation.py, since that behavior is a general BIP113 property and not specific to OP_CLTV (per our earlier discussion).
    • Rebased onto latest master.
    • The rebase pulled in #35089 (create_block's height= kwarg replacing the old create_coinbase(...) calling convention), which broke a few call sites this PR touches. Fixed those up to match the new convention.

    Let me know if this addresses your concerns or if there's anything else you'd like changed.

  37. Christewart renamed this:
    tests: Add unix timestamp tests for OP_CLTV
    tests: Add unix timestamp tests for OP_CLTV and max mediantimepast tests
    on Jul 5, 2026
Contributors
ChristewartDrahtBotmabu44fanquakesedited


mabu44

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-07-09 06:47 UTC