DEPENDENT: API: Expose bitcoinconsensus_verify

jtimon commented at 2:33 pm on April 10, 2015: contributor

To extend libconsensus’ functionality, I propose to add bitcoinconsensus_verify_header() the partial checks CheckBlockHeader() and ContextualCheckBlockHeader could be exposed as well.

Unlike VerifyScript, VerifyBlockHeader depends on the nodes storage, as it requires data from the blockchain previous to the header being verified. How to exactly interface that data with VerifyBlockHeader can be a delicate matter and it should be of course open to discussion. Here there’s just one proposal.

The offending dependency is CBlockIndex in chain.o, specially CBlockIndex::pprev which is a pointer to another object. All the other dependencies unacceptable for libconsensus have been removed first, to make things easier for someone who wants to propose to solve that final problem in some other way.

Dependencies: -MOVEONLY: Move most consensus function declarations to consensus/consensus.h #6048 -Consensus: Refactor: Consensus version of CheckBlockHeader() #6035 -Separate CValidationState from main #5669 -Chainparams: Refactor: Decouple IsSuperMajority from Params() #5968 -Consensus: Decouple ContextualCheckBlockHeader from checkpoints #5975 -Consensus: Turn CBlockIndex::GetMedianTimePast into independent function #6009 -Consensus: Consensus version of pow functions #6037

jtimon renamed this:
~~DEPENDENT: Consensus: API: Expose bitcoinconsensus_verify_header() in libconsensus~~
DEPENDENT: API: Expose bitcoinconsensus_verify_header() in libconsensus
on Apr 10, 2015

jtimon force-pushed on Apr 10, 2015

jtimon force-pushed on Apr 11, 2015

jtimon force-pushed on Apr 13, 2015

jtimon force-pushed on Apr 15, 2015

jtimon commented at 12:04 pm on April 15, 2015: contributor

Needed rebase

in src/consensus/blockverify.cpp: in f0860464ad outdated

10+#include "primitives/block.h"
11+#include "tinyformat.h"
12+
13+#include <algorithm>  
14+
15+static const unsigned int MEDIAN_TIME_SPAN = 11;

Diapolo commented at 12:52 pm on April 15, 2015:

IMHO this could get a comment.

jtimon commented at 5:50 pm on April 15, 2015:

Feel free to suggest something. Probably better to comment in the smaller PRs for now as this branch will likely require more rebases and force pushes. In this case, the relevant PR is #6009

in src/consensus/validation.h: in f0860464ad outdated

42+            return ret;
43+        nDoS += level;
44+        mode = MODE_INVALID;
45+        return ret;
46+    }
47+    bool Invalid(bool ret = false,

Diapolo commented at 12:55 pm on April 15, 2015:

This looks strange/ugly, you could IMHO let clang-format cleanup this pull :)?

jtimon commented at 5:48 pm on April 15, 2015:

This is from a moveonly commit in #5669 Hopefully we will do clang-format project wise for 0.11. @laanwj what were the requirements for that to happen?

in src/main.cpp: in f0860464ad outdated

1199@@ -1206,6 +1200,21 @@ bool IsInitialBlockDownload()
1200     return state;
1201 }
1202 
1203+static arith_uint256 GetBlockProof(const CBlockIndex& block)
1204+{
1205+    arith_uint256 bnTarget;
1206+    bool fNegative;
1207+    bool fOverflow;
1208+    bnTarget.SetCompact(block.nBits, &fNegative, &fOverflow);

Diapolo commented at 12:57 pm on April 15, 2015:

Block can never be NULL here?

jtimon commented at 5:50 pm on April 15, 2015:

This is a move-only commit as well.

in src/miner.cpp: in f0860464ad outdated

79@@ -78,17 +80,33 @@ class TxPriorityCompare
80     }
81 };
82 
83+uint32_t GetNextWorkRequiredLog(const CBlockIndex* pindexLast, const CBlockHeader *pblock, const Consensus::Params& consensusParams)
84+{
85+    uint32_t nextChallenge = GetNextWorkRequired(pindexLast, pblock, consensusParams, GetPrevIndex);
86+    /// debug print
87+    LogPrintf("GetNextWorkRequired RETARGET\n");

Diapolo commented at 12:59 pm on April 15, 2015:

Function name changed, now GetNextWorkRequiredLog, perhaps use func?

jtimon commented at 5:52 pm on April 15, 2015:

I don’t undesrtand what you mean. There’s two functions with this, one for miner and one for everyone else. I’m happy to remove the miner version if it’s fine to remove the logging there. Anyway, I will create a smaller PR only adapting the pow functions to consensus.

jtimon force-pushed on Apr 19, 2015

Consensus: Consensus version of pow functions 3dee0253b9

Consensus: Refactor: Consensus version of CheckBlockHeader() 504cae163e

Chainparams: Refactor: Decouple IsSuperMajority from Params() 4dc7f3c42f

Consensus: Separate CheckIndexAgainstCheckpoint() from ContextualCheckBlockHeader 5fab51f568

Consensus: Create consensus/consensus.h with some constants 7e8aa2ee52

Consensus: Refactor: Decouple CValidationState from main::AbortNode() 71e6f985a6

Consensus: Refactor: Turn CBlockIndex::GetMedianTimePast into independent function d407ced0d0

Consensus: Refactor: Consensus version of ContextualCheckBlockHeader() 392d2710a6

Consensus: MOVEONLY: Move CValidationState from main consensus/validation 7e7a79561f

Consensus: MOVEONLY: Move to blockverify.cpp:

from main.cpp:
-CheckBlockHeader
-ContextualCheckBlockHeader
-IsSuperMajority

from pow.cpp:
-CalculateNextWorkRequired
-CheckProofOfWork
-GetNextWorkRequired

928aae17e2

Consensus: Introduce Consensus::VerifyBlockHeader() 11ede96f59

Consensus: Cleanup: Destroy pow.o and move GetBlockProof(CBlockIndex) to chain.o 22e37bc079

Consensus: API proposal for Consensus::VerifyBlockHeader() 4528ec6961

Consensus: API: Expose bitcoinconsensus_verify_header() in libconsensus 00b9b227af

jtimon force-pushed on Apr 20, 2015

jtimon commented at 10:40 pm on April 22, 2015: contributor

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

jtimon closed this on Apr 22, 2015

jtimon referenced this in commit 1cc0e96e9c on Dec 9, 2015

DrahtBot locked this on Sep 8, 2021

DEPENDENT: API: Expose bitcoinconsensus_verify_header() in libconsensus #5995