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
[Feature] feat: allow-external-peers #3163
Changes from 6 commits
3221e52
c8a02c8
ce1ca02
6003adb
a682e2a
8b2db44
6e14920
8ecd4e1
a74d626
47ac52f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -264,6 +264,12 @@ impl<N: Network> Router<N> { | |
if self.is_connected(&peer_ip) { | ||
bail!("Dropping connection request from '{peer_ip}' (already connected)") | ||
} | ||
// Only allow trusted peers to connect if we are a validator | ||
// (unless allow_external_peers is set) | ||
let is_validator = self.node_type().is_validator(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: is_validator is no longer necessary as part of the check (because allow_external_peers takes precedence) |
||
if is_validator && !self.allow_external_peers() && !self.is_trusted(&peer_ip) { | ||
bail!("Dropping connection request from '{peer_ip}' (untrusted)") | ||
} | ||
// Ensure the peer is not restricted. | ||
if self.is_restricted(&peer_ip) { | ||
bail!("Dropping connection request from '{peer_ip}' (restricted)") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,6 +107,12 @@ pub trait Heartbeat<N: Network>: Outbound<N> { | |
return; | ||
} | ||
|
||
let is_validator = self.router().node_type().is_validator(); | ||
// Skip if the node is not requesting peers. | ||
if is_validator && !self.router().allow_external_peers() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: is_validator is no longer necessary as part of the check (because allow_external_peers takes precedence) |
||
return; | ||
} | ||
|
||
// Retrieve the trusted peers. | ||
let trusted = self.router().trusted_peers(); | ||
// Retrieve the bootstrap peers. | ||
|
@@ -216,9 +222,12 @@ pub trait Heartbeat<N: Network>: Outbound<N> { | |
for peer_ip in self.router().candidate_peers().into_iter().choose_multiple(rng, num_deficient) { | ||
self.router().connect(peer_ip); | ||
} | ||
// Request more peers from the connected peers. | ||
for peer_ip in self.router().connected_peers().into_iter().choose_multiple(rng, 3) { | ||
self.send(peer_ip, Message::PeerRequest(PeerRequest)); | ||
let is_validator = self.router().node_type().is_validator(); | ||
if !is_validator || self.router().allow_external_peers() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: is_validator is no longer necessary as part of the check (because allow_external_peers takes precedence) |
||
// Request more peers from the connected peers. | ||
for peer_ip in self.router().connected_peers().into_iter().choose_multiple(rng, 3) { | ||
self.send(peer_ip, Message::PeerRequest(PeerRequest)); | ||
} | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,6 +125,9 @@ pub trait Inbound<N: Network>: Reading + Outbound<N> { | |
if !self.router().cache.contains_outbound_peer_request(peer_ip) { | ||
bail!("Peer '{peer_ip}' is not following the protocol (unexpected peer response)") | ||
} | ||
if self.router().node_type().is_validator() && !self.router().allow_external_peers() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: is_validator is no longer necessary as part of the check (because allow_external_peers takes precedence) |
||
bail!("Not accepting peer response from '{peer_ip}' (validator gossip is disabled)"); | ||
} | ||
|
||
match self.peer_response(peer_ip, &message.peers) { | ||
true => Ok(()), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,6 +117,7 @@ impl<N: Network, C: ConsensusStorage<N>> Client<N, C> { | |
trusted_peers, | ||
Self::MAXIMUM_NUMBER_OF_PEERS as u16, | ||
matches!(storage_mode, StorageMode::Development(_)), | ||
false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should have spotted this sooner, but this is a bug. I think this should be true, and we should set it true for all the other invocations of |
||
) | ||
.await?; | ||
// Load the coinbase puzzle. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -110,6 +110,7 @@ impl<N: Network, C: ConsensusStorage<N>> Prover<N, C> { | |
trusted_peers, | ||
Self::MAXIMUM_NUMBER_OF_PEERS as u16, | ||
matches!(storage_mode, StorageMode::Development(_)), | ||
false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@vicsn Should this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes |
||
) | ||
.await?; | ||
// Load the coinbase puzzle. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.