Multiple fixes and optimizations for LLMQs and ChainLocks #2724
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In case we got INV items for the same CLSIG that we recreated through HandleNewRecoveredSig, (re-)requesting of the CLSIG from other peers becomes unnecessary.
We bail out early in a few situations from this method, so that Cleanup() might not be called while its at the bottom.
UdjinM6
reviewed
Feb 26, 2019
Otherwise we might end up re-requesting it for a very long time when the commitment INV was received shortly before it got mined.
Otherwise we end up with session data in node states lingering around until a fake "timeout" occurs (can be seen in the logs).
The old code was emitting a lot of messages in logs as it treated sigs for exactly the same session as a conflict. This commit fixes this by looking at the signHash before logging. Also handle a corner-case where a recovered sig might be deleted between the HasRecoveredSigForId and GetRecoveredSigById call.
Instead of just tracking when the first share was received, we now also track when the last (non-duplicate) share was received. Sessios will now timeout 5 minutes after the first share arrives, or 1 minute after the last one arrived.
codablock
force-pushed
the
pr_llmq_fixes
branch
from
971a2cc
to
8279b27
Compare
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Mar 10, 2019
* Indicate success when signing was unnecessary * Fix typo in name of LLMQ_400_60 * Move RemoveAskFor call for CLSIGs into ProcessNewChainLock In case we got INV items for the same CLSIG that we recreated through HandleNewRecoveredSig, (re-)requesting of the CLSIG from other peers becomes unnecessary. * Move Cleanup() call in CChainLocksHandler::UpdatedBlockTip up We bail out early in a few situations from this method, so that Cleanup() might not be called while its at the bottom. * Bail out from CChainLocksHandler::UpdatedBlockTip if we already got the CLSIG * Call RemoveAskFor when QFCOMMITMENT was received Otherwise we might end up re-requesting it for a very long time when the commitment INV was received shortly before it got mined. * Call RemoveSigSharesForSession when a recovered sig is received Otherwise we end up with session data in node states lingering around until a fake "timeout" occurs (can be seen in the logs). * Better handling of false-positive conflicts in CSigningManager The old code was emitting a lot of messages in logs as it treated sigs for exactly the same session as a conflict. This commit fixes this by looking at the signHash before logging. Also handle a corner-case where a recovered sig might be deleted between the HasRecoveredSigForId and GetRecoveredSigById call. * Don't run into session timeout when sig shares come in slow Instead of just tracking when the first share was received, we now also track when the last (non-duplicate) share was received. Sessios will now timeout 5 minutes after the first share arrives, or 1 minute after the last one arrived.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits. These are all fixes for issues I found while implementing LLMQ based InstantSend.