Add a registerListener() API #645

AndrejMitrovic · 2020-03-10T07:48:11Z

This was hard to pull off because of the complex design of the NetworkManager class (see #644).

Split off from #621

Part of #606 (but does not fully resolve it yet).

It was the only method with a unique name, but it was not doing any "handshaking" and was only retrieving the public key.

To allow incoming connections from nodes outside the known network list.

For future incoming connections, we will: - skip network discovery, as that connection may be untrusted - not automatically register as a listener

An outsider node which is not part of the network config of a given node will not receive any gossiping. This is a misdesign, and will be fixed in the next commit.

codecov · 2020-03-10T07:55:41Z

Codecov Report

Merging #645 into v0.x.x will increase coverage by 0.08%.
The diff coverage is 94.28%.

@@            Coverage Diff             @@
##           v0.x.x     #645      +/-   ##
==========================================
+ Coverage   90.02%   90.11%   +0.08%     
==========================================
  Files          62       62              
  Lines        4591     4622      +31     
==========================================
+ Hits         4133     4165      +32     
+ Misses        458      457       -1

Flag	Coverage Δ
#integration	`53.54% <92.3%> (+0.26%)`	⬆️
#unittests	`88.85% <91.42%> (ø)`	⬆️

Impacted Files	Coverage Δ
source/agora/network/NetworkClient.d	`94.87% <100%> (+0.13%)`	⬆️
source/agora/test/Base.d	`89% <100%> (+0.45%)`	⬆️
source/agora/node/Node.d	`100% <100%> (ø)`	⬆️
source/agora/network/NetworkManager.d	`78.2% <90%> (+3.03%)`	⬆️
source/agora/test/GossipProtocol.d	`91.66% <92.85%> (+0.75%)`	⬆️
source/agora/consensus/protocol/Nominator.d	`83.49% <0%> (-0.98%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a84b4b3...0f868bf. Read the comment docs.

AndrejMitrovic · 2020-03-11T00:32:56Z

By the way, I know this is not an ideal API. Anyone can pass any address string here.

It's possible to extract the address from the connection which invoked an API with vibe.d.

But for LocalRest it would require additional work. And I'm not sure what the best design would be.

So maybe we could keep registerListener like this for now with the address parameter, and later remove it.

TrustHenry · 2020-03-11T00:49:31Z

source/agora/network/NetworkClient.d


    ***************************************************************************/

-    public void handshake ()
+    public void registerListener (string address)


Suggested change

public void registerListener (string address)

public void registerListener (Address address)

Can you tell why used string here?

Oh I made a mistake while rebasing commits. I'll fix it.

Nice catch!

If a Node wants to listen for Gossip / Consensus messages, it needs to register itself as a listener. Previously a Node would gossip its messages to other nodes only if those nodes were in the network configuration of the Node. Part of: bosagora#606

AndrejMitrovic · 2020-03-11T00:59:01Z

Fixed string => address.

linked0

Nice code!
And, I'd like to know what is the worst thing when an outsider participates in the gossip protocol. I mean what happend in the worst case.

AndrejMitrovic added 4 commits March 10, 2020 16:43

Rename handshake => getPublicKey

a8dd4ad

It was the only method with a unique name, but it was not doing any "handshaking" and was only retrieving the public key.

Make max_listeners configurable in tests

b64a8d0

To allow incoming connections from nodes outside the known network list.

Add is_incoming parameter to tryConnecting()

7429166

For future incoming connections, we will: - skip network discovery, as that connection may be untrusted - not automatically register as a listener

Add a test-case for current gossiping behavior

7e7a2b5

An outsider node which is not part of the network config of a given node will not receive any gossiping. This is a misdesign, and will be fixed in the next commit.

AndrejMitrovic added type-feature An addition to the system introducing new functionalities type-refactoring An improvement on existing code without visible functional change labels Mar 10, 2020

TrustHenry reviewed Mar 11, 2020

View reviewed changes

Add a registerListener() API

0f868bf

If a Node wants to listen for Gossip / Consensus messages, it needs to register itself as a listener. Previously a Node would gossip its messages to other nodes only if those nodes were in the network configuration of the Node. Part of: bosagora#606

AndrejMitrovic force-pushed the exchange-address branch from e4c2904 to 0f868bf Compare March 11, 2020 00:59

linked0 self-requested a review March 11, 2020 02:27

linked0 approved these changes Mar 11, 2020

View reviewed changes

linked0 merged commit 4812c71 into bosagora:v0.x.x Mar 11, 2020

AndrejMitrovic mentioned this pull request Apr 6, 2020

Design and implement quorum balancing #240

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a registerListener() API #645

Add a registerListener() API #645

AndrejMitrovic commented Mar 10, 2020 •

edited

Loading

codecov bot commented Mar 10, 2020 •

edited

Loading

AndrejMitrovic commented Mar 11, 2020

TrustHenry Mar 11, 2020

AndrejMitrovic Mar 11, 2020

AndrejMitrovic Mar 11, 2020

AndrejMitrovic commented Mar 11, 2020

linked0 left a comment

	public void registerListener (string address)
	public void registerListener (Address address)

Add a registerListener() API #645

Add a registerListener() API #645

Conversation

AndrejMitrovic commented Mar 10, 2020 • edited Loading

codecov bot commented Mar 10, 2020 • edited Loading

Codecov Report

AndrejMitrovic commented Mar 11, 2020

TrustHenry Mar 11, 2020

Choose a reason for hiding this comment

AndrejMitrovic Mar 11, 2020

Choose a reason for hiding this comment

AndrejMitrovic Mar 11, 2020

Choose a reason for hiding this comment

AndrejMitrovic commented Mar 11, 2020

linked0 left a comment

Choose a reason for hiding this comment

AndrejMitrovic commented Mar 10, 2020 •

edited

Loading

codecov bot commented Mar 10, 2020 •

edited

Loading