Delegate Reprovide to Content Routing System

[guillaumemichel]

Content Routing Systems should directly handle the Reprovide Operation. Currently, the implementations making use of Content Routers have to keep track of the content they are providing and periodically call the Provide method. Content Routing Systems may have different Reprovide mechanics, and internal optimizations.

Examples:

• DHT implementation could implement reprovide optimizations such as Reprovide Sweep.
• IPNI doesn't need to reprovide content at all.

I suggest adding a StartProviding and a StopProviding methods to the ContentRouting interface. Content Routing Systems would expose a simple interface to provide content over time. Content Routing Systems would be responsible of republishing content where required.

Change to take place in routing.go

// ContentRouting is a value provider layer of indirection. It is used to find
// information about who has what content.
//
// Content is identified by CID (content identifier), which encodes a hash
// of the identified content in a future-proof manner.
type ContentRouting interface {
	// Provide adds the given cid to the content routing system. If 'true' is
	// passed, it also announces it, otherwise it is just kept in the local
	// accounting of which objects are being provided.
	Provide(context.Context, cid.Cid, bool) error

	// StartProviding adds the given cid to the content routing system, which 
	// periodically announces it until StopProviding is called.
	StartProviding(context.Context, cid.Cid) error

	// StopProviding removes the given cid from the content routing system.
	StopProviding(context.Context, cid.Cid) error

	// Search for peers who are able to provide a given key
	//
	// When count is 0, this method will return an unbounded number of
	// results.
	FindProvidersAsync(context.Context, cid.Cid, int) <-chan peer.AddrInfo
}

PS: I bet this change will have a lot of repercussions, hence I wanted to discuss it here first before opening a PR and breaking everything.

References:

• Reprovide Sweep go-libp2p-kad-dht#824
• Move (Re)Provider logic to Content Routers ipfs/go-ipfs-provider#49

[aschmahmann]

An alternative approach here is to make a new interface rather than try to extend the existing one.

IMO there are few issues with the routing interfaces that we can fix in roughly the same way.

Issues:

• libp2p mostly doesn't care about anything other than peer routing, and yet they hold these more generic interfaces they don't have much to do with
• Tools like go-libp2p-routing-helpers are built around these interfaces meaning that if you want to use them for something new you are encouraged to extend the routing interface
• We end up with helpers https://github.com/libp2p/go-libp2p-routing-helpers/blob/8adffb3129ab568afb7e099247099eb4560a8b96/composed.go#L19 to try and fill in the blanks when you're only trying to use part of the interface

Potential Fix:

• Lean into 1) Go allowing interfaces to be declared at the usage site, 2) Go now supporting generics
• You could define these interfaces where they are needed. You might actually want separate interfaces instead of a joint one (e.g. the Go stdlib has io.Reader io.Writer and io.Closer as separate functions, do you want Start/StopProviding in the same interface or two? You're likely not sure yet since you haven't written the consumers of those interfaces).
• You could lean into generics so that tools like the routing helpers will be reasonably reusable as you add more functionality.
  • For example, while right now this interface advertises CIDs you might discover in the future you want something else. The IPFS Public DHT actually uses multihashes not CIDs, and IPNI has all sorts of other arbitrary metadata shoved in advertisements. You probably want evolution of this interface in the future to break as few things as possible.

Side note: I doubt you want that bool in StartProviding. I'd check to see if the one used for Provide is even in use anymore and if so if there's a way to do it outside of the routing interface as it seems out of place.

[guillaumemichel]

An alternative approach here is to make a new interface rather than try to extend the existing one.

I agree that making new interfaces makes more sense.

libp2p mostly doesn't care about anything other than peer routing, and yet they hold these more generic interfaces they don't have much to do with

In theory, libp2p wouldn't need to deal with CIDs, or content routing at all. It must only handle peer routing (in a limited extent). IMO everything content routing should be handled at the IPFS level.

I think that all content routers should all expose the same interface to IPFS implementations, even if the mechanics underneath is different. I am not against the idea of routing helpers, but I think they belong to the IPFS space, and not libp2p. Hence I like the idea of having these content routing interfaces defined at the IPFS level. Generics could be useful to reuse some strategy in different content routers (below the interface IPFS-ContentRouting).

Side note: I doubt you want that bool in StartProviding. I'd check to see if the one used for Provide is even in use anymore and if so if there's a way to do it outside of the routing interface as it seems out of place.

+1

[MarcoPolo]

I think this is settled, so closing. If it's not we can re-open.