Remove BIP94 from regtest #31137

1. 
   mzumsande commented at 8:14 pm on October 22, 2024: contributor

   BIP94 which, amongst others, fixes the timewarp attack on testnet4, has been activated on regtest by #30681 (commit https://github.com/bitcoin/bitcoin/pull/30681/commits/e85f386c4b157b7d1ac16aface9bd2c614e62b46), in order to allow having a functional test for the new testnet4 behavior.

   As I argued in #30681#pullrequestreview-2275751672 I believe that regtest’s main task is to test mainnet behavior, and, as such, it shouldn’t diverge from mainnet consensus rules unless there is a very good reason (e.g. for situations where a soft fork has been merged and is a candidate for activation on mainnet). I don’t think testnet4 is a good enough reason - for example, writing test coverage for the actual unfixed timewarp issue will no longer be easily possible in a functional test, and if we end up fixing timwarp in a future softwork we might do it in a slightly different way than BIP94, which seems like it could easily get messy.

   Possibilities to fix this (if desired) include:

   • allowing to pass consensus parameters via command line (Reviving #17032 or do something similar), so BIP94 would only be active in the subtest where it’s needed.
   • rewriting the mining_basic.py subtest test_timewarp somehow
   • removing the test for testnet4, with the justification that testing mainnet should have priority over testing testnets.
2. maflcko added the label Tests on Oct 23, 2024
3. 
   laanwj commented at 8:07 am on October 24, 2024: member

   Concept ACK. i think it’s a good point that regtest, at least by default, should be as closest as possible to mainnet, not testnet.
4. 
   sipa commented at 12:08 pm on October 24, 2024: member

   For the first option, the BIP94 timewarp behavior could perhaps be turned into a (versionbits) deployment?

   Then mainnet and testnet3 can have it off (by not having a deployment defined), always on on testnet4, and it would be implicitly configurable on regtest using -vbparams?

5. 
   maflcko commented at 12:12 pm on October 24, 2024: member

   Anything is probably fine here. A pre-mined testnet4 chain can possibly be used for the test as well, instead.
6. 
   mzumsande commented at 6:07 pm on October 25, 2024: contributor

   I opened #31156 which just adds an arg. This seemed the least invasive solution to me.

   A pre-mined testnet4 chain can possibly be used for the test as well, instead.

   That should be possible too, but would require enabling setmocktime for testnet, which I didn’t love.

7. achow101 closed this on Oct 30, 2024

   ---

Contributors
mzumsande laanwj sipa maflcko

Labels
Tests