Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More Dash llmq backports #2941

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

panleone
Copy link

@panleone panleone commented Oct 29, 2024

Follow up of #2921

each commit backports a PR. you can find the number of the PR in the commit description

codablock and others added 9 commits October 29, 2024 15:38
* Implement and use SigShareMap instead of ordered map with helper methods

The old implementation was relying on the maps being ordered, which allowed
us to grab all sig shares for the same signHash by doing range queries on
the map. This has the disadvantage of being unnecessarily slow when the
maps get larger. Using an unordered map would be the naive solution, but
then it's not possible to query by range anymore.

The solution now is to have a specialized map "SigShareMap" which is
indexed by "SigShareKey". It's internally just an unordered map, indexed by
the sign hash and another unordered map for the value, indexed by the
quorum member index.

* Only use unordered maps/sets in CSigSharesManager

These are faster when maps/sets get larger.

* Use unorderes sets/maps in CSigningManager
…2712)

Otherwise we get some false-positive timeout messages in logs.
…ct#2724)

* 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.
Add more caching to CRecoveredSigsDb and use salted hashing for externally provided keys
Introduce "session announcements" and session IDs used in LLMQ P2P messages
…esManager (PIVX-Project#2729)

* Return bool in ProcessMessageXXX methods to indicate misbehaviour

* Send/Receive multiple messages as part of one P2P message in CSigSharesManager

Many messages, especially QSIGSHARESINV and QGETSIGSHARES, are very small
by nature (5-14 bytes for a 50 members LLMQ). The message headers are
24 bytes, meaning that we produce a lot of overhead for these small messages.
This sums up quite a bit when thousands of signing sessions are happening
in parallel.

This commit changes all related P2P messages to send a vector of messages
instead of a single message.

* Remove bogus lines

Included these by accident

* Unify handling of BanNode in ProcessMessageXXX methods

* Remove bogus check for fMasternodeMode

* Properly use == instead of misleading >= in SendMessages

* Put "didSend = true" near PushMessage
…-Project#2730)

Was wondering why verification was always 0ms...this explains it :)
… a share (PIVX-Project#2731)

* On timeout, print members proTxHashes from members which did not send a share

* Move inactive quorums check above timeout checks

This allows to reuse things in the next commit

* Avoid locking cs_main through GetQuorum by using a pre-filled map

* Use find() instead of [] to access quorums map
…X-Project#2733)

* Ignore sig share inv messages when we don't have the quorum vvec

* Update src/llmq/quorums_signing_shares.cpp

Co-Authored-By: codablock <[email protected]>
* Fix deadlock in CSigSharesManager::SendMessages

Locking "cs" at this location caused a (potential) deadlock due to changed
order of cs and cs_vNodes locking. This changes the method to not require
the session object anymore which removes the need for locking.

* Pass size of LLMQ instead of llmqType into CSigSharesInv::Init

This allows use of sizes which are not supported in chainparams.
Copy link
Member

@Duddino Duddino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK a29d294

Consistent with upstream commits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants