Skip to content

[0.17] blockheaders include block height, nodes validate it #431

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.

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

Merged
merged 2 commits into from
Nov 27, 2018
Merged
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions src/chain.h
Original file line number Diff line number Diff line change
@@ -210,6 +210,7 @@ class CBlockIndex
int32_t nVersion;
uint256 hashMerkleRoot;
uint32_t nTime;
uint32_t block_height;
uint32_t nBits;
uint32_t nNonce;

@@ -238,6 +239,7 @@ class CBlockIndex
nVersion = 0;
hashMerkleRoot = uint256();
nTime = 0;
block_height = 0;
nBits = 0;
nNonce = 0;
}
@@ -254,6 +256,7 @@ class CBlockIndex
nVersion = block.nVersion;
hashMerkleRoot = block.hashMerkleRoot;
nTime = block.nTime;
block_height = block.block_height;
nBits = block.nBits;
nNonce = block.nNonce;
}
@@ -284,6 +287,7 @@ class CBlockIndex
block.hashPrevBlock = pprev->GetBlockHash();
block.hashMerkleRoot = hashMerkleRoot;
block.nTime = nTime;
block.block_height = block_height;
block.nBits = nBits;
block.nNonce = nNonce;
return block;
@@ -403,6 +407,7 @@ class CDiskBlockIndex : public CBlockIndex
READWRITE(hashPrev);
READWRITE(hashMerkleRoot);
READWRITE(nTime);
READWRITE(block_height);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why this doesn't need to be conditional. Bitcoin block indexes sure won't have height here...

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure myself, but Greg discussed it with Jorge here: #431 (comment)

READWRITE(nBits);
READWRITE(nNonce);
}
@@ -414,6 +419,7 @@ class CDiskBlockIndex : public CBlockIndex
block.hashPrevBlock = hashPrev;
block.hashMerkleRoot = hashMerkleRoot;
block.nTime = nTime;
block.block_height = block_height;
block.nBits = nBits;
block.nNonce = nNonce;
return block.GetHash();
4 changes: 4 additions & 0 deletions src/chainparams.cpp
Original file line number Diff line number Diff line change
@@ -435,6 +435,10 @@ class CCustomParams : public CRegTestParams {
// No subsidy for custom chains by default
consensus.genesis_subsidy = args.GetArg("-con_blocksubsidy", 0);

// Note: This global is needed to avoid circular dependency
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

with chainparams (ie if the global was a field in chainparams), because chainparams depends on block.

// Defaults to true for custom chains.
g_con_blockheightinheader = gArgs.GetBoolArg("-con_blockheightinheader", true);

// All non-zero coinbase outputs must go to this scriptPubKey
std::vector<unsigned char> man_bytes = ParseHex(gArgs.GetArg("-con_mandatorycoinbase", ""));
consensus.mandatory_coinbase_destination = CScript(man_bytes.begin(), man_bytes.end()); // Blank script allows any coinbase destination
1 change: 1 addition & 0 deletions src/chainparamsbase.cpp
Original file line number Diff line number Diff line change
@@ -26,6 +26,7 @@ void SetupChainParamsBaseOptions()
gArgs.AddArg("-con_mandatorycoinbase", "All non-zero valued coinbase outputs must go to this scriptPubKey, if set.", false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-con_blocksubsidy", "Defines the amount of block subsidy to start with, at genesis block.", false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-con_connect_coinbase", "Connect outputs in genesis block to utxo database.", false, OptionsCategory::ELEMENTS);
gArgs.AddArg("-con_blockheightinheader", "Whether the chain includes the block height directly in the header, for easier validation of block height in low-resource environments. (default: true)", false, OptionsCategory::CHAINPARAMS);
}

static std::unique_ptr<CBaseChainParams> globalChainBaseParams;
2 changes: 1 addition & 1 deletion src/consensus/params.h
Original file line number Diff line number Diff line change
@@ -78,11 +78,11 @@ struct Params {
int64_t DifficultyAdjustmentInterval() const { return nPowTargetTimespan / nPowTargetSpacing; }
uint256 nMinimumChainWork;
uint256 defaultAssumeValid;

// Elements-specific chainparams
CScript mandatory_coinbase_destination;
CAmount genesis_subsidy;
bool connect_genesis_outputs;
// g_con_blockheightinheader global hack instead of proper arg due to circular dep
};
} // namespace Consensus

