Enforce BIP 68 from genesis #24567

Now that BIP 68 is active, it makes sense to enforce its rules on all blocks, even historic ones, regardless of the deployment status.

Benefits:

• BIP 68 rules are applied for all blocks (even blocks not yet created). This may benefit static analysis, code review, and development of new script features that build on BIP 68.
• Any future bugs introduced in the deployment code won't have any effect on the BIP 68 rules, as they are independent of deployment.
• Enforcing the BIP 68 rules regardless of the deployment status makes testing easier because invalid blocks after activation are also invalid before activation. So there is no need to differentiate the two cases.
• It gives belt-and-suspenders protection against a practically expensive and theoretically impossible IBD reorg attack where the node is eclipsed. nMinimumChainWork makes this attack practically expensive. Also, this attack is currently theoretically impossible because no vector is known to inject a consensus-invalid transaction into the mempool (or even have it mined).

For reference, previously something similar was done for the script verification flags P2SH and WITNESS in commit 0a8b7b4b33c9d78574627fc606267e2d8955cd1c. (And is being done for TAPROOT in #23536).

Considerations

Obviously this change can lead to consensus splits on the network in light of massive reorgs. Currently the last block before BIP 68 activation, that is the last block without the BIP 68 rules applied, is buried by a few months of POW. BIP90 considerations apply when looking at reorgs this large.

Please review #24565 first, while this stays in draft.

Doesn't seem to save much code, and requires revalidating years worth of blocks to ensure it doesn't break consensus. Not really seeing the point of this? Concept -1 from me.

Doesn't seem to save much code

Apologies, the motivation isn't to save code. I've added some text to OP.

Unlike #23536 I don't think this has much (any?) possible effect on wallet behaviour; bip68 only allows time locks up to ~15 months, and bip68 was ~68 months ago (slightly more than "a few months of POW"...), and unlike with taproot/segwit/p2sh any transaction that is invalid right now due to bip68, will eventually become valid.

Also, any static analysis tool that can't cope with conditional consensus logic just doesn't seem suitable for use on the bitcoin codebase in the first place to me.

So still -1 from me; these sorts of unnecessary changes around consensus code seem far more likely to introduce bugs (as #24421 has) than prevent them.

If a BIP 68 violating tx makes it into the mempool in current master (or a previous release), getblocktemplate will refuse to create a block on the current main chain tip. This pull doesn't change that. It enforces BIP 68 rules on all blocks (past and future ones) and block templates. I think this is a motivation for the changes here, since a block template that includes a BIP 68 violation on current master will happily go through for blocks that will never end up in the main chain (built on a stale tip, see the unit test) and it will fail for blocks that are built on the current tip.

If you're relying on someone giving you a bip68-invalid tx to avoid building blocks with a tip that's over 5 years out of date, I think you've already lost...

So still -1 from me; these sorts of unnecessary changes around consensus code seem far more likely to introduce bugs (as #24421 has) than prevent them.

24421 doesn't change consensus code, so I don't think it's a relevant comparison here. The bug introduced in that PR was a functional test simulating an impossible scenario.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #25311 (remove CBlockIndex copy construction by jamesob)
• #25073 (test: Cleanup miner_tests by MarcoFalke)
• #23897 (refactor: Move calculation logic out from CheckSequenceLocksAtTip() by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

<!--cf906140f33d8803c4a75a2196329ecb-->

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

<!--13523179cfe9479db18ec6c5d236f789-->

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

• Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
• Is it no longer relevant? ➡️ Please close.
• Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

I'll pick this up later