Rather than always having the genesis block be DEFINED, allow {0,0,true} to act as ALWAYS_ACTIVE and {0,0,false} to act as NEVER_ACTIVE, and {n,n,true} to act as a buried deployment.
This cleans up the implementation a bit I think, removing the need for special case handling. (Note that in the {n,n,true} case n will still be rounded up to the next retarget period however)
Rather than always having the genesis block be DEFINED, allow
{0,0,true} to act as ALWAYS_ACTIVE and {0,0,false} to act as
NEVER_ACTIVE, and {n,n,true} to act as a buried deployment.123 | + if (lockinontimeout) { 124 | + return ACTIVE; 125 | + } else { 126 | + return FAILED; 127 | + } 128 | + } else if (block.height >= startheight) {
I wonder if this is needed in the BIP, or just implementation details that complicate the BIP.
Fair question. I think it makes the special cases ("what if timeout is less than start? what if they're not equal but in the same retarget phase?") a bit clearer, without complicating the BIP very much. It also might let the implementation follow the BIP pseudocode more exactly which might make it easier to see both are correct.
119 |
120 | case DEFINED:
121 | if (block.height >= startheight) {
122 | - return STARTED;
123 | + if (block.height >= timeoutheight) {
124 | + return (lockinontimeout ? ACTIVE : FAILED);
The genesis block handling is slightly better than before, but imo this is more difficult to understand than the current "timoutheight must be 2 retarget periods after startheight".
Closing this -- it only covers cases that can't reasonably arise in practice on either mainnet or testnet, and can be treated as an implementation detail elsewhere.