Skip to content

Commit

Permalink
Implement accurate sigop / sighashbytes counting block consensus rules
Browse files Browse the repository at this point in the history
Replace the inaccurate / somewhat ineffective consensus rule for
number of signature operations per block with new consensus rules
that accurately count the number of ECDSA signature operations needed
to validate a block, and the number of bytes of data needed to compute
signature hashes (to mitigate the attack described in CVE-2013-2292).

BIP number for this to be determined. Constants were chosen such that
any 'non-attack' transaction valid under the old rules is also valid
under the new rules, but maximum possible block validation times
are well-bounded, but tied to block size increases.

Summary of old rules / new rules:

Old rules: 20,000 inaccurately-counted-sigops for a 1MB block
New: 80,000 accurately-counted sigops for an 8MB block

A scan of the last 100,000 blocks for high-sigop blocks gets
a maximum of 7,350 sigops in block 364,773 (in a single, huge,
~1MB transaction).

For reference, Pieter Wuille's libsecp256k1 validation code
validates about 10,000 signatures per second on a single
2.7GHZ CPU core.

Old rules: no limit for number of bytes hashed to generate
signature hashes

New rule: 1.3gigabytes hashed per 8MB block to generate
signature hashes

Block 364,422 contains a single ~1MB transaction that requires
1.2GB of data hashed to generate signature hashes.
  • Loading branch information
gavinandresen committed Jul 27, 2015
1 parent 9a133c8 commit 4eec022
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 12 deletions.
21 changes: 18 additions & 3 deletions src/consensus/params.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,25 @@ struct Params {
uint64_t nMaxSize = (nMaxSizeBase << doublings) + interpolate;
return nMaxSize;
}
/** Maximum number of signature ops in a block with timestamp nBlockTimestamp */
uint64_t MaxBlockSigops(uint64_t nBlockTimestamp, uint64_t nSizeForkActivationTime) const {
return MaxBlockSize(nBlockTimestamp, nSizeForkActivationTime)/50;
/** Pre-fork consensus rules use an inaccurate method of counting sigops **/
uint64_t MaxBlockLegacySigops(uint64_t nBlockTimestamp, uint64_t nSizeForkActivationTime) const {
if (nBlockTimestamp < nEarliestSizeForkTime || nBlockTimestamp < nSizeForkActivationTime)
return MaxBlockSize(nBlockTimestamp, nSizeForkActivationTime)/50;
return std::numeric_limits<uint64_t>::max(); // Post-fork uses accurate method
}
/** Post-fork consensus rule uses an accurate method of counting sigops **/
uint64_t MaxBlockAccurateSigops(uint64_t nBlockTimestamp, uint64_t nSizeForkActivationTime) const {
if (nBlockTimestamp < nEarliestSizeForkTime || nBlockTimestamp < nSizeForkActivationTime)
return std::numeric_limits<uint64_t>::max(); // Pre-fork doesn't care
return MaxBlockSize(nBlockTimestamp, nSizeForkActivationTime)/100;
}
/** Post-fork consensus rule to mitigate CVE-2013-2292 **/
uint64_t MaxBlockSighashBytes(uint64_t nBlockTimestamp, uint64_t nSizeForkActivationTime) const {
if (nBlockTimestamp < nEarliestSizeForkTime || nBlockTimestamp < nSizeForkActivationTime)
return std::numeric_limits<uint64_t>::max(); // Pre-fork doesn't care
return MaxBlockSize(nBlockTimestamp, nSizeForkActivationTime)*160;
}

int ActivateSizeForkMajority() const { return nActivateSizeForkMajority; }
uint64_t SizeForkGracePeriod() const { return nSizeForkGracePeriod; }
};
Expand Down
10 changes: 6 additions & 4 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1921,7 +1921,9 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin

CBlockUndo blockundo;

BlockValidationResourceTracker resourceTracker(std::numeric_limits<size_t>::max(), std::numeric_limits<size_t>::max());
uint64_t maxAccurateSigops = chainparams.GetConsensus().MaxBlockAccurateSigops(block.GetBlockTime(), sizeForkTime.load());
uint64_t maxSighashBytes = chainparams.GetConsensus().MaxBlockSighashBytes(block.GetBlockTime(), sizeForkTime.load());
BlockValidationResourceTracker resourceTracker(maxAccurateSigops, maxSighashBytes);
CCheckQueueControl<CScriptCheck> control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL);

int64_t nTimeStart = GetTimeMicros();
Expand All @@ -1938,7 +1940,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin

nInputs += tx.vin.size();
nSigOps += GetLegacySigOpCount(tx);
if (nSigOps > chainparams.GetConsensus().MaxBlockSigops(block.GetBlockTime(), sizeForkTime.load()))
if (nSigOps > chainparams.GetConsensus().MaxBlockLegacySigops(block.GetBlockTime(), sizeForkTime.load()))
return state.DoS(100, error("ConnectBlock(): too many sigops"),
REJECT_INVALID, "bad-blk-sigops");

