Implement constructor, initial bootstrap, and tests, for `PropagationNetwork`. #41

f2koi · 2022-08-12T19:00:47Z

Type of this PR

New feature (non-breaking change which adds functionality)
Change in tests

Describe your changes

Interface implementations

Among the following interfaces, async fn new(the constructor) and async fn create_recv_queue are implemented. The rest will be available after broadcasting logic is implemented.

#[async_trait]
pub trait AuthorizedNetwork {
    /// Joins the network with an authorized identity.
    async fn new([OMITTED]) -> Result<Self, String> where Self: Sized;
    /// Broadcasts a message to the network, after signed by the key given to this instance.
    async fn broadcast(&self, message: &[u8]) -> Result<BroadcastToken, String>;
    /// Stops a currently broadcasting message.
    async fn stop_broadcast(&self, token: BroadcastToken) -> Result<(), String>;
    /// Gets the current status of a broadcasting message.
    async fn get_broadcast_status(&self, token: BroadcastToken) -> Result<BroadcastStatus, String>;
    /// Creates a receiver for every message broadcasted to the network, except the one sent by this instance.
    async fn create_recv_queue(&self) -> Result<broadcast::Receiver<Vec<u8>>, ()>;
    /// Provides the estimated list of live nodes that are eligible and identified by their public keys.
    async fn get_live_list(&self) -> Result<Vec<PublicKey>, ()>;
}

Other implemented features

Provide configurations for some parameters used to construct and maintain the network.
Implement initial bootstrap, which is run once and while the PropagationNetwork is being constructed.
Create background task that listens on network events (such as incoming connection requests) and handles them to provide public interfaces.
Message passing between PropagationNetwork and the background task.

Unit tests

The following unit tests for initial bootstrap are added.

/// Test if every node fills its routing table with the addresses of all the other nodes
/// in a tiny network when they join the network sequentially.
/// (network_size = 5 < [`libp2p::kad::K_VALUE`] = 20)
async fn discovery_with_tiny_network_sequential();

/// Test if every node fills its routing table with the addresses of all the other nodes
/// in a small network when they join the network sequentially.
/// (network_size = [`libp2p::kad::K_VALUE`] = 20)
async fn discovery_with_small_network_sequential();

/// Test if every node fills its routing table with the addresses of all the other nodes
/// in a tiny network when they join the network at the same time.
/// (network_size = 5 < [`libp2p::kad::K_VALUE`] = 20)
async fn discovery_with_tiny_network_concurrent();

/// Test if every node fills its routing table with the addresses of all the other nodes
/// in a small network when they join the network at the same time.
/// (network_size = [`libp2p::kad::K_VALUE`] = 20)
async fn discovery_with_small_network_concurrent();

Remaining works

The followings are ordered by priority but can be changed.

Provide error types for this module(simperby-network)
Add tests for broadcasting
Implement public interfaces related to broadcasting

posgnu · 2022-08-13T18:54:19Z

This PR is good in terms of that it is self-contained, though, it is too big.
https://gist.github.com/mikepea/863f63d6e37281e329f8#aim-for-one-succinct-commit
https://gist.github.com/mikepea/863f63d6e37281e329f8#keep-it-small

junha1

Please use does ~ instead of do ~ for doc-comments.
What is K_VALUE?

network/src/propagation_network/mod.rs

junha1 · 2022-08-20T13:12:40Z

network/src/propagation_network/mod.rs

+}
+
+impl PropagationNetwork {
+    #[allow(dead_code)]


If this will be only for the tests ever, then just move it in mod tests.

f2koi · 2022-08-23T07:55:32Z

Automatically rebased on the latest commit of main.

f2koi · 2022-08-23T08:31:39Z

Please use does ~ instead of do ~ for doc-comments.

What is K_VALUE?

Fixed.
It is the maximum number of connections(except for one-time connections) or the maximum number of nodes in contact. I replaced it in the comment with descriptive text.

junha1 · 2022-08-23T17:57:27Z

network/Cargo.toml

+
+[dev-dependencies]
+rand = "0.8.5"


Add newline.

junha1 · 2022-08-23T17:57:44Z

network/src/propagation_network/behaviour.rs

+            floodsub: Floodsub::new(local_peer_id),
+        }
+    }
+    // Todo: Add constructor taking a configuration as an argument.


Just removed the comment because it is already achieved (config.rs).

junha1 · 2022-08-23T17:57:59Z

network/src/propagation_network/config.rs

