Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Enforce mandatory softfork flags for segwit block/tx #8635
Conversation
fanquake
added the
TX fees and policy
label
Aug 31, 2016
sipa
commented on an outdated diff
Sep 5, 2016
| @@ -2405,6 +2408,11 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin | ||
| std::vector<std::pair<uint256, CDiskTxPos> > vPos; | ||
| vPos.reserve(block.vtx.size()); | ||
| blockundo.vtxundo.reserve(block.vtx.size() - 1); | ||
| + bool fHaveWitness = false; |
sipa
Owner
|
sipa
commented on an outdated diff
Sep 5, 2016
| // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we | ||
| // need to turn both off, and compare against just turning off CLEANSTACK | ||
| // to see if the failure is specifically due to witness validation. | ||
| - if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true) && | ||
| - !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true)) { | ||
| + if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, NULL, fHaveWitness) && |
sipa
Owner
|
|
Rebased and removed the codes for block validation |
|
Is this a target of 0.13.1? |
laanwj
added this to the
0.13.1
milestone
Sep 8, 2016
MarcoFalke
added the
Needs backport
label
Sep 9, 2016
|
Looks like you snagged a couple commits from another PR in here. That intentional? Following this logic, any follow-up softfork from Core codebase would assume previous softforks will be followed then? |
|
@instagibbs I just rebased on top of that PR. It was not merged at that time. Should I rebase again? This one is special. Since segwit is a completely new tx format, there is no excuse for not respecting any known softforks, which have been enforced for months. Following this logic, it should have been mandatory for version 2 transactions to follow all existing softforks (expect segwit), since no one should produce or relay v2 tx without knowing BIP68. However, before the CSV softfork, we couldn't simply enforce mandatory flags since no change had been made in transaction version or format. |
jonasschnelli
removed this from the
0.13.1
milestone
Sep 15, 2016
MarcoFalke
removed the
Needs backport
label
Sep 20, 2016
|
Needs rebase. Not sure what the real value of this is, though...why is it a serious offense to relay such blocks? (we'd need careful to avoid any potential network-splitting issues with compact-fast-relay). |
jl2012 commentedAug 31, 2016
(untested) If a node is able to relay a block/tx with witness, it must know all existing script softfork rules: BIP66, 65, 112, 141, 143. All related verify flags should be mandatory for such blocks/txs