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

@chainsafe/blst module not found when importing @lodestar/light-client #6589

Open
acolytec3 opened this issue Mar 25, 2024 · 6 comments
Open
Labels
meta-bug Issues that identify a bug and require a fix.

Comments

@acolytec3
Copy link

Describe the bug

I'm working through updating Ultralight to use the latest versions of various chainsafe/lodestar libraries and encountered an issue where @chainsafe/blst isn't found. It appears to be an indirect import of @lodestar/light-client via @chainsafe/bls which Ultralight also does not currently use other than as a transitive dependency.

As noted here @chainsafe/blst is an optional/peer dep for @chainsafe/bls but not for @lodestar/light-client.

Expected behavior

Ideally, @lodestar/light-client should import the correct optional dependencies for @chainsafe/bls so downstream consumers don't need to specify this in package.json

Steps to reproduce

No response

Additional context

Here is an example from our CI where this issue occurs.

Operating system

Linux

Lodestar version or commit hash

"@lodestar/light-client": "^1.17.0"

@acolytec3 acolytec3 added the meta-bug Issues that identify a bug and require a fix. label Mar 25, 2024
@acolytec3
Copy link
Author

Note, for now, we've just added @chainsafe/blst as a direct dependency for ultralight so we can proceed but will update if/when this gets resolved on the lodestar side.

@g11tech
Copy link
Contributor

g11tech commented Mar 25, 2024

but i think lightclient will require bls to verify sync committee signature as most probably signed updates signatures

@jeluard
Copy link
Member

jeluard commented Mar 25, 2024

Lightclient uses bls, which itself optionally relies on blst. The issue here is related to blst.

@matthewkeil
Copy link
Member

matthewkeil commented Mar 25, 2024

@chainsafe/bls is designed isomorphicly and to work with either @chainsafe/blst or bls-eth-wasm depending on if its running client or server side. Looks like the blst side is setup as a peer dependency correctly but bls-eth-wasm is a required dep. Feels like those should be symmetrical if we can make that happen. And because light-client is meant to be an isomorphic library too, im thinking that it should probably match that same pattern. We could add a note to the docs to make it clear that the consumer of light-client (or bls depending on use case) can choose which one to include. One or the other is required but both do not need to get imported (for performance reasons on the server and for bundle size reasons on the client side).

I haven't attempted this yet but seem like it should be possible with the current setup. Or perhaps we can massage the imports in @chainsafe/bls in the case that there is an error thrown if we move bls-eth-wasm to a peer dependency and @chainsafe/blst is available to require but bls-eth-wasm is not.

@matthewkeil
Copy link
Member

Note, for now, we've just added @chainsafe/blst as a direct dependency for ultralight so we can proceed but will update if/when this gets resolved on the lodestar side.

@acolytec3 that will definitely get you going for now and is possibly the right idea long term depending on how everyone feels about the comment above.

@acolytec3
Copy link
Author

@chainsafe/bls is designed isomorphicly and to work with either @chainsafe/blst or bls-eth-wasm depending on if its running client or server side. Looks like the blst side is setup as a peer dependency correctly but bls-eth-wasm is a required dep. Feels like those should be symmetrical if we can make that happen. And because light-client is meant to be an isomorphic library too, im thinking that it should probably match that same pattern. We could add a note to the docs to make it clear that the consumer of light-client (or bls depending on use case) can choose which one to include. One or the other is required but both do not need to get imported (for performance reasons on the server and for bundle size reasons on the client side).

Good to know. Will just proceed with @chainsafe/blst for today and revisit if/when we end up running Ultralight in browser again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta-bug Issues that identify a bug and require a fix.
Projects
None yet
Development

No branches or pull requests

4 participants