Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHECKLOCKTIMEVERIFY (BIP65) IsSuperMajority() soft-fork #6351

Merged
merged 3 commits into from
Oct 23, 2015
Merged

CHECKLOCKTIMEVERIFY (BIP65) IsSuperMajority() soft-fork #6351

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
287f54f
Add CHECKLOCKTIMEVERIFY (BIP65) soft-fork logic
petertodd Jun 28, 2015
cde7ab2
Add RPC tests for the CHECKLOCKTIMEVERIFY (BIP65) soft-fork
petertodd Jun 28, 2015
65ef372
Add BIP65 to getblockchaininfo softforks list
petertodd Sep 27, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add CHECKLOCKTIMEVERIFY (BIP65) soft-fork logic 
Based on the earlier BIP66 soft-fork logic implemented by Pieter
Wuille's 5a47811
  • Loading branch information
@petertodd
petertodd committed Oct 8, 2015
commit 287f54fc90c29301faede8d4ac2ea24a91441917
14 changes: 13 additions & 1 deletion src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1740,11 +1740,18 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin

unsigned int flags = fStrictPayToScriptHash ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE;

// Start enforcing the DERSIG (BIP66) rules, for block.nVersion=3 blocks, when 75% of the network has upgraded:
// Start enforcing the DERSIG (BIP66) rules, for block.nVersion=3 blocks,
// when 75% of the network has upgraded:
if (block.nVersion >= 3 && IsSuperMajority(3, pindex->pprev, chainparams.GetConsensus().nMajorityEnforceBlockUpgrade, chainparams.GetConsensus())) {
flags |= SCRIPT_VERIFY_DERSIG;
}

// Start enforcing CHECKLOCKTIMEVERIFY, (BIP65) for block.nVersion=4
// blocks, when 75% of the network has upgraded:
if (block.nVersion >= 4 && IsSuperMajority(4, pindex->pprev, chainparams.GetConsensus().nMajorityEnforceBlockUpgrade, chainparams.GetConsensus())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't know why we wait for 75% to start using a policy rule.
I would do it from the start but only enforce it as a consensus rule after 95%.
I don't want to slow this down since it is a general softfork question (even if I'm right it can be solved after this PR), but I haven't been able to find an answer anywhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a policy rule.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While individual miners apply it but still not enforce it as a consensus rule (ie reject blocks that don't apply it), it is just mining policy.
What am I missing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seriously, what am I missing here? Why wait for 75% for enforcing the new rule in your own blocks (which by the way you are marking as being version 4)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is executed against all blocks, not just blocks you created. If another miner creates a nVersion=4 block with an invalid use of CHECKLOCKTIMEVERIFY if you reject it without a majority of hashing power also rejecting it you'll get forked.

In fact, it might be a good idea to go the other way and remove the 75% rule, accepting invalid nVersion=4 blocks until 95%

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, for some reason I was fixated on this 75% threshold only affecting your own blocks, but this is ConnectBlock so it obviously affects all blocks. Yes, I wouldn't oppose to wait for the 95% here too. But with versionbits this disappears so is probably not worth to discuss this much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I probably would have changed it to remove 75% a year ago... but at this stage I think it's harmless to leave in.

flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY;
}

CBlockUndo blockundo;

CCheckQueueControl<CScriptCheck> control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL);
Expand Down Expand Up @@ -2684,6 +2691,11 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta
return state.Invalid(error("%s : rejected nVersion=2 block", __func__),
REJECT_OBSOLETE, "bad-version");

// Reject block.nVersion=3 blocks when 95% (75% on testnet) of the network has upgraded:
if (block.nVersion < 4 && IsSuperMajority(4, pindexPrev, consensusParams.nMajorityRejectBlockOutdated, consensusParams))
return state.Invalid(error("%s : rejected nVersion=3 block", __func__),
REJECT_OBSOLETE, "bad-version");

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/primitives/block.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class CBlockHeader
{
public:
// header
static const int32_t CURRENT_VERSION=3;
static const int32_t CURRENT_VERSION=4;
int32_t nVersion;
uint256 hashPrevBlock;
uint256 hashMerkleRoot;
Expand Down
7 changes: 4 additions & 3 deletions src/script/bitcoinconsensus.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ typedef enum bitcoinconsensus_error_t
/** Script verification flags */
enum
{
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_NONE = 0,
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH = (1U << 0), // evaluate P2SH (BIP16) subscripts
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_DERSIG = (1U << 2), // enforce strict DER (BIP66) compliance
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_NONE = 0,
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH = (1U << 0), // evaluate P2SH (BIP16) subscripts
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_DERSIG = (1U << 2), // enforce strict DER (BIP66) compliance
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKLOCKTIMEVERIFY = (1U << 9), // enable CHECKLOCKTIMEVERIFY (BIP65)
};

/// Returns 1 if the input nIn of the serialized transaction pointed to by
Expand Down