Skip to content

Commit a247a63

Browse files
kwvgkwvg
committed
refactor: make CTxMemPool ProTx paths conditional on CDeterministicMNManager presence
Despite removeUnchecked not explicitly requiring CDeterministicMNManager, it has also been made conditional due to addUnchecked requiring the manager and allowing for some operations but not others when pertaining to a transaction type could allow inconsistencies to arise. Better to treat as one unit and skip both if manager isn't present.
1 parent 7aa8f54 commit a247a63

File tree

2 files changed

+96
-66
lines changed

2 files changed

+96
-66
lines changed

‎src/txmempool.cpp‎

Lines changed: 87 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -415,46 +415,8 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
415415
// Invalid ProTxes should never get this far because transactions should be
416416
// fully checked by AcceptToMemoryPool() at this point, so we just assume that
417417
// everything is fine here.
418-
if (tx.nType == TRANSACTION_PROVIDER_REGISTER) {
419-
auto proTx = *Assert(GetTxPayload<CProRegTx>(tx));
420-
if (!proTx.collateralOutpoint.hash.IsNull()) {
421-
mapProTxRefs.emplace(tx.GetHash(), proTx.collateralOutpoint.hash);
422-
}
423-
mapProTxAddresses.emplace(proTx.addr, tx.GetHash());
424-
mapProTxPubKeyIDs.emplace(proTx.keyIDOwner, tx.GetHash());
425-
mapProTxBlsPubKeyHashes.emplace(proTx.pubKeyOperator.GetHash(), tx.GetHash());
426-
if (!proTx.collateralOutpoint.hash.IsNull()) {
427-
mapProTxCollaterals.emplace(proTx.collateralOutpoint, tx.GetHash());
428-
} else {
429-
mapProTxCollaterals.emplace(COutPoint(tx.GetHash(), proTx.collateralOutpoint.n), tx.GetHash());
430-
}
431-
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) {
432-
auto proTx = *Assert(GetTxPayload<CProUpServTx>(tx));
433-
mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash());
434-
mapProTxAddresses.emplace(proTx.addr, tx.GetHash());
435-
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) {
436-
auto proTx = *Assert(GetTxPayload<CProUpRegTx>(tx));
437-
mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash());
438-
mapProTxBlsPubKeyHashes.emplace(proTx.pubKeyOperator.GetHash(), tx.GetHash());
439-
auto dmn = Assert(deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash));
440-
newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator);
441-
if (dmn->pdmnState->pubKeyOperator != proTx.pubKeyOperator) {
442-
newit->isKeyChangeProTx = true;
443-
}
444-
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) {
445-
auto proTx = *Assert(GetTxPayload<CProUpRevTx>(tx));
446-
mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash());
447-
auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash);
448-
assert(dmn);
449-
newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator);
450-
if (dmn->pdmnState->pubKeyOperator.Get() != CBLSPublicKey()) {
451-
newit->isKeyChangeProTx = true;
452-
}
453-
} else if (tx.nType == TRANSACTION_ASSET_UNLOCK) {
454-
auto assetUnlockTx = *Assert(GetTxPayload<CAssetUnlockPayload>(tx));
455-
mapAssetUnlockExpiry.insert({tx.GetHash(), assetUnlockTx.getHeightToExpiry()});
456-
} else if (tx.nType == TRANSACTION_MNHF_SIGNAL) {
457-
PrioritiseTransaction(tx.GetHash(), 0.1 * COIN);
418+
if (::deterministicMNManager) {
419+
addUncheckedProTx(newit, tx);
458420
}
459421
}
460422

@@ -590,6 +552,52 @@ bool CTxMemPool::removeSpentIndex(const uint256 txhash)
590552
return true;
591553
}
592554

