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

chore: Check version compatibility #11577

Closed
4 tasks
spalladino opened this issue Jan 28, 2025 · 2 comments · Fixed by #11611
Closed
4 tasks

chore: Check version compatibility #11577

spalladino opened this issue Jan 28, 2025 · 2 comments · Fixed by #11611
Assignees

Comments

@spalladino
Copy link
Contributor

  • Define what it means for a node or a client to be compatible. Consider a combination of protocol circuit vk roots, protocol contracts root, L1 addresses, L1 bytecode/abi.
  • Have the json rpc server return this info. Json rpc client should check this and throw if connecting to an incompatible server. Consider either doing an initial check when initializing the client, or returning version info in headers and checking on every request.
  • Include version info in the p2p topic, to prevent nodes from different versions to gossip between each other.
  • Check the configured bootnode version via p2p layer when starting up, so if versions do not match, we get more info rather than just "not joining the p2p network".
@Maddiaa0
Copy link
Member

Maddiaa0 commented Jan 29, 2025

Version of the node can also be set within the nodes enr.

copied for convenience:

  private onDiscovered(enr: ENR) {
    // check the peer is an aztec peer
    const value = enr.kvs.get(AZTEC_ENR_KEY);
    if (value) {
      const network = value[0];
      // check if the peer is on the same network
      if (network === AZTEC_NET) {
        this.emit(PeerEvent.DISCOVERED, enr);
      }
    }
  }

Enrs can have arbitrary key value pairs added to them, we can include the protocol version as one of these keys, at the moment, this AZTEC_NET key is set to Aztec.Devnet, but we can extent this arbitrarily to a protocol version!

@spalladino spalladino self-assigned this Jan 29, 2025
@spalladino
Copy link
Contributor Author

@Maddiaa0 I tried that approach in #11611 here, but it seems the peer gets added anyway, at least according to node.getAllPeers.

I can confirm the peer check is failing, since I see the following in the logs:

[18:06:29.512] VERBOSE: p2p:discv5_service:7893 Peer 6391176ceeb0ac36a5c173833a6fde59e658d7706dbad273d15c3984b5ee6c08 has incorrect version: Expected component version L1 chain ID to be 14 but received 0 {"compressedVersion":"00-0-00000000-0-0b5c2005-08bcdf74","expected":{"l1ChainId":14,"l1RollupAddress":"0x0000000000000000000000000000000000000000","l2ChainVersion":0,"l2ProtocolContractsTreeRoot":"0x0b5c20053c8ea2d3a90799672a8593c80386e4548ae518768cee575c73a11eeb","l2CircuitsVkTreeRoot":"0x08bcdf74ae56df28ee23e283ae65da0a4c0e9d104b647e100554adb14341f7bf"}}

Could there be something else adding the peer and bypassing that check? Or is getAllPeers the wrong method to use here for doing the check?

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 a pull request may close this issue.

2 participants