fix: Provide according to Reprovider.Strategy #10886

hsanjuan · 2025-07-29T12:48:15Z

This updates boxo to this PR ipfs/boxo#976, which decentralizes the providing responsibilities.

Then it perfoms the necessary changes in Kubo, which consist in initializing the Pinner, MFS and the blockstore with the provider.System.

Since the provider.System is created first, the reproviding KeyChanFunc is set later when we can create it.

Aims to Close #10837

Part of Most Kubo users can reliably advertise their data into the Amino DHT — IPFS/2025 ipshipyard/roadmaps#7 (respects user choice of strategy)

This updates boxo to this PR ipfs/boxo#976, which decentralizes the providing responsibilities. Then it perfoms the necessary changes in Kubo, which consist in initializing the Pinner, MFS and the blockstore with the provider.System. Since the provider.System is created first, the reproviding KeyChanFunc is set later when we can create it.

Rather than fx.Provide(). Since no one was needing the strategy for anything, it was not getting called, and not getting set in the reprovider.

This commit ensure that added blocks get provided with "pinned" strategy. Normally, blocks would get provided at the blockstore or pinner facility, but when adding with "pinned" strategy, the blockstore does not provide, and the pinner does not traverse the DAG so we need to provide directly from the Adder. This is resolved by wrapping the DAGService in a "providingDAGService" which provides every added block, when using the "pinned" strategy. There are alternatives like wiring a provider in coreunix but this seems simple enough and follows existing style as the DAGService is already wrapped as a Syncer. CoreAPI also overwrote the provider into a NoopProvider when using --offline. However, we have strategies to follow and it did not overwrite the blockstore. So --offline would provide anyways with strategy "all", and not provide with strategy "pinned". It seems reasonable to provide following the provider strategy when the node is online, regardless of the client flag. Otherwise we can change it later by overwriting the provider and the blockstore. cli/provider tests have been fixed and made to not rely on `--offline`.

…ccording-to-strategy

hsanjuan

Self review and clarifications

core/coreapi/coreapi.go

hsanjuan · 2025-08-06T08:26:11Z

core/coreapi/coreapi.go

