Skip to content

Commit 23de969

Browse files
PastaPastaPastaPastaPastaPasta
committed
Merge #6953: feat: instantsend cache heights and reduce cs_main contention
e918729 refactor: add CacheTipHeight, improve cache API safety (UdjinM6) 493d077 refactor: simplify optional usage (pasta) 0e1f114 instantsend: cache heights and reduce cs_main contention (pasta) Pull request description: ## Issue being fixed or feature implemented Today's testing on testnet showed under load, cs_main was a major contender: (however, note the major contenders from before #6912 / #6468 are now significantly reduced) ``` ======================================================================================================================== TOP 40 LOCATIONS BY TOTAL CONTENTION TIME (AGGREGATE) ======================================================================================================================== Lock Name Location Count Total(μs) Avg(μs) Max(μs) Nodes ------------------------------------------------------------------------------------------------------------------------ cs llmq/signing_shares.cpp:507 11952 81357095 6807.0 299734 8 ::cs_main instantsend/signing.cpp:210 75241 62985844 837.1 93240 8 cs_main index/base.cpp:340 29584 27040349 914.0 1613365 8 cs_main net_processing.cpp:5462 69291 19784473 285.5 1329191 8 cs llmq/signing_shares.cpp:1732 2355 17466035 7416.6 127471 8 ::cs_main llmq/quorums.cpp:1214 38975 15478543 397.1 883884 8 m_nodes_mutex net.cpp:2043 33457 14736059 440.4 31622 8 ::cs_main llmq/quorums.cpp:526 10587 10577503 999.1 2083501 8 pnode->cs_vSend net.cpp:2435 68528 9671050 141.1 37265 8 cs_main net_processing.cpp:4196 4945 8608157 1740.8 1326146 8 ::cs_main instantsend/instantsend.cpp:272 5602 8416269 1502.4 1260997 8 cs txmempool.cpp:1319 7942 8059684 1014.8 356199 8 ::cs_main validation.cpp:3747 309 6468144 20932.5 1225274 8 ::cs_main validation.cpp:6009 21855 3393152 155.3 228195 8 pnode->cs_vSend net.cpp:4709 7532 2299813 305.3 41543 8 m_nodes_mutex ./net.h:1374 6229 1998506 320.8 15593 8 inv_relay->m_tx_inventory_mutex net_processing.cpp:1169 7687 1871859 243.5 16292 8 cs_db instantsend/db.cpp:239 4323 1527297 353.3 20779 8 cs_cache spork.cpp:244 18083 1331472 73.6 27722 8 ::cs_main chainlock/signing.cpp:58 98 1312496 13392.8 1273691 8 cs_db ./instantsend/db.h:139 4381 1305738 298.0 29777 8 mutexMsgProc net.cpp:2604 4877 1243737 255.0 24641 8 ``` - Add unordered_lru_cache for block heights in CInstantSendManager - Provide GetTipHeight() (non-optional) and GetBlockHeight() helpers - Use cache in InstantSendSigner::CheckCanLock - Cache-first for cycleHash height in ISLOCK message path and batch verify - Cache-first mined-height for HasChainLock and WriteInstantSendLockMined ## What was done? ## How Has This Been Tested? ## Breaking Changes _Please describe any breaking changes your code introduces_ ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK e918729 Tree-SHA512: 1ea45e92c717dbefe5625bba3b27612b9ffbb21ea106d11005d78836fbe6ce00503e3f3fdb927fb405b09fd795f50137ca2beb07351bf94488d2af5694e543ba
2 parents 86e84d7 + e918729 commit 23de969

5 files changed

Lines changed: 149 additions & 38 deletions

File tree

‎src/instantsend/instantsend.cpp‎

Lines changed: 86 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -145,25 +145,32 @@ std::variant<uint256, CTransactionRef, std::monostate> CInstantSendManager::Proc
145145

