Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling libp2p doesn't work #344

Closed
2color opened this issue Dec 11, 2023 · 8 comments · Fixed by #382 or #386
Closed

Disabling libp2p doesn't work #344

2color opened this issue Dec 11, 2023 · 8 comments · Fixed by #382 or #386

Comments

@2color
Copy link
Member

2color commented Dec 11, 2023

Background

in #289, there's an example showing how to start helia with Libp2p disabled:

const helia = await createHelia({
  blockBrokers: [
    trustlessGateway()
  ],
  libp2p: {
    start: false
  }
})

The type for libp2p is defined here: https://github.com/libp2p/js-libp2p/blob/d994311cc9b59cdcbd5968bb7799b7fbe14a8961/packages/libp2p/src/index.ts#L125

However, when starting Helia like that, Helia still attempts to open quite a lot of libp2p connections to the bootstrappers and other nodes.

What should be the recommended configuration for this?

Reproduction

See the example here: https://stackblitz.com/edit/github-cadjaa?file=src%2Fprovider%2FHeliaProvider.jsx

@SgtPooki
Copy link
Member

I will provide a minimal config here that shows how to do this. This is done in helia-http-gateway currently, but its got a lot of other code around it.

@SgtPooki
Copy link
Member

The dials libp2p is doing by default can be disabled by using:

const helia = await createHelia({
  blockBrokers: [
    trustlessGateway()
  ],
  libp2p: {
    start: false,
    connectionManager: {
      minConnections: 0,
    },
  }
})

but it still "does stuff". You can see the console for libp2p debug logs that I enabled with localStorage.setItem('debug', 'libp2p*') in App.jsx

You can see that libp2p is starting, even though we set start to false. I think this is currently a bug with Helia/libp2p at the line

await this.libp2p.start()
. We should handle if users want to start libp2p, and if they pass libp2p: {start:false} we should prevent starting libp2p in Helia start, and require the user to manage it.

You can also do something like this:

        const helia = await createHelia({
          blockBrokers: [
            trustlessGateway({
              gateways: ['https://dweb.link', 'https://cf-ipfs.com'],
            }),
          ],
          libp2p: {
            start: false,
            connectionManager: {
              minConnections: 0,
            },
            services: {},
            peerDiscovery: [],
          },
        });
        await helia.libp2p.stop()

to silence libp2p further. I think any further work for this should be handled by #289

@SgtPooki
Copy link
Member

We may be able to provide some noop transport in the interim. You can't pass transports: [] because libp2p will yell that at least one is required.

@SgtPooki SgtPooki mentioned this issue Dec 14, 2023
3 tasks
@2color
Copy link
Member Author

2color commented Jan 10, 2024

Following the discussion in #372, what do you think we should do here @achingbrain?

As it stands, libp2p is a hard depenednecy, but exposing the start option of libp2p here is confusing, because Helia will start libp2p anyways in the helia.start method, which is called by createHelia by default:

await this.libp2p.start()
.

@achingbrain
Copy link
Member

The problem here is that we have two places start lives:

const node = await createHelia({
  start: false,
  libp2p: {
    start: false
  }
})

The first is from Helia's config, the second from libp2p's.

I guess we should probably use Omit to exclude the start parameter from the libp2p type, then Helia's start parameter can be used to control starting it.

I don't think we should support Helia being started but libp2p being stopped as valid configuration, I feel that way lies chaos.

I'm guessing this has come about from the desire to have a version of Helia that only does HTTP things - that will be enabled by #372 and libp2p will be excluded entirely from @helia/http so that may be a better fit for this than trying to control the lifecycle of individual components?

@2color
Copy link
Member Author

2color commented Jan 16, 2024

I'm guessing this has come about from the desire to have a version of Helia that only does HTTP things - that will be enabled by #372 and libp2p will be excluded entirely from @helia/http so that may be a better fit for this than trying to control the lifecycle of individual components?

Exactly. I think we can close this, once #372 is merged.

@2color
Copy link
Member Author

2color commented Jan 16, 2024

I guess we should probably use Omit to exclude the start parameter from the libp2p type, then Helia's start parameter can be used to control starting it.

Yeah, that seems sensible!

I don't think we should support Helia being started but libp2p being stopped as valid configuration, I feel that way lies chaos.

💯 . The only use-case would be @helia/http which'll abstract this.

@achingbrain
Copy link
Member

I think it's still worth Omiting the start param from the libp2p config so there's a canonical start option, but it can wait until after #372.

achingbrain added a commit that referenced this issue Jan 16, 2024
Omits the `start` key from the accepted libp2p init params, instead
use only the `start` key in the main Helia init object.

Fixes #344
achingbrain added a commit that referenced this issue Jan 17, 2024
Adds code from https://github.com/meandavejustice/helia-http to the monorepo.

Fixes: #289 and #344

BREAKING CHANGE: the `libp2p` property has been removed from the `Helia` interface in `@helia/interface` - it is still present on the return type of `createHelia` from the `helia` module

---------

Co-authored-by: David Justice <[email protected]>
Co-authored-by: Daniel Norman <[email protected]>
Co-authored-by: Daniel N <[email protected]>
achingbrain added a commit that referenced this issue Jan 17, 2024
Omits the `start` key from the accepted libp2p init params, instead
use only the `start` key in the main Helia init object.

Fixes #344
achingbrain added a commit that referenced this issue Jan 17, 2024
Omits the `start` key from the accepted libp2p init params, instead
use only the `start` key in the main Helia init object.

Fixes #344
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants