Prevent algo bits from triggering 'unknown new rules' message

[barrystyle]

There have been a few incorrect attempts at removing the 'unknown new rules' message.
This PR prevents any warning from being triggered if the target bit is less than the max block algo version (const BLOCK_VERSION_ALGO).

[SmartArray]

Your proposal looks interesting.

How comes Bitcoin doesn't have this control flow? Any ideas?

[barrystyle]

Have just noticed this in versionbits.h for bitcoin itself (https://github.com/bitcoin/bitcoin/blob/master/src/versionbits.h#L19-L20):

/** Total bits available for versionbits */
static const int32_t VERSIONBITS_NUM_BITS = 29;

Git blame shows our versionbits range was limited (d117ac5), but the actual intent (VERSIONBITS_NUM_BITS_TO_SKIP) was lost in merge.

[JaredTate]

Appreciate your help on this! Definitely a step towards a solution. So currently last I tested there were 2 unknown bit errors, bit 6 for DEPLOYMENT_ODO and 14 for DEPLOYMENT_NVERSIONBIPS. So not sure if this fix will fix the second, as I still think there might be an error in compute block or block version checker. My main laptop died a week and a half ago, and the parts I received didn't work so I have been without a main dev machine so I haven't been able to test this yet plus I have a bunch of personal stuff I am dealing with. I aim to try working on this next week. But this is a step in right direction, would love others to help out on this. This is last big issue I can find before working on a test release for 8.22.

[barrystyle]

Has anyone actually compiled/tried this? It resolves the issue.

[JaredTate]

Ok, I have had time to dig in deeper and this actually has revealed some unknown issues we can now correct. So appreciate you digging in to this Barry. We definitely have some worthwhile collaboration here.

The issue with adding:

            // dont trigger 'unknown new rules' warning if the bit falls within
            // the block algo version range (enum in primitives/block.h)
            if (bit <= BLOCK_VERSION_ALGO) continue;   

Is that then causes an issue for potential future algo swaps as there are reserved bits below BLOCK_VERSION_ALGO.

enum {
    // primary version
    BLOCK_VERSION_DEFAULT        = 2, 

    // algo
    BLOCK_VERSION_ALGO           = (15 << 8),
    BLOCK_VERSION_SCRYPT         = (0 << 8),
    BLOCK_VERSION_SHA256D        = (2 << 8),
    BLOCK_VERSION_GROESTL        = (4 << 8),
    BLOCK_VERSION_SKEIN          = (6 << 8),
    BLOCK_VERSION_QUBIT          = (8 << 8),
    //BLOCK_VERSION_EQUIHASH       = (10 << 8),
    //BLOCK_VERSION_ETHASH         = (12 << 8),
    BLOCK_VERSION_ODO            = (14 << 8),
};

If anything it should be Odo specific:

            // dont trigger 'unknown new rules' warning if the bit falls within
            // the block algo version range (enum in primitives/block.h)
            if (bit == BLOCK_VERSION_ODO) continue;   

However the actual fix is much more nuanced. It appears even BTC devs ran into same issue and the problem is a un initialized variable "nMinerConfirmationWindow" and we need to hard code the consensus.MinBIP9WarningHeight.

So basically this needs changed

consensus.MinBIP9WarningHeight = consensus.OdoHeight + consensus.nMinerConfirmationWindow;   

To

consensus.MinBIP9WarningHeight = 9152640; // Odo height + miner confirmation window,;   

However your message above about VERSIONBITS_NUM_BITS_TO_SKIP shows we never fully reverted the changes made in 2017. VERSIONBITS_NUM_BITS_TO_SKIP was actually reverted the very next day, however the one thing never reverted was static const int32_t VERSIONBITS_NUM_BITS = 16. That should be 29. I will take your PR changes and make a new PR with more changes to fix these. Once again thanks for your help.