Skip to content

Support WithIgnoreProviders() in provider query manager #10765

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 1, 2025
Merged

Support WithIgnoreProviders() in provider query manager #10765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ecca2eb
Support WithIgnoreProviders() in provider query manager
hsanjuan Mar 24, 2025
112eb61
changelog: add mention to IgnoreProviders
hsanjuan Apr 1, 2025
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
7 changes: 4 additions & 3 deletions config/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@ func InitWithIdentity(identity Identity) (*Config, error) {
},

Routing: Routing{
Type: nil,
Methods: nil,
Routers: nil,
Type: nil,
Methods: nil,
Routers: nil,
IgnoreProviders: []peer.ID{},
},

// setup the node mount points.
Expand Down
4 changes: 4 additions & 0 deletions config/routing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"os"
"runtime"
"strings"

peer "github.com/libp2p/go-libp2p/core/peer"
)

const (
Expand Down Expand Up @@ -41,6 +43,8 @@ type Routing struct {

LoopbackAddressesOnLanDHT Flag `json:",omitempty"`

IgnoreProviders []peer.ID
Copy link
Member

Choose a reason for hiding this comment

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

Something I realized while testing #10772 is that user is unable to set PeerID in json when this field type id peer.ID.

even tho peer.ID type in go is string, the value stored there is binary and not base-encoded 🙃

Proof: peer.ID.String() requires extra conversion from binary to base58: https://github.com/libp2p/go-libp2p/blob/50d714c94c043f2a326c6c8e16c7a3c4821b87ad/core/peer/peer.go#L52C8-L52C31

This means we we need to:

  1. store base-encoded values in JSON
  2. add conversion step from base-encoded to binary peer.ID

I've fixed that in 3c24a60#diff-6ef0b54f0ea8496cf31cb4bca6c5e8ca92facee272e34097201d832894bf9d7bR116, but good to keep that in mind in future config changes.

cc @gammazero for visibility


Routers Routers

Methods Methods
Expand Down
21 changes: 18 additions & 3 deletions core/node/bitswap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import (
"time"

"github.com/ipfs/boxo/bitswap"
"github.com/ipfs/boxo/bitswap/client"
bsnet "github.com/ipfs/boxo/bitswap/network/bsnet"
blockstore "github.com/ipfs/boxo/blockstore"
exchange "github.com/ipfs/boxo/exchange"
"github.com/ipfs/boxo/exchange/providing"
provider "github.com/ipfs/boxo/provider"
rpqm "github.com/ipfs/boxo/routing/providerquerymanager"
"github.com/ipfs/kubo/config"
irouting "github.com/ipfs/kubo/routing"
"github.com/libp2p/go-libp2p/core/host"
Expand Down Expand Up @@ -61,6 +63,7 @@ type bitswapIn struct {
fx.In

Mctx helpers.MetricsCtx
Cfg *config.Config
Host host.Host
Rt irouting.ProvideManyRouter
Bs blockstore.GCBlockstore
Expand All @@ -71,12 +74,24 @@ type bitswapIn struct {
// Additional options to bitswap.New can be provided via the "bitswap-options"
// group.
func Bitswap(provide bool) interface{} {
return func(in bitswapIn, lc fx.Lifecycle) *bitswap.Bitswap {
return func(in bitswapIn, lc fx.Lifecycle) (*bitswap.Bitswap, error) {
bitswapNetwork := bsnet.NewFromIpfsHost(in.Host)

var provider routing.ContentDiscovery
if provide {
provider = in.Rt
// We need to hardcode the default because it is an
// internal setting in boxo.
pqm, err := rpqm.New(bitswapNetwork,
in.Rt,
rpqm.WithMaxProviders(10),
Copy link
Member

@lidel lidel Apr 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@hsanjuan why do we hardcode 10 here?

Wasn't previous default DefaultMaxProviders = 0 (unlimited) from boxo/routing/providerquerymanager/providerquerymanager.go?

limiting to 10 feels like DoS vector, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is the value we always used:
https://github.com/ipfs/boxo/blob/main/bitswap/client/client.go#L209

When passing in the pqm explicitally we need to set it...

No change on that front. As to why 10... 🤷 In rainbow we do unlimited, but in Kubo we never ventured to change the default of 10.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! Feels we should avoid hardcoding magic numbers without docs, opened #10773 to sort this out + maybe align with rainbow.

rpqm.WithIgnoreProviders(in.Cfg.Routing.IgnoreProviders...),
)
if err != nil {
return nil, err
}
in.BitswapOpts = append(in.BitswapOpts, bitswap.WithClientOption(client.WithDefaultProviderQueryManager(false)))
provider = pqm

}
bs := bitswap.New(helpers.LifecycleCtx(in.Mctx, lc), bitswapNetwork, provider, in.Bs, in.BitswapOpts...)

Expand All @@ -85,7 +100,7 @@ func Bitswap(provide bool) interface{} {
return bs.Close()
},
})
return bs
return bs, nil
}
}

Expand Down
7 changes: 7 additions & 0 deletions docs/changelogs/v0.35.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ This release was brought to you by the [Shipyard](http://ipshipyard.com/) team.

### 🔦 Highlights

##### `Routing.IgnoreProviders`

This new option allows ignoring specific peer IDs when returned by the content
routing system as providers of content. See the
[documentation](https://github.com/ipfs/kubo/blob/master/docs/config.md#routingignoreproviders)
for for information.

#### 📦️ Important dependency updates

### 📝 Changelog
Expand Down
14 changes: 14 additions & 0 deletions docs/config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ config file at runtime.
- [`Routing.Type`](#routingtype)
- [`Routing.AcceleratedDHTClient`](#routingaccelerateddhtclient)
- [`Routing.LoopbackAddressesOnLanDHT`](#routingloopbackaddressesonlandht)
- [`Routing.IgnoreProviders`](#routingignoreproviders)
- [`Routing.Routers`](#routingrouters)
- [`Routing.Routers: Type`](#routingrouters-type)
- [`Routing.Routers: Parameters`](#routingrouters-parameters)
Expand Down Expand Up @@ -1718,6 +1719,19 @@ Default: `false`

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

### `Routing.IgnoreProviders`

An array of peerIDs. Any provider record associated to one of these peer IDs is ignored.

Apart from ignoring specific providers for reasons like misbehaviour etc. this
setting is useful to ignore providers as a way to indicate preference, when the same provider
is found under different peerIDs (i.e. one for HTTP and one for Bitswap retrieval).

Default: `[]`

Type: `array[peerID]`


### `Routing.Routers`

**EXPERIMENTAL: `Routing.Routers` configuration may change in future release**
Expand Down
Loading