feat: implement cmd/relay#224
Conversation
- Include argument parsing
- Ensures graceful shutdown
- Parse from url
- Fix error handling when private key is missing
- Add debug error message
- Add helper method for running background relay server - Add retries to wait until server is responsive
Ignored since metrics endpoint is not implemented
- Inline code - Reuse `with_relay_server` when possible
There was a problem hiding this comment.
Pull request overview
This PR implements the cmd/relay functionality by porting the relay command from the reference Go implementation. The relay server enables libp2p circuit relay functionality that allows Charon clients to discover and connect to their peers.
Changes:
- Adds new relay command to CLI with comprehensive configuration options for P2P, logging, and monitoring
- Implements relay server with QUIC support in addition to TCP
- Adds graceful shutdown handling via Ctrl+C signal
- Includes comprehensive test suite (one test ignored pending
/metricsendpoint implementation)
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
crates/relay-server/src/p2p.rs |
Updates to use new multiaddr methods from P2P config for TCP/UDP listening |
crates/relay-server/src/error.rs |
Adds P2PConfigError variant for improved error handling |
crates/relay-server/src/config.rs |
Changes data_dir from Option to PathBuf with default value ".charon" |
crates/p2p/src/p2p.rs |
Adds QUIC support to relay server and removes unnecessary keypair clone |
crates/cli/src/main.rs |
Implements top-level Ctrl+C handling and adds relay command route |
crates/cli/src/error.rs |
Changes CliError visibility to public and adds error variants |
crates/cli/src/commands/relay.rs |
New 558-line implementation of relay command with args, config conversion, and tests |
crates/cli/src/commands/mod.rs |
Exports new relay module |
crates/cli/src/cli.rs |
Adds Relay command variant to CLI structure |
crates/cli/Cargo.toml |
Adds relay-server, tracing, libp2p, and backon dependencies |
Cargo.toml |
Removes unused eth2_keystore optimization |
Cargo.lock |
Updates dependencies including alloy, futures, and other transitive deps |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
varex83
left a comment
There was a problem hiding this comment.
Small non-blocking comments, everything else looks great! LGTM!
|
Also one note - while testing it locally I had to run |
What problem did you find? CI did not show any issues on either platform. |
- Only run block when file is not found
It was related that the latest |
- Accept all interfaces (0.0.0.0) as valid
| // Go version checks if IP is nil (unspecified) | ||
| if socket_addr.ip().is_unspecified() { | ||
| return Err(P2PConfigError::UnspecifiedIP("TCP".to_string())); | ||
| } |
There was a problem hiding this comment.
This check was incorrect:
Go considers :1234 a valid TCP address with a nil IP, thus it was rejected with an error: https://github.com/ObolNetwork/charon/blob/749d2d7ab0b8ace34f2e686a5af5910c8adb4dc0/p2p/config.go#L103-L114
In Rust this string results in a parse error. Our check for is_unspecified() then was incorrect since it would reject 0.0.0.0 as address while the Go code does not.
To verify, I've ran the following test in Go and added its equivalent in Rust:
func TestResolveListenAddrAllInterfaces(t *testing.T) {
c := Config{
TCPAddrs: []string{
"0.0.0.0:0",
},
UDPAddrs: []string{
"0.0.0.0:0",
},
}
_, err := c.TCPMultiaddrs()
require.NoError(t, err)
_, err = c.UDPMultiaddrs()
require.NoError(t, err)
}
Closes #182
There is an ignored test due to a missing feature in the Relay (
/metricsendpoint). We'll revisit this later when we pick up this feature again.