Support WithIgnoreProviders() in provider query manager #10765
Conversation
Adds `Routing.IgnoreProviders`. This requires initializing a custom providerQueryManager and using it instead of the default created internally in Bitswap. Since the default is created with some internal default configuration options (MaxProviders), this hardcodes it.
hsanjuan
force-pushed
the
ignore-providers
branch
from
e955473 to
ecca2eb
Compare
| // internal setting in boxo. | ||
| pqm, err := rpqm.New(bitswapNetwork, | ||
| in.Rt, | ||
| rpqm.WithMaxProviders(10), |
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Thanks! Feels we should avoid hardcoding magic numbers without docs, opened #10773 to sort this out + maybe align with rainbow.
| @@ -41,6 +43,8 @@ type Routing struct { | |||
|
|
|||
| LoopbackAddressesOnLanDHT Flag `json:",omitempty"` | |||
|
|
|||
| IgnoreProviders []peer.ID | |||
There was a problem hiding this comment.
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:
- store base-encoded values in JSON
- 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
* Feat: http retrieval as experimental feature This introduces the http-retrieval capability as an experimental feature. It can be enabled in the configuration `Experimental.HTTPRetrieval.Enabled = true`. Documentation and changelog to be added later. * refactor: HTTPRetrieval.Enabled as Flag * docs(config): HTTPRetrieval section * refactor: reusable MockHTTPContentRouter * feat: HTTPRetrieval.TLSInsecureSkipVerify allows self-signed certificates in tests * feat(config): HTTPRetrieval.MaxBlockSize * test: end-to-end HTTPRetrieval.Enabled this spawns two http services on localhost: 1. HTTP router that returns HTTP provider when /routing/v1/providers/cid i queried 2. HTTP provider that returns a block when /ipfs/cid is queried 3. Configures Kubo to use (1) instead of cid.contact this seems to work (running test with DEBUG=true shows (1) was queried for the test CID and returned multiaddr of (2), but Kubo never requested test CID block from (2) – needs investigation * fix: enable /routing/v1/peers for non-cid.contact we artificially limited every delegated routing endpoint because of cid.contact being limited to one endpoint * feat: Routing.DelegatedRouters make it easy to override the hardcoded implicit HTTP routeur URL without having to set the entire custom Router.Routers and Router.Methods (http_retrieval_client_test.go still needs to be fixed in future commit) * test: flag remaining work * docs: review feedback * refactor: providerQueryMgr with bitswapNetworks this fixes two regressions: (1) introduced in #10717 where we only used bitswapLib2p query manager (this is why E2E did not act on http provider) (2) introduced in #10765 where it was not possible to set binary peerID in IgnoreProviders (we changed to []string) * refactor: Bitswap.Libp2pEnabled replaces Bitswap.Enabled with Bitswap.Libp2pEnabled adds tests that confirm it is possible to disable libp2p bitswap fully and only keep http in client mode also, removes the need for passing empty blockstore in client-only mode * docs: changelog --------- Co-authored-by: Marcin Rataj <[email protected]>
Adds
Routing.IgnoreProviders.This requires initializing a custom providerQueryManager and using it instead of the default created internally in Bitswap. Since the default is created with some internal default configuration options (MaxProviders), this hardcodes it.