Skip to content

Commit

Permalink
Merge #627: kill useless mapPeginsSpentToTxid
Browse files Browse the repository at this point in the history 
8591de9 kill useless mapPeginsSpentToTxid (Gregory Sanders)

Pull request description:

  Completely redundant since #260 when peg-in inputs have taken on deterministic prevouts, so normal mempool logic has taken over for accounting for double-spending peg-in proofs in the mempool.

  resolves #624

Tree-SHA512: e5c0e7570657bd1418f54fdd47a6f3f815a5779e28ff15ba37c18e425780415385af64efa57be882ba12ad748e7a664fb723eae0e819bf1d5f197bb0b82b086b
  • Loading branch information
@stevenroose
stevenroose committed May 29, 2019
2 parents d913803 + 8591de9 commit e1bf441
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 136 deletions.
93 changes: 2 additions & 91 deletions src/test/mempool_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
/* after tx6 is mined, tx7 should move up in the sort */
std::vector<CTransactionRef> vtx;
vtx.push_back(MakeTransactionRef(tx6));
std::set<std::pair<uint256, COutPoint>> setPeginsSpentDummy;
pool.removeForBlock(vtx, 1, setPeginsSpentDummy);
pool.removeForBlock(vtx, 1);

sortedOrder.erase(sortedOrder.begin()+1);
// Ties are broken by hash
Expand Down Expand Up @@ -420,93 +419,6 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
CheckSort<ancestor_score>(pool, sortedOrder);
}

// ELEMENTS:
BOOST_AUTO_TEST_CASE(PeginSpentTest)
{
CBlockPolicyEstimator feeEst;
CTxMemPool pool(&feeEst);
LOCK(pool.cs);

std::set<std::pair<uint256, COutPoint> > setPeginsSpent;
TestMemPoolEntryHelper entry;

std::pair<uint256, COutPoint> pegin1, pegin2, pegin3;
GetRandBytes(pegin1.first.begin(), pegin1.first.size());
GetRandBytes(pegin2.first.begin(), pegin2.first.size());
GetRandBytes(pegin3.first.begin(), pegin3.first.size());
GetRandBytes(pegin1.second.hash.begin(), pegin1.second.hash.size());
GetRandBytes(pegin2.second.hash.begin(), pegin2.second.hash.size());
pegin3.second.hash = pegin2.second.hash;
pegin1.second.n = 0;
pegin2.second.n = 0;
pegin3.second.n = 1;

CMutableTransaction tx;
tx.vin.resize(1);
tx.vout.resize(1);
tx.vout[0].nValue = 0;
pool.addUnchecked(entry.PeginsSpent(setPeginsSpent).FromTx(tx));
BOOST_CHECK(pool.mapPeginsSpentToTxid.empty());

setPeginsSpent = {pegin1};
GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size());
tx.vout.resize(2);
tx.vout[1].nValue = 0;
const uint256 tx2Hash(tx.GetHash());
pool.addUnchecked(entry.PeginsSpent(setPeginsSpent).FromTx(tx));
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin1].ToString(), tx2Hash.ToString());

setPeginsSpent = {pegin2};
GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size());
tx.vout.resize(3);
tx.vout[2].nValue = 0;
const uint256 tx3Hash(tx.GetHash());
pool.addUnchecked(entry.PeginsSpent(setPeginsSpent).FromTx(tx));
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin2].ToString(), tx3Hash.ToString());

setPeginsSpent = {pegin3};
GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size());
tx.vout.resize(4);
tx.vout[3].nValue = 0;
CTransactionRef txref(MakeTransactionRef(tx));
pool.removeForBlock({txref}, 1, setPeginsSpent);

BOOST_CHECK_EQUAL(pool.size(), 3);
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid.size(), 2);
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin1].ToString(), tx2Hash.ToString());
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin2].ToString(), tx3Hash.ToString());

setPeginsSpent = {pegin1};
GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size());
tx.vout.resize(5);
tx.vout[4].nValue = 0;
txref = MakeTransactionRef(tx);
pool.removeForBlock({txref}, 2, setPeginsSpent);

BOOST_CHECK_EQUAL(pool.size(), 2);
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid.size(), 1);
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin2].ToString(), tx3Hash.ToString());

setPeginsSpent = {pegin1, pegin3};
GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size());
tx.vout.resize(6);
tx.vout[5].nValue = 0;
const uint256 tx4Hash(tx.GetHash());
pool.addUnchecked(entry.PeginsSpent(setPeginsSpent).FromTx(tx));
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin1].ToString(), tx4Hash.ToString());
BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin3].ToString(), tx4Hash.ToString());

setPeginsSpent = {pegin2, pegin3};
GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size());
tx.vout.resize(7);
tx.vout[6].nValue = 0;
txref = MakeTransactionRef(tx);
pool.removeForBlock({txref}, 3, setPeginsSpent);

BOOST_CHECK_EQUAL(pool.size(), 1);
BOOST_CHECK(pool.mapPeginsSpentToTxid.empty());
}

BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
{
CTxMemPool pool;
Expand Down Expand Up @@ -637,8 +549,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE);
BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000);
// ... we should keep the same min fee until we get a block
std::set<std::pair<uint256, COutPoint>> setPeginsSpentDummy;
pool.removeForBlock(vtx, 1, setPeginsSpentDummy);
pool.removeForBlock(vtx, 1);
SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE);
BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), llround((maxFeeRateRemoved.GetFeePerK() + 1000)/2.0));
// ... then feerate should drop 1/2 each halflife
Expand Down
11 changes: 5 additions & 6 deletions src/test/policyestimator_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
CAmount basefee(2000);
CAmount deltaFee(100);
std::vector<CAmount> feeV;
std::set<std::pair<uint256, COutPoint>> setPeginsSpentDummy;

// Populate vectors of increasing fees
for (int j = 0; j < 10; j++) {
Expand Down Expand Up @@ -75,7 +74,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
txHashes[9-h].pop_back();
}
}
mpool.removeForBlock(block, ++blocknum, setPeginsSpentDummy);
mpool.removeForBlock(block, ++blocknum);
block.clear();
// Check after just a few txs that combining buckets works as expected
if (blocknum == 3) {
Expand Down Expand Up @@ -114,7 +113,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
// Mine 50 more blocks with no transactions happening, estimates shouldn't change
// We haven't decayed the moving average enough so we still have enough data points in every bucket
while (blocknum < 250)
mpool.removeForBlock(block, ++blocknum, setPeginsSpentDummy);
mpool.removeForBlock(block, ++blocknum);

BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0));
for (int i = 2; i < 10;i++) {
Expand All @@ -134,7 +133,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
txHashes[j].push_back(hash);
}
}
mpool.removeForBlock(block, ++blocknum, setPeginsSpentDummy);
mpool.removeForBlock(block, ++blocknum);
}

for (int i = 1; i < 10;i++) {
Expand All @@ -151,7 +150,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
txHashes[j].pop_back();
}
}
mpool.removeForBlock(block, 266, setPeginsSpentDummy);
mpool.removeForBlock(block, 266);
block.clear();
BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0));
for (int i = 2; i < 10;i++) {
Expand All @@ -172,7 +171,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)

}
}
mpool.removeForBlock(block, ++blocknum, setPeginsSpentDummy);
mpool.removeForBlock(block, ++blocknum);
block.clear();
}
BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0));
Expand Down
37 changes: 2 additions & 35 deletions src/txmempool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,13 +403,6 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces

vTxHashes.emplace_back(tx.GetWitnessHash(), newit);
newit->vTxHashesIdx = vTxHashes.size() - 1;

// ELEMENTS:
typedef std::pair<uint256, COutPoint> PeginPair;
for(const PeginPair& it : entry.setPeginsSpent) {
std::pair<std::map<std::pair<uint256, COutPoint>, uint256>::iterator, bool> ret = mapPeginsSpentToTxid.insert(std::make_pair(it, tx.GetHash()));
assert(ret.second);
}
}

void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
Expand All @@ -419,12 +412,6 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
for (const CTxIn& txin : it->GetTx().vin)
mapNextTx.erase(txin.prevout);

// ELEMENTS:
typedef std::pair<uint256, COutPoint> PeginPair;
for (const PeginPair& it2 : it->setPeginsSpent) {
mapPeginsSpentToTxid.erase(it2);
}

