feed_qr_info reports previous-cycle rotation commitments that fail validation through QRInfoFeedResult::previous_cycle_invalid_count, but nothing acts on the number.
Where it stands
MasternodeListEngine::validate_and_store_previous_cycle_quorums is best-effort enrichment: a previous-cycle rotated quorum that fails its aggregate signature check is left out of the stored cycle and counted, and the feed continues. The count exists so a caller can judge the peer that served the data. In dash-spv the only consumer is a tracing::info! line in MasternodesManager::handle_message.
The result is that a peer serving corrupt previous-cycle commitments costs it nothing and costs us the cycle:
- The QRInfo is recorded as processed (
last_processed_qrinfo_tip), so should_process_qrinfo drops anything else arriving for that tip.
next_pipeline_mode fires at most one QRInfo per new tip inside the DKG mining window, so there is no retry against a different peer within the same tip.- The dropped indices stay unverifiable until a later QRInfo happens to carry them again, which means InstantSend locks selecting those quorums fail in the meantime.
Why PR #934 did not fix it
PR #934 makes the count trustworthy (it no longer blames the peer for a quarter signature our own elimination pass had to guess), but every minimal way to act on it is either a no-op or a hazard:
- Not recording
last_processed_qrinfo_tip when the count is non-zero achieves nothing on its own: should_process_qrinfo also requires a matching in-flight request, and no new request is issued for the same tip.
- Re-requesting on the same tip has no peer-selection input at that layer, so it can loop against the peer that just served the bad data.
Making this actionable needs peer attribution and a retry path that can pick a different peer, which is a larger change than PR #934 should carry.
Suggested direction
- Attribute the QRInfo response to the peer it came from and penalise it when
previous_cycle_invalid_count > 0, reusing the existing peer reputation/scoring rather than adding a parallel mechanism.
- Allow one re-request of the same tip when the response was penalised, routed to a different peer, bounded so a cycle cannot spend more than a couple of attempts on it.
- Keep the storage behaviour as is. Storing the verified subset is correct and the retry only needs to fill in what was dropped.
feed_qr_inforeports previous-cycle rotation commitments that fail validation throughQRInfoFeedResult::previous_cycle_invalid_count, but nothing acts on the number.Where it stands
MasternodeListEngine::validate_and_store_previous_cycle_quorumsis best-effort enrichment: a previous-cycle rotated quorum that fails its aggregate signature check is left out of the stored cycle and counted, and the feed continues. The count exists so a caller can judge the peer that served the data. Indash-spvthe only consumer is atracing::info!line inMasternodesManager::handle_message.The result is that a peer serving corrupt previous-cycle commitments costs it nothing and costs us the cycle:
last_processed_qrinfo_tip), soshould_process_qrinfodrops anything else arriving for that tip.next_pipeline_modefires at most one QRInfo per new tip inside the DKG mining window, so there is no retry against a different peer within the same tip.Why PR #934 did not fix it
PR #934 makes the count trustworthy (it no longer blames the peer for a quarter signature our own elimination pass had to guess), but every minimal way to act on it is either a no-op or a hazard:
last_processed_qrinfo_tipwhen the count is non-zero achieves nothing on its own:should_process_qrinfoalso requires a matching in-flight request, and no new request is issued for the same tip.Making this actionable needs peer attribution and a retry path that can pick a different peer, which is a larger change than PR #934 should carry.
Suggested direction
previous_cycle_invalid_count > 0, reusing the existing peer reputation/scoring rather than adding a parallel mechanism.