[LLMQ][BugFix] Fix infinite loop in quorum relay members selection algorithm #2744
Conversation
This file contains hidden or 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
furszy
changed the title
furszy
force-pushed
the
2022_GetQuorumRelayMembers_infinite_loop
branch
from
e17129a to
c9a0ebb
Compare
furszy
changed the title
furszy
force-pushed
the
2022_GetQuorumRelayMembers_infinite_loop
branch
from
79ecd82 to
90743c8
Compare
…descriptive and readable.
…t the last quorum index. Can just look it up on chainActive.
1) No need to re-calculate the quorum members internally, all the function' callers already have calculated the list. 2) As 'onlyOutbound' argument is always true. There is no need to loop over all the quorum members. The function only calculates the local DMN outbound relay connections.
Also more strict checking of expected size in unit test
…l case of quorum size of two members.
by initializing CDeterministicMNCPtr lists. Also speed up get_quorum_relay_members test loop by precomputing the log2 and checking quorum sizes in decreasing order. Also test up to 2000-mns quorums
furszy
force-pushed
the
2022_GetQuorumRelayMembers_infinite_loop
branch
from
64f5f21 to
b4e7fe1
Compare
Author
|
Rebased on master, plus added three cool commits from zebra that simplify the new test code. Ready to go. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Follow-up work to my on-going review of #2722 (review). Starts in eeefe79c.
The current intra-quorum relay members selection algorithm receives a quorum member and selects a number of relay connections equals to the quorum size bits count.
So, for example: each DMN, in a quorum of 400 members, deterministically selects 9 other members and mark them as intra-quorum-relay members.
In the test LLMQ params case, with a quorum of 2, this algorithm falls into an infinite loop. Blocking the node's LLMQ session forever.
The process cannot select the input member for intra-quorum relay and expects to return a minimum of 2 elements.
Added a test case commit first so the issue can be easily verified/tested.
Aside from that, added some other performance improvements and cleanups for the LLMQ sessions:
CDKGSessionHandler::UpdateBlockTip: No need to walk-backwards through the chain to get the last quorum index. Can just look it up on chainActive`.