feat: add socket connection between nodes #99

yoxira · 2025-02-03T02:55:39Z

https://trello.com/c/5Jt28f7V/63-feat-socket-support-node-node

Description

The node chooses peers to connect to in the same way as for HTTP broadcasting—randomly, to avoid centralization.
Every 30 minutes, the node attempts to replace 20% of its WebSocket connections.
Received transactions affect the peer's health check.
Nodes authenticate own ip by providing nonce.

Config

Added new required config properties:

{
  "peers": {
    // ....
    "options": {
      "maxWsConnections": 15 // maximum number of peers to receive transactions from
      // ...
    }
  },
  "wsNode": {
    "enabled": true, // enable broadcasting to other nodes using ws
    "maxConnections": 25 // maximum number of peers to broadcast transactions to
  }
}

API

The GET /api/peers now returns info about connection type for incoming (syncing) and outcoming (broadcasting) transactions:

{
  "peers": [
    {
      "isBroadcastingViaSocket": false,
      "syncProtocol": "ws" // or 'http'
    },
  ]
}

api/ws/server.js

bludnic · 2025-02-07T21:17:01Z

config.default.json

@@ -77,6 +77,7 @@
      "blackList": []
    },
    "options": {
+      "maxWsConnections": 15,


Isn't better to move it to wsNode config and use more descriptive naming, like:

"wsNode": { "enabled": true, "maxIncomingConnections": 15, // maximum number of peers to receive transactions from "maxOutgoingConnections": 25, // maximum number of peers to broadcast transactions to }

It's really confusing to have options.maxWsConnections and wsNode.maxConnections.

bludnic · 2025-02-07T21:23:49Z

api/ws/transport.js

+const maxReconnectDelay = 60000;
+const defaultReconnectionDelay = 5000;
+
+function TransportWsApi(modules, library, options) {


Can you add a JSDoc to describe the purpose of TransportWsApi?

And also JSDoc for prototype's methods below.

bludnic · 2025-02-07T21:25:58Z

app.js

@@ -606,6 +604,21 @@ d.run(function () {
      });
    }],

+    /**
+     * Listens for new transacttions using websocket and links peers to the websocket server


typo: transacttions -> transactions

bludnic · 2025-02-07T21:30:01Z

logic/peers.js

+Peers.prototype.getByNonce = function (nonce) {
+  for (const [peerString, peer] of Object.entries(__private.peers)) {
+    if (peer.nonce === nonce) {
+      return __private.peers[peerString];


Suggested change

return __private.peers[peerString];

return peer;

Same behaviour?

If so, you can use Object.values instead of Object.entries

adamant-al · 2025-02-20T15:33:08Z

Please elaborate:

Received transactions affect the peer's health check.

adamant-al · 2025-02-20T15:36:04Z

Please elaborate:

Nodes authenticate own ip by providing nonce.

Does REST node exchange uses the same authentication? What's it for?
Does the node get its nonce once and forever, updates it on restart, updates it on some event?
Is the node's nonce stays the same across all nodes?

adamant-al · 2025-02-20T15:40:02Z

(Updated, see the comment below #99 (comment))

The GET /api/peers now returns info about connection type for incoming (syncing) and outcoming (broadcasting) transactions:

{
  "peers": [
    {
      "isBroadcastingViaSocket": false,
      "syncProtocol": "ws" // or 'http'
    },
  ]
}

If syncProtocol is "ws", it means the node accepts new txs both via ws and http? Maybe then something like "syncProtocol": "wsAndHttp" // or 'http'?

adamant-al · 2025-02-20T15:43:34Z

What is the limit [of number of peers for broadcasting] for REST connections?
It's hard coded in constants.js file:
maxPeers: 100

Why then we have peers.options.limits.max in the default config?

Can you please explain what are peers.options, broadcasts and transactions params from config?

adamant-al · 2025-02-20T17:54:51Z

The GET /api/peers now returns info about connection type for incoming (syncing) and outcoming (broadcasting) transactions:
{
  "peers": [
    {
      "isBroadcastingViaSocket": false,
      "syncProtocol": "ws" // or 'http'
    },
  ]
}
If syncProtocol is "ws", it means the node accepts new txs both via ws and http? Maybe then something like "syncProtocol": "wsAndHttp" // or 'http'?

I see that current implementation assumes that a node is connected either via ws, or http.

One of reasons while adding wsNode is redundancy. It means some nodes may connect with ws AND http. Imaging that a network has low amount of nodes, and all of them ws-enabled.

Can we improve that some nodes will connect both ws and http?

api/ws/server.js

adamant-al · 2025-02-20T15:48:36Z

api/ws/transport.js

+const maxReconnectDelay = 60000;
+const defaultReconnectionDelay = 5000;
+
+function TransportWsApi(modules, library, options) {


And also JSDoc for prototype's methods below.

adamant-al · 2025-02-20T16:21:49Z

api/ws/transport.js

+const { io } = require('socket.io-client');
+const Peer = require('../../logic/peer.js');
+
+const maxReconnectDelay = 60000;


Consider moving it to config

adamant-al · 2025-02-20T16:28:53Z

api/ws/transport.js

+
+  // Connect to multiple peers
+  self.getRandomPeers(self.maxConnections, (err, peers) => {
+    if (err || !peers.length) {


Preferred to log this event / debug

adamant-al · 2025-02-20T17:24:48Z

api/ws/transport.js

+  // Find a new peer to replace it
+  self.getRandomPeer((err, newPeer) => {
+    if (err || !newPeer) {
+      self.logger.debug('WebSocket: Failed to find replacement peer');


'WebSocket: Failed to find replacement' ⟶ Add 'for peer ws://${peer.ip}:${peer.port} + err || no peers'?

adamant-al · 2025-02-20T17:33:54Z

api/ws/transport.js

+};
+
+TransportWsApi.prototype.updatePeers = function() {
+  const self = this;


Preferred to log this event / debug

adamant-al · 2025-02-20T17:35:14Z

api/ws/transport.js

+};
+
+TransportWsApi.prototype.rotatePeers = function () {
+  const self = this;


Preferred to log this event / debug

adamant-al · 2025-02-20T17:37:59Z

api/ws/transport.js

+      return;
+    }
+
+    self.logger.debug(`Rotating ${newPeers.length} out of ${totalConnections} peers.`);


Consider to clarify logging everywhere that it's about ws.

E.g., [wsNode] Rotating ${newPeers.length} out of ${totalConnections} peers.

adamant-al · 2025-02-20T17:39:36Z

api/ws/transport.js

+TransportWsApi.prototype.startRotation = function() {
+  this.rotationInterval = setInterval(() => {
+    this.rotatePeers();
+  }, 1000 * 60 * 30); // 30 minutes


adamant-al · 2025-02-20T17:55:32Z

modules/peers.js

+ * Changes the connection type to ws
+ * @param {Peer} peer
+ */
+Peers.prototype.switchToWs = function (peer) {


See my comment about redundancy
#99 (comment)

yoxira · 2025-02-24T22:41:02Z

Does REST node exchange uses the same authentication? What's it for?

The authentication is needed to link the incoming WebSocket connection to the existing peer. REST provides port instead.

Does the node get its nonce once and forever, updates it on restart, updates it on some event?

A nonce is generated on app start and never changes.

Is the node's nonce stays the same across all nodes?

Yes

Please elaborate:

Received transactions affect the peer's health check.

If a node receives an invalid transaction over WebSocket, the peer will be removed from the peer list, as it would be using REST.

yoxira · 2025-02-24T23:14:46Z

I see that current implementation assumes that a node is connected either via ws, or http.

A node can be connected using both ws and http at the same time. It's up to a broadcasting node to decide whether to double-send the transactions or not. In the current implementation, a node doesn't send the transactions twice at all, while the old nodes always do.

I suggest still preferring nodes with http-only connections when broadcasting, but also choosing some % of ws-connected nodes to broadcast to using http.

yoxira added 6 commits February 3, 2025 07:53

feat: add socket connection between nodes

99f7f13

fix: config schema

dbba163

fix: unable to connect to wss

c8dc984

feat: add wsNode options with maxConnections for ws server

1faae2e

fix: config schema

156fc31

chore: filter peers by protocol to find potential socket servers

bed5335

yoxira added the feature label Feb 3, 2025

yoxira added 7 commits February 4, 2025 02:31

chore: rename connection type properties

817318e

chore: fix syntax error

f86df74

chore: add getByNonce and fix getSocketCount methods

cd706a6

feat: update socket connections when peer list is updated

3216814

fix: changing peers connection state when updating peers on discovery

490051f

fix: do not print about chaning immutable properties of a peer

d0abf40

feat: add rotation of the socket connections

84b023e

yoxira requested review from bludnic and adamant-al February 5, 2025 09:43

yoxira added 2 commits February 5, 2025 14:44

chore: add new properties in the default testnet config

a3e0157

chore: change default config values for ws connections

5ee24d3

yoxira marked this pull request as ready for review February 5, 2025 09:59

bludnic reviewed Feb 7, 2025

View reviewed changes

adamant-al requested changes Feb 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add socket connection between nodes #99

feat: add socket connection between nodes #99

yoxira commented Feb 3, 2025 •

edited

Loading

bludnic Feb 7, 2025

adamant-al Feb 20, 2025

bludnic Feb 7, 2025

adamant-al Feb 20, 2025 •

edited

Loading

bludnic Feb 7, 2025

bludnic Feb 7, 2025

bludnic Feb 7, 2025

adamant-al commented Feb 20, 2025

adamant-al commented Feb 20, 2025

adamant-al commented Feb 20, 2025 •

edited

Loading

adamant-al commented Feb 20, 2025

adamant-al commented Feb 20, 2025

adamant-al Feb 20, 2025 •

edited

Loading

adamant-al Feb 20, 2025

adamant-al Feb 20, 2025

adamant-al Feb 20, 2025

adamant-al Feb 20, 2025

adamant-al Feb 20, 2025

adamant-al Feb 20, 2025

adamant-al Feb 20, 2025

adamant-al Feb 20, 2025

yoxira commented Feb 24, 2025

yoxira commented Feb 24, 2025

feat: add socket connection between nodes #99

Are you sure you want to change the base?

feat: add socket connection between nodes #99

Conversation

yoxira commented Feb 3, 2025 • edited Loading

Description

Config

API

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamant-al Feb 20, 2025 • edited Loading

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

adamant-al commented Feb 20, 2025

adamant-al commented Feb 20, 2025

adamant-al commented Feb 20, 2025 • edited Loading

adamant-al commented Feb 20, 2025

adamant-al commented Feb 20, 2025

adamant-al Feb 20, 2025 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoxira commented Feb 24, 2025

yoxira commented Feb 24, 2025

yoxira commented Feb 3, 2025 •

edited

Loading

adamant-al Feb 20, 2025 •

edited

Loading

adamant-al commented Feb 20, 2025 •

edited

Loading

adamant-al Feb 20, 2025 •

edited

Loading