Fix node selection improvement #1815

petreze · 2023-08-23T08:39:26Z

Description:
This PR improves the node selection of the SDK

Related issue(s):

Fixes #1813

Notes for reviewer:

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

Signed-off-by: Petar Tonev <[email protected]>

rokn · 2023-08-23T11:53:32Z

src/client/ManagedNetwork.js

            let node = this.getNode();
-            if (
-                !keys.has(node.getKey()) ||
-                !nodeAddresses.has(node.address._address)
-            ) {
+            if (!keys.has(node.getKey())) {
                keys.add(node.getKey());
-                nodeAddresses.add(node.address._address);
                nodes.push(node);
            } else {
                i--;


Why do we even need this? in Go we are just assigning the first count nodes for the transaction. Even if we don't want this and we want random nodes there are better more deterministic ways to get them. For example filling an array with all healthy nodes and picking and removing 1 by 1 out of it in a random fashion

We do this check to ensure that we don't add the same node twice or more

Yeah but we can do this with a deterministic approach, always getting count nodes. I understand it's just the way it was till now, but if we are changing this we could as well just do this a bit better. Something like:

const healthy = // copy the healthy nodes array const chosenNodes = [] for (let i = 0; i < count; i++) { const idx = Math.floor(Math.random() * healthy.length); // add to the chosen nodes // remove from healthy array copy }