146146
uint256 hashBlock{};
147147
const auto tx = GetTransaction(nullptr, &mempool, islock->txid, Params().GetConsensus(), hashBlock);
148-
const CBlockIndex* pindexMined{nullptr};
149148
const bool found_transaction{tx != nullptr};
150149
// we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally
151-
if (found_transaction && !hashBlock.IsNull()) {
152-
pindexMined = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(hashBlock));
153-
150+
std::optional<int> minedHeight = GetBlockHeight(hashBlock);
151+
if (found_transaction) {
152+
if (!minedHeight.has_value()) {
153+
const CBlockIndex* pindexMined = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(hashBlock));
154+
if (pindexMined != nullptr) {
155+
CacheBlockHeight(pindexMined);
156+
minedHeight = pindexMined->nHeight;
157+
}
158+
}
154159
// Let's see if the TX that was locked by this islock is already mined in a ChainLocked block. If yes,
155160
// we can simply ignore the islock, as the ChainLock implies locking of all TXs in that chain
156-
if (pindexMined != nullptr && clhandler.HasChainLock(pindexMined->nHeight, pindexMined->GetBlockHash())) {
157-
LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txlock=%s, islock=%s: dropping islock as it already got a ChainLock in block %s, peer=%d\n", __func__,
158-
islock->txid.ToString(), hash.ToString(), hashBlock.ToString(), from);
161+
if (minedHeight.has_value() && clhandler.HasChainLock(*minedHeight, hashBlock)) {
162+
LogPrint(BCLog::INSTANTSEND, /* Continued */
163+
"CInstantSendManager::%s -- txlock=%s, islock=%s: dropping islock as it already got a "
164+
"ChainLock in block %s, peer=%d\n",
165+
__func__, islock->txid.ToString(), hash.ToString(), hashBlock.ToString(), from);
159166
return std::monostate{};
160167
}
161168
}
162169

