Skip to content

Conversation

theochap
Copy link
Member

@theochap theochap commented Oct 1, 2025

Description

It is impossible to run sysgo with more than an op-reth node at a time. This is a fix that ensures we can:

  • deploy networks with more than one op-reth node
  • networks can be successfully deployed.

Rationale:

  • Disabling discv4 discovery in op-reth is not enough. The former configuration would deploy op-reth with discv5 using the default address/port, which then would fail when trying to deploy more than one node and this node tries to bind to an already allocated port
  • admin_addPeer does not also connect peers automatically - manually added peers through the admin api will not appear in admin_peers. The check below was failing because we expected that peers added through admin_addPeer would also appear when calling admin_peers.

@theochap theochap requested review from a team as code owners October 1, 2025 20:18
@joshklop joshklop self-requested a review October 1, 2025 20:31
@theochap theochap changed the title fix(devstack/op-reth): fix devstack configuration for op-reth fix(devstack/op-reth): fix devstack configuration for op-reth/networks with more than one validator Oct 3, 2025
@joshklop
Copy link
Contributor

joshklop commented Oct 3, 2025

admin_addPeer does not also connect peers automatically - manually added peers through the admin api will not appear in admin_peers.

Is this the case in op-reth, op-geth, or both? From my reading of the relevant code in op-geth, it appears that admin_addPeer does connect the peers automatically and cause them to show up in admin_peers.

@theochap
Copy link
Member Author

theochap commented Oct 3, 2025

admin_addPeer does not also connect peers automatically - manually added peers through the admin api will not appear in admin_peers.

Is this the case in op-reth, op-geth, or both? From my reading of the relevant code in op-geth, it appears that admin_addPeer does connect the peers automatically and cause them to show up in admin_peers.

This is the case at least in op-reth. For op-geth, I don't know exactly what is the behavior there, but this check was failing systematically when I was trying to deploy networks with more than two nodes. And without it everything seems to work properly, which makes me doubt of why we had the check in the first place.

@Inphi
Copy link
Contributor

Inphi commented Oct 7, 2025

admin_addPeer always returns true regardless of if the peer was actually added. The check on admin_peers RPC ensures creates a synchronization point that guarantees the peer was indeed added before ConnectP2P returns.
If the admin_peers RPC isn't working for reth, I suggest only eliding it for that EL type. Also, consider using admin_addTrustedPeer instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants