From e5e7cf180f3dbb978245afa4822547294298f733 Mon Sep 17 00:00:00 2001 From: gammazero <11790789+gammazero@users.noreply.github.com> Date: Tue, 29 Apr 2025 10:11:04 -1000 Subject: [PATCH] eat(config): ability to disable Bitswap fully or just 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 https://github.com/ipfs/boxo/pull/912 --- config/bitswap.go | 10 +++ config/config.go | 2 + core/node/bitswap.go | 52 ++++++++++-- core/node/groups.go | 7 +- docs/changelogs/v0.35.md | 3 + test/cli/bitswap_client_test.go | 136 ++++++++++++++++++++++++++++++++ 6 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 config/bitswap.go create mode 100644 test/cli/bitswap_client_test.go diff --git a/config/bitswap.go b/config/bitswap.go new file mode 100644 index 00000000000..0b80b696c84 --- /dev/null +++ b/config/bitswap.go @@ -0,0 +1,10 @@ +package config + +// Bitswap holds Bitswap configuration options. +type Bitswap struct { + // Enabled controls both client and server (enabled by default). + Enabled Flag `json:",omitempty"` + // ServerEnabled controls if the node responds to WANTs (depends on + // Enabled, enabled by default). + ServerEnabled Flag `json:",omitempty"` +} diff --git a/config/config.go b/config/config.go index 3db7573d06c..68e57e1c3dd 100644 --- a/config/config.go +++ b/config/config.go @@ -42,6 +42,8 @@ type Config struct { Version Version Internal Internal // experimental/unstable options + + Bitswap Bitswap `json:",omitempty"` } const ( diff --git a/core/node/bitswap.go b/core/node/bitswap.go index 2408fe3715e..0888a221143 100644 --- a/core/node/bitswap.go +++ b/core/node/bitswap.go @@ -2,6 +2,7 @@ package node import ( "context" + "io" "time" "github.com/ipfs/boxo/bitswap" @@ -12,13 +13,15 @@ import ( "github.com/ipfs/boxo/exchange/providing" provider "github.com/ipfs/boxo/provider" rpqm "github.com/ipfs/boxo/routing/providerquerymanager" + blocks "github.com/ipfs/go-block-format" + "github.com/ipfs/go-cid" + "github.com/ipfs/go-datastore" "github.com/ipfs/kubo/config" + "github.com/ipfs/kubo/core/node/helpers" irouting "github.com/ipfs/kubo/routing" "github.com/libp2p/go-libp2p/core/host" "github.com/libp2p/go-libp2p/core/routing" "go.uber.org/fx" - - "github.com/ipfs/kubo/core/node/helpers" ) // Docs: https://github.com/ipfs/kubo/blob/master/docs/config.md#internalbitswap @@ -72,13 +75,15 @@ type bitswapIn struct { } // Bitswap creates the BitSwap server/client instance. -// Additional options to bitswap.New can be provided via the "bitswap-options" -// group. +// If Bitswap.ServerEnabled is false, the node will act only as a client +// using an empty blockstore to prevent serving blocks to other peers. func Bitswap(provide bool) interface{} { return func(in bitswapIn, lc fx.Lifecycle) (*bitswap.Bitswap, error) { bitswapNetwork := bsnet.NewFromIpfsHost(in.Host) + var blockstoree blockstore.Blockstore = in.Bs var provider routing.ContentDiscovery + if provide { var maxProviders int = DefaultMaxProviders if in.Cfg.Internal.Bitswap != nil { @@ -93,10 +98,16 @@ func Bitswap(provide bool) interface{} { return nil, err } in.BitswapOpts = append(in.BitswapOpts, bitswap.WithClientOption(client.WithDefaultProviderQueryManager(false))) + in.BitswapOpts = append(in.BitswapOpts, bitswap.WithServerEnabled(true)) provider = pqm - + } else { + provider = nil + // When server is disabled, use an empty blockstore to prevent serving blocks + blockstoree = blockstore.NewBlockstore(datastore.NewMapDatastore()) + in.BitswapOpts = append(in.BitswapOpts, bitswap.WithServerEnabled(false)) } - bs := bitswap.New(helpers.LifecycleCtx(in.Mctx, lc), bitswapNetwork, provider, in.Bs, in.BitswapOpts...) + + bs := bitswap.New(helpers.LifecycleCtx(in.Mctx, lc), bitswapNetwork, provider, blockstoree, in.BitswapOpts...) lc.Append(fx.Hook{ OnStop: func(ctx context.Context) error { @@ -107,9 +118,12 @@ func Bitswap(provide bool) interface{} { } } -// OnlineExchange creates new LibP2P backed block exchange. -func OnlineExchange() interface{} { +// Returns a no-op exchange if Bitswap is disabled. +func OnlineExchange(isBitswapActive bool) interface{} { return func(in *bitswap.Bitswap, lc fx.Lifecycle) exchange.Interface { + if !isBitswapActive { + return &noopExchange{closer: in} + } lc.Append(fx.Hook{ OnStop: func(ctx context.Context) error { return in.Close() @@ -144,3 +158,25 @@ func ProvidingExchange(provide bool) interface{} { return exch } } + +type noopExchange struct { + closer io.Closer +} + +func (e *noopExchange) GetBlock(ctx context.Context, c cid.Cid) (blocks.Block, error) { + return nil, nil +} + +func (e *noopExchange) GetBlocks(ctx context.Context, cids []cid.Cid) (<-chan blocks.Block, error) { + ch := make(chan blocks.Block) + close(ch) + return ch, nil +} + +func (e *noopExchange) NotifyNewBlocks(ctx context.Context, blocks ...blocks.Block) error { + return nil +} + +func (e *noopExchange) Close() error { + return e.closer.Close() +} diff --git a/core/node/groups.go b/core/node/groups.go index 0e28444be95..8bc8bfeeece 100644 --- a/core/node/groups.go +++ b/core/node/groups.go @@ -335,13 +335,14 @@ func Online(bcfg *BuildCfg, cfg *config.Config, userResourceOverrides rcmgr.Part recordLifetime = d } - /* don't provide from bitswap when the strategic provider service is active */ - shouldBitswapProvide := !cfg.Experimental.StrategicProviding + isBitswapEnabled := cfg.Bitswap.Enabled.WithDefault(true) && cfg.Bitswap.ServerEnabled.WithDefault(true) + // Don't provide from bitswap when the strategic provider service is active + shouldBitswapProvide := isBitswapEnabled && !cfg.Experimental.StrategicProviding return fx.Options( fx.Provide(BitswapOptions(cfg)), fx.Provide(Bitswap(shouldBitswapProvide)), - fx.Provide(OnlineExchange()), + fx.Provide(OnlineExchange(cfg.Bitswap.Enabled.WithDefault(true))), // Replace our Exchange with a Providing exchange! fx.Decorate(ProvidingExchange(shouldBitswapProvide)), fx.Provide(DNSResolver), diff --git a/docs/changelogs/v0.35.md b/docs/changelogs/v0.35.md index e20d4759cb0..b1d39fa7224 100644 --- a/docs/changelogs/v0.35.md +++ b/docs/changelogs/v0.35.md @@ -120,4 +120,7 @@ but for reprovides only. ### ๐Ÿ“ Changelog +- Completely disable Bitswap functionality through the `Bitswap.Enabled` flag +- Control server behavior separately with `Bitswap.ServerEnabled`, allowing nodes to operate in client-only mode + ### ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors diff --git a/test/cli/bitswap_client_test.go b/test/cli/bitswap_client_test.go new file mode 100644 index 00000000000..541cd70bcb7 --- /dev/null +++ b/test/cli/bitswap_client_test.go @@ -0,0 +1,136 @@ +package cli + +import ( + "testing" + "time" + + "github.com/ipfs/kubo/config" + "github.com/ipfs/kubo/test/cli/harness" + "github.com/ipfs/kubo/test/cli/testutils" + "github.com/stretchr/testify/assert" +) + +func TestBitswapConfig(t *testing.T) { + t.Parallel() + + // Create test data that will be shared between nodes + testData := testutils.RandomBytes(100) + + t.Run("server enabled (default)", func(t *testing.T) { + t.Parallel() + h := harness.NewT(t) + provider := h.NewNode().Init().StartDaemon() + requester := h.NewNode().Init().StartDaemon() + + hash := provider.IPFSAddStr(string(testData)) + requester.Connect(provider) + + res := requester.IPFS("cat", hash) + assert.Equal(t, testData, res.Stdout.Bytes(), "retrieved data should match original") + }) + + t.Run("server disabled", func(t *testing.T) { + t.Parallel() + h := harness.NewT(t) + + provider := h.NewNode().Init() + provider.SetIPFSConfig("Bitswap.ServerEnabled", false) + provider = provider.StartDaemon() + + requester := h.NewNode().Init().StartDaemon() + + hash := provider.IPFSAddStr(string(testData)) + requester.Connect(provider) + + // If the data was available, it would be retrieved immediately. + // Therefore, after the timeout, we can assume the data is not available + // i.e. the server is disabled + timeout := time.After(3 * time.Second) + dataChan := make(chan []byte) + + go func() { + res := requester.RunIPFS("cat", hash) + dataChan <- res.Stdout.Bytes() + }() + + select { + case data := <-dataChan: + assert.NotEqual(t, testData, data, "retrieved data should not match original") + case <-timeout: + t.Log("Test passed: operation timed out after 3 seconds as expected") + } + }) + + t.Run("server disabled and client enabled", func(t *testing.T) { + t.Parallel() + h := harness.NewT(t) + + provider := h.NewNode().Init() + provider.SetIPFSConfig("Bitswap.ServerEnabled", false) + provider.StartDaemon() + + requester := h.NewNode().Init().StartDaemon() + hash := requester.IPFSAddStr(string(testData)) + + provider.Connect(requester) + + // Even when the server is disabled, the client should be able to retrieve data + res := provider.RunIPFS("cat", hash) + assert.Equal(t, testData, res.Stdout.Bytes(), "retrieved data should match original") + }) + + t.Run("bitswap completely disabled", func(t *testing.T) { + t.Parallel() + h := harness.NewT(t) + + requester := h.NewNode().Init() + requester.UpdateConfig(func(cfg *config.Config) { + cfg.Bitswap.Enabled = config.False + cfg.Bitswap.ServerEnabled = config.False + }) + requester.StartDaemon() + + provider := h.NewNode().Init().StartDaemon() + hash := provider.IPFSAddStr(string(testData)) + + requester.Connect(provider) + res := requester.RunIPFS("cat", hash) + assert.Equal(t, []byte{}, res.Stdout.Bytes(), "cat should not return any data") + + // Verify that basic operations still work with bitswap disabled + res = requester.IPFS("id") + assert.Equal(t, 0, res.ExitCode(), "basic IPFS operations should work") + res = requester.IPFS("bitswap", "stat") + assert.Equal(t, 0, res.ExitCode(), "bitswap stat should work even with bitswap disabled") + res = requester.IPFS("bitswap", "wantlist") + assert.Equal(t, 0, res.ExitCode(), "bitswap wantlist should work even with bitswap disabled") + + // Verify local operations still work + hashNew := requester.IPFSAddStr("random") + res = requester.IPFS("cat", hashNew) + assert.Equal(t, []byte("random"), res.Stdout.Bytes(), "cat should return the added data") + }) + + // t.Run("bitswap protocols disabled", func(t *testing.T) { + // t.Parallel() + // harness.EnableDebugLogging() + // h := harness.NewT(t) + + // provider := h.NewNode().Init() + // provider.SetIPFSConfig("Bitswap.ServerEnabled", false) + // provider = provider.StartDaemon() + // requester := h.NewNode().Init().StartDaemon() + // requester.Connect(provider) + // // Parse and check ID output + // res := provider.IPFS("id", "-f", "") + // protocols := strings.Split(strings.TrimSpace(res.Stdout.String()), "\n") + + // // No bitswap protocols should be present + // for _, proto := range protocols { + // assert.NotContains(t, proto, bsnet.ProtocolBitswap, "bitswap protocol %s should not be advertised when server is disabled", proto) + // assert.NotContains(t, proto, bsnet.ProtocolBitswapNoVers, "bitswap protocol %s should not be advertised when server is disabled", proto) + // assert.NotContains(t, proto, bsnet.ProtocolBitswapOneOne, "bitswap protocol %s should not be advertised when server is disabled", proto) + // assert.NotContains(t, proto, bsnet.ProtocolBitswapOneZero, "bitswap protocol %s should not be advertised when server is disabled", proto) + // } + // }) +}