163170
if (found_transaction) {
164171
db.WriteNewInstantSendLock(hash, islock);
165-
if (pindexMined) {
166-
db.WriteInstantSendLockMined(hash, pindexMined->nHeight);
172+
if (minedHeight.has_value()) {
173+
db.WriteInstantSendLockMined(hash, *minedHeight);
167174
}
168175
} else {
169176
// put it in a separate pending map and try again later
@@ -251,6 +258,8 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr<const CBlock>& pb
251258
return;
252259
}
253260

261+
CacheTipHeight(pindex);
262+
254263
if (m_mn_sync.IsBlockchainSynced()) {
255264
const bool has_chainlock = clhandler.HasChainLock(pindex->nHeight, pindex->GetBlockHash());
256265
for (const auto& tx : pblock->vtx) {
@@ -278,6 +287,13 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr<const CBlock>& pb
278287
void CInstantSendManager::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock,
279288
const CBlockIndex* pindexDisconnected)
280289
{
290+
{
291+
LOCK(cs_height_cache);
292+
m_cached_block_heights.erase(pindexDisconnected->GetBlockHash());
293+
}
294+
295+
CacheTipHeight(pindexDisconnected->pprev);
296+
281297
db.RemoveBlockInstantSendLocks(pblock, pindexDisconnected);
282298
}
283299

@@ -417,6 +433,8 @@ void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindexChainLock)
417433

418434
void CInstantSendManager::UpdatedBlockTip(const CBlockIndex* pindexNew)
419435
{
436+
CacheTipHeight(pindexNew);
437+
420438
bool fDIP0008Active = pindexNew->pprev && pindexNew->pprev->nHeight >= Params().GetConsensus().DIP0008Height;
421439

422440
if (AreChainLocksEnabled(spork_manager) && fDIP0008Active) {
@@ -597,7 +615,7 @@ void CInstantSendManager::RemoveConflictingLock(const uint256& islockHash, const
597615
{
598616
LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: Removing ISLOCK and its chained children\n", __func__,
599617
islock.txid.ToString(), islockHash.ToString());
600-
int tipHeight = WITH_LOCK(::cs_main, return m_chainstate.m_chain.Height());
618+
const int tipHeight = GetTipHeight();
601619

602620
auto removedIslocks = db.RemoveChainedInstantSendLocks(islockHash, islock.txid, tipHeight);
603621
for (const auto& h : removedIslocks) {
@@ -703,6 +721,64 @@ size_t CInstantSendManager::GetInstantSendLockCount() const
703721
return db.GetInstantSendLockCount();
704722
}
705723

724+
void CInstantSendManager::CacheBlockHeightInternal(const CBlockIndex* const block_index) const
725+
{
726+
AssertLockHeld(cs_height_cache);
727+
m_cached_block_heights.insert(block_index->GetBlockHash(), block_index->nHeight);
728+
}
729+
730+
void CInstantSendManager::CacheBlockHeight(const CBlockIndex* const block_index) const
731+
{
732+
LOCK(cs_height_cache);
733+
CacheBlockHeightInternal(block_index);
734+
}
735+
736+
std::optional<int> CInstantSendManager::GetBlockHeight(const uint256& hash) const
737+
{
738+
if (hash.IsNull()) {
739+
return std::nullopt;
740+
}
741+
{
742+
LOCK(cs_height_cache);
743+
int cached_height{0};
744+
if (m_cached_block_heights.get(hash, cached_height)) return cached_height;
745+
}
746+
747+
const CBlockIndex* pindex = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(hash));
748+
if (pindex == nullptr) {
749+
return std::nullopt;
750+
}
751+
752+
CacheBlockHeight(pindex);
753+
return pindex->nHeight;
754+
}
755+
756+
void CInstantSendManager::CacheTipHeight(const CBlockIndex* const tip) const
757+
{
758+
LOCK(cs_height_cache);
759+
if (tip) {
760+
CacheBlockHeightInternal(tip);
761+
m_cached_tip_height = tip->nHeight;
762+
} else {
763+
m_cached_tip_height = -1;
764+
}
765+
}
766+
767+
int CInstantSendManager::GetTipHeight() const
768+
{
769+
{
770+
LOCK(cs_height_cache);
771+
if (m_cached_tip_height >= 0) {
772+
return m_cached_tip_height;
773+
}
774+
}
775+
776+
const CBlockIndex* tip = WITH_LOCK(::cs_main, return m_chainstate.m_chain.Tip());
777+
778+
CacheTipHeight(tip);
779+
return tip ? tip->nHeight : -1;
780+
}
781+
706782
bool CInstantSendManager::IsInstantSendEnabled() const
707783
{
708784
return !fReindex && !fImporting && spork_manager.IsSporkActive(SPORK_2_INSTANTSEND_ENABLED);

‎src/instantsend/instantsend.h‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717
#include <unordered_lru_cache.h>
1818

1919
#include <atomic>
20-
#include <unordered_map>
20+
#include <optional>
2121
#include <unordered_set>
2222
#include <variant>
2323
#include <vector>
2424

25+
#include <saltedhasher.h>
26+
2527
class CBlockIndex;
2628
class CChainState;
2729
class CDataStream;
@@ -91,6 +93,14 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
9193
mutable Mutex cs_timingsTxSeen;
9294
Uint256HashMap<int64_t> timingsTxSeen GUARDED_BY(cs_timingsTxSeen);
9395

96+
mutable Mutex cs_height_cache;
97+
static constexpr size_t MAX_BLOCK_HEIGHT_CACHE{16384};
98+
mutable unordered_lru_cache<uint256, int, StaticSaltedHasher, MAX_BLOCK_HEIGHT_CACHE> m_cached_block_heights
99+
GUARDED_BY(cs_height_cache);
100+
mutable int m_cached_tip_height GUARDED_BY(cs_height_cache){-1};
101+
102+
void CacheBlockHeightInternal(const CBlockIndex* const block_index) const EXCLUSIVE_LOCKS_REQUIRED(cs_height_cache);
103+
94104
public:
95105
CInstantSendManager() = delete;
96106
CInstantSendManager(const CInstantSendManager&) = delete;
@@ -122,7 +132,7 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
122132
void RemoveMempoolConflictsForLock(const uint256& hash, const instantsend::InstantSendLock& islock)
123133
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
124134
void ResolveBlockConflicts(const uint256& islockHash, const instantsend::InstantSendLock& islock)
125-
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
135+
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry, !cs_height_cache);
126136

127137
void HandleFullyConfirmedBlock(const CBlockIndex* pindex)
128138
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
@@ -142,14 +152,15 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
142152
CSigningManager& Sigman() { return sigman; }
143153
[[nodiscard]] std::variant<uint256, CTransactionRef, std::monostate> ProcessInstantSendLock(
144154
NodeId from, const uint256& hash, const instantsend::InstantSendLockPtr& islock)
145-
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
155+
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry, !cs_height_cache);
146156

147157
void TransactionAddedToMempool(const CTransactionRef& tx)
148158
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry, !cs_timingsTxSeen);
149-
void TransactionRemovedFromMempool(const CTransactionRef& tx);
159+
void TransactionRemovedFromMempool(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
150160
void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex)
151-
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry, !cs_timingsTxSeen);
152-
void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexDisconnected);
161+
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry, !cs_timingsTxSeen, !cs_height_cache);
162+
void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexDisconnected)
163+
EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
153164

