Post
Topic
Board Development & Technical Discussion
Re: Regtest Consensus Forking Behavior Introduced in Bitcoin Core in May 2014
by
davec
on 20/05/2015, 15:16:14 UTC
Testing is very important, but adding additional code to accommodate makes the simulation even _less_ faithful, and more likely to miss real issues (or potentially even introduce real issues).  I do regression testing with a ASIC miner and the main network code (with checkpoints=0, of course). Testing with the actual production-time behavior is the gold standard and cannot be replaced with shortcutted version without compromise.  (FWIW, testnet which, in spite of its own stupid shortcuts, is somewhat closer to Bitcoin has test cases in the chain for adjustment extremes).

You're making a supposition here that the code contains special casing for the regtest network.  I would agree with you if the retarget code had an extra branch condition that was only executed on the regtest network as then you would only be testing that branch, which is not the same branch as the one that mainnet/testnet would use.  However, that is not the case here since it is parameterized.  I personally think testnet's approach of having special cased branches regarding difficulty is more dangerous from a testing standpoint than the parameterized code in question.

The only reason you were able to make this comment at all is because regtest exists, and the only reason regtest mode exists is because it was specifically created for the block tester harness that runs on externally hosted ci setup, e.g. its intended use.
...
Use of it has expanded since then--  I think today a somewhat different approach would make more sense for the regtest shortcutting and would result in a smaller divergence from the normal network behavior.  (e.g. when in testing mode, mask out the highest bits of the block hashes before the target check).  So quite the opposite, testing is important enough that one should actually be testing the actual Bitcoin network code and not a altcoinified mockup that makes testing easier, or to the extent a modified version for testability is used great care should be taken to minimize the number of differences (and  to not add risk to production code).

I agree that regtest specifically was created for the block tester and then later expanded.  This is why we created a separate simnet in btcd for actual simulation testing because the regtest mode has behavior specific to the block tester harness and we didn't want to conflate them.  Unfortunately, when we've discussed simnet in #bitcoin-dev, more than one Bitcoin Core dev suggested to just use regtest for that purpose.  So, based upon your comments, I think there is some confusion by the BC community in general on how regtest is supposed to be used.

We can certainly discuss other approaches for the future, but the entire current RPC test harness is based on it, so for better or worse, this is way things currently work.

How you could extract "testing is not important" from my comments about a whole alternative network mode created specifically for testing is beyond me-- specifically given the amount of effort I put in previously in convincing you that agreement testing was essential.

I'm not sure how a sharp guy such as yourself would expect any other conclusion.  You stated that the behavior was known at the time of the patch and merged anyways.  Therefore a conscious decision was made to commit a change that knowingly broke retargetting on the regtest network.  The only logical conclusion from that information is that proper retargetting on the regtest network was not deemed important enough to avoid merging the change.  Obviously, there were reasons for that decision (some of which you elucidated), but whatever the circumstances leading to the decision, the conclusion is the same.