Skip to content

Add electrum feature flag #558

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

Closed

Conversation

aylagreystone
Copy link
Contributor

@aylagreystone aylagreystone commented Jun 4, 2025

Electrum brings a dependency on aws-lc-sys which is causing lots of issues cross-compiling a project using ldk-node for many target platforms.

This introduces an electrum feature flag and makes it a default feature as to not change the current default behavior but allows an ldk-node user to use default-features = false if they are not using electrum and do not wish to take the extra unnecessary dependencies.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jun 4, 2025

👋 I see @wpaulino was un-assigned.
If you'd like another reviewer assignemnt, please click here.

@ldk-reviews-bot ldk-reviews-bot requested a review from wpaulino June 4, 2025 21:47
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far we omitted introducing features to allow dis/enabling via the builder. We will however introduce features going forward (likely with 0.7), but they need to be carefully considered (also due to the interplay with the uniffi feature) so we will not do a one-off just for electrum.

Electrum brings a dependency on aws-lc-sys which is causing lots of issues cross-compiling a project using ldk-node for many target platforms.

It's actually not 'lots of issues', AFAIK it's really just Swift bindings generation that don't work seamlessly with aws-lc-rs (please correct me if you hit other instances), which is otherwise clearly preferable to its ring counterpart. We already added the capability to select the rustls backend for lightning-transaction-sync with lightningdevkit/rust-lightning#3587, so this will be fixed going forward with the LDK 0.2 upgrade, but until then I'd recommend just running on a backport of that PR and switching the the backend to rustls-ring for any Swift builds (which is what we did for 0.5).

Going ahead and closing this for now, but let me know if you need further assistance resolving this on your side.

@tnull tnull closed this Jun 5, 2025
@tnull tnull removed the request for review from wpaulino June 5, 2025 08:11
@aylagreystone
Copy link
Contributor Author

Makes sense.

FWIW it wasn't related to Swift generation in my case. I'm using napi-rs to expose ldk-node functionality in javascript and NAPI builds native modules for a whole host of platform targets. Lots of them were failing because of aws-lc-rs: https://github.com/moneydevkit/lightning-js/actions/runs/15451440363/job/43494266800

With this electrum-feature branch they all passed fine: https://github.com/moneydevkit/lightning-js/actions/runs/15454322485

I suspect I can probably fix most of them by adjusting the base image / build environment github actions is using to be able to compile / cross-compile aws-lc-rs but this seemed easier and I thought maybe ldk-node would be okay with minimizing dependencies for end-users.

I'll just build off the fork for now and revisit the build issues later.

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

Successfully merging this pull request may close these issues.

3 participants