@@ -70,7 +73,8 @@ type CoreAPI struct {
 	ipldPathResolver   pathresolver.Resolver
 	unixFSPathResolver pathresolver.Resolver

-	provider provider.System
+	provider          provider.System
+	providingStrategy string


Carry the strategy so we can react on it when adding content.

hsanjuan · 2025-08-06T08:29:15Z

core/coreapi/pin.go

-	if err := api.provider.Provide(ctx, dagNode.Cid(), true); err != nil {
-		return err
-	}
-


Previously, at the end of the Pin process, coreapi would trigger a Provide for the root CID (regardless of prividing strategy).

We now provide according to strategy, and this does not need to happen here (happens in the pinner).

hsanjuan · 2025-08-06T08:31:07Z

core/coreapi/unixfs.go

+	var dserv ipld.DAGService = merkledag.NewDAGService(bserv)
+
+	// wrap the DAGService in a providingDAG service which provides every block written.
+	// note about strategies:
+	//   - "all"/"flat" gets handled directly at the blockstore so no need to provide
+	//   - "roots" gets handled in the pinner
+	//   - "mfs" gets handled in mfs
+	// We need to provide the "pinned" cases only. Added blocks are not
+	// going to be provided by the blockstore (wrong strategy for that),
+	// nor by the pinner (it does not traverse the pinned DAG).
+	if settings.Pin && !settings.OnlyHash &&
+		(api.providingStrategy == "pinned" || api.providingStrategy == "pinned+mfs") {
+		dserv = &providingDagService{dserv, api.provider}
+	}


Per the comment, we need to ensure we provide blocks while Adding with the "pinned" strategy as this cannot happen in the Pinner (it doesn't traverse the DAG itself). For that, we wrap the DagService.

hsanjuan · 2025-08-06T08:33:56Z

core/coreapi/unixfs.go

@@ -183,7 +200,7 @@ func (api *UnixfsAPI) Add(ctx context.Context, files files.Node, opts ...options
 		if err != nil {
 			return path.ImmutablePath{}, err
 		}
-		mr, err := mfs.NewRoot(ctx, md, emptyDirNode, nil)
+		mr, err := mfs.NewRoot(ctx, md, emptyDirNode, nil, nil)


new nil is the provider, which we leave disabled for MFS here. We are using a providing DAG service when needed.

Could we potentially passed a provider around and initialize MFS with it here and have it provide this way, rather than wrapping the DAGService? Potentially yes, but I'm not sure what happens with file-chunks and if MFS touches them at all. If not, I would need to wire the chunkers too etc. Overall it seemed more complex and fragile to do it that way.

core/node/core.go

core/node/provider.go

hsanjuan · 2025-08-06T08:45:26Z

core/node/provider.go

+
+		in.Provider.SetKeyProvider(kcf)
+
+		var strategyChanged bool


The strategy-change detection has been moved here. No changes in code blocks below.

test/cli/harness/run.go

Things like block/put are now provided correctly. Tests rely on those things not being provided and a manual call to "routing provide" being needed. Fix things by setting "roots" as strategy for tests.

gammazero

See comments.

gammazero · 2025-08-06T21:35:42Z

core/coreapi/unixfs.go

@@ -126,6 +141,8 @@ func (api *UnixfsAPI) Add(ctx context.Context, files files.Node, opts ...options
 		}
 	}

+	// reminder: the dag server gets wrapped again (!) as a batching dagservice in coreunix.Adder.


Where does this happen? Does this refer to where a batch is created in the dag put command?

Here:

kubo/core/coreunix/add.go

Line 52 in 96aa052

bufferedDS := ipld.NewBufferedDAG(ctx, ds)

There is a normal DAGService, which gets wrapped in a providing DAGService, which gets wrapped in a SyncDAGService, which gets wrapped in a Buffered DAGService.

added a comment which documents this: c8d1f73#diff-f95c7ebeb06be691a9a243030e11f7d3a1c4297086e8ebfa20ac3c7936878a32R145

core/coreapi/unixfs.go

core/coreiface/tests/routing.go

core/node/core.go

guillaumemichel

Looks good!

A few comments

core/node/provider.go

docs/changelogs/v0.37.md

test/cli/provider_test.go

core/node/groups.go

* more tests * work with bit flags rather than string for the strategies * fix typos

…ccording-to-strategy

just a precaution, since we don't want pinner provide to hide issues with "all" strategy

extra info from comments to #10886

ipfs/boxo#976

did too many things, this makes it easier to understand and maintain

lidel

Pushed some extra go comments with context from this and boxo PRs + small refactor.

Lgtm, but want to run some additional manual tests before merging.
Will post once done.

core/node/provider.go

just making sure dag and block puts are wired "the same way" as add command

lidel

Thank you @hsanjuan, this should make future changes easier to reason about. Lgtm, pushed extra docs/tests.

Ok to squash&merge, but would like you to take one final look at split I did in 3b481cd and give 👍

If no concerns my plan is to merge this by the end of this week (end of Friday 8th) to unblock other PRs that depend on latest boxo (e.g. #10851 (comment)).

hsanjuan · 2025-08-08T08:48:25Z

Split looks good

hsanjuan self-assigned this Jul 29, 2025

hsanjuan force-pushed the fix/10837-provide-according-to-strategy branch from dc44336 to 72a97c7 Compare July 29, 2025 12:55

Update to latest boxo PR 976

13719ee

hsanjuan mentioned this pull request Jul 31, 2025

Remove providing Exchange. Call Provide() from relevant places. ipfs/boxo#976

Merged

provider setup: fx.Invoke() the strategy

08b83f2

Rather than fx.Provide(). Since no one was needing the strategy for anything, it was not getting called, and not getting set in the reprovider.

This comment was marked as outdated.

Sign in to view

hsanjuan added 4 commits August 4, 2025 15:50

test/cli: fix provider tests

692e15a

test: add provider strategy tests

a99563a

Merge remote-tracking branch 'origin/master' into fix/10837-provide-a…

0bdae0e

…ccording-to-strategy

hsanjuan marked this pull request as ready for review August 6, 2025 08:24

hsanjuan requested a review from a team as a code owner August 6, 2025 08:24

hsanjuan commented Aug 6, 2025

View reviewed changes

core: initialize MFS with provider only when required by strategy

64d5096

hsanjuan force-pushed the fix/10837-provide-according-to-strategy branch from d1de66a to 64d5096 Compare August 6, 2025 09:15

hsanjuan added 2 commits August 6, 2025 11:30

changelog: add information about respecting reprovider.Strategy

c155be0

coreapi/rpc tests: fix tests with latest providing improvements

15f3c13

Things like block/put are now provided correctly. Tests rely on those things not being provided and a manual call to "routing provide" being needed. Fix things by setting "roots" as strategy for tests.

hsanjuan changed the title ~~Provide according to strategy~~ Provide according to Reprovider.Strategy Aug 6, 2025

changelog: add top-index link to Reprovider.Strategy entry

96aa052

gammazero approved these changes Aug 6, 2025

View reviewed changes

guillaumemichel approved these changes Aug 7, 2025

View reviewed changes

Address comments on #10886 around the reprovider

3fbee50

* more tests * work with bit flags rather than string for the strategies * fix typos

hsanjuan requested review from gammazero and guillaumemichel August 7, 2025 13:24

Merge remote-tracking branch 'origin/master' into fix/10837-provide-a…

5522b1e

…ccording-to-strategy

lidel mentioned this pull request Aug 7, 2025

Release 0.37 #10867

Closed

51 tasks

lidel added 3 commits August 7, 2025 20:18

test(provider): ipfs add with --pin=false

cc799d8

just a precaution, since we don't want pinner provide to hide issues with "all" strategy

docs: preserve comments from PR review

c8d1f73

extra info from comments to #10886

docs: extra context from boxo#976

6d5e254

ipfs/boxo#976

lidel self-requested a review August 7, 2025 18:42

lidel changed the title ~~Provide according to Reprovider.Strategy~~ fix: Provide according to Reprovider.Strategy Aug 7, 2025

refactor: split setReproviderKeyProvider

3b481cd

did too many things, this makes it easier to understand and maintain

lidel reviewed Aug 7, 2025

View reviewed changes

core/node/provider.go Outdated Show resolved Hide resolved

core/node/provider.go Outdated Show resolved Hide resolved

lidel added 2 commits August 7, 2025 22:30

docs: changelog

b9d7763

test(provider): dag|block put --pin=false

8405fcb

just making sure dag and block puts are wired "the same way" as add command

lidel mentioned this pull request Aug 7, 2025

feat(ipns): support passing custom sequence number during publishing #10851

Merged

lidel approved these changes Aug 7, 2025

View reviewed changes

guillaumemichel approved these changes Aug 8, 2025

View reviewed changes

hsanjuan merged commit a673c2e into master Aug 8, 2025
16 checks passed

hsanjuan deleted the fix/10837-provide-according-to-strategy branch August 8, 2025 08:56

lidel mentioned this pull request Aug 11, 2025

Most Kubo users can reliably advertise their data into the Amino DHT — IPFS/2025 ipshipyard/roadmaps#7

Open

guillaumemichel mentioned this pull request Aug 12, 2025

Move Reprovider config to Provider #10909

Open

3 tasks

Uh oh!

fix: Provide according to Reprovider.Strategy #10886

fix: Provide according to Reprovider.Strategy #10886

Uh oh!

Conversation

hsanjuan commented Jul 29, 2025 • edited by lidel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

hsanjuan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gammazero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guillaumemichel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lidel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lidel left a comment

Choose a reason for hiding this comment

Uh oh!

hsanjuan commented Aug 8, 2025

Uh oh!

Uh oh!

Uh oh!

hsanjuan commented Jul 29, 2025 •

edited by lidel

Loading