555+
void CTxMemPool::addUncheckedProTx(indexed_transaction_set::iterator& newit, const CTransaction& tx)
556+
{
557+
assert(::deterministicMNManager);
558+
559+
if (tx.nType == TRANSACTION_PROVIDER_REGISTER) {
560+
auto proTx = *Assert(GetTxPayload<CProRegTx>(tx));
561+
if (!proTx.collateralOutpoint.hash.IsNull()) {
562+
mapProTxRefs.emplace(tx.GetHash(), proTx.collateralOutpoint.hash);
563+
}
564+
mapProTxAddresses.emplace(proTx.addr, tx.GetHash());
565+
mapProTxPubKeyIDs.emplace(proTx.keyIDOwner, tx.GetHash());
566+
mapProTxBlsPubKeyHashes.emplace(proTx.pubKeyOperator.GetHash(), tx.GetHash());
567+
if (!proTx.collateralOutpoint.hash.IsNull()) {
568+
mapProTxCollaterals.emplace(proTx.collateralOutpoint, tx.GetHash());
569+
} else {
570+
mapProTxCollaterals.emplace(COutPoint(tx.GetHash(), proTx.collateralOutpoint.n), tx.GetHash());
571+
}
572+
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) {
573+
auto proTx = *Assert(GetTxPayload<CProUpServTx>(tx));
574+
mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash());
575+
mapProTxAddresses.emplace(proTx.addr, tx.GetHash());
576+
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) {
577+
auto proTx = *Assert(GetTxPayload<CProUpRegTx>(tx));
578+
mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash());
579+
mapProTxBlsPubKeyHashes.emplace(proTx.pubKeyOperator.GetHash(), tx.GetHash());
580+
auto dmn = Assert(deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash));
581+
newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator);
582+
if (dmn->pdmnState->pubKeyOperator != proTx.pubKeyOperator) {
583+
newit->isKeyChangeProTx = true;
584+
}
585+
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) {
586+
auto proTx = *Assert(GetTxPayload<CProUpRevTx>(tx));
587+
mapProTxRefs.emplace(proTx.proTxHash, tx.GetHash());
588+
auto dmn = Assert(deterministicMNManager->GetListAtChainTip().GetMN(proTx.proTxHash));
589+
newit->validForProTxKey = ::SerializeHash(dmn->pdmnState->pubKeyOperator);
590+
if (dmn->pdmnState->pubKeyOperator.Get() != CBLSPublicKey()) {
591+
newit->isKeyChangeProTx = true;
592+
}
593+
} else if (tx.nType == TRANSACTION_ASSET_UNLOCK) {
594+
auto assetUnlockTx = *Assert(GetTxPayload<CAssetUnlockPayload>(tx));
595+
mapAssetUnlockExpiry.insert({tx.GetHash(), assetUnlockTx.getHeightToExpiry()});
596+
} else if (tx.nType == TRANSACTION_MNHF_SIGNAL) {
597+
PrioritiseTransaction(tx.GetHash(), 0.1 * COIN);
598+
}
599+
}
600+
593601
void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
594602
{
595603
if (reason != MemPoolRemovalReason::BLOCK) {
@@ -615,6 +623,23 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
615623
} else
616624
vTxHashes.clear();
617625

626+
if (::deterministicMNManager) {
627+
removeUncheckedProTx(it->GetTx());
628+
}
629+
630+
totalTxSize -= it->GetTxSize();
631+
m_total_fee -= it->GetFee();
632+
cachedInnerUsage -= it->DynamicMemoryUsage();
633+
cachedInnerUsage -= memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst());
634+
mapTx.erase(it);
635+
nTransactionsUpdated++;
636+
if (minerPolicyEstimator) {minerPolicyEstimator->removeTx(hash, false);}
637+
removeAddressIndex(hash);
638+
removeSpentIndex(hash);
639+
}
640+
641+
void CTxMemPool::removeUncheckedProTx(const CTransaction& tx)
642+
{
618643
auto eraseProTxRef = [&](const uint256& proTxHash, const uint256& txHash) {
619644
auto its = mapProTxRefs.equal_range(proTxHash);
620645
for (auto it = its.first; it != its.second;) {
@@ -626,40 +651,30 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
626651
}
627652
};
628653

629-
if (it->GetTx().nType == TRANSACTION_PROVIDER_REGISTER) {
630-
auto proTx = *Assert(GetTxPayload<CProRegTx>(it->GetTx()));
654+
if (tx.nType == TRANSACTION_PROVIDER_REGISTER) {
655+
auto proTx = *Assert(GetTxPayload<CProRegTx>(tx));
631656
if (!proTx.collateralOutpoint.IsNull()) {
632-
eraseProTxRef(it->GetTx().GetHash(), proTx.collateralOutpoint.hash);
657+
eraseProTxRef(tx.GetHash(), proTx.collateralOutpoint.hash);
633658
}
634659
mapProTxAddresses.erase(proTx.addr);
635660
mapProTxPubKeyIDs.erase(proTx.keyIDOwner);
636661
mapProTxBlsPubKeyHashes.erase(proTx.pubKeyOperator.GetHash());
637662
mapProTxCollaterals.erase(proTx.collateralOutpoint);
638-
mapProTxCollaterals.erase(COutPoint(it->GetTx().GetHash(), proTx.collateralOutpoint.n));
639-
} else if (it->GetTx().nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) {
640-
auto proTx = *Assert(GetTxPayload<CProUpServTx>(it->GetTx()));
641-
eraseProTxRef(proTx.proTxHash, it->GetTx().GetHash());
663+
mapProTxCollaterals.erase(COutPoint(tx.GetHash(), proTx.collateralOutpoint.n));
664+
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) {
665+
auto proTx = *Assert(GetTxPayload<CProUpServTx>(tx));
666+
eraseProTxRef(proTx.proTxHash, tx.GetHash());
642667
mapProTxAddresses.erase(proTx.addr);
643-
} else if (it->GetTx().nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) {
644-
auto proTx = *Assert(GetTxPayload<CProUpRegTx>(it->GetTx()));
645-
eraseProTxRef(proTx.proTxHash, it->GetTx().GetHash());
668+
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) {
669+
auto proTx = *Assert(GetTxPayload<CProUpRegTx>(tx));
670+
eraseProTxRef(proTx.proTxHash, tx.GetHash());
646671
mapProTxBlsPubKeyHashes.erase(proTx.pubKeyOperator.GetHash());
647-
} else if (it->GetTx().nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) {
648-
auto proTx = *Assert(GetTxPayload<CProUpRevTx>(it->GetTx()));
649-
eraseProTxRef(proTx.proTxHash, it->GetTx().GetHash());
650-
} else if (it->GetTx().nType == TRANSACTION_ASSET_UNLOCK) {
651-
mapAssetUnlockExpiry.erase(it->GetTx().GetHash());
672+
} else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) {
673+
auto proTx = *Assert(GetTxPayload<CProUpRevTx>(tx));
674+
eraseProTxRef(proTx.proTxHash, tx.GetHash());
675+
} else if (tx.nType == TRANSACTION_ASSET_UNLOCK) {
676+
mapAssetUnlockExpiry.erase(tx.GetHash());
652677
}
653-
654-
totalTxSize -= it->GetTxSize();
655-
m_total_fee -= it->GetFee();
656-
cachedInnerUsage -= it->DynamicMemoryUsage();
657-
cachedInnerUsage -= memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst());
658-
mapTx.erase(it);
659-
nTransactionsUpdated++;
660-
if (minerPolicyEstimator) {minerPolicyEstimator->removeTx(hash, false);}
661-
removeAddressIndex(hash);
662-
removeSpentIndex(hash);
663678
}
664679

