Skip to content

Commit

Permalink
p2p: require explicit enabling of manifest enrs as bootnodes (#144)
Browse files Browse the repository at this point in the history
This makes using manifest ENRs as bootnodes explicit; disabling by default.
- Manifest ENRs will probably be invalid bootnodes in most cases. So using them in most cases doens't help.
- When a node acts as bootnode for the others, it should have zero bootnodes itself.

category: feature
ticket: #145
  • Loading branch information
corverroos committed Feb 28, 2022
1 parent 40ea6a8 commit 7c045b9
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 17 deletions.
32 changes: 18 additions & 14 deletions app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func TestPingCluster(t *testing.T) {
t.Run("bind_enrs", func(t *testing.T) {
pingCluster(t, pingTest{
Slow: false,
BootManifest: true,
BindENRAddrs: true,
})
})
Expand All @@ -61,28 +62,30 @@ func TestPingCluster(t *testing.T) {
pingCluster(t, pingTest{
Slow: false,
BindENRAddrs: false,
BootManifest: false,
Bootnodes: []string{external.URLv4()},
})
})

// TODO(corver): Figure out how to provide stale ENRs AND external bootnodes.
//// Nodes bind to non-ENR addresses, with external bootnode AS WELL AS stale ENRs.
//// Discv5 times out resolving stale ENRs, then resolves peers via external node.
//// This is slow due to discv5 internal timeouts, run with -slow.
// t.Run("external_and_stale_enrs", func(t *testing.T) {
// external := startExtBootnode(t)
//
// pingCluster(t, pingTest{
// Slow: true,
// BindENRAddrs: false,
// ExtBootnodes: []string{external.URLv4()},
// })
// })
// Nodes bind to non-ENR addresses, with external bootnode AS WELL AS stale ENRs.
// Discv5 times out resolving stale ENRs, then resolves peers via external node.
// This is slow due to discv5 internal timeouts, run with -slow.
t.Run("external_and_stale_enrs", func(t *testing.T) {
external := startExtBootnode(t)

pingCluster(t, pingTest{
Slow: true,
BindENRAddrs: false,
BootManifest: true,
Bootnodes: []string{external.URLv4()},
})
})
}

type pingTest struct {
Slow bool
BindENRAddrs bool
BootManifest bool
Bootnodes []string
}

Expand Down Expand Up @@ -121,7 +124,8 @@ func pingCluster(t *testing.T, test pingTest) {
PingCallback: asserter.Callback(t, i),
},
P2P: p2p.Config{
UDPBootnodes: test.Bootnodes,
UDPBootnodes: test.Bootnodes,
UDPBootManifest: test.BootManifest,
},
}

Expand Down
1 change: 1 addition & 0 deletions cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func bindGeneralFlags(flags *pflag.FlagSet, dataDir *string) {
func bindP2PFlags(flags *pflag.FlagSet, config *p2p.Config) {
flags.StringVar(&config.DBPath, "p2p-udp-peerdb", "", "Path to store a discv5 peer database. Empty default results in in-memory database.")
flags.StringSliceVar(&config.UDPBootnodes, "p2p-udp-bootnodes", nil, "Comma-separated list of discv5 bootnode URLs or ENRs. Manifest ENRs are used if empty. Example URL: enode://<hex node id>@10.3.58.6:30303?discport=30301")
flags.BoolVar(&config.UDPBootManifest, "p2p-udp-bootmanifest", false, "Enables using manifest ENRs as discv5 boot nodes. Allows skipping external bootnode if key generation ceremony included correct IPs")
flags.StringVar(&config.UDPAddr, "p2p-udp-address", "127.0.0.1:30309", "Listening UDP address (ip and port) for Discv5 discovery")
flags.StringSliceVar(&config.TCPAddrs, "p2p-tcp-address", []string{"127.0.0.1:13900"}, "Comma-separated list of listening TCP addresses (ip and port) for LibP2P traffic")
flags.StringVar(&config.Allowlist, "p2p-allowlist", "", "Comma-separated list of CIDR subnets for allowing only certain peer connections. Example: 192.168.0.0/16 would permit connections to peers on your local network only. The default is to accept all connections.")
Expand Down
1 change: 1 addition & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Flags:
--p2p-denylist string Comma-separated list of CIDR subnets for disallowing certain peer connections. Example: 192.168.0.0/16 would disallow connections to peers on your local network. The default is to accept all connections.
--p2p-tcp-address strings Comma-separated list of listening TCP addresses (ip and port) for LibP2P traffic (default [127.0.0.1:13900])
--p2p-udp-address string Listening UDP address (ip and port) for Discv5 discovery (default "127.0.0.1:30309")
--p2p-udp-bootmanifest Enables using manifest ENRs as discv5 boot nodes. Allows skipping external bootnode if key generation ceremony included correct IPs
--p2p-udp-bootnodes strings Comma-separated list of discv5 bootnode URLs or ENRs. Manifest ENRs are used if empty. Example URL: enode://<hex node id>@10.3.58.6:30303?discport=30301
--p2p-udp-peerdb string Path to store a discv5 peer database. Empty default results in in-memory database.
--validator-api-address string Listening address (ip and port) for validator-facing traffic proxying the beacon-node API (default "127.0.0.1:3500")
Expand Down
4 changes: 3 additions & 1 deletion p2p/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ import (
type Config struct {
// DBPath defines the discv5 peer database file path.
DBPath string
// UDPBootnodes defines the discv5 boot node URLs (in addition to manifest ENRs).
// UDPBootnodes defines the discv5 boot node URLs.
UDPBootnodes []string
// UDPBootManifest enables using manifest ENRS as discv5 boot nodes.
UDPBootManifest bool
// UDPAddr defines the discv5 udp listen address.
UDPAddr string
// TCPAddrs defines the lib-p2p tcp listen addresses.
Expand Down
3 changes: 1 addition & 2 deletions p2p/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ func NewUDPNode(config Config, ln *enode.LocalNode, key *ecdsa.PrivateKey,
bootnodes = append(bootnodes, node)
}

// Use manifest ENRs as bootnodes if nothing else provided.
if len(bootnodes) == 0 {
if config.UDPBootManifest {
for _, record := range enrs {
record := record
node, err := enode.New(enode.V4ID{}, &record)
Expand Down

0 comments on commit 7c045b9

Please sign in to comment.