Skip to content

Commit d15ce14

Browse files
fix(llmq): retain signHash marker on recSig truncation and drop late sigShares
- Problem: After we process a recovered signature, TruncateRecoveredSig removes the id/signHash indexes (rs_r/rs_s). A very late sigShare for that session then sees HasRecoveredSigForId as false, recreates a session, and eventually times out, even though we already finished the session. - Behavior change: Keep the rs_s entry (signHash -> 1) when truncating recovered sigs so HasRecoveredSigForSession(signHash) stays true after truncation. Cleanup still removes the remaining keys when appropriate. - Handling late shares: CSigSharesManager::ProcessMessageSigShare now also checks HasRecoveredSigForSession and logs/drops sigShares for sessions already completed, preventing “zombie” sessions/timeouts. - Notes: This leaves the recSig hash path (rs_h) unchanged; full deletions (conflict/cleanup) still remove all indexes. No extra caching insertions to keep behavior parallel to rs_h.
1 parent 23de969 commit d15ce14

File tree

3 files changed

+16
-7
lines changed

3 files changed

+16
-7
lines changed

src/llmq/signing.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
183183
batch.Erase(k2);
184184
if (deleteHashKey) {
185185
batch.Erase(k3);
186+
batch.Erase(k4);
186187
}
187-
batch.Erase(k4);
188188

189189
if (deleteTimeKey) {
190190
CDataStream writeTimeDs(SER_DISK, CLIENT_VERSION);
@@ -199,8 +199,8 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
199199

200200
LOCK(cs_cache);
201201
hasSigForIdCache.erase(std::make_pair(recSig.getLlmqType(), recSig.getId()));
202-
hasSigForSessionCache.erase(signHash.Get());
203202
if (deleteHashKey) {
203+
hasSigForSessionCache.erase(signHash.Get());
204204
hasSigForHashCache.erase(recSig.GetHash());
205205
}
206206
}

src/llmq/signing.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ class CSigningManager
204204

205205
// This is called when a recovered signature can be safely removed from the DB. This is only safe when some other
206206
// mechanism prevents possible conflicts. As an example, ChainLocks prevent conflicts in confirmed TXs InstantSend votes
207-
// This won't completely remove all traces of the recovered sig but instead leave the hash entry in the DB. This
208-
// allows AlreadyHave to keep returning true. Cleanup will later remove the remains
207+
// This won't completely remove all traces of the recovered sig but instead leave the hash and signHash entries in the
208+
// DB. This allows AlreadyHave/late-share filtering to keep returning true. Cleanup will later remove the remains
209209
void TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id);
210210

211211
private:

src/llmq/signing_shares.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -503,14 +503,23 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& s
503503
return;
504504
}
505505

506+
const auto signHash = sigShare.GetSignHash();
507+
const bool alreadyRecovered = sigman.HasRecoveredSigForId(sigShare.getLlmqType(), sigShare.getId()) ||
508+
sigman.HasRecoveredSigForSession(signHash);
509+
506510
{
507511
LOCK(cs);
508512

509-
if (sigShares.Has(sigShare.GetKey())) {
513+
if (alreadyRecovered) {
514+
LogPrint(BCLog::LLMQ_SIGS,
515+
"CSigSharesManager::%s -- dropping sigShare for recovered session. signHash=%s, id=%s, "
516+
"msgHash=%s, member=%d, node=%d\n",
517+
__func__, signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(),
518+
sigShare.getQuorumMember(), fromId);
510519
return;
511520
}
512521

513-
if (sigman.HasRecoveredSigForId(sigShare.getLlmqType(), sigShare.getId())) {
522+
if (sigShares.Has(sigShare.GetKey())) {
514523
return;
515524
}
516525

@@ -519,7 +528,7 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& s
519528
}
520529

521530
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, id=%s, msgHash=%s, member=%d, node=%d\n", __func__,
522-
sigShare.GetSignHash().ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), sigShare.getQuorumMember(), fromId);
531+
signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), sigShare.getQuorumMember(), fromId);
523532
}
524533

525534
bool CSigSharesManager::PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager,

0 commit comments

Comments
 (0)