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

1. 
   jtimon commented at 12:28 pm on March 26, 2015: contributor

   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

2. jtimon renamed this:
   WIP: Put all the consensus code together
   WIP: Encapsulate consensus code
   on Mar 26, 2015
3. 
   gavinandresen commented at 4:07 pm on March 26, 2015: contributor

   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.

4. jtimon force-pushed on Mar 27, 2015
5. 
   jtimon commented at 10:36 am on March 28, 2015: contributor

   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.
6. jtimon force-pushed on Mar 28, 2015
7. jtimon force-pushed on Apr 1, 2015
8. jtimon force-pushed on Apr 1, 2015
9. jtimon force-pushed on Apr 2, 2015
10. jtimon force-pushed on Apr 5, 2015
11. laanwj added the label Improvement on Apr 6, 2015
12. jtimon force-pushed on Apr 6, 2015
13. jtimon force-pushed on Apr 8, 2015
14. jtimon force-pushed on Apr 10, 2015
15. jtimon renamed this:
    WIP: Encapsulate consensus code
    API: Expose bitcoinconsensus_verify_block() in libconsensus
    on Apr 10, 2015
16. jtimon renamed this:
    API: Expose bitcoinconsensus_verify_block() in libconsensus
    DEPENDENT: API: Expose bitcoinconsensus_verify_block() in libconsensus
    on Apr 10, 2015
17. jtimon force-pushed on Apr 10, 2015
18. 
    jtimon commented at 2:57 pm on April 10, 2015: contributor

    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.
19. 
    gavinandresen commented at 3:45 pm on April 10, 2015: contributor

    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)

20. 
    jtimon commented at 4:19 pm on April 10, 2015: contributor

    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.

21. jtimon force-pushed on Apr 13, 2015
22. jtimon force-pushed on Apr 17, 2015
23. jtimon force-pushed on Apr 17, 2015
24. Chainparams: Refactor: Decouple IsSuperMajority from Params() 1397c71216
25. jtimon renamed this:
    DEPENDENT: API: Expose bitcoinconsensus_verify_block() in libconsensus
    WIP: DEPENDENT: API: Expose bitcoinconsensus_verify_block() in libconsensus
    on Apr 19, 2015
26. Consensus: Separate CheckIndexAgainstCheckpoint() from ContextualCheckBlockHeader 77e1d15278
27. Consensus: Create consensus/consensus.h with some constants 09e5ab555e
28. Consensus: Refactor: Decouple CValidationState from main::AbortNode() e9fedd95cb
29. Consensus: MOVEONLY: Move CValidationState from main consensus/validation 64df4c6d56
30. Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast into independent function b9d53adfad
31. Consensus: Consensus version of pow functions f537c56e85
32. Consensus: Refactor: Consensus version of CheckBlockHeader() 46cc312fc0
33. Consensus: Refactor: Consensus version of ContextualCheckBlockHeader() 0df3baf27d
34. Consensus: MOVEONLY: Move to consensus.cpp:

    from main.cpp:
    CheckBlockHeader, IsSuperMajority, ContextualCheckBlockHeader
    
    from miner.cpp
    GetNextWorkRequired, CalculateNextWorkRequired, CheckProofOfWork

    fa71dfc90a
35. Consensus: Cleanup: Destroy pow.o and turn its remaining into static functions in main.cpp and miner.cpp d1a8ab4db7
36. Consensus: Introduce Consensus::VerifyBlockHeader() 8c4c0d9293
37. Consensus: API proposal for Consensus::VerifyBlockHeader() d850a6bc88
38. Consensus: API: Expose bitcoinconsensus_verify_header() in libconsensus 7e0cd1405b
39. Consensus: Refactor: CheckTransaction() -> Consensus::CheckTx()

    Decouple it from util.h [bool error(char*)] and BOOST_FOREACH

    10a39eb847
40. Consensus: Refactor: Introduce GetSpendHeight() 216974fe17
41. Consensus: Refactor: Separate Consensus::CheckTxInputs from CheckInputs f959b186d6
42. Consensus: Introduce Consensus::CheckTxInputsScripts() and use it separately from main::CheckInputs when possible ddd505a4f6
43. Consensus: Refactor: Separate CheckFinalTx() from main::IsFinalTx() 965792e14c
44. Consensus: Introduce Consensus::GetSigOpCount(CTransaction, CCoinsViewEfficient) c82754bf69
45. Consensus: Introduce Consensus::VerifyTx() a6d1e6e60d
46. MOVEONLY: Consensus: Move to consensus/txverify.cpp:

    from main.cpp:
    -CheckFinalTx
    -Consensus::CheckTx
    -Consensus::CheckTxInputs
    -GetLegacySigOpCount
    -GetP2SHSigOpCount

    c29e94b48e
47. Consensus: Introduce minimal UTXO interface with function pointers fb85f35ea4
48. Consensus: Refactor: Turn main::CheckBlock() into Consensus::CheckBlock() b15090850a
49. Consensus: Refactor: Consensus version of Consensus::ContextualCheckBlock() 3c3cedb592
50. Chainparams: Refactor: Decouple main::GetBlockValue() from Params() [renamed GetBlockSubsidy]

    Remove redundant getter CChainParams::SubsidyHalvingInterval()

    8ba559693e
51. Consensus: Refactor: Introduce Consensus::GetFlags

    and use it instead of MANDATORY_SCRIPT_VERIFY_FLAGS in miner::CreateNewBlock()

    9ba7d06410
52. Consensus: Refactor: Introduce Consensus::EnforceBIP30 753349f9d9
53. MOVEONLY: Consensus: Make IsSuperMajority static and move to blockverify.cpp:

    from main:
    -CheckBlock
    -ContextualCheckBlock
    -GetBlockSubsidy

    6e65cdc790
54. Consensus: Introduce Consensus::VerifyBlock() 40fd10a7fd
55. jtimon force-pushed on Apr 19, 2015
56. 
    jtimon commented at 0:10 am on April 23, 2015: contributor

    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.
57. jtimon closed this on Apr 23, 2015

    ---

58. DrahtBot locked this on Sep 8, 2021

Contributors
jtimon gavinandresen

Labels
Refactoring