feat(config): ability to disable Bitswap fully or just server #10782

gystemd · 2025-04-10T21:58:18Z

Replaced by #10793

This pull request introduces changes to the Bitswap configuration and functionality, including the addition of new configuration options, modifications to the Bitswap server/client behavior, and the inclusion of new tests to verify these changes.

Bitswap Configuration Enhancements:

Added a new BitswapConfig struct in config/bitswap.go to hold Bitswap configuration options, including Enabled and ServerEnabled flags.
Updated the Config struct in config/config.go to include the new BitswapConfig field.

Bitswap Server/Client Behavior:

Modified the Bitswap function in core/node/bitswap.go to respect the ServerEnabled flag, using an empty blockstore when the server is disabled to prevent serving blocks to other peers.
Updated the OnlineExchange function to return a no-op exchange when Bitswap is disabled.
Added a noopExchange struct to handle cases where Bitswap is disabled, ensuring no blocks are served or retrieved.

Testing:

Introduced a new test file test/cli/bitswap_config_test.go with multiple test cases to verify the behavior of Bitswap configuration, including scenarios where the server is enabled, disabled, or Bitswap is completely disabled.

…ablement

lidel

Thank you @gystemd, this partially works, but does not fully disable the server (it is still possible for other peers to send us WANT and GET for CIDs we dont have).

See ideas (A) and (B) below, I suspect we will end up with (B) but let's see what is easier.

config/bitswap.go

core/node/bitswap.go

lidel · 2025-04-11T15:02:57Z

test/cli/bitswap_config_test.go

We need a test that confirms ipfs id peerid no longer includes /ipfs/bitswap protocols when Bitswap.ServerEnabled is set to false (ensuring we no longer announce ourselves as server)

Original PR: #10782 This pull request introduces changes to the Bitswap configuration and functionality, including the addition of new configuration options, modifications to the Bitswap server/client behavior, and the inclusion of new tests to verify these changes. * Added a new `BitswapConfig` struct in `config/bitswap.go` to hold Bitswap configuration options, including `Enabled` and `ServerEnabled` flags. * Updated the `Config` struct in `config/config.go` to include the new `BitswapConfig` field. * Modified the `Bitswap` function in `core/node/bitswap.go` to respect the `ServerEnabled` flag, using an empty blockstore when the server is disabled to prevent serving blocks to other peers. * Updated the `OnlineExchange` function to return a no-op exchange when Bitswap is disabled. * Added a `noopExchange` struct to handle cases where Bitswap is disabled, ensuring no blocks are served or retrieved. * Introduced a new test file `test/cli/bitswap_config_test.go` with multiple test cases to verify the behavior of Bitswap configuration, including scenarios where the server is enabled, disabled, or Bitswap is completely disabled. - Closes #10717 - Depends on ipfs/boxo#912

lidel · 2025-04-30T17:01:43Z

core/node/bitswap.go

