Conversation

Three tests use non-push opcodes in scriptSigs, so those have
the fork explicitly disabled.

The 64-byte transaction check is executed in a new
ContextualBlockPreCheck which must be run before CheckBlock (at
least in the final checking before writing the block to disk).
This function is a bit awkward but is seemingly the simplest way
to implement the new check, with the caveat that, because the
new function is called before CheckBlock, it can never return a
non-CorruptionPossible error state.

This prepares us for a potential future timewarp-fixing softfork by
ensuring that we always refuse to mine blocks which refuse to
exploit timewarp, no matter the behavior of other miners. Note that
we allow timestamp to go backwards by 600 seconds on the
difficulty-adjustment blocks to avoid bricking any existing
hardware which relies on the ability to roll nTime by up to 600
seconds.
Note that it is possible that the eventual softfork to fix timewarp
has a looser resetriction than the 600 seconds enforced here,
however it seems unlikely we will apply a tighter one, and its fine
if we restrict things further on the mining end than in consensus.

Previously, if we fail both due to a
STANDARD_NOT_MANDATORY_SCRIPT_VERIFY_FLAGS and a
MANDATORY_SCRIPT_VERIFY_FLAGS, we would potentially return the
non-mandatory error as the reason for
mandatory-script-verify-flag-failed. This ensures we always return
the correct error.

Some notes:
* Three tests use non-push opcodes in scriptSigs, so those have
the fork explicitly disabled.
* The 64-byte transaction check is executed in a new
ContextualBlockPreCheck which must be run before CheckBlock (at
least in the final checking before writing the block to disk).
This function is a bit awkward but is seemingly the simplest way
to implement the new check, with the caveat that, because the
new function is called before CheckBlock, it can never return a
non-CorruptionPossible error state.

This comment has been minimized.

edited

Given the issues with the mailing list, and the lack of a PR adding the BIP, I'm going to write some thoughts I have regarding the general idea of the proposal here. My apologies if this seems like the wrong forum.

I'm a bit surprised that the possibility of using forward blocks to achieve future scaling is not mentioned in the BIP or PR, since the absolute removal of the time-warp attack vector closes off the possibility of using the bug for forward- and backwards-compatible soft-fork scaling in the future, as I covered in my talk at Scaling Bitcoin last year.

While developing the idea of forward blocks I sent a vulnerability report email to some of the senior Bitcoin Core developers. In it I outlayed some analysis of the attack vectors, and a suggested solution which both prevents the sort of attacks which could be devastating to the network, while still allowing uses of the time-warp "feature" to achieve on-chain scaling when it becomes necessary at some point in the future. I'll quote that part of the email here:

...It turns out this is to the advantage of bitcoin users as the time warp is an integral part of how we can make a block size increase and/or PoW change without a hard fork, and thereby allow such proposals be decided on their own merits rather than as a yea or nay decision on hard forks generally.

...This is an issue that has been known about for some time and persists as unfixed because, as I understand it, the likelihood of attack was low and likelihood of coordinated user response high. And since ideas like Forward Blocks require the time-warp bug to achieve forwards-compatible scaling of settlement capacity, I would think it short sighted to advocate for "fixing" the bug now.

However in discussing forward blocks with another developer, I happened upon a solution that would allow for the worst scenarios to be avoided while not blocking off the later deployment of forward blocks: If the current block timestamp is more than 4 hours beyond the median time of the past 11 blocks, then the difficulty adjustment errors if the adjustment would exceed a smaller limiting factor than the normal +/- 4x. Specifically I recommend ensuring that the block timestamp is within the range [start + 600*2015 - 4800, start + 600*2015 + 4800]. 4800 = 80 * 60 = 1 hour 20 minutes. This would allow decreases in the inter-block time of no more than 12.6% the first year of purposeful adjustments, and is easy to implement.

(Potentially the lower limit could be removed as it would be nice for the difficulty to be able to quickly reset, but asymmetric adjustments have been a source of error in the past.)

This would prevent ... [censored bug disclosure] ... while still allowing a rate of growth for forward blocks or other proposals to that is conservative slow enough as to not cause catastrophic failure. Requiring the block timestamp to be 4 hours ahead of the median time past ensures this rule is very unlikely to be engaged during normal operation of a healthy network, for which normal [unconstrained] difficulty adjustments can still occur.

This approach additionally has the advantage of being less disruptive than what is implemented in this PR. No rule changes are engaged at all until such time as the adjustment block's timestamp is 4 hours ahead of median time past, so there is no risk to un-upgraded miners outside of a time-warp regime because a block more than 2 hours in the future is considered invalid. I think this modified rule would make it safe to switch to BIP8 instead of BIP9 for deployment.

This comment has been minimized.

This is not the appropriate forum for having that discussion. I'll include further motivation in the email to the mailing list, so lets wait for the mailing list to be online before we open it up further.

This comment has been minimized.

Can you split the soft-fork rules into separate commits, away from the activation logic? E.g. it's hard to spot the OP_CODESEPERATOR change if you're not familiar with #11423 that made it non-standard.

Shameless review plug for #12134, so we can test the interaction with older nodes.

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.