Skip to content

[Fix] Handle changing IPs of trusted peers#4229

Draft
kaimast wants to merge 3 commits intostagingfrom
fix/changed-address
Draft

[Fix] Handle changing IPs of trusted peers#4229
kaimast wants to merge 3 commits intostagingfrom
fix/changed-address

Conversation

@kaimast
Copy link
Copy Markdown
Contributor

@kaimast kaimast commented Apr 24, 2026

This PR aims to tackle (some of) #4228. But there are some open questions that need to be addressed.

It makes the following changes:

  • handle_trusted_validators now properly waits for connection attempts to succeed/fail and prints any associated errors. We already do this for bootstrap peers, and most likely forgot to make the change for the other function.
  • CandidatePeers now track the last known Aleo address. This way, we can detect when a peer changes IP addresses.
  • The PR now also generated a warning if a node connects with an Aleo address that is associated with a trusted peer, but from a different IP

Discussion/Open Questions:

  • Are there legitimate cases where multiple nodes appear in the network with the same Aleo address? If so, the solution in this PR might be too simplistic.
  • Does this add a new potential attack vector? The handshake should prevent a third party from using someone's Aleo address, but we should still ensure that this new behavior cannot be exploited in some way.

@vicsn vicsn requested a review from ljedrz April 24, 2026 07:29
@vicsn
Copy link
Copy Markdown
Collaborator

vicsn commented Apr 24, 2026

We can assume one IP address per validator.

Comment thread node/bft/src/gateway.rs
Comment on lines +1065 to +1069
if let Some(aleo_addr) = c.last_known_aleo_addr {
if self.is_connected_address(aleo_addr) {
return None;
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The more robust (and simpler) way to check whether there's already a peer connected with the given Aleo address is to use Resolver::get_peer_ip_for_address - the Resolver only ever holds the addresses of connected peers.

Copy link
Copy Markdown
Collaborator

@ljedrz ljedrz left a comment

Choose a reason for hiding this comment

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

This seems like the exact kind of issue that would manifest in the way we've observed. My suggestion is to use a preexisting tool, but other than that, the approach is sound.

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