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

[Mempool] Improve removal of invalid transactions after reorgs #6915

Merged
merged 8 commits into from
Dec 1, 2015
Merged

[Mempool] Improve removal of invalid transactions after reorgs #6915

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0c9959a
Add failing test checking timelocked-txn removal during reorg
TheBlueMatt Aug 27, 2015
9b060e5
Fix removal of time-locked transactions during reorg
TheBlueMatt Aug 27, 2015
b0a064c
Fix comment in removeForReorg
TheBlueMatt Sep 6, 2015
474b84a
Make indentation in ActivateBestChainStep readable
TheBlueMatt Sep 9, 2015
bb8ea1f
removeForReorg calls once-per-disconnect-> once-per-reorg
TheBlueMatt Sep 9, 2015
7e49f5f
Track coinbase spends in CTxMemPoolEntry
sdaftuar Oct 29, 2015
b7fa4aa
Don't call removeForReorg if DisconnectTip fails
sdaftuar Nov 12, 2015
2d8860e
Fix removeForReorg to use MedianTimePast
sdaftuar Nov 23, 2015
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
Prev Previous commit
Next Next commit
Track coinbase spends in CTxMemPoolEntry 
This allows us to optimize CTxMemPool::removeForReorg.
  • Loading branch information
@sdaftuar
sdaftuar committed Nov 30, 2015
commit 7e49f5f8b4e237d7212d027a7bea4bbd52c9e7b6
13 changes: 12 additions & 1 deletion src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,18 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
CAmount inChainInputValue;
double dPriority = view.GetPriority(tx, chainActive.Height(), inChainInputValue);

CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), pool.HasNoInputsOf(tx), inChainInputValue);
// Keep track of transactions that spend a coinbase, which we re-scan
// during reorgs to ensure COINBASE_MATURITY is still met.
bool fSpendsCoinbase = false;
BOOST_FOREACH(const CTxIn &txin, tx.vin) {
const CCoins *coins = view.AccessCoins(txin.prevout.hash);
if (coins->IsCoinBase()) {
fSpendsCoinbase = true;
break;
}
}

CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbase);
unsigned int nSize = entry.GetTxSize();

// Don't accept it if it can't get into a block
Expand Down
24 changes: 13 additions & 11 deletions src/test/miner_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
{
tx.vout[0].nValue -= 1000000;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
bool spendsCoinbase = (i == 0) ? true : false; // only first tx spends coinbase
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx));
tx.vin[0].prevout.hash = hash;
}
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
Expand All @@ -139,7 +140,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
{
tx.vout[0].nValue -= 10000000;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
bool spendsCoinbase = (i == 0) ? true : false; // only first tx spends coinbase
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx));
tx.vin[0].prevout.hash = hash;
}
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
Expand All @@ -158,15 +160,15 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
tx.vin[0].prevout.hash = txFirst[1]->GetHash();
tx.vout[0].nValue = 4900000000LL;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
tx.vin[0].prevout.hash = hash;
tx.vin.resize(2);
tx.vin[1].scriptSig = CScript() << OP_1;
tx.vin[1].prevout.hash = txFirst[0]->GetHash();
tx.vin[1].prevout.n = 0;
tx.vout[0].nValue = 5900000000LL;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
delete pblocktemplate;
mempool.clear();
Expand All @@ -177,7 +179,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
tx.vin[0].scriptSig = CScript() << OP_0 << OP_1;
tx.vout[0].nValue = 0;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(false).FromTx(tx));
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
delete pblocktemplate;
mempool.clear();
Expand All @@ -190,12 +192,12 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
script = CScript() << OP_0;
tx.vout[0].scriptPubKey = GetScriptForDestination(CScriptID(script));
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
tx.vin[0].prevout.hash = hash;
tx.vin[0].scriptSig = CScript() << (std::vector<unsigned char>)script;
tx.vout[0].nValue -= 1000000;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(false).FromTx(tx));
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
delete pblocktemplate;
mempool.clear();
Expand All @@ -206,10 +208,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
tx.vout[0].nValue = 4900000000LL;
tx.vout[0].scriptPubKey = CScript() << OP_1;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
tx.vout[0].scriptPubKey = CScript() << OP_2;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
delete pblocktemplate;
mempool.clear();
Expand All @@ -235,7 +237,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
tx.vout[0].scriptPubKey = CScript() << OP_1;
tx.nLockTime = chainActive.Tip()->nHeight+1;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
BOOST_CHECK(!CheckFinalTx(tx, LOCKTIME_MEDIAN_TIME_PAST));

// time locked
Expand All @@ -249,7 +251,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
tx2.vout[0].scriptPubKey = CScript() << OP_1;
tx2.nLockTime = chainActive.Tip()->GetMedianTimePast()+1;
hash = tx2.GetHash();
mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx2));
mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx2));
BOOST_CHECK(!CheckFinalTx(tx2, LOCKTIME_MEDIAN_TIME_PAST));

BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
Expand Down
2 changes: 1 addition & 1 deletion src/test/test_bitcoin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(CMutableTransaction &tx, CTxMemPo
CAmount inChainValue = hasNoDependencies ? txn.GetValueOut() : 0;

return CTxMemPoolEntry(txn, nFee, nTime, dPriority, nHeight,
hasNoDependencies, inChainValue);
hasNoDependencies, inChainValue, spendsCoinbase);
}

void Shutdown(void* parg)
Expand Down
4 changes: 3 additions & 1 deletion src/test/test_bitcoin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ struct TestMemPoolEntryHelper
double dPriority;
unsigned int nHeight;
bool hadNoDependencies;
bool spendsCoinbase;

TestMemPoolEntryHelper() :
nFee(0), nTime(0), dPriority(0.0), nHeight(1),
hadNoDependencies(false) { }
hadNoDependencies(false), spendsCoinbase(false) { }

CTxMemPoolEntry FromTx(CMutableTransaction &tx, CTxMemPool *pool = NULL);

Expand All @@ -78,5 +79,6 @@ struct TestMemPoolEntryHelper
TestMemPoolEntryHelper &Priority(double _priority) { dPriority = _priority; return *this; }
TestMemPoolEntryHelper &Height(unsigned int _height) { nHeight = _height; return *this; }
TestMemPoolEntryHelper &HadNoDependencies(bool _hnd) { hadNoDependencies = _hnd; return *this; }
TestMemPoolEntryHelper &SpendsCoinbase(bool _flag) { spendsCoinbase = _flag; return *this; }
};
#endif
8 changes: 5 additions & 3 deletions src/txmempool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ using namespace std;

CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee,
int64_t _nTime, double _entryPriority, unsigned int _entryHeight,
bool poolHasNoInputsOf, CAmount _inChainInputValue):
bool poolHasNoInputsOf, CAmount _inChainInputValue,
bool _spendsCoinbase):
tx(_tx), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight),
hadNoDependencies(poolHasNoInputsOf), inChainInputValue(_inChainInputValue)
hadNoDependencies(poolHasNoInputsOf), inChainInputValue(_inChainInputValue),
spendsCoinbase(_spendsCoinbase)
{
nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION);
nModSize = tx.CalculateModifiedSize(nTxSize);
Expand Down Expand Up @@ -488,7 +490,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
const CTransaction& tx = it->GetTx();
if (!IsFinalTx(tx, nMemPoolHeight, GetAdjustedTime())) {
transactionsToRemove.push_back(tx);
} else {
} else if (it->GetSpendsCoinbase()) {
BOOST_FOREACH(const CTxIn& txin, tx.vin) {
indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash);
if (it2 != mapTx.end())
Expand Down
5 changes: 4 additions & 1 deletion src/txmempool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class CTxMemPoolEntry
unsigned int entryHeight; //! Chain height when entering the mempool
bool hadNoDependencies; //! Not dependent on any other txs when it entered the mempool
CAmount inChainInputValue; //! Sum of all txin values that are already in blockchain
bool spendsCoinbase; //! keep track of transactions that spend a coinbase

// Information about descendants of this transaction that are in the
// mempool; if we remove this transaction we must remove all of these
Expand All @@ -80,7 +81,7 @@ class CTxMemPoolEntry
public:
CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee,
int64_t _nTime, double _entryPriority, unsigned int _entryHeight,
bool poolHasNoInputsOf, CAmount _inChainInputValue);
bool poolHasNoInputsOf, CAmount _inChainInputValue, bool spendsCoinbase);
CTxMemPoolEntry(const CTxMemPoolEntry& other);

const CTransaction& GetTx() const { return this->tx; }
Expand Down Expand Up @@ -109,6 +110,8 @@ class CTxMemPoolEntry
uint64_t GetCountWithDescendants() const { return nCountWithDescendants; }
uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; }
CAmount GetFeesWithDescendants() const { return nFeesWithDescendants; }

bool GetSpendsCoinbase() const { return spendsCoinbase; }
};

// Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index.
Expand Down