Skip to content
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

chore: upgrade go-libp2p-kad-dht #10378

Merged
merged 6 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion client/rpc/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ func (np NodeProvider) MakeAPISwarm(t *testing.T, ctx context.Context, fullIdent
c := n.ReadConfig()
c.Experimental.FilestoreEnabled = true
n.WriteConfig(c)

n.StartDaemon("--enable-pubsub-experiment", "--offline="+strconv.FormatBool(!online))

if online {
Expand Down
1 change: 1 addition & 0 deletions config/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ is useful when using the daemon in test environments.`,
}

c.Swarm.DisableNatPortMap = true
c.Routing.LoopbackAddressesOnLanDHT = True

c.Bootstrap = []string{}
c.Discovery.MDNS.Enabled = false
Expand Down
5 changes: 4 additions & 1 deletion config/routing.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import (
)

var (
DefaultAcceleratedDHTClient = false
DefaultAcceleratedDHTClient = false
DefaultLoopbackAddressesOnLanDHT = false
)

// Routing defines configuration options for libp2p routing.
Expand All @@ -21,6 +22,8 @@ type Routing struct {

AcceleratedDHTClient Flag `json:",omitempty"`

LoopbackAddressesOnLanDHT Flag `json:",omitempty"`

Routers Routers

Methods Methods
Expand Down
2 changes: 2 additions & 0 deletions core/node/libp2p/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/libp2p/go-libp2p/core/routing"
routedhost "github.com/libp2p/go-libp2p/p2p/host/routed"

"github.com/ipfs/kubo/config"
"github.com/ipfs/kubo/core/node/helpers"
"github.com/ipfs/kubo/repo"

Expand Down Expand Up @@ -60,6 +61,7 @@ func Host(mctx helpers.MetricsCtx, lc fx.Lifecycle, params P2PHostIn) (out P2PHo
BootstrapPeers: bootstrappers,
OptimisticProvide: cfg.Experimental.OptimisticProvide,
OptimisticProvideJobsPoolSize: cfg.Experimental.OptimisticProvideJobsPoolSize,
LoopbackAddressesOnLanDHT: cfg.Routing.LoopbackAddressesOnLanDHT.WithDefault(config.DefaultLoopbackAddressesOnLanDHT),
}
opts = append(opts, libp2p.Routing(func(h host.Host) (routing.PeerRouting, error) {
args := routingOptArgs
Expand Down
11 changes: 10 additions & 1 deletion core/node/libp2p/routingopt.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type RoutingOptionArgs struct {
BootstrapPeers []peer.AddrInfo
OptimisticProvide bool
OptimisticProvideJobsPoolSize int
LoopbackAddressesOnLanDHT bool
}

type RoutingOption func(args RoutingOptionArgs) (routing.Routing, error)
Expand Down Expand Up @@ -116,10 +117,18 @@ func constructDHTRouting(mode dht.ModeOpt) RoutingOption {
if args.OptimisticProvideJobsPoolSize != 0 {
dhtOpts = append(dhtOpts, dht.OptimisticProvideJobsPoolSize(args.OptimisticProvideJobsPoolSize))
}
wanOptions := []dht.Option{
dht.BootstrapPeers(args.BootstrapPeers...),
}
lanOptions := []dht.Option{}
if args.LoopbackAddressesOnLanDHT {
lanOptions = append(lanOptions, dht.AddressFilter(nil))
}
Comment on lines +120 to +126
Copy link
Member

@lidel lidel Apr 2, 2024

Choose a reason for hiding this comment

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

💭 LoopbackAddressesOnLanDHT Feels overly specific given how generic go-libp2p config option is:

We have similar setup for WAN.

I would expect this a Flag to be named close to what we do in libp2p config, Routing.DisableNoAddressFiltersOnLanDHT and have similar one for WAN (Routing.DisableAddressFiltersOnWanDHT) just in case we break people running custom overlay networks that don't follow official RFCs (giving them an escape hatch to self-fix, without having to report bugs to us).

I don't know how real this problem is, we would have to itnterview someone working with IPFS and overlays, so don't want to block on this.

For now, if we make LoopbackAddressesOnLanDHT a Flag we can leave this as-is, and fix later, if/once someone reports we broke their setup.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed to Flag. I would keep it as-is for now and see if someone complains. Otherwise we'll have to add many options. I had already discussed yesterday with @aschmahmann and we had decided on removing the WAN option for now.

return dual.New(
args.Ctx, args.Host,
dual.DHTOption(dhtOpts...),
dual.WanDHTOption(dht.BootstrapPeers(args.BootstrapPeers...)),
dual.WanDHTOption(wanOptions...),
dual.LanDHTOption(lanOptions...),
)
}
}
Expand Down
7 changes: 7 additions & 0 deletions docs/changelogs/v0.28.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- [RPC client: removed deprecated DHT API](#rpc-client-removed-deprecated-dht-api)
- [Gateway: `/api/v0` is removed](#gateway-apiv0-is-removed)
- [Removed deprecated Object API commands](#removed-deprecated-object-api-commands)
- [No longer publishes loopback and private addresses on DHT](#no-longer-publishes-loopback-and-private-addresses-on-dht)
- [📝 Changelog](#-changelog)
- [👨‍👩‍👧‍👦 Contributors](#-contributors)

Expand All @@ -28,6 +29,12 @@ If you have a legacy software that relies on this behavior, and want to expose p

The Object API commands deprecated back in [2021](https://github.com/ipfs/kubo/issues/7936) have been removed, except for `object diff`, `object patch add-link` and `object patch rm-link`, whose alternatives have not yet been built (see issues [4801](https://github.com/ipfs/kubo/issues/4801) and [4782](https://github.com/ipfs/kubo/issues/4782)).

##### Kubo ignores loopback addresses on LAN DHT and private addresses on WAN DHT

Kubo no longer keeps track of loopback and private addresses on the LAN and WAN DHTs, respectively. This means that other nodes will not try to dial likely undialable addresses.

hacdias marked this conversation as resolved.
Show resolved Hide resolved
To support testing scenarios where multiple Kubo instances run on the same machine, [`Routing.LoopbackAddressesOnLanDHT`](https://github.com/ipfs/kubo/blob/master/docs/config.md#routingloopbackaddressesonlandht) is set to `true` when the `test` profile is applied.

### 📝 Changelog

### 👨‍👩‍👧‍👦 Contributors
13 changes: 13 additions & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ config file at runtime.
- [`Routing`](#routing)
- [`Routing.Type`](#routingtype)
- [`Routing.AcceleratedDHTClient`](#routingaccelerateddhtclient)
- [`Routing.LoopbackAddressesOnLanDHT`](#routingloopbackaddressesonlandht)
- [`Routing.Routers`](#routingrouters)
- [`Routing.Routers: Type`](#routingrouters-type)
- [`Routing.Routers: Parameters`](#routingrouters-parameters)
Expand Down Expand Up @@ -1612,6 +1613,18 @@ Default: `false`

Type: `flag`

### `Routing.LoopbackAddressesOnLanDHT`

**EXPERIMENTAL: `Routing.LoopbackAddressesOnLanDHT` configuration may change in future release**

Whether loopback addresses (e.g. 127.0.0.1) should not be ignored on the local LAN DHT.

Most users do not need this setting. It can be useful during testing, when multiple Kubo nodes run on the same machine but some of them do not have `Discovery.MDNS.Enabled`.
Copy link
Member

Choose a reason for hiding this comment

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

ℹ️ I played a bit with various setups and this seems to be useful only when mDNS discovery is not available. Had two nodes with MDNS, and they discovered each other and connected over /ip6/::1/udp/4001/quic-v1/ just fine.

Added this note so our config docs explain why and when the option matters, so users save time and not bother with it.


Default: `false`

Type: `bool` (missing means `false`)

### `Routing.Routers`

**EXPERIMENTAL: `Routing.Routers` configuration may change in future release**
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ require (
github.com/libp2p/go-doh-resolver v0.4.0
github.com/libp2p/go-libp2p v0.33.2
github.com/libp2p/go-libp2p-http v0.5.0
github.com/libp2p/go-libp2p-kad-dht v0.24.4
github.com/libp2p/go-libp2p-kad-dht v0.25.2
github.com/libp2p/go-libp2p-kbucket v0.6.3
github.com/libp2p/go-libp2p-pubsub v0.10.0
github.com/libp2p/go-libp2p-pubsub-router v0.6.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,8 @@ github.com/libp2p/go-libp2p-gostream v0.6.0 h1:QfAiWeQRce6pqnYfmIVWJFXNdDyfiR/qk
github.com/libp2p/go-libp2p-gostream v0.6.0/go.mod h1:Nywu0gYZwfj7Jc91PQvbGU8dIpqbQQkjWgDuOrFaRdA=
github.com/libp2p/go-libp2p-http v0.5.0 h1:+x0AbLaUuLBArHubbbNRTsgWz0RjNTy6DJLOxQ3/QBc=
github.com/libp2p/go-libp2p-http v0.5.0/go.mod h1:glh87nZ35XCQyFsdzZps6+F4HYI6DctVFY5u1fehwSg=
github.com/libp2p/go-libp2p-kad-dht v0.24.4 h1:ktNiJe7ffsJ1wX3ULpMCwXts99mPqGFSE/Qn1i8pErQ=
github.com/libp2p/go-libp2p-kad-dht v0.24.4/go.mod h1:ybWBJ5Fbvz9sSLkNtXt+2+bK0JB8+tRPvhBbRGHegRU=
github.com/libp2p/go-libp2p-kad-dht v0.25.2 h1:FOIk9gHoe4YRWXTu8SY9Z1d0RILol0TrtApsMDPjAVQ=
github.com/libp2p/go-libp2p-kad-dht v0.25.2/go.mod h1:6za56ncRHYXX4Nc2vn8z7CZK0P4QiMcrn77acKLM2Oo=
github.com/libp2p/go-libp2p-kbucket v0.3.1/go.mod h1:oyjT5O7tS9CQurok++ERgc46YLwEpuGoFq9ubvoUOio=
github.com/libp2p/go-libp2p-kbucket v0.6.3 h1:p507271wWzpy2f1XxPzCQG9NiN6R6lHL9GiSErbQQo0=
github.com/libp2p/go-libp2p-kbucket v0.6.3/go.mod h1:RCseT7AH6eJWxxk2ol03xtP9pEHetYSPXOaJnOiD8i0=
Expand Down
1 change: 1 addition & 0 deletions test/cli/harness/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ func (n *Node) Init(ipfsArgs ...string) *Node {
cfg.Addresses.Gateway = []string{n.GatewayListenAddr.String()}
cfg.Swarm.DisableNatPortMap = true
cfg.Discovery.MDNS.Enabled = n.EnableMDNS
cfg.Routing.LoopbackAddressesOnLanDHT = config.True
})
return n
}
Expand Down
4 changes: 4 additions & 0 deletions test/sharness/lib/iptb-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ startup_cluster() {
other_args="$@"
bound=$(expr "$num_nodes" - 1)

test_expect_success "set Routing.LoopbackAddressesOnLanDHT to true" '
iptb run [0-$bound] -- ipfs config --json "Routing.LoopbackAddressesOnLanDHT" true
'

if test -n "$other_args"; then
test_expect_success "start up nodes with additional args" "
iptb start -wait [0-$bound] -- ${other_args[@]}
Expand Down
3 changes: 2 additions & 1 deletion test/sharness/t0131-multinode-client-routing.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ run_single_file_test() {
NNODES=10

test_expect_success "set up testbed" '
iptb testbed create -type localipfs -count $NNODES -force -init
iptb testbed create -type localipfs -count $NNODES -force -init &&
iptb run -- ipfs config --json "Routing.LoopbackAddressesOnLanDHT" true
'

test_expect_success "start up nodes" '
Expand Down
3 changes: 2 additions & 1 deletion test/sharness/t0142-testfilter.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ AF="/ip4/127.0.0.0/ipcidr/24"
NUM_NODES=3

test_expect_success "set up testbed" '
iptb testbed create -type localipfs -count $NUM_NODES -force -init
iptb testbed create -type localipfs -count $NUM_NODES -force -init &&
iptb run -- ipfs config --json "Routing.LoopbackAddressesOnLanDHT" true
'

test_expect_success 'filter 127.0.0.0/24 on node 1' '
Expand Down
1 change: 1 addition & 0 deletions test/sharness/t0181-private-network.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ LIBP2P_FORCE_PNET=1 test_launch_ipfs_daemon

test_expect_success "set up iptb testbed" '
iptb testbed create -type localipfs -count 5 -force -init &&
iptb run -- ipfs config --json "Routing.LoopbackAddressesOnLanDHT" true &&
iptb run -- ipfs config --json Addresses.Swarm '"'"'["/ip4/127.0.0.1/tcp/0"]'"'"'
'

Expand Down
3 changes: 2 additions & 1 deletion test/sharness/t0182-circuit-relay.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ test_description="Test circuit relay"
# start iptb + wait for peering
NUM_NODES=3
test_expect_success 'init iptb' '
iptb testbed create -type localipfs -count $NUM_NODES -init
iptb testbed create -type localipfs -count $NUM_NODES -init &&
iptb run -- ipfs config --json "Routing.LoopbackAddressesOnLanDHT" true
'

# Network toplogy: A <-> Relay <-> B
Expand Down
1 change: 1 addition & 0 deletions test/sharness/t0184-http-proxy-over-p2p.sh
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ function curl_send_multipart_form_request() {

test_expect_success 'configure nodes' '
iptb testbed create -type localipfs -count 2 -force -init &&
iptb run -- ipfs config --json "Routing.LoopbackAddressesOnLanDHT" true &&
ipfsi 0 config --json Experimental.Libp2pStreamMounting true &&
ipfsi 1 config --json Experimental.Libp2pStreamMounting true &&
ipfsi 0 config --json Experimental.P2pHttpProxy true &&
Expand Down
3 changes: 2 additions & 1 deletion test/sharness/t0276-cidv0v1.sh
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ test_expect_success "check that we can access the file when converted to CIDv1"
#

test_expect_success "set up iptb testbed" '
iptb testbed create -type localipfs -count 2 -init
iptb testbed create -type localipfs -count 2 -init &&
iptb run -- ipfs config --json "Routing.LoopbackAddressesOnLanDHT" true
'

test_expect_success "start nodes" '
Expand Down
Loading