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

Consensus: Remove ISM #8391

Merged
merged 1 commit into from
Aug 4, 2016
+83 −107
Merged

Consensus: Remove ISM #8391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 20 additions & 20 deletions qa/rpc-tests/bip65-cltv-p2p.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ def get_tests(self):
self.nodeaddress = self.nodes[0].getnewaddress()
self.last_block_time = int(time.time())

''' 98 more version 3 blocks '''
''' 398 more version 3 blocks '''
test_blocks = []
for i in range(98):
for i in range(398):
block = create_block(self.tip, create_coinbase(height), self.last_block_time + 1)
block.nVersion = 3
block.rehash()
Expand Down Expand Up @@ -118,24 +118,6 @@ def get_tests(self):
height += 1
yield TestInstance([[block, True]])

'''
Check that the new CLTV rules are enforced in the 751st version 4
block.
'''
spendtx = self.create_transaction(self.nodes[0],
self.coinbase_blocks[1], self.nodeaddress, 1.0)
cltv_invalidate(spendtx)
spendtx.rehash()

block = create_block(self.tip, create_coinbase(height), self.last_block_time + 1)
block.nVersion = 4
block.vtx.append(spendtx)
block.hashMerkleRoot = block.calc_merkle_root()
block.rehash()
block.solve()
self.last_block_time += 1
yield TestInstance([[block, False]])

''' Mine 199 new version blocks on last valid tip '''
test_blocks = []
for i in range(199):
Expand Down Expand Up @@ -169,6 +151,24 @@ def get_tests(self):
height += 1
yield TestInstance([[block, True]])

'''
Check that the new CLTV rules are enforced in the 951st version 4
block.
'''
spendtx = self.create_transaction(self.nodes[0],
self.coinbase_blocks[1], self.nodeaddress, 1.0)
cltv_invalidate(spendtx)
spendtx.rehash()

block = create_block(self.tip, create_coinbase(height), self.last_block_time + 1)
block.nVersion = 4
block.vtx.append(spendtx)
block.hashMerkleRoot = block.calc_merkle_root()
block.rehash()
block.solve()
self.last_block_time += 1
yield TestInstance([[block, False]])

''' Mine 1 old version block, should be invalid '''
block = create_block(self.tip, create_coinbase(height), self.last_block_time + 1)
block.nVersion = 3
Expand Down
3 changes: 2 additions & 1 deletion qa/rpc-tests/bip65-cltv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ def run_test(self):
cnt = self.nodes[0].getblockcount()

# Mine some old-version blocks
self.nodes[1].generate(100)
self.nodes[1].generate(200)
cnt += 100
self.sync_all()
if (self.nodes[0].getblockcount() != cnt + 100):
raise AssertionError("Failed to mine 100 version=3 blocks")
Expand Down
42 changes: 21 additions & 21 deletions qa/rpc-tests/bipdersig-p2p.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ def get_tests(self):
self.nodeaddress = self.nodes[0].getnewaddress()
self.last_block_time = int(time.time())

''' 98 more version 2 blocks '''
''' 298 more version 2 blocks '''
test_blocks = []
for i in range(98):
for i in range(298):
block = create_block(self.tip, create_coinbase(height), self.last_block_time + 1)
block.nVersion = 2
block.rehash()
Expand Down Expand Up @@ -124,25 +124,7 @@ def get_tests(self):
self.last_block_time += 1
self.tip = block.sha256
height += 1
yield TestInstance([[block, True]])

'''
Check that the new DERSIG rules are enforced in the 751st version 3
block.
'''
spendtx = self.create_transaction(self.nodes[0],
self.coinbase_blocks[1], self.nodeaddress, 1.0)
unDERify(spendtx)
spendtx.rehash()

block = create_block(self.tip, create_coinbase(height), self.last_block_time + 1)
block.nVersion = 3
block.vtx.append(spendtx)
block.hashMerkleRoot = block.calc_merkle_root()
block.rehash()
block.solve()
self.last_block_time += 1
yield TestInstance([[block, False]])
yield TestInstance([[block, True]])

''' Mine 199 new version blocks on last valid tip '''
test_blocks = []
Expand Down Expand Up @@ -177,6 +159,24 @@ def get_tests(self):
height += 1
yield TestInstance([[block, True]])

'''
Check that the new DERSIG rules are enforced in the 951st version 3
block.
'''
spendtx = self.create_transaction(self.nodes[0],
self.coinbase_blocks[1], self.nodeaddress, 1.0)
unDERify(spendtx)
spendtx.rehash()

