feat/fix(llmq): Ensure connections between IS quorums #4917
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
|
Can you explain the why a little bit more? |
|
Basically the issue is that two quorums of the same llmq type might not be able to "talk" to each other reliably. We only ensure connections inside the quorum I think. But my fix is too broad atm, should limit such connections per llmqtype or maybe do this for IS llmq types only (cause that's where it can be actually helpful I guess). Making it a draft for now, going to play with it a bit more. |
|
Should add tests for something like this, also this is more of a feature not a fix |
Every masternode will now "watch" a single node from _every other_ quorum in addition to intra-quorum connections. This should make propagation of recsigs produced by one quorum to other quorums much more reliable.
…match the actual behaviour (and avoid confusion with `CLLMQUtils::EnsureQuorumConnections`)
…umConnections` avoid calling slow `ScanQuorums` (no caching atm) inside the loop
|
Seems fine, but I'd like to see functional tests happy on CI |
|
@PastaPastaPasta I restarted tests a couple of times, few of them are green now (I believe test failures were unrelated). |
PastaPastaPasta
approved these changes
Jul 18, 2022
|
|
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jul 18, 2022
* fix(llmq): Ensure connections between quorums Every masternode will now "watch" a single node from _every other_ quorum in addition to intra-quorum connections. This should make propagation of recsigs produced by one quorum to other quorums much more reliable. * fix: Do this only for masternodes which participate in IS quorums * refactor: rename `CQuorumManager::EnsureQuorumConnections` to better match the actual behaviour (and avoid confusion with `CLLMQUtils::EnsureQuorumConnections`) * refactor: move IS quorums watch logic into `CQuorumManager::CheckQuorumConnections` avoid calling slow `ScanQuorums` (no caching atm) inside the loop * tests: check that inter-quorum connections are added * use `ranges::any_of`
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jul 18, 2022
* fix(llmq): Ensure connections between quorums Every masternode will now "watch" a single node from _every other_ quorum in addition to intra-quorum connections. This should make propagation of recsigs produced by one quorum to other quorums much more reliable. * fix: Do this only for masternodes which participate in IS quorums * refactor: rename `CQuorumManager::EnsureQuorumConnections` to better match the actual behaviour (and avoid confusion with `CLLMQUtils::EnsureQuorumConnections`) * refactor: move IS quorums watch logic into `CQuorumManager::CheckQuorumConnections` avoid calling slow `ScanQuorums` (no caching atm) inside the loop * tests: check that inter-quorum connections are added * use `ranges::any_of`
5 tasks
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Nov 16, 2023
* fix(llmq): Ensure connections between quorums Every masternode will now "watch" a single node from _every other_ quorum in addition to intra-quorum connections. This should make propagation of recsigs produced by one quorum to other quorums much more reliable. * fix: Do this only for masternodes which participate in IS quorums * refactor: rename `CQuorumManager::EnsureQuorumConnections` to better match the actual behaviour (and avoid confusion with `CLLMQUtils::EnsureQuorumConnections`) * refactor: move IS quorums watch logic into `CQuorumManager::CheckQuorumConnections` avoid calling slow `ScanQuorums` (no caching atm) inside the loop * tests: check that inter-quorum connections are added * use `ranges::any_of`
gades
pushed a commit
to piratecash/pirate
that referenced
this pull request
Dec 9, 2023
* fix(llmq): Ensure connections between quorums Every masternode will now "watch" a single node from _every other_ quorum in addition to intra-quorum connections. This should make propagation of recsigs produced by one quorum to other quorums much more reliable. * fix: Do this only for masternodes which participate in IS quorums * refactor: rename `CQuorumManager::EnsureQuorumConnections` to better match the actual behaviour (and avoid confusion with `CLLMQUtils::EnsureQuorumConnections`) * refactor: move IS quorums watch logic into `CQuorumManager::CheckQuorumConnections` avoid calling slow `ScanQuorums` (no caching atm) inside the loop * tests: check that inter-quorum connections are added * use `ranges::any_of`
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.
TLDR: Every masternode will now "watch" a single node from every other quorum in addition to intra-quorum connections. This should make propagation of recsigs produced by one quorum to other quorums much more reliable which is critical for creation of IS-locks for example.
To lock a tx you lock inputs first (each can be locked by a completely different quorum potentially) and then the tx itself is locked by another quorum when it receives qrecsigs for each input. No qrecsigs for any input, no is lock.
With tiny quorums on tiny networks e.g. 5 nodes total, 3 nodes to form a quorum there is always an overlap (same mns participate in multiple quorums) so tests pass. But for 15 nodes total and 4-5 nodes in a quorum there might be no overlap (so no qrecsigs are relayed to the tx-signing quorum) and tests fail. This should also be helpful for small devnets with tiny quorums.