if (vTxHashes.size() > 1) {
vTxHashes[it->vTxHashesIdx] = std::move(vTxHashes.back());
vTxHashes[it->vTxHashesIdx].second->vTxHashesIdx = it->vTxHashesIdx;
Expand Down Expand Up @@ -561,7 +548,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
/**
* Called when a block is connected. Removes from mempool and updates the miner fee estimator.
*/
void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight, const std::set<std::pair<uint256, COutPoint>>& setPeginsSpent, bool pak_transition)
void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight, bool pak_transition)
{
LOCK(cs);
std::vector<const CTxMemPoolEntry*> entries;
Expand All @@ -587,22 +574,6 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
ClearPrioritisation(tx->GetHash());
}

// ELEMENTS:
// Eject any conflicting pegins
for (std::set<std::pair<uint256, COutPoint> >::const_iterator it = setPeginsSpent.begin(); it != setPeginsSpent.end(); it++) {
std::map<std::pair<uint256, COutPoint>, uint256>::const_iterator it2 = mapPeginsSpentToTxid.find(*it);
if (it2 != mapPeginsSpentToTxid.end()) {
uint256 tx_id = it2->second;
txiter txit = mapTx.find(tx_id);
assert(txit != mapTx.end());
const CTransaction& tx = txit->GetTx();
setEntries stage;
stage.insert(txit);
RemoveStaged(stage, true);
removeRecursive(tx, MemPoolRemovalReason::CONFLICT);
ClearPrioritisation(tx_id);
}
}
// Eject any newly-invalid peg-outs based on changing block commitment
const CChainParams& chainparams = Params();
if (pak_transition && chainparams.GetEnforcePak()) {
Expand Down Expand Up @@ -786,10 +757,6 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
for (std::set<std::pair<uint256, COutPoint> >::const_iterator it = setGlobalPeginsSpent.begin(); it != setGlobalPeginsSpent.end(); it++) {
assert(!pcoins->IsPeginSpent(*it));
}
for (std::map<std::pair<uint256, COutPoint>, uint256>::const_iterator it = mapPeginsSpentToTxid.begin(); it != mapPeginsSpentToTxid.end(); it++) {
assert(setGlobalPeginsSpent.erase(it->first));
}
assert(setGlobalPeginsSpent.size() == 0);
// END ELEMENTS
//

Expand Down Expand Up @@ -988,7 +955,7 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const {

// ELEMENTS:
bool CCoinsViewMemPool::IsPeginSpent(const std::pair<uint256, COutPoint> &outpoint) const {
return mempool.mapPeginsSpentToTxid.count(outpoint) || base->IsPeginSpent(outpoint);
return base->IsPeginSpent(outpoint);
}

size_t CTxMemPool::DynamicMemoryUsage() const {
Expand Down
4 changes: 1 addition & 3 deletions src/txmempool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,6 @@ class CTxMemPool
const setEntries & GetMemPoolParents(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs);
const setEntries & GetMemPoolChildren(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs);
uint64_t CalculateDescendantMaximum(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs);
// ELEMENTS:
std::map<std::pair<uint256, COutPoint>, uint256> mapPeginsSpentToTxid;
private:
typedef std::map<txiter, setEntries, CompareIteratorByHash> cacheMap;

Expand Down Expand Up @@ -554,7 +552,7 @@ class CTxMemPool
void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight,
const std::set<std::pair<uint256, COutPoint>>& setPeginsSpent, bool pak_transition=false);
bool pak_transition=false);

void clear();
void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free
Expand Down
2 changes: 1 addition & 1 deletion src/validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2650,7 +2650,7 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp
// ELEMENTS: We also eject now-invalid peg-outs based on block transition if not config list set
// If config is set, this means all peg-outs have been filtered for that list already and other
// functionaries aren't matching your list. Operator should restart with no list or new matching list.
mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, setPeginsSpent, (paklist && !g_paklist_config));
mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, (paklist && !g_paklist_config));
disconnectpool.removeForBlock(blockConnecting.vtx);
// Update chainActive & related variables.
chainActive.SetTip(pindexNew);
Expand Down

0 comments on commit e1bf441

Please sign in to comment.