Remove --nodiscover from non bootnodes #320
Conversation
Signed-off-by: rodion <rodion.lim@partior.com>
rodion-lim-partior
force-pushed
the
feature/set516_correct_no_discover
branch
from
84b6c6b
to
3635acf
Compare
There was a problem hiding this comment.
@rodion-lim-partior thanks for proposing the PR. please check the suggested changes I made below. after that it should work as expected.
note that with this setup, such that the bootnode (the node at index 0) has discovery turn on whereas the rest of the nodes have discovery turned off, only the bootnode is connected to the other nodes, while any other nodes are only connected to the bootnode, but not to each other. while this is not ideal, it serves the purpose because the only block signer in the network is the node at index 0. This means the network will function properly, as long as no other nodes are voted in as additional block signers.
Getting all nodes to be connected with each other will take more work. If that's desirable or becomes mandatory in the future, we can work on adding that.
| connectTimeout := 15 | ||
| if memberIndex != 0 { | ||
| discoveryCmd = fmt.Sprintf(`bootnode=$(curl http://quorum_0:%s -s --connect-timeout %[2]d --max-time %[2]d --retry 5 --retry-connrefused --retry-delay 0 --retry-max-time 60 --fail --header "Content-Type: application/json" --data '{"jsonrpc":"2.0", "method": "admin_nodeInfo", "params": [], "id": 1}' | grep -o "enode://[a-z0-9@.:]*") | ||
| BOOTNODE_CMD="--bootnodes $bootnode" | ||
| BOOTNODE_CMD="BOOTNODE_CMD=--bootnodes $bootnode" |
There was a problem hiding this comment.
| BOOTNODE_CMD="BOOTNODE_CMD=--bootnodes $bootnode" | |
| BOOTNODE_CMD="--bootnodes $bootnode --nodiscover" |
| BOOTNODE_CMD=${BOOTNODE_CMD/127.0.0.1/quorum_0}`, QuorumPort, connectTimeout) | ||
| } else { | ||
| discoveryCmd = "BOOTNODE_CMD=--nodiscover" |
There was a problem hiding this comment.
| discoveryCmd = "BOOTNODE_CMD=--nodiscover" | |
| discoveryCmd = `BOOTNODE_CMD=""` |
|
Will merge these changes, and @jimthematrix will include his proposed changes in a new PR. I believe getting this working for v1.3.1 is critical so going ahead and merging |
There was a problem hiding this comment.
As explained above, merging and Jim will raise a PR with proposed fixes! Thanks for the contribution @rodion-lim-partior 😃
|
@jimthematrix, apologies for the late reply, the original PR that I raised for adding Quorum/Tessera support is in line with your suggested changes. I kept |
Currently when starting multiple members Quorum network, there is an error being thrown when trying to register organization and nodes for members whom are not part of the bootnode (i.e. bootnodes are used for auto discovery of peers).
Removing --nodiscover flag from all members prevent nodes from being discovered by the bootnode. This PR is raised to retain the --nodiscover flag on the bootnode itself, while removing the flag on the remaining nodes so that they can be connected in the same network.