3 changes: 3 additions & 0 deletions src/miner.cpp
Original file line number Diff line number Diff line change
@@ -168,6 +168,9 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
// Fill in header
pblock->hashPrevBlock = pindexPrev->GetBlockHash();
UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev);
if (g_con_blockheightinheader) {
pblock->block_height = nHeight;
Copy link
Contributor

Choose a reason for hiding this comment

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

Assignment probably doesn't need to be conditional.

}
pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, chainparams.GetConsensus());
pblock->nNonce = 0;
pblocktemplate->vTxSigOpsCost[0] = WITNESS_SCALE_FACTOR * GetLegacySigOpCount(*pblock->vtx[0]);
2 changes: 2 additions & 0 deletions src/primitives/block.cpp
Original file line number Diff line number Diff line change
@@ -10,6 +10,8 @@
#include <utilstrencodings.h>
#include <crypto/common.h>

bool g_con_blockheightinheader = false;

uint256 CBlockHeader::GetHash() const
{
return SerializeHash(*this);
10 changes: 10 additions & 0 deletions src/primitives/block.h
Original file line number Diff line number Diff line change
@@ -10,6 +10,8 @@
#include <serialize.h>
#include <uint256.h>

extern bool g_con_blockheightinheader;

/** Nodes collect new transactions into a block, hash them into a hash tree,
* and scan through nonce values to make the block's hash satisfy proof-of-work
* requirements. When they solve the proof-of-work, they broadcast the block
@@ -25,6 +27,9 @@ class CBlockHeader
uint256 hashPrevBlock;
uint256 hashMerkleRoot;
uint32_t nTime;
// Height in header as well as in coinbase for easier hsm validation
// Is set for serialization with `-con_blockheightinheader=1`
uint32_t block_height;
uint32_t nBits;
uint32_t nNonce;

@@ -41,6 +46,9 @@ class CBlockHeader
READWRITE(hashPrevBlock);
READWRITE(hashMerkleRoot);
READWRITE(nTime);
if (g_con_blockheightinheader) {
READWRITE(block_height);
}
READWRITE(nBits);
READWRITE(nNonce);
}
@@ -51,6 +59,7 @@ class CBlockHeader
hashPrevBlock.SetNull();
hashMerkleRoot.SetNull();
nTime = 0;
block_height = 0;
nBits = 0;
nNonce = 0;
}
@@ -111,6 +120,7 @@ class CBlock : public CBlockHeader
block.hashPrevBlock = hashPrevBlock;
block.hashMerkleRoot = hashMerkleRoot;
block.nTime = nTime;
block.block_height = block_height;
block.nBits = nBits;
block.nNonce = nNonce;
return block;
1 change: 1 addition & 0 deletions src/txdb.cpp
Original file line number Diff line number Diff line change
@@ -263,6 +263,7 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams,
CBlockIndex* pindexNew = insertBlockIndex(diskindex.GetBlockHash());
pindexNew->pprev = insertBlockIndex(diskindex.hashPrev);
pindexNew->nHeight = diskindex.nHeight;
pindexNew->block_height = diskindex.block_height;
pindexNew->nFile = diskindex.nFile;
pindexNew->nDataPos = diskindex.nDataPos;
pindexNew->nUndoPos = diskindex.nUndoPos;
5 changes: 5 additions & 0 deletions src/validation.cpp
Original file line number Diff line number Diff line change
@@ -3271,6 +3271,11 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta
if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast())
return state.Invalid(false, REJECT_INVALID, "time-too-old", "block's timestamp is too early");

// Check height in header against prev
if (g_con_blockheightinheader && (uint32_t)nHeight != block.block_height)
return state.Invalid(error("%s: block height in header is incorrect", __func__),
REJECT_INVALID, "bad-header-height");

// Check timestamp
if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME)
return state.Invalid(false, REJECT_INVALID, "time-too-new", "block timestamp too far in the future");
1 change: 1 addition & 0 deletions test/bitcoin_functional/functional/test_framework/util.py
Original file line number Diff line number Diff line change
@@ -308,6 +308,7 @@ def initialize_datadir(dirname, n, chain):
f.write("con_blocksubsidy=5000000000\n")
f.write("con_connect_coinbase=0\n")
f.write("anyonecanspendaremine=0\n")
f.write("con_blockheightinheader=0\n")
os.makedirs(os.path.join(datadir, 'stderr'), exist_ok=True)
os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True)
return datadir
Loading