-
Notifications
You must be signed in to change notification settings - Fork 412
Do not auto-select the lightning std
feature from tx-sync crate
#2069
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
Do not auto-select the lightning std
feature from tx-sync crate
#2069
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #2069 +/- ##
==========================================
- Coverage 87.19% 87.10% -0.09%
==========================================
Files 100 100
Lines 44560 45009 +449
Branches 44560 45009 +449
==========================================
+ Hits 38853 39206 +353
- Misses 5707 5803 +96
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM mod outstanding comments.
|
||
# Obviously lightning-transaction-sync doesn't support no-std, but it should build | ||
# even if lightning is built with no-std. | ||
lightning-transaction-sync = { path = "../lightning-transaction-sync", optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that currently you'd either need to enable esplora-blocking
or esplora-async
to have the acutal code built.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's okay, I don't actually want the code built, I want to make sure lightning builds :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, just wanted to mention since the comment is technically slightly off. But really doesn't matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean it does build, there's just no code built :p
We have some downstream folks who are using LDK in wasm compiled via the normal rust wasm path. To ensure nothing breaks they want to use `no-std` on the lightning crate, disabling time calls as those panic. However, the HTTP logic in `lightning-transaction-sync` gets automatically stubbed out by the HTTP client crates when targeting wasm via `wasm_bindgen`, so it works fine despite the std restrictions. In order to make both work, `lightning-transaction-sync` can remain `std`, but needs to not automatically enable the `std` flag on the `lightning` crate, ie by setting `default-features = false`. We do so here.
b15dfdc
to
1a18b88
Compare
We have some downstream folks who are using LDK in wasm compiled via the normal rust wasm path. To ensure nothing breaks they want to use
no-std
on the lightning crate, disabling time calls as those panic. However, the HTTP logic inlightning-transaction-sync
gets automatically stubbed out by the HTTP client crates when targeting wasm viawasm_bindgen
, so it works fine despite the std restrictions.In order to make both work,
lightning-transaction-sync
can remainstd
, but needs to not automatically enable thestd
flag on thelightning
crate, ie by settingdefault-features = false
. We do so here.