net: resolve seed names by host name, add as CLI opt - #134
Conversation
Avoids trying to connect to invalid peers in a never ending loop
Take in seed nodes as host names in addition to raw IP addresses. This makes it easier to reconfigure seeds without having to release new Thunder versions. Re-resolve on startup. Expose passing custom seed nodes via CLI.
d47215c to
1613bba
Compare
| // The mailbox stream sets `received_msg_successfully` once a | ||
| // message bearing our magic bytes has been received; forward | ||
| // that edge to the net task once, so it can persist the peer. | ||
| if !sent_validated |
There was a problem hiding this comment.
Why is this bool still here? Clearly not needed since you have self.received_msg_successfully
| )] | ||
| pub struct ParsePeerAddress { | ||
| input: String, | ||
| reason: &'static str, |
There was a problem hiding this comment.
This is sloppy, please use an enum for all of the possible failure cases
| }; | ||
| const DEFAULT_SEED_NODE_PORT: u16 = 4000 + THIS_SIDECHAIN as u16; | ||
|
|
||
| const SIGNET_SEED_NODES: &[(&str, u16)] = &[ |
There was a problem hiding this comment.
Why change the name? These are addresses, not nodes.
Also, this should be &[PeerAddress].
| ); | ||
| &[SIGNET_MINING_SERVER, BIP300_XYZ] | ||
| }; | ||
| const DEFAULT_SEED_NODE_PORT: u16 = 4000 + THIS_SIDECHAIN as u16; |
There was a problem hiding this comment.
DEFAULT_PORT would be a better name
| #[derive( | ||
| Clone, Debug, Eq, Hash, PartialEq, serde::Deserialize, serde::Serialize, | ||
| )] | ||
| pub struct PeerAddress { |
There was a problem hiding this comment.
This could be in types/net.rs instead. This would also make it usable in thunder_app_rpc_api.
| } | ||
| } | ||
|
|
||
| /// Known peers and seed nodes are not dialed here: the net task reads |
There was a problem hiding this comment.
They were not dialed before this PR! Now the known_peers DB is not initialized, either.
|
|
||
| tracing::trace!(%addr, "wrote peer to database"); | ||
| // Not written to `known_peers` here: the handshake proves nothing about | ||
| // which network the peer is on. Recorded once it emits `Validated`. |
There was a problem hiding this comment.
This comment is unnecessary, and could be worded better, ie. "Written to known_peers once we receive a message from the peer that starts with the magic network bytes."
| // attempting to reorg to the new tip, on the corresponding oneshot | ||
| // receiver. | ||
| NewTipReady(Tip, Option<SocketAddr>, Option<oneshot::Sender<bool>>), | ||
| // Dial an initial peer, trying its resolved socket addresses |
There was a problem hiding this comment.
I don't think the net task should handle this. Each peer connection task should handle dialing.
| .get(&addr) | ||
| .cloned() | ||
| .unwrap_or_else(|| PeerAddress::from(addr)); | ||
| // The peer is connected, so its other resolved |
There was a problem hiding this comment.
It is actually fine to dial fallback addresses if a peer disconnects, and it is no longer possible to connect to the first validated address
| /// more than once, and is dialed in addition to the network's built-in seed | ||
| /// nodes. | ||
| #[arg(long = "seed-node")] | ||
| seed_nodes: Vec<thunder::net::PeerAddress>, |
There was a problem hiding this comment.
Please rename this to add-peer, mirroring bitcoind's -addnode startup option.
2 participants
Take in seed nodes as host names in addition to raw IP addresses. This
makes it easier to reconfigure seeds without having to release new
Thunder versions. Re-resolve on startup. Expose passing custom seed
nodes via CLI.