The three protocol layer classes look quite different from each other even though they should all be doing the same thing in the southbound side: listening to the same events from the layer below, parsing them and generating events to the upper layer.

• All events generated by the connection layer should be listened to in all 3 classes
• Southbound event listener methods should be named the same in all 3 classes

To implement this, I propose implementing the UML diagram's EndpointListener interface as a real class that would do/enforce this all in one place.

after #51

to use them across the project

The composition of the layer stack needs to be moved away from the logic layer into the executable layer (into /tracker.js etc.). Logic layer should not need to know about the existence of the connection layer.

for logic, protocol and connection layers

Currently the codebase works on the assumption that there is a single tracker with a specific private key, hostname, and port. This is good for now.

First step towards generalization would be to keep the assumption that there is a single tracker, but allow the private key, port, and hostname to be different.

Things that need to be changed

• utils.js#BOOTNODES constant must be removed and bootstrap tracker given as parameter.
• utils.js#isTracker - we need a mechanism to detect whether a peer is a tracker or not. Current mechanism is hardcording.

Integrate Data API

Publish private npm package

The UML diagram should reflect the use of EventEmitter events instead of the current Java-style listening of events.

node.js -> 8.12.0 LTS
npm -> 6.4.1

The composition of the layer stack should be done in one place so it is easy to make changes to the composition.

At the moment the Peer handling is very much dependent on Libp2p specific data structures even on the Logic layer. It is still undecided in which way the Peer will be handled in an implementation-independent manner. I see at leat two possible solutions here: 1. an implementation-independent Peer class that encapsulates all necessary information about a Peer, or 2. passing only peerId (string) cross-layer, and trying to minimise the information the higher layers need to know about peers.

Which option should we go for in your opinion? Any other solutions come to mind?

We should have a thorough discussion about the pros and cons of switching to typescript. The only con that comes to my mind is the compilation step, whereas the pros are numerous: type safety, interfaces, better code editors.. the modern javascript that we are writing now is already very similar to typescript, so why not take the final step?

Move decoding and encoding of messages to the protocol layer.
https://github.com/streamr-dev/network/blob/177cecc7bebc1c88d741030085734c3f78ac17b2/src/connection/Connection.js#L91

• Collect event logs from appropriately formatted debug messages.
• Analyze and visualize Graph with tool, e.g., gephi.org

tests for:

• creating connection
• tracker
• tracker + x2 nodes + publisher

Partitioning can happen within a stream. Effectively the identifier of a stream then becomes (streamId, partition). Usually we only deal with one partition so the identifier collapses to (streamId, 0).

We should decide consistent case for filenames. At the moment some filenames are written in lower case whereas others are in camel case. I would use lower case for all filenames, as this works cross-platform (yes, some people are still using Windows).

Try projects workflow.

1. Set up network with at least 3 nodes N1, N2, N3, and a tracker T.
2. Producer P1 starts producing data to a new stream S1 via node N1.
3. Consumer C1 connects to node N2 and subscribes to stream S1.
4. Consumer C2 connects to node N3 and subscribe to stream S1.
5. Producer P2 starts producing data to stream S2 via node N1.

Then verify that

• C1, C2 receives all messages for stream S1
• C1, C2 do not receive any messages for stream S2

Description

Node does not connect to tracker.

Reproduce

1. Start tracker with DEBUG=streamr:* npm run tracker.
2. Start node with DEBUG=streamr:* npm run node

Behaviour

Node does not seem to connect tracker (tested for 10 min)

Output of node:

  streamr:connection:libp2p libp2p bundle created +0ms
  streamr:connection:connection listening on: /ip4/127.0.0.1/tcp/30301/ipfs/QmamXjmfnN7EYgAtukfDM9bHYdnBwtiTLshEWWhe2qhyAA +0ms
  streamr:logic:node node: streamr-node/v1.0.3/darwin-x64/nodejs is running
  streamr:logic:node
  streamr:logic:node
  streamr:logic:node  +0ms

Output of tracker:

  streamr:connection:libp2p libp2p bundle created +0ms
  streamr:connection:connection listening on: /ip4/127.0.0.1/tcp/30300/ipfs/QmQ2zigjQikYnyYUSXZydNXrDRhBut2mubwJBaLXobMt3A +0ms
  streamr:logic:tracker tracker: streamr-tracker/v1.0.3/darwin-x64/nodejs is running
  streamr:logic:tracker
  streamr:logic:tracker
  streamr:logic:tracker  +0ms

Expected behaviour

Expecting to see connected to .... in node and tracker logs.

now when we are closing connection while we are writing/reading data from stream it generates random errors

• for private methods
• for event listeners

I think variable message is undefined here. Should this be messages or looping through each message of messages?
https://github.com/streamr-dev/network/blob/e0b512c87b346b3afefd09f464123cc322c4a055/src/connection/Connection.js#L87

Does this event occur both when we connect to a peer and when a peer connects to us?
https://github.com/streamr-dev/network/blob/e0b512c87b346b3afefd09f464123cc322c4a055/src/connection/Connection.js#L78

