index: Fix for indexers skipping genesis block. #14085

This fixes a bug where indexers would skip processing of the genesis block. Preserves the current behavior of omitting genesis block transaction from the index.

@sipa has NACK'd this previously when I offered a "fix".

Reasoning is something like: Txindex was used previous to Core maintaining a pruned utxo set, and thus never had use for the genesis block transaction, since it was never entered into the utxo set.

I personally concept ACK, just giving background.

Even if we decide to exclude the genesis transaction from the txindex specifically, we should still fix in the BaseIndex and add an exception in the TxIndex subclass.

What was the previous behaviour (e.g. in version 0.15 with -txindex)?

That transaction isn't really in the blockchain in almost any meaningful sense: It has no effect on the system state. e.g. it shouldn't show up in address indexing either, because although it looks spendable it's not spendable.

@MarcoFalke I'm pretty sure prior behavior was to include it in the index. <- That's wrong, it was omitted from the index. I ran a 0.15.1 node to verify.

The genesis transaction was never included in any index, intentionally.

IMO, a merkle root of 4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b indicates the block has 0 transactions. ;)

~0 on this

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #14121 (Index for BIP 157 block filters by jimpo)
• #14111 (index: Create IndexRunner class for activing indexes. by jimpo)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

~0 on this as well - it could be considered if there is ever an indexer that needs to observe the genesis block somehow, but I don't see why that would be the case.

@sipa The block filter headers commit the the filter header of the previous block and BIP 157 defines a filter header for the genesis block, which is the motivation for this PR. The BIP could be amended to define the filter header of the genesis as 0, but I feel like this is a silly reason to change the BIP. Also, the genesis block is a block -- it's very unintuitive to me that it would get skipped by default just because it's a special block with no effect on the chain state.

@jimpo I see, that's a perfectly good reason. There is no restriction on downloading the genesis block either.

utACK ed12d5df1ba52b5ef3dd3799de26bb5e1d3fc654

utACK ed12d5df

utACK https://github.com/bitcoin/bitcoin/commit/ed12d5df1ba52b5ef3dd3799de26bb5e1d3fc654

@jimpo The genesis block de facto has ZERO transactions. IMO it is a bug in BIP 157 if it considers that it does.

@luke-jr Per BIP 157, the genesis block does have a filter and it has a single element, an output. Since this output is unspendable, this does seem like a bug on reflection. Would everyone be happier if I change BIP 157 so that there is no filter for the genesis block and it's filter header becomes 0x00{32}? @roasbeef

This has enough (ut)ACKs, but the last post makes it unclear to me whether to merge this now, probably better to wait for BIP clarification or not?

@jimpo I don't see a good reason to modify the format of the filters this late in the cycle. A number of individuals are already running BIP 157/158 on mainnet as other full node implementations are already able to serve them. I think we all understand that the genesis block is unspendable, but it does in fact have transactions, though they don't modify the UTXO set. Why add another special case?

@laanwj I think the BIP is fine and I haven't heard serious objections. I think this should get merged and if there is a call to change the genesis filter in the BIP it's a one-line change (similar to what was done with txindex). I still think 100% that the base index should not skip the genesis block, which is the main thing this fixes.

This is ready for merge IMO.