+func (e *noopExchange) GetBlock(ctx context.Context, c cid.Cid) (blocks.Block, error) {
+	return nil, nil


Tested latest commit 258fb25 and not returning error here seems to be why we have a panic when executing ipfs cat when Bitswap.Enabled is set to false:

$ ipfs daemon ... Daemon is ready ... ipfs cat for lon-local cid is executed from other terminal 2025-04-30T17:00:40.931+0200 ERROR cmds/http http/handler.go:96 a panic has occurred in the commands handler! 2025-04-30T17:00:40.931+0200 ERROR cmds/http http/handler.go:97 runtime error: invalid memory address or nil pointer dereference 2025-04-30T17:00:40.933+0200 ERROR cmds/http http/handler.go:98 stack trace: goroutine 50865 [running]: runtime/debug.Stack() runtime/debug/stack.go:26 +0x5e github.com/ipfs/go-ipfs-cmds/http.(*handler).ServeHTTP.func1() github.com/ipfs/go-ipfs-cmds@v0.14.1/http/handler.go:98 +0xef panic({0x2e5e7c0?, 0x4b6e100?}) runtime/panic.go:792 +0x132 github.com/ipfs/boxo/blockstore.(*idstore).Put(0xc0011d9400, {0x385dbf0, 0xc002d40060}, {0x0, 0x0}) github.com/ipfs/boxo@v0.30.0/blockstore/idstore.go:93 +0x27 github.com/ipfs/boxo/blockservice.getBlock({0x385dbf0, 0xc002d40060}, {{0xc0022d8fc0?, 0x1d?}}, {0x3871b68, 0xc00130a270}, 0xc001462740) github.com/ipfs/boxo@v0.30.0/blockservice/blockservice.go:278 +0x242 github.com/ipfs/boxo/blockservice.(*Session).GetBlock(0xc002e559c0, {0x385dbf0, 0xc0004afe00}, {{0xc0022d8fc0?, 0x248f580?}}) github.com/ipfs/boxo@v0.30.0/blockservice/blockservice.go:462 +0x33c github.com/ipfs/boxo/ipld/merkledag.(*sesGetter).Get(0xc002ed7940, {0x385dbf0, 0xc0004afe00}, {{0xc0022d8fc0?, 0xc001462788?}}) github.com/ipfs/boxo@v0.30.0/ipld/merkledag/merkledag.go:143 +0x2e github.com/ipfs/boxo/ipld/merkledag.(*ComboService).Get(0xc002356300?, {0x385dbf0?, 0xc0004afe00?}, {{0xc0022d8fc0?, 0xc00074cdc0?}}) github.com/ipfs/boxo@v0.30.0/ipld/merkledag/rwservice.go:31 +0x2a github.com/ipfs/kubo/core/coreapi.(*CoreAPI).ResolveNode(0xc002356300, {0x385dbf0, 0xc0004afdd0}, {0x385db48, 0xc00074cdc0}) github.com/ipfs/kubo/core/coreapi/path.go:31 +0x338 github.com/ipfs/kubo/core/coreapi.(*UnixfsAPI).Get(0xc000b48480, {0x385dc28, 0xc0042f09b0}, {0x385db48, 0xc00074cdc0}) github.com/ipfs/kubo/core/coreapi/unixfs.go:210 +0x318 github.com/ipfs/kubo/core/commands.cat({0x385dc28, 0xc0042f09b0}, {0x387bba0, 0xc000b48480}, {0xc002ed7840, 0x1, 0x0?}, 0x0, 0xffffffffffffffff) github.com/ipfs/kubo/core/commands/cat.go:136 +0x17b

Seems to be an easy fix, I'll push one soon.

Fix: b11a65a

Fixes panic described in: ipfs#10782 (comment)

lidel

Thank you for your work and patience @gystemd.

Since we switched to latest boxo release, we don't need non-fork local branch PR anymore, using this one
Pushed small fix for a panic described in #10782 (comment) and extra test check, but overall lgtm.
We may want to move noopExchange to boxo/exchange in the future, but fine to keep it here for now (not worth blocking this PR on that).
This is good enough for testing, we can tackle removal of libp2p identify announcement when Bitswap.Enabled=false in future PR.

I'm merging this to include in 0.35.0-rc1 and do some tests in staging environment.

we have those in section above

gystemd added 2 commits April 10, 2025 23:52

feat: add Bitswap configuration and related tests

14c7ccc

fix: update Bitswap function to use 'provide' parameter for server en…

682b47b

…ablement

gystemd requested a review from a team as a code owner April 10, 2025 21:58

gystemd and others added 3 commits April 11, 2025 00:00

Merge branch 'master' into feat/bitswap/disable

a2c34bd

docs: update changelog for Bitswap functionality changes

366b56e

Merge branch 'bitswap-disable' into feat/bitswap/disable

bc4a386

lidel changed the title ~~Add Bitswap disable option~~ feat(config): ability to disable Bitswap fully or just server Apr 11, 2025

lidel requested changes Apr 11, 2025

View reviewed changes

gystemd added 2 commits April 17, 2025 02:31

fix: update Bitswap server enablement logic and improve related tests

411ed04

fix: rename BitswapConfig to Bitswap and update references

7c6b791

gystemd mentioned this pull request Apr 17, 2025

feat(bitswap): add option to disable Bitswap server ipfs/boxo#911

Merged

gammazero added 2 commits April 29, 2025 09:01

Update boxo PR

fd65781

update boxo pr

6de6673

gammazero mentioned this pull request Apr 29, 2025

feat(config): ability to disable Bitswap fully or just server #10793

Closed

gammazero and others added 3 commits April 29, 2025 20:10

update boxo

60bf94a

Fix metrics names

e812e3e

Merge branch 'master' into feat/bitswap/disable

188c1b9

Giulio Piva and others added 3 commits April 30, 2025 15:45

chore: formatting

258fb25

refactor: use consts

e7f501e

docs: config and changelog

9de888b

lidel reviewed Apr 30, 2025

View reviewed changes

lidel added 2 commits April 30, 2025 19:07

fix: ipfs cat panic when Bitswap.Enabled=false

b11a65a

Fixes panic described in: ipfs#10782 (comment)

chore: fix go vet

0473134

lidel force-pushed the feat/bitswap/disable branch from 7b862f5 to 0473134 Compare April 30, 2025 19:58

lidel approved these changes Apr 30, 2025

View reviewed changes

chore: remove redundant entry

f71926f

we have those in section above

lidel merged commit e8ff2d5 into ipfs:master Apr 30, 2025
12 checks passed

BrewTestBot mentioned this pull request May 21, 2025

ipfs 0.35.0 Homebrew/homebrew-core#224309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(config): ability to disable Bitswap fully or just server #10782

feat(config): ability to disable Bitswap fully or just server #10782

Uh oh!

gystemd commented Apr 10, 2025 •

edited by gammazero

Loading

Uh oh!

lidel left a comment

Uh oh!

Uh oh!

Uh oh!

lidel Apr 11, 2025

Uh oh!

lidel Apr 30, 2025

Uh oh!

lidel Apr 30, 2025

Uh oh!

lidel left a comment

Uh oh!

Uh oh!

Uh oh!

		func (e *noopExchange) GetBlock(ctx context.Context, c cid.Cid) (blocks.Block, error) {
		return nil, nil

Uh oh!

feat(config): ability to disable Bitswap fully or just server #10782

feat(config): ability to disable Bitswap fully or just server #10782

Uh oh!

Conversation

gystemd commented Apr 10, 2025 • edited by gammazero Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitswap Configuration Enhancements:

Bitswap Server/Client Behavior:

Testing:

Uh oh!

lidel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lidel Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

lidel Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

lidel Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

lidel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gystemd commented Apr 10, 2025 •

edited by gammazero

Loading