From 56424040a469f775f14be0c81d7d62ebcfd8a0d7 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 22 Oct 2012 09:22:15 +0200 Subject: Bugfix: off-by-one error in coinbase maturity check --- src/main.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index ad5a7b0f1..723b8d75e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1319,7 +1319,9 @@ bool CTransaction::CheckInputs(CCoinsViewCache &inputs, enum CheckSig_mode csmod if (!HaveInputs(inputs)) return error("CheckInputs() : %s inputs unavailable", GetHash().ToString().substr(0,10).c_str()); - CBlockIndex *pindexBlock = inputs.GetBestBlock(); + // While checking, GetBestBlock() refers to the parent block. + // This is also true for mempool checks. + int nSpendHeight = inputs.GetBestBlock()->nHeight + 1; int64 nValueIn = 0; int64 nFees = 0; for (unsigned int i = 0; i < vin.size(); i++) @@ -1329,8 +1331,8 @@ bool CTransaction::CheckInputs(CCoinsViewCache &inputs, enum CheckSig_mode csmod // If prev is coinbase, check that it's matured if (coins.IsCoinBase()) { - if (pindexBlock->nHeight - coins.nHeight < COINBASE_MATURITY) - return error("CheckInputs() : tried to spend coinbase at depth %d", pindexBlock->nHeight - coins.nHeight); + if (nSpendHeight - coins.nHeight < COINBASE_MATURITY) + return error("CheckInputs() : tried to spend coinbase at depth %d", nSpendHeight - coins.nHeight); } // Check for negative or overflow input values -- cgit v1.2.3 From 9e957fb3b17e93a37db80544550a6689a3647295 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 22 Oct 2012 21:46:00 +0200 Subject: Bugfix: add missing fee check --- src/main.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 723b8d75e..dd2f491ae 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1597,6 +1597,9 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust blockundo.vtxundo.push_back(txundo); } + if (vtx[0].GetValueOut() > GetBlockValue(pindex->nHeight, nFees)) + return false; + if (fJustCheck) return true; -- cgit v1.2.3 From 1e64c2d585eeb6217d08a49c4cc7a89e0afd0421 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 23 Oct 2012 00:10:12 +0200 Subject: Bugfix: off-by-one in priority calculation --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index dd2f491ae..4386893ee 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3830,7 +3830,7 @@ CBlock* CreateNewBlock(CReserveKey& reservekey) int64 nValueIn = coins.vout[txin.prevout.n].nValue; nTotalIn += nValueIn; - int nConf = pindexPrev->nHeight - coins.nHeight; + int nConf = pindexPrev->nHeight - coins.nHeight + 1; dPriority += (double)nValueIn * nConf; } -- cgit v1.2.3 From c2ed184f987fbb157880fce2f91ebc8514fd6e76 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 23 Oct 2012 00:21:16 +0200 Subject: Added some comments Some clarifications after a code review by Mike Hearn. --- src/main.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 4386893ee..26cbab848 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -698,6 +698,8 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, return false; // do all inputs exist? + // Note that this does not check for the presence of actual outputs (see the next check for that), + // only helps filling in pfMissingInputs (to determine missing vs spent). BOOST_FOREACH(const CTxIn txin, tx.vin) { if (!view.HaveCoins(txin.prevout.hash)) { if (pfMissingInputs) @@ -706,6 +708,7 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, } } + // are the actual inputs available? if (!tx.HaveInputs(view)) return error("CTxMemPool::accept() : inputs already spent"); @@ -1956,9 +1959,13 @@ bool CBlock::CheckBlock(bool fCheckPOW, bool fCheckMerkleRoot) const if (!tx.CheckTransaction()) return DoS(tx.nDoS, error("CheckBlock() : CheckTransaction failed")); + // Build the merkle tree already. We need it anyway later, and it makes the + // block cache the transaction hashes, which means they don't need to be + // recalculated many times during this block's validation. + BuildMerkleTree(); + // Check for duplicate txids. This is caught by ConnectInputs(), // but catching it earlier avoids a potential DoS attack: - BuildMerkleTree(); set uniqueTx; for (unsigned int i=0; i Date: Tue, 23 Oct 2012 01:16:26 +0200 Subject: Bugfix: actually use CCoinsViewMemPool --- src/main.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 26cbab848..be1e947ad 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -691,7 +691,13 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, if (fCheckInputs) { - CCoinsViewCache &view = *pcoinsTip; + CCoinsView dummy; + CCoinsViewCache view(dummy); + + { + LOCK(cs); + CCoinsViewMemPool viewMemPool(*pcoinsTip, *this); + view.SetBackend(viewMemPool); // do we already have it? if (view.HaveCoins(hash)) @@ -711,6 +717,13 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, // are the actual inputs available? if (!tx.HaveInputs(view)) return error("CTxMemPool::accept() : inputs already spent"); + + // Bring the best block into scope + view.GetBestBlock(); + + // we have all inputs cached now, so switch back to dummy, so we don't need to keep lock on mempool + view.SetBackend(dummy); + } // Check for non-standard pay-to-script-hash in inputs if (!tx.AreInputsStandard(view) && !fTestNet) @@ -741,7 +754,6 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, int64 nNow = GetTime(); { - LOCK(cs); // Use an exponentially decaying ~10-minute window: dFreeCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime)); nLastTime = nNow; -- cgit v1.2.3