block = create_block(self.tip, create_coinbase(height), self.last_block_time + 1)
block.nVersion = 3
block.vtx.append(spendtx)
block.hashMerkleRoot = block.calc_merkle_root()
block.rehash()
block.solve()
self.last_block_time += 1
yield TestInstance([[block, False]])

''' Mine 1 old version block, should be invalid '''
block = create_block(self.tip, create_coinbase(height), self.last_block_time + 1)
block.nVersion = 2
Expand Down
17 changes: 7 additions & 10 deletions src/chainparams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,10 @@ class CMainParams : public CChainParams {
CMainParams() {
strNetworkID = "main";
consensus.nSubsidyHalvingInterval = 210000;
consensus.nMajorityEnforceBlockUpgrade = 750;
consensus.nMajorityRejectBlockOutdated = 950;
consensus.nMajorityWindow = 1000;
consensus.BIP34Height = 227931;
consensus.BIP34Hash = uint256S("0x000000000000024b89b42a942fe0d9fea3bb44ab7bd1b19115dd6a759c0808b8");
consensus.BIP65Height = 388381; // 000000000000000004c2b624ed5d7756c508d90fd0da2c7c679febfa6c4735f0
consensus.BIP66Height = 363725; // 00000000000000000379eaa19dce8c9b722d46ae6a57c2f1a988119488b50931
consensus.powLimit = uint256S("00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks
consensus.nPowTargetSpacing = 10 * 60;
Expand Down Expand Up @@ -167,11 +166,10 @@ class CTestNetParams : public CChainParams {
CTestNetParams() {
strNetworkID = "test";
consensus.nSubsidyHalvingInterval = 210000;
consensus.nMajorityEnforceBlockUpgrade = 51;
consensus.nMajorityRejectBlockOutdated = 75;
consensus.nMajorityWindow = 100;
consensus.BIP34Height = 21111;
consensus.BIP34Hash = uint256S("0x0000000023b3a96d3484e5abb3755c413e7d41500f8e2a5c3f0dd01299cd8ef8");
consensus.BIP65Height = 581885; // 00000000007f6655f22f98e72ed80d8b06dc761d5da09df0fa1dc4be4f861eb6
consensus.BIP66Height = 330776; // 000000002104c8c45e99a8853285a3b592602a3ccde2b832481da85e9e4ba182
Copy link
Member

Choose a reason for hiding this comment

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

Why is it not possible to check the hash for main and test?

Copy link
Member

Choose a reason for hiding this comment

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

Where would that be useful? The hash is only used as an exception to BIP30 rule at https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L2358

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, in fact, what that's doing is that if we're past BIP34Height but at BIP34Height we don't have the hardcoded block, then keep checking bip30 as if bip34 hadn't been activated.
But now in https://github.com/bitcoin/bitcoin/pull/8391/files#diff-7ec3c68a81efff79b6ca22ac1f1eabbaL3550 if we have a reorg below BIP34Height, BIP34 will be activated after BIP34Height regardless of the original activation being different, thus the optimization of not checking BIP30 if BIP34 is activated could be used as well. So BIP34Hash can go after this change, I believe.
If we have a reorg to BIP34Height things are terribly broken anyway.

consensus.powLimit = uint256S("00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks
consensus.nPowTargetSpacing = 10 * 60;
Expand Down Expand Up @@ -247,11 +245,10 @@ class CRegTestParams : public CChainParams {
CRegTestParams() {
strNetworkID = "regtest";
consensus.nSubsidyHalvingInterval = 150;
consensus.nMajorityEnforceBlockUpgrade = 750;
consensus.nMajorityRejectBlockOutdated = 950;
consensus.nMajorityWindow = 1000;
consensus.BIP34Height = -1; // BIP34 has not necessarily activated on regtest
consensus.BIP34Height = 100000000; // BIP34 has not activated on regtest (far in the future so block v1 are not rejected in tests)
consensus.BIP34Hash = uint256();
consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in rpc activation tests)
consensus.BIP66Height = 1251; // BIP66 activated on regtest (Used in rpc activation tests)
consensus.powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks
consensus.nPowTargetSpacing = 10 * 60;
Expand Down
8 changes: 4 additions & 4 deletions src/consensus/params.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ struct BIP9Deployment {
struct Params {
uint256 hashGenesisBlock;
int nSubsidyHalvingInterval;
/** Used to check majorities for block version upgrade */
int nMajorityEnforceBlockUpgrade;
int nMajorityRejectBlockOutdated;
int nMajorityWindow;
/** Block height and hash at which BIP34 becomes active */
int BIP34Height;
uint256 BIP34Hash;
/** Block height at which BIP65 becomes active */
Copy link
Contributor

Choose a reason for hiding this comment

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

We can make an array with an enum, like with vDeployments. Say, vPastDeployments, vOldDeployments, vBuriedDeployments, or something of the sort.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer keeping change of logic and refactoring done in different PR. I want this PR to be easy to review, so I'm using the previous way of doing with BIP34Height.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, you can leave BIP34 as it is and put the new things in the array if you think that is less disruptive, no?
Anyway, if you feel strongly against doing it now, we can do it later. It's just feel cleaner not to put code to destroy it "right afterwards" (I can foresee the same "we can do the refactor later" argument when moving BIP113 there...).

int BIP65Height;
/** Block height at which BIP66 becomes active */
int BIP66Height;
/**
* Minimum blocks including miner confirmation of the total of 2016 blocks in a retargetting period,
* (nPowTargetTimespan / nPowTargetSpacing) which is also used for BIP9 deployments.
Expand Down
44 changes: 13 additions & 31 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,6 @@ map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(cs_main);
map<COutPoint, set<map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(cs_main);
void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* Returns true if there are nRequired or more blocks of minVersion or above
* in the last Consensus::Params::nMajorityWindow blocks, starting at pstart and going backwards.
*/
static bool IsSuperMajority(int minVersion, const CBlockIndex* pstart, unsigned nRequired, const Consensus::Params& consensusParams);
static void CheckBlockIndex(const Consensus::Params& consensusParams);

/** Constant stuff for coinbase transactions we create: */
Expand Down Expand Up @@ -2372,15 +2367,13 @@ 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:
if (block.nVersion >= 3 && IsSuperMajority(3, pindex->pprev, chainparams.GetConsensus().nMajorityEnforceBlockUpgrade, chainparams.GetConsensus())) {
// Start enforcing the DERSIG (BIP66) rule
if (pindex->nHeight >= chainparams.GetConsensus().BIP66Height) {
Copy link
Member

@instagibbs instagibbs Jul 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

nit: Any reason not to leave version check in? The fewer differences the better, imo. (clearly this isn't a HF, just asking)

Copy link
Contributor Author

@NicolasDorier NicolasDorier Jul 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, will later make refactoring for libconsensus easier because this does not depend on Block anymore.
Moreover, it is impossible that block.nVersion to be below 3 at this point. I'm using the 95% threshold for this test (compared to the 75% of before), so if the block.nVersion were below 3 it would have been rejected before by https://github.com/bitcoin/bitcoin/pull/8391/files#diff-7ec3c68a81efff79b6ca22ac1f1eabbaR3516

Copy link
Member

@sipa sipa Jul 22, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@sipa? Hm? I was just asking about nVersion check.

That said, future refactoring purposes is good enough for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, as @NicolasDorier points out the block.nVersion check can be safely removed from here because blocks with invalid versions will be rejected in ContextualCheckBlockHeader.

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())) {
// Start enforcing CHECKLOCKTIMEVERIFY (BIP65) rule
if (pindex->nHeight >= chainparams.GetConsensus().BIP65Height) {
flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY;
}

Expand Down Expand Up @@ -3504,6 +3497,7 @@ std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBloc

bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev, int64_t nAdjustedTime)
{
const int nHeight = pindexPrev == NULL ? 0 : pindexPrev->nHeight + 1;
// Check proof of work
if (block.nBits != GetNextWorkRequired(pindexPrev, &block, consensusParams))
return state.DoS(100, false, REJECT_INVALID, "bad-diffbits", false, "incorrect proof of work");
Expand All @@ -3517,10 +3511,12 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta
return state.Invalid(false, REJECT_INVALID, "time-too-new", "block timestamp too far in the future");

// Reject outdated version blocks when 95% (75% on testnet) of the network has upgraded:
for (int32_t version = 2; version < 5; ++version) // check for version 2, 3 and 4 upgrades
if (block.nVersion < version && IsSuperMajority(version, pindexPrev, consensusParams.nMajorityRejectBlockOutdated, consensusParams))
return state.Invalid(false, REJECT_OBSOLETE, strprintf("bad-version(0x%08x)", version - 1),
strprintf("rejected nVersion=0x%08x block", version - 1));
// check for version 2, 3 and 4 upgrades
if((block.nVersion < 2 && nHeight >= consensusParams.BIP34Height) ||
(block.nVersion < 3 && nHeight >= consensusParams.BIP66Height) ||
(block.nVersion < 4 && nHeight >= consensusParams.BIP65Height))
return state.Invalid(false, REJECT_OBSOLETE, strprintf("bad-version(0x%08x)", block.nVersion),
strprintf("rejected nVersion=0x%08x block", block.nVersion));

return true;
}
Expand All @@ -3547,9 +3543,8 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const CB
}
}

// Enforce block.nVersion=2 rule that the coinbase starts with serialized block height
// if 750 of the last 1,000 blocks are version 2 or greater (51/100 if testnet):
if (block.nVersion >= 2 && IsSuperMajority(2, pindexPrev, consensusParams.nMajorityEnforceBlockUpgrade, consensusParams))
// Enforce rule that the coinbase starts with serialized block height
if (nHeight >= consensusParams.BIP34Height)
{
CScript expect = CScript() << nHeight;
if (block.vtx[0].vin[0].scriptSig.size() < expect.size() ||
Expand Down Expand Up @@ -3722,19 +3717,6 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha
return true;
}

static bool IsSuperMajority(int minVersion, const CBlockIndex* pstart, unsigned nRequired, const Consensus::Params& consensusParams)
{
unsigned int nFound = 0;
for (int i = 0; i < consensusParams.nMajorityWindow && nFound < nRequired && pstart != NULL; i++)
{
if (pstart->nVersion >= minVersion)
++nFound;
pstart = pstart->pprev;
}
return (nFound >= nRequired);
}


bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp)
{
{
Expand Down
36 changes: 16 additions & 20 deletions src/rpc/blockchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -817,22 +817,23 @@ UniValue verifychain(const UniValue& params, bool fHelp)
}

/** Implementation of IsSuperMajority with better feedback */
static UniValue SoftForkMajorityDesc(int minVersion, CBlockIndex* pindex, int nRequired, const Consensus::Params& consensusParams)
static UniValue SoftForkMajorityDesc(int version, CBlockIndex* pindex, const Consensus::Params& consensusParams)
{
int nFound = 0;
CBlockIndex* pstart = pindex;
for (int i = 0; i < consensusParams.nMajorityWindow && pstart != NULL; i++)
UniValue rv(UniValue::VOBJ);
bool activated = false;
switch(version)
{
if (pstart->nVersion >= minVersion)
++nFound;
pstart = pstart->pprev;
case 2:
activated = pindex->nHeight >= consensusParams.BIP34Height;
break;
case 3:
activated = pindex->nHeight >= consensusParams.BIP66Height;
break;
case 4:
activated = pindex->nHeight >= consensusParams.BIP65Height;
break;
}

UniValue rv(UniValue::VOBJ);
rv.push_back(Pair("status", nFound >= nRequired));
rv.push_back(Pair("found", nFound));
rv.push_back(Pair("required", nRequired));
rv.push_back(Pair("window", consensusParams.nMajorityWindow));
rv.push_back(Pair("status", activated));
return rv;
}

Expand All @@ -841,8 +842,7 @@ static UniValue SoftForkDesc(const std::string &name, int version, CBlockIndex*
UniValue rv(UniValue::VOBJ);
rv.push_back(Pair("id", name));
rv.push_back(Pair("version", version));
rv.push_back(Pair("enforce", SoftForkMajorityDesc(version, pindex, consensusParams.nMajorityEnforceBlockUpgrade, consensusParams)));
rv.push_back(Pair("reject", SoftForkMajorityDesc(version, pindex, consensusParams.nMajorityRejectBlockOutdated, consensusParams)));
rv.push_back(Pair("reject", SoftForkMajorityDesc(version, pindex, consensusParams)));
return rv;
}

Expand Down Expand Up @@ -897,13 +897,9 @@ UniValue getblockchaininfo(const UniValue& params, bool fHelp)
" {\n"
" \"id\": \"xxxx\", (string) name of softfork\n"
" \"version\": xx, (numeric) block version\n"
" \"enforce\": { (object) progress toward enforcing the softfork rules for new-version blocks\n"
" \"reject\": { (object) progress toward rejecting pre-softfork blocks\n"
" \"status\": xx, (boolean) true if threshold reached\n"
" \"found\": xx, (numeric) number of blocks with the new version found\n"
" \"required\": xx, (numeric) number of blocks required to trigger\n"
" \"window\": xx, (numeric) maximum size of examined window of recent blocks\n"
" },\n"
" \"reject\": { ... } (object) progress toward rejecting pre-softfork blocks (same fields as \"enforce\")\n"
" }, ...\n"
" ],\n"
" \"bip9_softforks\": { (object) status of BIP9 softforks in progress\n"
Expand Down