154165
bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks);
155166
bool GetInstantSendLockByHash(const uint256& hash, instantsend::InstantSendLock& ret) const
@@ -159,14 +170,20 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
159170
void NotifyChainLock(const CBlockIndex* pindexChainLock)
160171
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
161172
void UpdatedBlockTip(const CBlockIndex* pindexNew)
162-
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
173+
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry, !cs_height_cache);
163174

164-
void RemoveConflictingLock(const uint256& islockHash, const instantsend::InstantSendLock& islock);
175+
void RemoveConflictingLock(const uint256& islockHash, const instantsend::InstantSendLock& islock)
176+
EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
165177
void TryEmplacePendingLock(const uint256& hash, const NodeId id, const instantsend::InstantSendLockPtr& islock) override
166178
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks);
167179

168180
size_t GetInstantSendLockCount() const;
169181

182+
void CacheBlockHeight(const CBlockIndex* const block_index) const EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
183+
std::optional<int> GetBlockHeight(const uint256& hash) const override EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
184+
void CacheTipHeight(const CBlockIndex* const tip) const EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
185+
int GetTipHeight() const override EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);
186+
170187
bool IsInstantSendEnabled() const override;
171188
/**
172189
* If true, MN should sign all transactions, if false, MN should not sign

‎src/instantsend/net_instantsend.cpp‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,18 +34,18 @@ void NetInstantSend::ProcessMessage(CNode& pfrom, const std::string& msg_type, C
3434
return;
3535
}
3636

37-
int block_height = [this, islock] {
37+
auto cycleHeightOpt = m_is_manager.GetBlockHeight(islock->cycleHash);
38+
if (!cycleHeightOpt) {
3839
const auto blockIndex = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(islock->cycleHash));
3940
if (blockIndex == nullptr) {
40-
return -1;
41+
// Maybe we don't have the block yet or maybe some peer spams invalid values for cycleHash
42+
m_peer_manager->PeerMisbehaving(pfrom.GetId(), 1);
43+
return;
4144
}
42-
return blockIndex->nHeight;
43-
}();
44-
if (block_height < 0) {
45-
// Maybe we don't have the block yet or maybe some peer spams invalid values for cycleHash
46-
m_peer_manager->PeerMisbehaving(pfrom.GetId(), 1);
47-
return;
45+
m_is_manager.CacheBlockHeight(blockIndex);
46+
cycleHeightOpt = blockIndex->nHeight;
4847
}
48+
const int block_height = *cycleHeightOpt;
4949

5050
// Deterministic islocks MUST use rotation based llmq
5151
auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend;
@@ -125,16 +125,18 @@ Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks(
125125
continue;
126126
}
127127

128-
const auto blockIndex = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(islock->cycleHash));
129-
if (blockIndex == nullptr) {
128+
auto cycleHeightOpt = m_is_manager.GetBlockHeight(islock->cycleHash);
129+
if (!cycleHeightOpt) {
130130
batchVerifier.badSources.emplace(nodeId);
131131
continue;
132132
}
133133

134134
int nSignHeight{-1};
135135
const auto dkgInterval = llmq_params.dkgInterval;
136-
if (blockIndex->nHeight + dkgInterval < m_chainstate.m_chain.Height()) {
137-
nSignHeight = blockIndex->nHeight + dkgInterval - 1;
136+
const int tipHeight = m_is_manager.GetTipHeight();
137+
const int cycleHeight = *cycleHeightOpt;
138+
if (cycleHeight + dkgInterval < tipHeight) {
139+
nSignHeight = cycleHeight + dkgInterval - 1;
138140
}
139141
// For RegTest non-rotating quorum cycleHash has directly quorum hash
140142
auto quorum = llmq_params.useRotation ? llmq::SelectQuorumForSigning(llmq_params, m_chainstate.m_chain, m_qman,

‎src/instantsend/signing.cpp‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -204,16 +204,28 @@ bool InstantSendSigner::CheckCanLock(const COutPoint& outpoint, bool printDebug,
204204
return false;
205205
}
206206

207-
const CBlockIndex* pindexMined;
208-
int nTxAge;
209-
{
210-
LOCK(::cs_main);
211-
pindexMined = m_chainstate.m_blockman.LookupBlockIndex(hashBlock);
212-
nTxAge = m_chainstate.m_chain.Height() - pindexMined->nHeight + 1;
207+
const auto blockHeight = m_isman.GetBlockHeight(hashBlock);
208+
if (!blockHeight) {
209+
if (printDebug) {
210+
LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: failed to determine mined height for parent TX %s\n", __func__,
211+
txHash.ToString(), outpoint.hash.ToString());
212+
}
213+
return false;
214+
}
215+
216+
const int tipHeight = m_isman.GetTipHeight();
217+
218+
if (tipHeight < *blockHeight) {
219+
if (printDebug) {
220+
LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: cached tip height %d is below block height %d for parent TX %s\n",
221+
__func__, txHash.ToString(), tipHeight, *blockHeight, outpoint.hash.ToString());
222+
}
223+
return false;
213224
}
214225

215-
if (nTxAge < nInstantSendConfirmationsRequired &&
216-
!m_clhandler.HasChainLock(pindexMined->nHeight, pindexMined->GetBlockHash())) {
226+
const int nTxAge = tipHeight - *blockHeight + 1;
227+
228+
if (nTxAge < nInstantSendConfirmationsRequired && !m_clhandler.HasChainLock(*blockHeight, hashBlock)) {
217229
if (printDebug) {
218230
LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: outpoint %s too new and not ChainLocked. nTxAge=%d, nInstantSendConfirmationsRequired=%d\n", __func__,
219231
txHash.ToString(), outpoint.ToStringShort(), nTxAge, nInstantSendConfirmationsRequired);

‎src/instantsend/signing.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include <instantsend/lock.h>
99
#include <llmq/signing.h>
1010

11+
#include <optional>
12+
1113
class CMasternodeSync;
1214
class CSporkManager;
1315
class CTxMemPool;
@@ -34,6 +36,8 @@ class InstantSendSignerParent
3436
virtual bool IsLocked(const uint256& txHash) const = 0;
3537
virtual InstantSendLockPtr GetConflictingLock(const CTransaction& tx) const = 0;
3638
virtual void TryEmplacePendingLock(const uint256& hash, const NodeId id, const InstantSendLockPtr& islock) = 0;
39+
virtual std::optional<int> GetBlockHeight(const uint256& hash) const = 0;
40+
virtual int GetTipHeight() const = 0;
3741
};
3842

3943
class InstantSendSigner final : public llmq::CRecoveredSigsListener

0 commit comments

Comments
 (0)