Skip to content

Conversation

@jtimon
Copy link
Contributor

@jtimon jtimon commented Feb 20, 2016

This unifies the recent locktime validation flags with the script ones for easier consensus deployment via BIP9 as implemented in #7566. It facilitates deploying BIP113 in this way by moving the check to ConnectBlock() and use the unified consensus validation flags.

Like #7566, it finally introduces "the src/consensus/consensus.cpp file" which is a place intended to attract new consensus code (as opposed to keep putting it in main.cpp like bip113 and bip68).

This PR introduces Consensus::CheckTxPreInputs(), which does all the consensus checks common for coinbase and noncoinbase transactions (which implies none of the checks use the inputs of the transction nor the utxo state).
Note that this introduces a redundant call to main::CheckTransaction() in main::ConnectBlock() which also calls main::CheckBlock() [which calls main::CheckTransaction()].
We can eat either:

a) Eat the redundant main::CheckTransaction() call per tx per block.
b) Remove it from main::CheckBlock()
c) Don't call main::CheckTransaction() from Consensus::CheckTxPreInputs() in this PR

I don't want to decide myself.

In addition to Consensus::CheckTxPreInputs(), the PR introduces Consensus::VerifyTx(), which in its current incomplete implementation seems pretty stupid, as it only calls Consensus::CheckTxPreInputs().
But there's many more things that could be encapsulated besides that, for example, in https://github.com/jtimon/bitcoin/commits/jt see:

jtimon@a61cec8
jtimon@a408a5d
jtimon@5817f75
jtimon@6aed9aa
jtimon@0321823
jtimon@8c28d1a
jtimon@473696c

I believe #7552 should wait for a subset of those commits or equivalent to avoid duplicating code or exposing an incomplete API that is harder to change later.

Context: The main goal of this PR is advancing in libconsensus phase 2 decribed in the to-be-released-because-its-fortunately-already-outdated promised document with pictures. In the absence of a publishable document with pictures for a plan to encapsulate, expose and separate libbitcoinconsensus from bitcoin core, here's a summary including open PRs and other maintained branches:

** TODO libconsensus-p2: Put all the consensus critical code excluding storage in the consensus building package #NO_PR [no_branch]
*** REBASE [1/4] libconsensus-p2a: Build consensus/consensus.o with the consensus package #NO_PR [libconsensus-p2a]
Last version: 72cdd1f
**** PR bip9/bip113/libconsensus-p2a: Deployment preparations forBIP113 + #7552 + Introduce Consensus::VerifyTx() #7565 [libconsensus-p2a-verifytx-bip113-0.12.99]
**** REBASE libconsensus-p2a: Decouple pow.o from chain.o and move it to the consensus package #7563 [libconsensus-p2a-chain-cpp-interface-0.12.99]
**** REBASE libconsensus-p2a: Preparations to decouple libconsensus from coins.o #7564 [libconsensus-p2a-coins-cpp-interface-0.12.99]
** libconsensus-p3: Complete C API for consensus non-storage functionality (within the same package) #NO_PR [libconsensus-p3]
** libconsensus-p4: Separate libconsensus into its own repository like libsecp256k1 #NO_PR [no_branch]
** libconsensus-p5: Stop reusing libconsensus' internal code in Bitcoin Core
** libconsensus-p6: Import libconsensus from Bitcoin Core as an external dependency

legend:
[branch-name] = github/jtimon/bitcoin/branch-name
p2 = phase 2
p2a = phase 2-A

(Extracted from https://github.com/jtimon/doc/blob/master/branches/bitcoin.org if you use org-mode in emacs or vim)

@jtimon jtimon force-pushed the libconsensus-p2a-verifytx-bip113-0.12.99 branch 2 times, most recently from fd134a8 to 7bf7f1a Compare February 20, 2016 02:48
@jtimon jtimon force-pushed the libconsensus-p2a-verifytx-bip113-0.12.99 branch from 7bf7f1a to 13b7dbf Compare February 20, 2016 04:45
@jtimon jtimon force-pushed the libconsensus-p2a-verifytx-bip113-0.12.99 branch 2 times, most recently from cf97bbb to 8f58826 Compare February 23, 2016 00:35
…ts()

Also move the locktime flags with the script ones.
@jtimon jtimon force-pushed the libconsensus-p2a-verifytx-bip113-0.12.99 branch from 8f58826 to a355de8 Compare February 25, 2016 03:08
@jtimon
Copy link
Contributor Author

jtimon commented Mar 3, 2016

I don't know why travis is unhappy yet, but it doesn't seem like there's a lot of interest anyway. I will reopen when there's more interest or after the promised doc with pictures.

@jtimon jtimon closed this Mar 3, 2016
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants