WIP: DEPENDENT: API: Expose bitcoinconsensus_verify_block() in libconsensus #5946

To fully verify a block one needs to be able to verify block headers and transactions first. Therefor VerifyBlock() can only be exposed after VerifyTransaction() and VerifyBlockHeader() are exposed or ready to be exposed as well. We can work on exposing VerifyTransaction() and VerifyBlockHeader() in parallel. This PR contains all the work I'm doing on those rebased together plus some extra commits towards exposing a VerifyBlock later.

Dependencies: -API: Expose bitcoinconsensus_verify_header() in libconsensus #5995

RE: people who want to see the big picture:

A design document for libconsensus would be a much more efficient way for people like me to get the big picture. And seeing the API you're aiming for would be really nifty, too.

Yes, I'm trying to separate what could get into the next libconsensus relatively easily from what will require some API discussion first. I was planning on just writing to the mailing list for the later, but having an editable document in the wiki or somewhere with the whole proposed API (even if it has question marks in some functions) will probably be better. I will do that after I reorganize the code in 2 or 3 clear stages.

I updated the PR text with something more descriptive, including a PR where there's actually an API to judge. I'm not very happy with it as it is myself, because it ends with a little bit of a hack to get rid of the chain.o dependency. But it should serve to get an idea of the kind of API I would like to propose, at least for VerifyBlockHeader.

Ok, some big-picture design questions I have after spending a few minutes looking through the blizzard of commits:

Why factor out CBlockIndexBase and add a PrevIndexGetter everywhere?

That seems like a weird hybrid way of doing things. Why the move away from blockindex->pprev ? Is there a problem with memory management or storing pointers? If there is, then I'd expect to just store uint256 hashes, and when a pointer to a CBlockIndexBase is needed have some GetBlockHeader(hash) or GetBlockIndex(hash) function(s).

Or if you're trying to save memory not storing a prevHash-- why not just have GetPrevBlock(parent_hash) as part of the API? Are you worried it would be too slow?

On a higher level: what is libconsensus supposed to be do versus not do? I'm gathering it is to encapsulate all of the validation logic, but leave storing/managing all the data up to higher layers. How are the layers supposed to be connected-- pass in lookup functions to the libconsensus methods, as needed? Register lookup methods at startup? (that tends to create a friendlier API, in my opinion, but maybe one that is harder to test)

Why factor out CBlockIndexBase and add a PrevIndexGetter everywhere?

Well, that's the particular interface I chose to replace CBlockIndex. I don't like much myself but I wanted to give an example of a C API at the end of #5995. As explained there, that's the last thing is done (there) to help people replace those last 2 commits with their own proposal. But we can start to prepare that removing all other dependencies that are not CBlockIndex first.

Why the move away from blockindex->pprev ? Is there a problem with memory management or storing pointers?

Well, maybe we want to expose a struct with pprev directly in the C API. That's what needs discussion, how are other codebases using libconsensus expected to feed VerifyBlockHeader with chain index data. I think the function pointer I chose is quite flexible. Maybe another codebase maintains a dictionary nHeight -> blockHeader and other codebases give a pointer to a function that takes the height index, substracts one and looks in that dictionary. It's just an idea. What is clear is that we cannot expose CBlockIndex as a C++ class to the C API.

If there is, then I'd expect to just store uint256 hashes, and when a pointer to a CBlockIndexBase is needed have some GetBlockHeader(hash) or GetBlockIndex(hash) function(s).

That's another solution that in fact I like more, and then other implementations could replace that getter with whatever they want. But this will also require more changes than my little hack.

Or if you're trying to save memory not storing a prevHash-- why not just have GetPrevBlock(parent_hash) as part of the API? Are you worried it would be too slow?

Yeah, if it wasn't for the extra memory of storing prevHash we should already have a common base for CBlockIndex and CBlockHeader. The other suggestions are possible too, that's the kind of discussion I would like to have in #5995 .

On a higher level: what is libconsensus supposed to be do versus not do? I'm gathering it is to encapsulate all of the validation logic, but leave storing/managing all the data up to higher layers.

That's what I envision as well. Although, @luke-jr for example thinks that we should also expose a validation + basic storage library. In any case that would be something to do later.

How are the layers supposed to be connected-- pass in lookup functions to the libconsensus methods, as needed? Register lookup methods at startup? (that tends to create a friendlier API, in my opinion, but maybe one that is harder to test)

Yes, I think having lookup function pointers as parameters of the validation functions is what makes more sense. It is very flexible for other implementations and it allows bitcoin core to adapt to that without necessarily having to radically change its internal structures.

The question is which function pointers should replace CBlockIndex and CCoinsViewCache. I think everything else will be mostly uncontroversial.

I realized this is mostly unreadable until something similar to #6051 is merged. I will close this for now but I will still maintain the list of dependencies.