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

kill useless mapPeginsSpentToTxid #627

Merged
merged 1 commit into from
May 29, 2019
Merged

kill useless mapPeginsSpentToTxid #627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
93 changes: 2 additions & 91 deletions src/test/mempool_tests.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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