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

Allow switchable bls to detect the right implementation #137

Open
nazarhussain opened this issue Jul 28, 2022 · 2 comments
Open

Allow switchable bls to detect the right implementation #137

nazarhussain opened this issue Jul 28, 2022 · 2 comments

Comments

@nazarhussain
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Found out during the testing that on some environment top-level-await are not properly supported. Which are used to initialize the BLS object if the package is imported directly.

Describe the solution you'd like

The best approach to fit in all cases is to import the BLS from the switchable. And when is appropriate use the init to initialize the object.

import bls, {init} from "@chainsafe/bls/switchable";

That init does accept one parameter and it's redundant to identify and pass the parameter for the right implementation. This logic of detecting is already part of the package so we can reuse here. We can provide third option here auto which could be the default value as well. So user don't need to write custom logic and switchable will detect the right implementation.

Describe alternatives you've considered
The alternative safe approach is to use the await import. But that can't be done with correct user experience as code outside that block will not access to the bls object.

Additional context
ChainSafe/lodestar#2723

@dapplion
Copy link
Contributor

CC: @wemeetagain what do you think? This is the approach we were using before ESM modules

@wemeetagain
Copy link
Member

As far as the title of the issue, definitely agree, the switchable thing could/should have an 'auto' setting that is default.

As far as refactoring lodestar to use the switchable thing, I'm more hesitant, would like to have more info first.

I'd like to know which environments don't support top-level-await. According to https://caniuse.com/?search=top%20level%20await it seems that all modern browsers support it? What exactly are we missing by using top-level-await? This is a feature we would have liked to have in our pocket and possibly use elsewhere. Which packages in the lodestar monorepo need to be changed to support these environments?

We originally used the switchable thing everywhere and there are tradeoffs to this approach. Basically boils down to the pattern (explicit initialization + singleton) being non-standard, unintuitive, and more verbose. That isn't just a code hygiene issue, it manifests in different ways during debugging and in downstream use.

  • Some consumable modules (validator and beacon node) required bls initialization before use - unintuitive for us and for downstream use
  • All tests that use bls directly or indirectly need to initialize bls - added some bloat to the tests and was a common troubleshooting timesink (eg: a new test failed, was bls properly initialized?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants