miner: always treat SegWit as active #31625

1. 
   Sjors commented at 11:16 am on January 9, 2025: member

   The getblocktemplate RPC would check if SegWit has activated yet at the tip. This has been the case for more than seven years.
2. miner: always treat SegWit as active

   The getblocktemplate RPC would check if SegWit has activated yet
   at the tip. This has been the case for more than seven years.

   30b2dc559f
3. 
   DrahtBot commented at 11:16 am on January 9, 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/31625.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31117 (miner: Reorg Testnet4 minimum difficulty blocks by fjahr)
   • #26201 (Remove Taproot activation height by Sjors)

   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.

4. DrahtBot added the label Mining on Jan 9, 2025
5. 
   maflcko commented at 11:24 am on January 9, 2025: member

   Not sure about this. If someone goes back in the main chain to an earlier block to mine on top of it for testing (or fun), this could lead to the mined block being rejected from the own node. Conceptually, there is an argument to be self-consistent, but I am not sure how to weight that against removing the lines of code.
6. 
   Sjors commented at 12:01 pm on January 9, 2025: member

   the mined block being rejected from the own node

   Currently getblocktemplate has a guard on mainnet against miner.isInitialBlockDownload(), which among other things checks MinimumChainWork. So without patching the code you can’t generate a pre-segwit block using getblocktemplate.

   But if you patched that check away, why would a SegWit block be rejected?

   You’d also need a (bunch of) ASIC(s) for such an experiment.

7. 
   maflcko commented at 12:03 pm on January 9, 2025: member

   But if you patched that check away, why would a SegWit block be rejected?

   Witness data is not allowed per consensus rules before segwit activation.

8. 
   Sjors commented at 12:28 pm on January 9, 2025: member

   Ok, so someone could roll back their node to before SegWit activation, patch getblocktemplate to skip the IsInitialBlockDownload() check (or remove the MinimumChainWork and max_tip_age checks from from the latter).

   If they then call getblocktemplate and if they have any SegWit transactions in their mempool (I didn’t check if that’s actually possible), it would generate a template with SegWit transactions and a commitment. Since it verifies the template at the end, the RPC call would fail with unexpected-witness.

   They could get around that by changing consensus.SegwitHeight to 0, if their goal is to write an alternative history where SegWit was always active. If instead they want to create an alternative history where SegWit never activated, they would need to exclude SegWit transactions from their mempool. If on top of that they want to make a statement by including SegWit-violating transactions, it would be a bit more complicated. I don’t think that’s a use-case we need to cater for though.

9. 
   maflcko commented at 12:34 pm on January 9, 2025: member

   Ok, so someone could roll back their node to before SegWit activation, patch getblocktemplate to skip the IsInitialBlockDownload() check (or remove the MinimumChainWork and max_tip_age checks from from the latter).

   I don’t think a patch is needed. ibd will latch, so in theory one could use that. I am not claiming this is a valid use case, just that internal self-consistency was the reason why this hasn’t been done previously, IIRC.

10. 
    Sjors commented at 9:30 am on January 10, 2025: member

    It would be good if someone has a link to the earlier reasoning.
11. 
    maflcko commented at 10:31 am on January 10, 2025: member

    Not sure if it was ever mentioned publicly where a link exists.

    Obviously, if you want to reproduce the inconsistency, it would be easier in regtest, and the change may theoretically break someone’s test deployment.

12. 
    Sjors commented at 8:49 am on January 14, 2025: member

    It seems useful to illustrate the change here by regenerating the alternative mainnet blocks introduced in #31583, switching their coinbase outputs to taproot addresses. Before this PR the last block, which spends a coinbase output, would fail with unexpected-witness.

    Marking as draft when that lands.

13. Sjors marked this as a draft on Jan 14, 2025
14. 
    Sjors commented at 10:01 am on January 14, 2025: member

    It would also seem more consistent to drop the unexpected-witness check entirely, rather than just change the miner code. That would dramatically increase the scope of this PR though. Leaving up for grabs for now.
15. Sjors closed this on Jan 14, 2025

    ---

Contributors
Sjors DrahtBot maflcko

Labels
Mining