Send & receive SCP quorum sets to fix SCP message propagation #621
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
AndrejMitrovic
added
type-bug
Codecov Report
@@ Coverage Diff @@
## v0.x.x #621 +/- ##
==========================================
- Coverage 91.34% 91.33% -0.02%
==========================================
Files 65 65
Lines 5167 5192 +25
==========================================
+ Hits 4720 4742 +22
- Misses 447 450 +3
Continue to review full report at Codecov.
|
AndrejMitrovic
added
the
type-feature
|
Some of these commits should be split off into another PR. |
AndrejMitrovic
added
the
status-blocked
AndrejMitrovic
commented
Mar 5, 2020
| @@ -365,6 +381,8 @@ public class NetworkManager | |||
| try | |||
| { | |||
| node.handshake(this.getAddress()); | |||
| if (node.quorum_hash != StellarHash.init) | |||
| this.quorum_sets[node.quorum_hash] = node.quorum_set; | |||
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
AndrejMitrovic
force-pushed
the
new-clients
branch
from
ee10fcb
to
745e00e
Compare
AndrejMitrovic
added
status-blocked
AndrejMitrovic
changed the title
AndrejMitrovic
force-pushed
the
new-clients
branch
2 times, most recently
from
427a75b
to
b87068e
Compare
AndrejMitrovic
changed the title
|
I will split this into two, and remove the necessity to exchange SCP structs in the API. |
This was referenced Mar 10, 2020
AndrejMitrovic
force-pushed
the
new-clients
branch
from
b87068e
to
695eb64
Compare
AndrejMitrovic
force-pushed
the
new-clients
branch
from
695eb64
to
5447d0b
Compare
This was referenced Apr 1, 2020
AndrejMitrovic
force-pushed
the
new-clients
branch
from
5447d0b
to
2aedea6
Compare
|
This needs to be reworked with regards to #702. |
AndrejMitrovic
removed
the
type-feature
AndrejMitrovic
changed the title
Temporary measure until we use our own hashes.
This is a temporary solution to make getQSet() in our implementation of the SCPDriver to work properly. In the future, the set of quorum hashes and sets will be known based on the quorum balancing algorithm and these APIs can then be removed..
If there was a new client connecting to use, it might have a quorum set associated with a hash that we previously haven't seen. If a new hash is found, it's added to the cache.
The previous set of fixes included: - Gossip all messages to connected peers, and not just to *our* quorum set - Retrieve all hash => QuorumSet mappings, through a set of new APIs. Together with these fixes, the cylic quorum configuration finally works. In addition, the ExtraChecks for the quorum configurations can finally be re-enabled. Fixes bosagora#606
AndrejMitrovic
force-pushed
the
new-clients
branch
from
2aedea6
to
c59bd05
Compare
AndrejMitrovic
added a commit
to AndrejMitrovic/agora
that referenced
this pull request
Apr 16, 2020
TrustHenry
reviewed
Apr 21, 2020
| { | ||
| auto stellar_hash = uint512(hash); | ||
| auto set = this.nominator.getQSet(stellar_hash); | ||
| assert(set.ptr !is null); |
There was a problem hiding this comment.
If StellarHash is not new, an error occurs.
Can it be used for malicious attacks?
|
As mentioned in #621 (comment), this PR has become unnecessary. We don't need to look up quorum sets explicitly because they will be known in advance once #240 is implemented. |
AndrejMitrovic
added a commit
to AndrejMitrovic/agora
that referenced
this pull request
Apr 29, 2020
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.
Fixes #606
Requires:
Validator: Connect to all quorum set validators #643Fix network discovery & start Node asynchronously #648