It's been on the agenda for some time to disable POW check for genesis block. As signet blocks will not have valid proof of work for the genesis block, it makes sense to do this change unconditionally.
This is a part of #16411 but as it is a minor consensus tweak, it probably deserves its own PR.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
No conflicts as of last run.
3033 | @@ -3031,10 +3034,13 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P 3034 | if (block.fChecked) 3035 | return true; 3036 | 3037 | - // Check that the header is valid (particularly PoW). This is mostly 3038 | - // redundant with the call in AcceptBlockHeader. 3039 | - if (!CheckBlockHeader(block, state, consensusParams, fCheckPOW)) 3040 | - return false; 3041 | + // We skip POW checks for genesis block 3042 | + if (block.GetHash() != consensusParams.hashGenesisBlock) {
Let's not skip all header checks (even though CheckBlockHeader only checks PoW at the moment).
Ok. I rewrote code to pass the POW check bool based also on if genesis block or not.
Thanks. However, given #9102 it sounds like we actually should skip all checks. If so, then CheckBlockHeader could check if it's the genesis block and skip all checks. The comment here could say: "Check that the header is valid (particularly PoW, and except for the genesis block)"
273 | @@ -274,7 +274,7 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, 274 | pindexNew->nStatus = diskindex.nStatus; 275 | pindexNew->nTx = diskindex.nTx; 276 | 277 | - if (!CheckProofOfWork(pindexNew->GetBlockHash(), pindexNew->nBits, consensusParams)) 278 | + if (pindexNew->GetBlockHash() != consensusParams.hashGenesisBlock && !CheckProofOfWork(pindexNew->GetBlockHash(), pindexNew->nBits, consensusParams))
See #16551
I think this problem can be generalized to "what does Bitcoin Core do if someone sends us the same block header repeatedly", in which case this change is no longer relevant, as the software shouldn't do a bunch of work to validate the same header over and over.
To prevent repetition, can you link to some of these earlier discussions?
It's been on the agenda for some time to disable POW check for genesis block. As signet blocks will not have valid proof of work for the genesis block, it makes sense to do this change unconditionally.
How is this different from #9102 and why can't signet genesis blocks not be ground for pow?
@MarcoFalke They can, but in order to set up a new signet network, you would need to actually sign and mine the genesis block. There is no readily available tool to do that, but such a tool could be created. It would complicate the initial setup process.
To clarify, this becomes a bit of a chicken and egg problem: without a signet network you can't sign blocks and without a signed genesis block you can't startup a new signet network.
I'd prefer to write a simple cpp while loop that is compiled into util_signet binary or exposed via RPC than to change the consensus rules. I'd argue that this wouldn't complicate the initial setup process.
@MarcoFalke Signet operators would have to include the signature and nonce value for the genesis block, FWIW. Right now all they provide is the script itself (and any potential seed nodes).
I will be neutral on this because I think the genesis block should be treated like any other block as much as possible (good for learning purposes as well). Otherwise I like the (tiny) performance increase on it.
I'm not a Core dev so my opinion probably doesn't mean much, but I think it's better to check PoW. That way we keep consistency. Though hardcoded, the genesis block is still a block and there is no reason why we shouldn't validate its PoW. The performance gains from this change are probably negligible anyway. We only risk introducing bugs.
@Relaxo143
You are most welcome :)
Anyone may participate in peer review which is expressed by comments in the pull request. see CONTRIBUTING.md
@emilengler Thank you! I like how everyone is allowed to express their opinion and thus contribute to this amazing project.
@Relaxo143 You being a core dev or not is completely irrelevant. The only thing that matters is the content of what you are saying. We regularly have non-developers chime in on developer matters because these things often blend together.
That being said, my proposal is to remove the proof of work check for the genesis block, because this will otherwise cause problems when a feature is added later. Your comment does not at all address this point, which makes it of limited value, I'm afraid.
It looks like I may be moving towards not touching the proof of work checks and finding some way to grind the genesis block for signet, though, so this change will probably not go through. I just have to figure out a good way to do that.
It looks like I may be moving towards not touching the proof of work checks and finding some way to grind the genesis block for signet, though, so this change will probably not go through.
I have a light preference for that as well, if only to kick this can down the road.
Repeating my inline comment, since that might get burried: the discussion in #9102 suggests - I haven't thought about it deeply - that we should not verify the genesis block at all, and any existing check - including a PoW check - is a regression. But sometimes leaving a regression alone is better than fixing it (devil you know).
@Relaxo143 You being a core dev or not is completely irrelevant. The only thing that matters is the content of what you are saying. We regularly have non-developers chime in on developer matters because these things often blend together.
That being said, my proposal is to remove the proof of work check for the genesis block, because this will otherwise cause problems when a feature is added later. Your comment does not at all address this point, which makes it of limited value, I'm afraid.
It looks like I may be moving towards not touching the proof of work checks and finding some way to grind the genesis block for signet, though, so this change will probably not go through. I just have to figure out a good way to do that.
What feature are you talking about exactly? Does it have to do with signet? If yes, then you are right - I did not address that because I don't know anything about signet. That being said, maybe the change should only be applied to node configs who use signet and not to default nodes who run on mainnet?
Sorry for the necromancy, but I missed it at the time. I want to extend on some points brought up in #16411 (comment) and I don't want that PR to re-hash the topic one more time.
Let's go through some of the objections this or similar proposals, had:
#9102 (comment) tldr: it doesn't have tests But now it is tested in #8994 , or a modified #16411 can also test it
I'd prefer to write a simple cpp while loop that is compiled into util_signet binary or exposed via RPC than to change the consensus rules. I'd argue that this wouldn't complicate the initial setup process.
But now we can see it does. Only replicating the mining_signet test with say a 2 of 2 challenge would require the write tester to mine a genesis block. And we could save some code and not use the signet_nonce field, see: https://github.com/kallewoof/bitcoin/commit/010cca031dfb890f92ff78c3bf6a45326ae38edf
In the case of #8994 , having to mine each genesis block makes the new feature much less usable and makes the fact of hashing the chain name counterproductive (since a new hash will mean having to mine a genesis block again).
If I missed any other objection, please reiterate it. Sorry for the rant.
For the record, I would love to get rid of the genesisnonce parameter, but I can see both sides of these arguments.