probably using express

Set up basic Travis CI to run tests when

• A PR is created or the branch of an existing PR has changes
• When master changes (PR merged or hot fix commits)

Naming of the classes and public methods should be the same in the code and the UML model. My logic in the naming of classes was that the main classes on the different layers share a common layer-specific ending. On the logic layer the main classes end with "Engine", on the protocol layer with "Handler" and on the connection layer with "Endpoint". This way it is easy to see from the class file alone which layer it belongs to without checking the directory structure.

Is it OK to others if I modify the naming in the code to match that of the UML model?

https://github.com/streamr-dev/network/blob/e0b512c87b346b3afefd09f464123cc322c4a055/src/connection/Connection.js#L43

npm test

[email protected] test /Users/ptesavol/projects/streamr-network
jest

FAIL test/integration/tracker.test.js
● Test suite failed to run

Cannot find module '../../src/logic/peer' from 'tracker.test.js'

  1 | const assert = require('assert')
  2 | const Tracker = require('../../src/logic/Tracker')
> 3 | const Peer = require('../../src/logic/peer')
    |              ^
  4 | 
  5 | jest.setTimeout(30000)
  6 | 

  at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:221:17)
  at Object.require (test/integration/tracker.test.js:3:14)

FAIL test/unit/tracker.test.js
● tracker creation › should be able to start and stop successfully

TypeError: this.connection.on is not a function

  10 |         this.connection = connection
  11 | 
> 12 |         this.connection.on('streamr:peer:connect', (peer) => this.onNewConnection(peer))
     |                         ^
  13 |         this.connection.on('streamr:message-received', ({
  14 |             sender,
  15 |             message

  at new on (src/protocol/TrackerServer.js:12:25)
  at new Tracker (src/logic/Tracker.js:17:35)
  at Object.done (test/unit/tracker.test.js:11:19)

Test Suites: 2 failed, 2 total
Tests: 1 failed, 1 total
Snapshots: 0 total
Time: 1.713s
Ran all test suites.
npm ERR! Test failed. See above for more details.

This only happens occasionally. Apparently publisher.test.js exits before LibP2p has closed its listening port, and tracker.test.js complains about EADDRINUSE.

I'll change the port in tracker.test.js to 30336 just to get the tests go through, but need to keep this in mind, probably there is a bug in LibP2p that calls the final callback before the listening socket has actually closed.

Test output:

PASS test/unit/topology.test.js
PASS test/unit/encoder.test.js
PASS test/unit/publisher.test.js
FAIL test/unit/tracker.test.js
● tracker creation › should be able to start and stop successfully

listen EADDRINUSE 127.0.0.1:30335

We should lock down terminology for consistency of terms in codebase.

Do we use:
Publisher / Subscriber

or

Producer / Consumer

or

something else?

and update documentation

I think unit/connection.test.js and unit/network.node.test.js should be moved to integration tests. They take longer to run and interact with LibP2p in a significant way.

I personally like unit tests to be pretty fast so I can run them often while I develop. And then leave more of the heavy lifting to integration tests. What do you guys think?

Not sure how this pull-stream stuff works, is it sync or async. We should verify that event streamr:message-sent is emitted once message has been sent.

https://github.com/streamr-dev/network/blob/177cecc7bebc1c88d741030085734c3f78ac17b2/src/connection/Connection.js#L101

Currently Connection calls dialProtocol() every time when sending data:
https://github.com/streamr-dev/network/blob/71d555f178ed1e944691e9075b5299f0ee999813/src/connection/Connection.js#L38

This should be replaced with the use of reusable "pull-pushable" as done in the libp2p example at https://github.com/libp2p/js-libp2p/blob/e0520213979abcc5e58330c83b784c6b8d596500/examples/chat/src/dialer.js#L59

I guess the idea is that logic layer wouldn't send messages directly via connection object, but instead use an interface provided by protocol layer to do this.

Add numbering to messages so Nodes know what messages they have already received.

What do you think about renaming isNode to includeNodeOptions (but only in this class)?
https://github.com/streamr-dev/network/blob/e0b512c87b346b3afefd09f464123cc322c4a055/src/connection/Libp2pBundle.js#L31

• First node asks tracker who is responsible for stream.
• If no one is responsible, then tracker assigns responsibility to this node.

I think an await is needed here, but not possible because constructor cannot be async?
https://github.com/streamr-dev/network/blob/e0b512c87b346b3afefd09f464123cc322c4a055/src/connection/Connection.js#L24

Maybe we should export an async function from this module that acts as the "constructor" of this class for the outside world. That async would return a Promise with the Connection object instance as its value. We could probably get rid of this.isStarted variable while doing this.

Let's discuss if this make sense or not.

This might seem like splitting hairs, but it would be good to establish the meaning of the words Peer, Node, and Tracker. Then make sure these terms are used in a consistent manner throughout the codebase.

As I see it:

• Peer is a catch-all term for any actor in the peer-to-peer network?
• Tracker and Node are specific-kinds of Peers?

Write unit tests for different message types and their expected bodies.