Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Chainid to p2p filter #355

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Add Chainid to p2p filter #355

wants to merge 13 commits into from

Conversation

ping-ke
Copy link
Collaborator

@ping-ke ping-ke commented Jan 2, 2025

Issue:
Currently, we use l1.chain_id to filter ENR in the discovery UDPv5 table, which cannot filter out the es-node ENR that has been connected before. Also, the run*.sh does not set the l1.chain_id, and the es-node will use 1 as the default value.
So now, we only use the contract address as a filter in the SyncClient.AddPeer filters out es-nodes that do not belong to the same network. However, when we change SWC from Alpha to Beta, the contract address will not change. So, Alpha and Beta es-node networks will join together if any Beta es-node does not clear its discovery DB (esnode_discovery_db) before it starts.

How to fix:

  1. Fetch L1 chain ID from L1 RPC instead of using flags
  2. Filter all ENRs which do not have the same L1 Chain ID
  3. Add L2 Chain ID to the P2P topic when getting ShardList for an Inbound connection. If they do not belong to the same network, the get ShardList request will not get a response, and the connection will be closed.
  4. Clear addresses in PeerStore DB if their p2p version are not the same

Note: After this change, the Bootnode ENR will be changed, so all the es-nodes in the network need to sync to the latest code and restart.

How to test:

  1. Bootnode with this chain ID change + es-node with this chain ID change should connect and sync.
  2. Bootnode with this chain ID change + es-node without this chain ID change should not connect and sync.
  3. Bootnode without this chain ID change + es-node with this chain ID change should not connect and sync.
  4. Bootnode switch to this chain ID change without clear esnode_discovery_db and esnode_peerstore_db, es-node without this chain ID change should not connect and sync.
  5. es-node switch to this chain ID change without clear esnode_discovery_db and esnode_peerstore_db, it should
    connect to bootnode with this chain ID change and it should not connect to bootnode without this chain ID change.

@ping-ke ping-ke marked this pull request as ready for review January 16, 2025 04:22

// clear pstore from DB if the version is not match
// the DB path is set by p2p.peerstore.path flag, and the default value is esnode_peerstore_db
go func() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this function, and in what scenarios would we need to increase the p2pversion?

@qzhodl
Copy link
Collaborator

qzhodl commented Jan 26, 2025

Is there a note that outlines what boot node and full node operators should do for the beta testnet if they take this PR? Additionally, should we update our runL2.sh script to include the new ENR?

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.

2 participants