+    /// Returns a default.
+    ///
+    /// To customize configurations, call `default` and chain the functions with the name of `with_<fieldname>`.
+    pub fn default() -> Self {


Why not trait Default?

Moved it into impl Default block.

junha1 · 2022-08-23T18:00:35Z

network/src/propagation_network/mod.rs

+        if let Ok(keypair_inner) = ed25519::Keypair::decode(&mut keypair_bytes) {
+            Ok(Keypair::Ed25519(keypair_inner))
+        } else {
+            Err("Invalid public/private keypair was given.".to_string())


Using a non-capitalized sentence is the (loose) convention for error msgs. Please apply it.

Applied it to all err messages and panic messages.

junha1 · 2022-08-23T18:03:01Z

network/src/propagation_network/config.rs

+    pub(crate) peer_discovery_interval: Duration,
+    /// Capacity for the message queue that passes messages from other nodes
+    /// to its simperby node.
+    pub(crate) message_queue_capacity: usize,


It is common to refer struct fields with The in the doc comments.
(e.g., The addresses to ..)

Applied it to all structs.

f2koi requested review from posgnu and junha1 August 12, 2022 19:00

f2koi mentioned this pull request Aug 13, 2022

Survey applicable functions in libp2p #2

Open

f2koi removed the request for review from posgnu August 14, 2022 01:13

f2koi force-pushed the main branch 4 times, most recently from cfd4844 to 0a8a624 Compare August 19, 2022 13:36

junha1 reviewed Aug 21, 2022

View reviewed changes

f2koi and others added 17 commits August 23, 2022 16:53

refactor: use tokio::broadcast for message queue

c7e4ef2

feat: add default constructor for Behaviour

20329f4

feat: setup constructor for PropagationNetwork

0cd133e

feat: add methods to get peers and listeners

814d27b

docs: remove redundant argument

d81293c

test: make all tests asynchronous

19dc0dd

test: add unit tests for initial bootstrap

f79bd37

refactor: derive Debug for network events

bf7bcab

feat: implement initial bootstrap

4ea1ffc

test: activate tests for initial bootstrap

62d3189

feat: add network configuration interface

f4d7fd5

test: add unit tests for initial bootstrap

4838485

feat: retry initial bootstrap if failed

da0426e

test: assign possibly available ports when testing

955ad28

test: activate tests for initial bootstrap

512c28e

refactor: split constructor

61c13a8

refactor: split background task

6631a69

f2koi force-pushed the main branch from 0a8a624 to 6631a69 Compare August 23, 2022 07:53

f2koi force-pushed the main branch from 8f41e79 to 8b2a120 Compare August 23, 2022 08:26

junha1 reviewed Aug 23, 2022

View reviewed changes

f2koi added 4 commits August 24, 2022 21:12

docs: restyle comments and error messages

5e813cd

refactor: reduce port range

b5a6d48

refactor: change location of test helpers

ef79b88

refactor: implement Default trait

cb1bdde

f2koi force-pushed the main branch from 8b2a120 to cb1bdde Compare August 24, 2022 12:14

junha1 approved these changes Aug 24, 2022

View reviewed changes

junha1 merged commit 5cac70a into postech-dao:main Aug 24, 2022

f2koi mentioned this pull request Aug 24, 2022

Define unit tests for peer discovery without internet #11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement constructor, initial bootstrap, and tests, for `PropagationNetwork`. #41

Implement constructor, initial bootstrap, and tests, for `PropagationNetwork`. #41

f2koi commented Aug 12, 2022

posgnu commented Aug 13, 2022 •

edited

junha1 left a comment

junha1 Aug 20, 2022

f2koi Aug 23, 2022

f2koi commented Aug 23, 2022 •

edited

f2koi commented Aug 23, 2022

junha1 Aug 23, 2022

f2koi Aug 24, 2022

junha1 Aug 23, 2022

f2koi Aug 24, 2022 •

edited

junha1 Aug 23, 2022

f2koi Aug 24, 2022

junha1 Aug 23, 2022

f2koi Aug 24, 2022

junha1 Aug 23, 2022

f2koi Aug 24, 2022


		[dev-dependencies]
		rand = "0.8.5"

Implement constructor, initial bootstrap, and tests, for PropagationNetwork. #41

Implement constructor, initial bootstrap, and tests, for PropagationNetwork. #41

Conversation

f2koi commented Aug 12, 2022

Type of this PR

Describe your changes

Interface implementations

Other implemented features

Unit tests

Remaining works

posgnu commented Aug 13, 2022 • edited

junha1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

f2koi commented Aug 23, 2022 • edited

f2koi commented Aug 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

f2koi Aug 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Implement constructor, initial bootstrap, and tests, for `PropagationNetwork`. #41

Implement constructor, initial bootstrap, and tests, for `PropagationNetwork`. #41

posgnu commented Aug 13, 2022 •

edited

f2koi commented Aug 23, 2022 •

edited

f2koi Aug 24, 2022 •

edited