Permalink
Browse files

Merge pull request #6915

2d8860e Fix removeForReorg to use MedianTimePast (Suhas Daftuar)
b7fa4aa Don't call removeForReorg if DisconnectTip fails (Suhas Daftuar)
7e49f5f Track coinbase spends in CTxMemPoolEntry (Suhas Daftuar)
bb8ea1f removeForReorg calls once-per-disconnect-> once-per-reorg (Matt Corallo)
474b84a Make indentation in ActivateBestChainStep readable (Matt Corallo)
b0a064c Fix comment in removeForReorg (Matt Corallo)
9b060e5 Fix removal of time-locked transactions during reorg (Matt Corallo)
0c9959a Add failing test checking timelocked-txn removal during reorg (Matt Corallo)
  • Loading branch information...
laanwj committed Dec 1, 2015
2 parents 9afbd96 + 2d8860e commit 2ef5ffa59afaf9b1d30cc0c97e6b1ab2b7ab30f4
@@ -82,7 +82,7 @@
'rawtransactions.py',
'rest.py',
'mempool_spendcoinbase.py',
- 'mempool_coinbase_spends.py',
+ 'mempool_reorg.py',
'httpbasics.py',
'multi_rpc.py',
'zapwallettxes.py',
@@ -52,32 +52,46 @@ def run_test(self):
# 3. Indirect (coinbase and child both in chain) : spend_103 and spend_103_1
# Use invalidatblock to make all of the above coinbase spends invalid (immature coinbase),
# and make sure the mempool code behaves correctly.
- b = [ self.nodes[0].getblockhash(n) for n in range(102, 105) ]
+ b = [ self.nodes[0].getblockhash(n) for n in range(101, 105) ]
coinbase_txids = [ self.nodes[0].getblock(h)['tx'][0] for h in b ]
- spend_101_raw = self.create_tx(coinbase_txids[0], node1_address, 50)
- spend_102_raw = self.create_tx(coinbase_txids[1], node0_address, 50)
- spend_103_raw = self.create_tx(coinbase_txids[2], node0_address, 50)
+ spend_101_raw = self.create_tx(coinbase_txids[1], node1_address, 50)
+ spend_102_raw = self.create_tx(coinbase_txids[2], node0_address, 50)
+ spend_103_raw = self.create_tx(coinbase_txids[3], node0_address, 50)
+
+ # Create a block-height-locked transaction which will be invalid after reorg
+ timelock_tx = self.nodes[0].createrawtransaction([{"txid": coinbase_txids[0], "vout": 0}], {node0_address: 50})
+ # Set the time lock
+ timelock_tx = timelock_tx.replace("ffffffff", "11111111", 1)
+ timelock_tx = timelock_tx[:-8] + hex(self.nodes[0].getblockcount() + 2)[2:] + "000000"
+ timelock_tx = self.nodes[0].signrawtransaction(timelock_tx)["hex"]
+ assert_raises(JSONRPCException, self.nodes[0].sendrawtransaction, timelock_tx)
# Broadcast and mine spend_102 and 103:
spend_102_id = self.nodes[0].sendrawtransaction(spend_102_raw)
spend_103_id = self.nodes[0].sendrawtransaction(spend_103_raw)
self.nodes[0].generate(1)
+ assert_raises(JSONRPCException, self.nodes[0].sendrawtransaction, timelock_tx)
# Create 102_1 and 103_1:
spend_102_1_raw = self.create_tx(spend_102_id, node1_address, 50)
spend_103_1_raw = self.create_tx(spend_103_id, node1_address, 50)
# Broadcast and mine 103_1:
spend_103_1_id = self.nodes[0].sendrawtransaction(spend_103_1_raw)
- self.nodes[0].generate(1)
+ last_block = self.nodes[0].generate(1)
+ timelock_tx_id = self.nodes[0].sendrawtransaction(timelock_tx)
# ... now put spend_101 and spend_102_1 in memory pools:
spend_101_id = self.nodes[0].sendrawtransaction(spend_101_raw)
spend_102_1_id = self.nodes[0].sendrawtransaction(spend_102_1_raw)
self.sync_all()
- assert_equal(set(self.nodes[0].getrawmempool()), set([ spend_101_id, spend_102_1_id ]))
+ assert_equal(set(self.nodes[0].getrawmempool()), set([ spend_101_id, spend_102_1_id, timelock_tx_id ]))
+
+ for node in self.nodes:
+ node.invalidateblock(last_block[0])
+ assert_equal(set(self.nodes[0].getrawmempool()), set([ spend_101_id, spend_102_1_id, spend_103_1_id ]))
# Use invalidateblock to re-org back and make all those coinbase spends
# immature/invalid:
View
@@ -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
@@ -2310,12 +2321,11 @@ void static UpdateTip(CBlockIndex *pindexNew) {
}
}
-/** Disconnect chainActive's tip. You want to manually re-limit mempool size after this */
+/** Disconnect chainActive's tip. You probably want to call mempool.removeForReorg and manually re-limit mempool size after this, with cs_main held. */
bool static DisconnectTip(CValidationState& state, const Consensus::Params& consensusParams)
{
CBlockIndex *pindexDelete = chainActive.Tip();
assert(pindexDelete);
- mempool.check(pcoinsTip);
// Read block from disk.
CBlock block;
if (!ReadBlockFromDisk(block, pindexDelete, consensusParams))
@@ -2350,8 +2360,6 @@ bool static DisconnectTip(CValidationState& state, const Consensus::Params& cons
// UpdateTransactionsFromBlock finds descendants of any transactions in this
// block that were added back and cleans up the mempool state.
mempool.UpdateTransactionsFromBlock(vHashUpdate);
- mempool.removeCoinbaseSpends(pcoinsTip, pindexDelete->nHeight);
- mempool.check(pcoinsTip);
// Update chainActive and related variables.
UpdateTip(pindexDelete->pprev);
// Let wallets know transactions went from 1-confirmed to
@@ -2375,7 +2383,6 @@ static int64_t nTimePostConnect = 0;
bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const CBlock* pblock)
{
assert(pindexNew->pprev == chainActive.Tip());
- mempool.check(pcoinsTip);
// Read block from disk.
int64_t nTime1 = GetTimeMicros();
CBlock block;
@@ -2412,7 +2419,6 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams,
// Remove conflicting transactions from the mempool.
list<CTransaction> txConflicted;
mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, txConflicted, !IsInitialBlockDownload());
- mempool.check(pcoinsTip);
// Update chainActive & related variables.
UpdateTip(pindexNew);
// Tell wallet about transactions that went from mempool
@@ -2525,46 +2531,49 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c
bool fContinue = true;
int nHeight = pindexFork ? pindexFork->nHeight : -1;
while (fContinue && nHeight != pindexMostWork->nHeight) {
- // Don't iterate the entire list of potential improvements toward the best tip, as we likely only need
- // a few blocks along the way.
- int nTargetHeight = std::min(nHeight + 32, pindexMostWork->nHeight);
- vpindexToConnect.clear();
- vpindexToConnect.reserve(nTargetHeight - nHeight);
- CBlockIndex *pindexIter = pindexMostWork->GetAncestor(nTargetHeight);
- while (pindexIter && pindexIter->nHeight != nHeight) {
- vpindexToConnect.push_back(pindexIter);
- pindexIter = pindexIter->pprev;
- }
- nHeight = nTargetHeight;
-
- // Connect new blocks.
- BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) {
- if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : NULL)) {
- if (state.IsInvalid()) {
- // The block violates a consensus rule.
- if (!state.CorruptionPossible())
- InvalidChainFound(vpindexToConnect.back());
- state = CValidationState();
- fInvalidFound = true;
- fContinue = false;
- break;
+ // Don't iterate the entire list of potential improvements toward the best tip, as we likely only need
+ // a few blocks along the way.
+ int nTargetHeight = std::min(nHeight + 32, pindexMostWork->nHeight);
+ vpindexToConnect.clear();
+ vpindexToConnect.reserve(nTargetHeight - nHeight);
+ CBlockIndex *pindexIter = pindexMostWork->GetAncestor(nTargetHeight);
+ while (pindexIter && pindexIter->nHeight != nHeight) {
+ vpindexToConnect.push_back(pindexIter);
+ pindexIter = pindexIter->pprev;
+ }
+ nHeight = nTargetHeight;
+
+ // Connect new blocks.
+ BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) {
+ if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : NULL)) {
+ if (state.IsInvalid()) {
+ // The block violates a consensus rule.
+ if (!state.CorruptionPossible())
+ InvalidChainFound(vpindexToConnect.back());
+ state = CValidationState();
+ fInvalidFound = true;
+ fContinue = false;
+ break;
+ } else {
+ // A system error occurred (disk space, database error, ...).
+ return false;
+ }
} else {
- // A system error occurred (disk space, database error, ...).
- return false;
- }
- } else {
- PruneBlockIndexCandidates();
- if (!pindexOldTip || chainActive.Tip()->nChainWork > pindexOldTip->nChainWork) {
- // We're in a better position than we were. Return temporarily to release the lock.
- fContinue = false;
- break;
+ PruneBlockIndexCandidates();
+ if (!pindexOldTip || chainActive.Tip()->nChainWork > pindexOldTip->nChainWork) {
+ // We're in a better position than we were. Return temporarily to release the lock.
+ fContinue = false;
+ break;
+ }
}
}
}
- }
- if (fBlocksDisconnected)
+ if (fBlocksDisconnected) {
+ mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
mempool.TrimToSize(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
+ }
+ mempool.check(pcoinsTip);
// Callbacks/notifications for a new best chain.
if (fInvalidFound)
@@ -2672,6 +2681,7 @@ bool InvalidateBlock(CValidationState& state, const Consensus::Params& consensus
// ActivateBestChain considers blocks already in chainActive
// unconditionally valid already, so force disconnect away from it.
if (!DisconnectTip(state, consensusParams)) {
+ mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
return false;
}
}
@@ -2689,6 +2699,7 @@ bool InvalidateBlock(CValidationState& state, const Consensus::Params& consensus
}
InvalidChainFound(pindex);
+ mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS);
return true;
}
View
@@ -467,7 +467,7 @@ bool InvalidateBlock(CValidationState& state, const Consensus::Params& consensus
/** Remove invalidity status from a block and its descendants. */
bool ReconsiderBlock(CValidationState& state, CBlockIndex *pindex);
-/** The currently-connected chain of blocks. */
+/** The currently-connected chain of blocks (protected by cs_main). */
extern CChain chainActive;
/** Global variable that points to the active CCoinsView (protected by cs_main) */
View
@@ -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));
@@ -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));
@@ -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();
@@ -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();
@@ -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.begin(), script.end());
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();
@@ -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();
@@ -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
@@ -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));
@@ -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)
View
@@ -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);
@@ -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
Oops, something went wrong.

0 comments on commit 2ef5ffa

Please sign in to comment.