Expand All @@ -1954,7 +1956,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
// this is to prevent a "rogue miner" from creating
// an incredibly-expensive-to-validate block.
nSigOps += GetP2SHSigOpCount(tx, view);
if (nSigOps > chainparams.GetConsensus().MaxBlockSigops(block.GetBlockTime(), sizeForkTime.load()))
if (nSigOps > chainparams.GetConsensus().MaxBlockLegacySigops(block.GetBlockTime(), sizeForkTime.load()))
return state.DoS(100, error("ConnectBlock(): too many sigops"),
REJECT_INVALID, "bad-blk-sigops");
}
Expand Down Expand Up @@ -2818,7 +2820,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
{
nSigOps += GetLegacySigOpCount(tx);
}
if (nSigOps > Params().GetConsensus().MaxBlockSigops(block.GetBlockTime(), sizeForkTime.load()))
if (nSigOps > Params().GetConsensus().MaxBlockLegacySigops(block.GetBlockTime(), sizeForkTime.load()))
return state.DoS(100, error("CheckBlock(): out-of-bounds SigOpCount"),
REJECT_INVALID, "bad-blk-sigops", true);

Expand Down
16 changes: 12 additions & 4 deletions src/miner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
if(!pblocktemplate.get())
return NULL;
CBlock *pblock = &pblocktemplate->block; // pointer for convenience
BlockValidationResourceTracker resourceTracker(std::numeric_limits<size_t>::max(), std::numeric_limits<size_t>::max());

uint64_t maxAccurateSigops = chainparams.GetConsensus().MaxBlockAccurateSigops(pblock->GetBlockTime(), sizeForkTime.load());
uint64_t maxSighashBytes = chainparams.GetConsensus().MaxBlockSighashBytes(pblock->GetBlockTime(), sizeForkTime.load());
BlockValidationResourceTracker resourceTracker(maxAccurateSigops, maxSighashBytes);

// -regtest only: allow overriding block.nVersion with
// -blockversion=N to test forking scenarios
Expand Down Expand Up @@ -257,7 +260,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)

// Legacy limits on sigOps:
unsigned int nTxSigOps = GetLegacySigOpCount(tx);
if (nBlockSigOps + nTxSigOps >= chainparams.GetConsensus().MaxBlockSigops(nBlockTime, sizeForkTime.load()))
if (nBlockSigOps + nTxSigOps >= chainparams.GetConsensus().MaxBlockLegacySigops(nBlockTime, sizeForkTime.load()))
continue;

// Skip free transactions if we're past the minimum block size:
Expand All @@ -284,15 +287,20 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
CAmount nTxFees = view.GetValueIn(tx)-tx.GetValueOut();

nTxSigOps += GetP2SHSigOpCount(tx, view);
if (nBlockSigOps + nTxSigOps >= chainparams.GetConsensus().MaxBlockSigops(nBlockTime, sizeForkTime.load()))
if (nBlockSigOps + nTxSigOps >= chainparams.GetConsensus().MaxBlockLegacySigops(nBlockTime, sizeForkTime.load()))
continue;

// Note that flags: we don't want to set mempool/IsStandard()
// policy here, but we still have to ensure that the block we
// create only contains transactions that are valid in new blocks.
CValidationState state;
if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, &resourceTracker))
continue;
{
if (!resourceTracker.IsWithinLimits())
break; // Ran out of sigops / sighash bytes, done building block
else
continue;
}

UpdateCoins(tx, state, view, nHeight);

Expand Down
4 changes: 3 additions & 1 deletion src/rpcmining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,10 @@ Value getblocktemplate(const Array& params, bool fHelp)
result.push_back(Pair("mintime", (int64_t)pindexPrev->GetMedianTimePast()+1));
result.push_back(Pair("mutable", aMutable));
result.push_back(Pair("noncerange", "00000000ffffffff"));
result.push_back(Pair("sigoplimit", Params().GetConsensus().MaxBlockSigops(nBlockTime, sizeForkTime.load())));
result.push_back(Pair("sigoplimit", Params().GetConsensus().MaxBlockLegacySigops(nBlockTime, sizeForkTime.load())));
result.push_back(Pair("sizelimit", Params().GetConsensus().MaxBlockSize(nBlockTime, sizeForkTime.load())));
result.push_back(Pair("accuratesigoplimit", Params().GetConsensus().MaxBlockAccurateSigops(nBlockTime, sizeForkTime.load())));
result.push_back(Pair("sighashlimit", Params().GetConsensus().MaxBlockSighashBytes(nBlockTime, sizeForkTime.load())));
result.push_back(Pair("curtime", nBlockTime));
result.push_back(Pair("bits", strprintf("%08x", pblock->nBits)));
result.push_back(Pair("height", (int64_t)(pindexPrev->nHeight+1)));
Expand Down

0 comments on commit 4eec022

Please sign in to comment.