Merge pull request #773 from gavinandresen/p2shSigOpCount

Simplify counting of P2SH sigops to match BIP 16
This commit is contained in:
Gavin Andresen 2012-01-25 05:21:20 -08:00
commit b6a35d2d52
4 changed files with 53 additions and 52 deletions

View file

@ -503,20 +503,17 @@ bool CTransaction::AcceptToMemoryPool(CTxDB& txdb, bool fCheckInputs, bool* pfMi
if (!AreInputsStandard(mapInputs) && !fTestNet) if (!AreInputsStandard(mapInputs) && !fTestNet)
return error("AcceptToMemoryPool() : nonstandard transaction input"); return error("AcceptToMemoryPool() : nonstandard transaction input");
// Note: if you modify this code to accept non-standard transactions, then
// you should add code here to check that the transaction does a
// reasonable number of ECDSA signature verifications.
int64 nFees = GetValueIn(mapInputs)-GetValueOut(); int64 nFees = GetValueIn(mapInputs)-GetValueOut();
int nSigOps = GetSigOpCount(mapInputs);
unsigned int nSize = ::GetSerializeSize(*this, SER_NETWORK); unsigned int nSize = ::GetSerializeSize(*this, SER_NETWORK);
// Don't accept it if it can't get into a block // Don't accept it if it can't get into a block
if (nFees < GetMinFee(1000, true, GMF_RELAY)) if (nFees < GetMinFee(1000, true, GMF_RELAY))
return error("AcceptToMemoryPool() : not enough fees"); return error("AcceptToMemoryPool() : not enough fees");
// Checking ECDSA signatures is a CPU bottleneck, so to avoid denial-of-service
// attacks disallow transactions with more than one SigOp per 65 bytes.
// 65 bytes because that is the minimum size of an ECDSA signature
if (nSigOps > nSize / 65 || nSize < 100)
return error("AcceptToMemoryPool() : transaction with out-of-bounds SigOpCount");
// Continuously rate-limit free transactions // Continuously rate-limit free transactions
// This mitigates 'penny-flooding' -- sending thousands of free transactions just to // This mitigates 'penny-flooding' -- sending thousands of free transactions just to
// be annoying or make other's transactions take longer to confirm. // be annoying or make other's transactions take longer to confirm.
@ -1022,7 +1019,7 @@ int64 CTransaction::GetValueIn(const MapPrevTx& inputs) const
} }
int CTransaction::GetSigOpCount(const MapPrevTx& inputs) const int CTransaction::GetP2SHSigOpCount(const MapPrevTx& inputs) const
{ {
if (IsCoinBase()) if (IsCoinBase())
return 0; return 0;
@ -1030,14 +1027,16 @@ int CTransaction::GetSigOpCount(const MapPrevTx& inputs) const
int nSigOps = 0; int nSigOps = 0;
for (int i = 0; i < vin.size(); i++) for (int i = 0; i < vin.size(); i++)
{ {
nSigOps += GetOutputFor(vin[i], inputs).scriptPubKey.GetSigOpCount(vin[i].scriptSig); const CTxOut& prevout = GetOutputFor(vin[i], inputs);
if (prevout.scriptPubKey.IsPayToScriptHash())
nSigOps += prevout.scriptPubKey.GetSigOpCount(vin[i].scriptSig);
} }
return nSigOps; return nSigOps;
} }
bool CTransaction::ConnectInputs(MapPrevTx inputs, bool CTransaction::ConnectInputs(MapPrevTx inputs,
map<uint256, CTxIndex>& mapTestPool, const CDiskTxPos& posThisTx, map<uint256, CTxIndex>& mapTestPool, const CDiskTxPos& posThisTx,
const CBlockIndex* pindexBlock, bool fBlock, bool fMiner) const CBlockIndex* pindexBlock, bool fBlock, bool fMiner, bool fStrictPayToScriptHash)
{ {
// Take over previous transactions' spent pointers // Take over previous transactions' spent pointers
// fBlock is true when this is called from AcceptBlock when a new best-block is added to the blockchain // fBlock is true when this is called from AcceptBlock when a new best-block is added to the blockchain
@ -1074,20 +1073,6 @@ bool CTransaction::ConnectInputs(MapPrevTx inputs,
if (!MoneyRange(txPrev.vout[prevout.n].nValue) || !MoneyRange(nValueIn)) if (!MoneyRange(txPrev.vout[prevout.n].nValue) || !MoneyRange(nValueIn))
return DoS(100, error("ConnectInputs() : txin values out of range")); return DoS(100, error("ConnectInputs() : txin values out of range"));
bool fStrictPayToScriptHash = true;
if (fBlock)
{
// To avoid being on the short end of a block-chain split,
// don't do secondary validation of pay-to-script-hash transactions
// until blocks with timestamps after paytoscripthashtime:
int64 nEvalSwitchTime = GetArg("paytoscripthashtime", 1329264000); // Feb 15, 2012
fStrictPayToScriptHash = (pindexBlock->nTime >= nEvalSwitchTime);
}
// if !fBlock, then always be strict-- don't accept
// invalid-under-new-rules pay-to-script-hash transactions into
// our memory pool (don't relay them, don't include them
// in blocks we mine).
// Skip ECDSA signature verification when connecting blocks (fBlock=true) // Skip ECDSA signature verification when connecting blocks (fBlock=true)
// before the last blockchain checkpoint. This is safe because block merkle hashes are // before the last blockchain checkpoint. This is safe because block merkle hashes are
// still computed and checked, and any change will be caught at the next checkpoint. // still computed and checked, and any change will be caught at the next checkpoint.
@ -1198,6 +1183,12 @@ bool CBlock::ConnectBlock(CTxDB& txdb, CBlockIndex* pindex)
if (!CheckBlock()) if (!CheckBlock())
return false; return false;
// To avoid being on the short end of a block-chain split,
// don't do secondary validation of pay-to-script-hash transactions
// until blocks with timestamps after paytoscripthashtime:
int64 nEvalSwitchTime = GetArg("-paytoscripthashtime", 1329264000); // Feb 15, 2012
bool fStrictPayToScriptHash = (pindex->nTime >= nEvalSwitchTime);
//// issue here: it doesn't know the version //// issue here: it doesn't know the version
unsigned int nTxPos = pindex->nBlockPos + ::GetSerializeSize(CBlock(), SER_DISK) - 1 + GetSizeOfCompactSize(vtx.size()); unsigned int nTxPos = pindex->nBlockPos + ::GetSerializeSize(CBlock(), SER_DISK) - 1 + GetSizeOfCompactSize(vtx.size());
@ -1206,6 +1197,10 @@ bool CBlock::ConnectBlock(CTxDB& txdb, CBlockIndex* pindex)
int nSigOps = 0; int nSigOps = 0;
BOOST_FOREACH(CTransaction& tx, vtx) BOOST_FOREACH(CTransaction& tx, vtx)
{ {
nSigOps += tx.GetLegacySigOpCount();
if (nSigOps > MAX_BLOCK_SIGOPS)
return DoS(100, error("ConnectBlock() : too many sigops"));
CDiskTxPos posThisTx(pindex->nFile, pindex->nBlockPos, nTxPos); CDiskTxPos posThisTx(pindex->nFile, pindex->nBlockPos, nTxPos);
nTxPos += ::GetSerializeSize(tx, SER_DISK); nTxPos += ::GetSerializeSize(tx, SER_DISK);
@ -1216,17 +1211,19 @@ bool CBlock::ConnectBlock(CTxDB& txdb, CBlockIndex* pindex)
if (!tx.FetchInputs(txdb, mapQueuedChanges, true, false, mapInputs, fInvalid)) if (!tx.FetchInputs(txdb, mapQueuedChanges, true, false, mapInputs, fInvalid))
return false; return false;
int nTxOps = tx.GetSigOpCount(mapInputs); if (fStrictPayToScriptHash)
nSigOps += nTxOps; {
if (nSigOps > MAX_BLOCK_SIGOPS) // Add in sigops done by pay-to-script-hash inputs;
return DoS(100, error("ConnectBlock() : too many sigops")); // this is to prevent a "rogue miner" from creating
// There is a different MAX_BLOCK_SIGOPS check in AcceptBlock(); // an incredibly-expensive-to-validate block.
// a block must satisfy both to make it into the best-chain nSigOps += tx.GetP2SHSigOpCount(mapInputs);
// (AcceptBlock() is always called before ConnectBlock()) if (nSigOps > MAX_BLOCK_SIGOPS)
return DoS(100, error("ConnectBlock() : too many sigops"));
}
nFees += tx.GetValueIn(mapInputs)-tx.GetValueOut(); nFees += tx.GetValueIn(mapInputs)-tx.GetValueOut();
if (!tx.ConnectInputs(mapInputs, mapQueuedChanges, posThisTx, pindex, true, false)) if (!tx.ConnectInputs(mapInputs, mapQueuedChanges, posThisTx, pindex, true, false, fStrictPayToScriptHash))
return false; return false;
} }
@ -1510,9 +1507,6 @@ bool CBlock::CheckBlock() const
if (!tx.CheckTransaction()) if (!tx.CheckTransaction())
return DoS(tx.nDoS, error("CheckBlock() : CheckTransaction failed")); return DoS(tx.nDoS, error("CheckBlock() : CheckTransaction failed"));
// Pre-pay-to-script-hash (before version 0.6), this is how sigops
// were counted; there is another check in ConnectBlock when
// transaction inputs are fetched to count pay-to-script-hash sigops:
int nSigOps = 0; int nSigOps = 0;
BOOST_FOREACH(const CTransaction& tx, vtx) BOOST_FOREACH(const CTransaction& tx, vtx)
{ {
@ -3046,8 +3040,7 @@ CBlock* CreateNewBlock(CReserveKey& reservekey)
map<uint256, CTxIndex> mapTestPool; map<uint256, CTxIndex> mapTestPool;
uint64 nBlockSize = 1000; uint64 nBlockSize = 1000;
uint64 nBlockTx = 0; uint64 nBlockTx = 0;
int nBlockSigOps1 = 100; // pre-0.6 count of sigOps int nBlockSigOps = 100;
int nBlockSigOps2 = 100; // post-0.6 count of sigOps
while (!mapPriority.empty()) while (!mapPriority.empty())
{ {
// Take highest priority transaction off priority queue // Take highest priority transaction off priority queue
@ -3061,8 +3054,8 @@ CBlock* CreateNewBlock(CReserveKey& reservekey)
continue; continue;
// Legacy limits on sigOps: // Legacy limits on sigOps:
int nTxSigOps1 = tx.GetLegacySigOpCount(); int nTxSigOps = tx.GetLegacySigOpCount();
if (nBlockSigOps1 + nTxSigOps1 >= MAX_BLOCK_SIGOPS) if (nBlockSigOps + nTxSigOps >= MAX_BLOCK_SIGOPS)
continue; continue;
// Transaction fee required depends on block size // Transaction fee required depends on block size
@ -3081,8 +3074,8 @@ CBlock* CreateNewBlock(CReserveKey& reservekey)
if (nFees < nMinFee) if (nFees < nMinFee)
continue; continue;
int nTxSigOps2 = tx.GetSigOpCount(mapInputs); nTxSigOps += tx.GetP2SHSigOpCount(mapInputs);
if (nBlockSigOps2 + nTxSigOps2 >= MAX_BLOCK_SIGOPS) if (nBlockSigOps + nTxSigOps >= MAX_BLOCK_SIGOPS)
continue; continue;
if (!tx.ConnectInputs(mapInputs, mapTestPoolTmp, CDiskTxPos(1,1,1), pindexPrev, false, true)) if (!tx.ConnectInputs(mapInputs, mapTestPoolTmp, CDiskTxPos(1,1,1), pindexPrev, false, true))
@ -3094,8 +3087,7 @@ CBlock* CreateNewBlock(CReserveKey& reservekey)
pblock->vtx.push_back(tx); pblock->vtx.push_back(tx);
nBlockSize += nTxSize; nBlockSize += nTxSize;
++nBlockTx; ++nBlockTx;
nBlockSigOps1 += nTxSigOps1; nBlockSigOps += nTxSigOps;
nBlockSigOps2 += nTxSigOps2;
// Add transactions that depend on this one to the priority queue // Add transactions that depend on this one to the priority queue
uint256 hash = tx.GetHash(); uint256 hash = tx.GetHash();

View file

@ -528,14 +528,13 @@ public:
*/ */
int GetLegacySigOpCount() const; int GetLegacySigOpCount() const;
/** Count ECDSA signature operations the new (0.6-and-later) way /** Count ECDSA signature operations in pay-to-script-hash inputs.
This is a better measure of how expensive it is to process this transaction.
@param[in] mapInputs Map of previous transactions that have outputs we're spending @param[in] mapInputs Map of previous transactions that have outputs we're spending
@return maximum number of sigops required to validate this transaction's inputs @return maximum number of sigops required to validate this transaction's inputs
@see CTransaction::FetchInputs @see CTransaction::FetchInputs
*/ */
int GetSigOpCount(const MapPrevTx& mapInputs) const; int GetP2SHSigOpCount(const MapPrevTx& mapInputs) const;
/** Amount of bitcoins spent by this transaction. /** Amount of bitcoins spent by this transaction.
@return sum of all outputs (note: does not include fees) @return sum of all outputs (note: does not include fees)
@ -699,11 +698,12 @@ public:
@param[in] pindexBlock @param[in] pindexBlock
@param[in] fBlock true if called from ConnectBlock @param[in] fBlock true if called from ConnectBlock
@param[in] fMiner true if called from CreateNewBlock @param[in] fMiner true if called from CreateNewBlock
@param[in] fStrictPayToScriptHash true if fully validating p2sh transactions
@return Returns true if all checks succeed @return Returns true if all checks succeed
*/ */
bool ConnectInputs(MapPrevTx inputs, bool ConnectInputs(MapPrevTx inputs,
std::map<uint256, CTxIndex>& mapTestPool, const CDiskTxPos& posThisTx, std::map<uint256, CTxIndex>& mapTestPool, const CDiskTxPos& posThisTx,
const CBlockIndex* pindexBlock, bool fBlock, bool fMiner); const CBlockIndex* pindexBlock, bool fBlock, bool fMiner, bool fStrictPayToScriptHash=true);
bool ClientConnectInputs(); bool ClientConnectInputs();
bool CheckTransaction() const; bool CheckTransaction() const;
bool AcceptToMemoryPool(CTxDB& txdb, bool fCheckInputs=true, bool* pfMissingInputs=NULL); bool AcceptToMemoryPool(CTxDB& txdb, bool fCheckInputs=true, bool* pfMissingInputs=NULL);

View file

@ -255,7 +255,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
} }
CTransaction txFrom; CTransaction txFrom;
txFrom.vout.resize(5); txFrom.vout.resize(6);
// First three are standard: // First three are standard:
CScript pay1; pay1.SetBitcoinAddress(key[0].GetPubKey()); CScript pay1; pay1.SetBitcoinAddress(key[0].GetPubKey());
@ -267,12 +267,18 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
txFrom.vout[1].scriptPubKey = pay1; txFrom.vout[1].scriptPubKey = pay1;
txFrom.vout[2].scriptPubKey = pay1of3; txFrom.vout[2].scriptPubKey = pay1of3;
// Last two non-standard: // Last three non-standard:
CScript empty; CScript empty;
keystore.AddCScript(empty); keystore.AddCScript(empty);
txFrom.vout[3].scriptPubKey = empty; txFrom.vout[3].scriptPubKey = empty;
// Can't use SetPayToScriptHash, it checks for the empty Script. So: // Can't use SetPayToScriptHash, it checks for the empty Script. So:
txFrom.vout[4].scriptPubKey << OP_HASH160 << Hash160(empty) << OP_EQUAL; txFrom.vout[4].scriptPubKey << OP_HASH160 << Hash160(empty) << OP_EQUAL;
CScript oneOfEleven;
oneOfEleven << OP_1;
for (int i = 0; i < 11; i++)
oneOfEleven << key[0].GetPubKey();
oneOfEleven << OP_11 << OP_CHECKMULTISIG;
txFrom.vout[5].scriptPubKey.SetPayToScriptHash(oneOfEleven);
mapInputs[txFrom.GetHash()] = make_pair(CTxIndex(), txFrom); mapInputs[txFrom.GetHash()] = make_pair(CTxIndex(), txFrom);
@ -292,16 +298,21 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 2)); BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 2));
BOOST_CHECK(txTo.AreInputsStandard(mapInputs)); BOOST_CHECK(txTo.AreInputsStandard(mapInputs));
BOOST_CHECK_EQUAL(txTo.GetP2SHSigOpCount(mapInputs), 1);
CTransaction txToNonStd; CTransaction txToNonStd;
txToNonStd.vout.resize(1); txToNonStd.vout.resize(1);
txToNonStd.vout[0].scriptPubKey.SetBitcoinAddress(key[1].GetPubKey()); txToNonStd.vout[0].scriptPubKey.SetBitcoinAddress(key[1].GetPubKey());
txToNonStd.vin.resize(1); txToNonStd.vin.resize(2);
txToNonStd.vin[0].prevout.n = 4; txToNonStd.vin[0].prevout.n = 4;
txToNonStd.vin[0].prevout.hash = txFrom.GetHash(); txToNonStd.vin[0].prevout.hash = txFrom.GetHash();
txToNonStd.vin[0].scriptSig << Serialize(empty); txToNonStd.vin[0].scriptSig << Serialize(empty);
txToNonStd.vin[1].prevout.n = 5;
txToNonStd.vin[1].prevout.hash = txFrom.GetHash();
txToNonStd.vin[1].scriptSig << OP_0 << Serialize(oneOfEleven);
BOOST_CHECK(!txToNonStd.AreInputsStandard(mapInputs)); BOOST_CHECK(!txToNonStd.AreInputsStandard(mapInputs));
BOOST_CHECK_EQUAL(txToNonStd.GetP2SHSigOpCount(mapInputs), 11);
txToNonStd.vin[0].scriptSig.clear(); txToNonStd.vin[0].scriptSig.clear();
BOOST_CHECK(!txToNonStd.AreInputsStandard(mapInputs)); BOOST_CHECK(!txToNonStd.AreInputsStandard(mapInputs));

View file

@ -78,7 +78,6 @@ BOOST_AUTO_TEST_CASE(test_Get)
t1.vout[0].scriptPubKey << OP_1; t1.vout[0].scriptPubKey << OP_1;
BOOST_CHECK(t1.AreInputsStandard(dummyInputs)); BOOST_CHECK(t1.AreInputsStandard(dummyInputs));
BOOST_CHECK_EQUAL(t1.GetSigOpCount(dummyInputs), 3);
BOOST_CHECK_EQUAL(t1.GetValueIn(dummyInputs), (50+21+22)*CENT); BOOST_CHECK_EQUAL(t1.GetValueIn(dummyInputs), (50+21+22)*CENT);
} }
@ -103,7 +102,6 @@ BOOST_AUTO_TEST_CASE(test_GetThrow)
t1.vout[0].scriptPubKey << OP_1; t1.vout[0].scriptPubKey << OP_1;
BOOST_CHECK_THROW(t1.AreInputsStandard(missingInputs), runtime_error); BOOST_CHECK_THROW(t1.AreInputsStandard(missingInputs), runtime_error);
BOOST_CHECK_THROW(t1.GetSigOpCount(missingInputs), runtime_error);
BOOST_CHECK_THROW(t1.GetValueIn(missingInputs), runtime_error); BOOST_CHECK_THROW(t1.GetValueIn(missingInputs), runtime_error);
} }