665680
// Calculates descendants of entry that are not already in setDescendants, and adds to
@@ -829,6 +844,8 @@ void CTxMemPool::removeProTxCollateralConflicts(const CTransaction &tx, const CO
829844

830845
void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx)
831846
{
847+
assert(::deterministicMNManager);
848+
832849
// Remove TXs that refer to a MN for which the collateral was spent
833850
auto removeSpentCollateralConflict = [&](const uint256& proTxHash) {
834851
// Can't use equal_range here as every call to removeRecursive might invalidate iterators
@@ -966,7 +983,9 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
966983
RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
967984
}
968985
removeConflicts(*tx);
969-
removeProTxConflicts(*tx);
986+
if (::deterministicMNManager) {
987+
removeProTxConflicts(*tx);
988+
}
970989
ClearPrioritisation(tx->GetHash());
971990
}
972991
lastRollingFeeUpdate = GetTime();
@@ -1240,6 +1259,8 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const
12401259
}
12411260

12421261
bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const {
1262+
assert(::deterministicMNManager);
1263+
12431264
LOCK(cs);
12441265

12451266
auto hasKeyChangeInMempool = [&](const uint256& proTxHash) {

‎src/txmempool.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,7 @@ class CTxMemPool
759759
TxMempoolInfo info(const uint256& hash) const;
760760
std::vector<TxMempoolInfo> infoAll() const;
761761

762+
/** @pre Caller must ensure that CDeterministicMNManager exists */
762763
bool existsProviderTxConflict(const CTransaction &tx) const;
763764

764765
size_t DynamicMemoryUsage() const;
@@ -816,6 +817,12 @@ class CTxMemPool
816817
/** Sever link between specified transaction and direct children. */
817818
void UpdateChildrenForRemoval(txiter entry) EXCLUSIVE_LOCKS_REQUIRED(cs);
818819

820+
/**
821+
* addUnchecked extension for Dash-specific transactions (ProTx).
822+
* Depends on CDeterministicMNManager.
823+
*/
824+
void addUncheckedProTx(indexed_transaction_set::iterator& newit, const CTransaction& tx);
825+
819826
/** Before calling removeUnchecked for a given transaction,
820827
* UpdateForRemoveFromMempool must be called on the entire (dependent) set
821828
* of transactions being removed at the same time. We use each
@@ -825,6 +832,8 @@ class CTxMemPool
825832
* removal.
826833
*/
827834
void removeUnchecked(txiter entry, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
835+
void removeUncheckedProTx(const CTransaction& tx);
836+
828837
public:
829838
/** visited marks a CTxMemPoolEntry as having been traversed
830839
* during the lifetime of the most recently created Epoch::Guard

0 commit comments

Comments
 (0)