Bug in devtax_address_for_height #362

https://git.hush.is/hush/hush3/src/branch/dev/src/hush_globals.h#L349

The index 2 should be index 0, there is no index 2. I still need to spin up a test chain to confirm this bug is what I think and then I will change the index and verify it fixes it.

This bug would not happen on most full nodes but it would happen to solo miners or mining pools that use the getblocktemplate RPC

I am going to explain this bug in as much detail as I can here, because wow it is a tricky bug. The good news is that even if we didn't see this bug it is a new feature that is not used by anybody ( need @fekt to confirm that ) and so I don't believe it would cause consensus problems, but it's much too close for comfort and very VERY confusing.

When I first realized that index 2 should be index 0 I said to myself "how does this not crash the node? It is doing an out of bound array access." I tested getblocktemplate on various blocks and indeed the node does not crash and gives back seemingly valid data. It shows data like this :

    "foundersrewardaddress": "RVMygLshHa8qvFX1orEi5FGji99BVXUQFy",
    "foundersrewardscriptpub": "76a914dafd59fd09a4e7cb813fc4dde4515a605e3aceae88ac",

which seems valid. But it's not, there is an off-by-one error and that scriptpub actually corresponds to the scriptpub of the previous address in the list.

If we had a one dimensional array the out of bounds access might cause a crash or give undefined behavior, but because we have a 2D array index 2 is actually the same as index 0 of the next row in the array. This is described in more detail at https://stackoverflow.com/questions/48219108/multidimensional-array-out-of-bound-access

So the scriptpub is actually correct but the address is not, caused by the off-by-one error of using index 2 instead of index 0.

But the inquisitive mind may ask "What happens if it is a block height which causes it to read index 2 at the very end of the list?" Indeed, I had the same question and on block height 238 of TUSH3 it will look for DEVTAX_DATA[ 238 % 20 + 1][2] which is DEVTAX_DATA[19][2] (the +1 comes from getblocktemplate always showing the data for the next block) and then getblocktemplate output shows

    "foundersrewardaddress": "",
    "foundersrewardscriptpub": "76a914dc4a3eb079349b4bb25864af4ddc5cd52faf382d88ac",

because it attempted to read the index one past the very end of the 2D array.

I am going to explain this bug in as much detail as I can here, because wow it is a tricky bug. The good news is that even if we didn't see this bug it is a new feature that is not used by anybody ( need @fekt to confirm that ) and so I don't believe it would cause consensus problems, but it's much too close for comfort and very VERY confusing. When I first realized that index 2 should be index 0 I said to myself "how does this not crash the node? It is doing an out of bound array access." I tested `getblocktemplate` on various blocks and indeed the node does not crash and gives back seemingly valid data. It shows data like this : ``` "foundersrewardaddress": "RVMygLshHa8qvFX1orEi5FGji99BVXUQFy", "foundersrewardscriptpub": "76a914dafd59fd09a4e7cb813fc4dde4515a605e3aceae88ac", ``` which seems valid. But it's not, there is an off-by-one error and that scriptpub actually corresponds to the scriptpub of the previous address in the list. If we had a one dimensional array the out of bounds access might cause a crash or give undefined behavior, but because we have a 2D array index 2 is actually the same as index 0 of the next row in the array. This is described in more detail at https://stackoverflow.com/questions/48219108/multidimensional-array-out-of-bound-access So the scriptpub is actually correct but the address is not, caused by the off-by-one error of using index 2 instead of index 0. But the inquisitive mind may ask "What happens if it is a block height which causes it to read index 2 at the very end of the list?" Indeed, I had the same question and on block height 238 of TUSH3 it will look for `DEVTAX_DATA[ 238 % 20 + 1][2]` which is `DEVTAX_DATA[19][2]` (the +1 comes from `getblocktemplate` always showing the data for the *next* block) and then `getblocktemplate` output shows ``` "foundersrewardaddress": "", "foundersrewardscriptpub": "76a914dc4a3eb079349b4bb25864af4ddc5cd52faf382d88ac", ``` because it attempted to read the index one past the very end of the 2D array.

Fixed on the dev branch. Watch out for nasal demons!