@TheBlueMatt This will probably require a comparison tool update to remove tests for the BIP34 changeover logic.

You know, conceptually it feels like what we really want here is for the block validation logic to only apply BIP34, but allow the test to fail if we're validating a block prior to a BIP34-specific checkpoint.

What you've written now has the weird effect that a huge reorg can create a different blockchain where BIP34 rules are disabled prior to a certain block height. A pedantic difference sure in this case, but quite possible on testnet. In the event of a security fix it might even matter - suppose the fix is for an exploit that lets you mine blocks unexpectedly easily. In that case we'd definitely have to apply the new rules based on a specific hash, rather than block height.

Speaking of checkpoints... Rather than just removing them I was thinking they should be renamed something like "validation shortcuts", with the behavior that validation is skipped until we're past the shortcut. However in the event of a large reorg, we do allow the reorg to happen, but instead validate all reorged blocks 100% Thus we end up in a situation where checkpoints no longer define history, but they are used as a easily-auditable shortcut to allow validation to be skipped.

@petertodd Good point - ideally the BIP34 check is indeed guarded by a block hash rather than just height. It does add a complication in the decision logic though, as it means we need to wait until the switchover header is downloaded and validated before we can do block validation. I'm not convinced that complication is worth the benefit in this case.

I don't think we can remove checkpoints entirely. They're still needed to prevent being spammed with huge amounts of low-height valid header chains that fill nodes' memory. Using like you suggest would be a nice and easy step though - and doesn't suffer from the complication listed above (if the headers leading to the checkpoint aren't known/validated yet, we just use the stricter rule and do full validation).

I agree it's not worth it yet; more thinking down the road when we've had a bunch of these little upgrades. My understanding with headers first was it'd be totally reasonable to download the entire set of block headers, and only then start downloading blocks.

We can't yet I agree, although it's easy to forsee us being able to in the nearish future when we have better ways of determining the total work represented by a peer's claimed headers. The main thing I don't want to see is us move to a model where we skip validation as an optimization, but we do that based on a large amount of hashing power rather than an explicit "shortcut/checkpoint/whatever" out of the control of hashing power.

I would think we may want to keep IsSuperMajority around for future softforks, and I agree testnet should not be hard-coded like this.

@luke-jr Oh, I'm not suggesting we remove it, just suggesting there may be a simpler way to deal with older rule changes.

@luke-jr Oh wait, you mean @sipa's removal - well, that can be recovered from git history.

re: testnet, we should be careful to make sure testnet and mainnet use as similar code as possible - it might make sense to have the testnet rule switchover even be not block zero, certainly to keep the logic there w/ block zero as the switchover.

BIP34 checks are uninteresting enough (in terms of what you get from violating them) that I wouldn't want any additional complexity for them, and so I think it's fine to retcon the rule into being a straight up height threshold. Such a test is pretty much the easiest behaviour change to implement, and to implement mostly correctly.

When we simplified BIP30 we were unable to do this (well, unable to keep it) and had to block-pin it because someone feeding you an early chain fork with BIP30 violations was problematic, but that doesn't apply here.

The shortcut you're talking about sounds somewhat like one of the things I'd previously proposed for post-headers-first: Abbreviate validation on blocks on the longest chain where the block is work-dominant over any competition. Work dominant means that chain after the block under consideration has at least threshold more work than any fork you know about which doesn't include that block. We'd talked about a threshold based on X days of work (e.g. 60 days) at the highest difficulty ever seen subject to some minimum. Pieter has been charting how much work there is in the whole chain at each time relative to that time: http://bitcoin.sipa.be/powdays-50k.png

Some of these things should go along with reworking wallet facing security measures. I'd rather move some of the protective logic away from the consensus code and toward the wallet. E.g. I wouldn't consider refusing a 100 block reorg in the consensus code, such efforts are inevitably attack vectors, but I think having a wallet go into a safe mode where nothing shows confirmed (or additional confirms) and no transactions can be made without a manual kick would be a pretty advisable thing to do.

@luke-jr I wouldn't want to use that implementation anymore for future rule changes, both semantically (it unnecessarily constrains the block version number further) and implementation-wise (it needs to scan over 1000 block index entries for every block check).

@sipa What alternative are you thinking of?

We're using IsSuperMajority for the BIP66 block version 2 to 3 switch, so the implementation here needs to be changed.

Rebased as #5966