Skip to content

Bump electrum_client dependency and add new TLS-backend features - #3587

Merged
tnull merged 3 commits into
lightningdevkit:mainfrom
tnull:2025-02-bump-electrum-client
May 7, 2025
Merged

Bump electrum_client dependency and add new TLS-backend features #3587
tnull merged 3 commits into
lightningdevkit:mainfrom
tnull:2025-02-bump-electrum-client

Conversation

@tnull

@tnulltnull commented Feb 4, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

We bump our electrum_client dependency to v0.22 which is the newest version and also aligns with recent BDK releases.

Also, recently rustls introduced a new default crypto provider in aws-lc-rc. Since there have been reports on this new provider not building properly on certain platforms electrum-client introduced
additional features allowing the user to configure what backend to use for TLS.

Here, we simply re-expose these features to our users, which also mirrors what we do for Esplora already. Note we avoid changing the behavior of the pre-existing electrum feature to make this a backwards compatible change.

@tnull
tnullforce-pushed the 2025-02-bump-electrum-client branch 2 times, most recently from 84fd91e to d12378dCompareFebruary 4, 2025 10:16
@codecov

codecovBot commented Feb 4, 2025•
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.00%. Comparing base (43de15e) to head (ab0537f).
Report is 75 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #3587 +/- ##
==========================================
+ Coverage 89.20% 91.00% +1.79% 
==========================================
Files 155 156 +1 Lines 119229 135548 +16319 Branches 119229 135548 +16319 ==========================================
+ Hits 106363 123358 +16995 + Misses 10266 9715 -551 + Partials 2600 2475 -125

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tnull
tnullforce-pushed the 2025-02-bump-electrum-client branch 3 times, most recently from 4c96307 to e4ecb76CompareFebruary 4, 2025 10:32
@tnull

tnull commented Feb 4, 2025

Copy link
Copy Markdown
ContributorAuthor

Draft for now as in dependency hell :(

@tnull
tnull marked this pull request as draft February 4, 2025 11:22
@tnull
tnullforce-pushed the 2025-02-bump-electrum-client branch 4 times, most recently from 09e929a to 1d09868CompareMarch 13, 2025 09:44
@tnull
tnullforce-pushed the 2025-02-bump-electrum-client branch 4 times, most recently from 094b538 to db8c7c2CompareMarch 19, 2025 12:09
@tnull

tnull commented Mar 26, 2025

Copy link
Copy Markdown
ContributorAuthor

Blocked on rust-bitcoin/corepc#111

@tnull
tnullforce-pushed the 2025-02-bump-electrum-client branch from f2b80af to 2cc608aCompareMarch 27, 2025 08:49
tnull added 2 commits April 28, 2025 16:39
@tnull
Bump electrum_client dependency to v0.22
af5359f
.. which is the newest version and also aligns with recent BDK releases.
@tnull
Upgrade bitcoind used in CI to v28.1
c257589
@tnull
tnullforce-pushed the 2025-02-bump-electrum-client branch from 2cc608a to c257589CompareApril 28, 2025 14:39
@tnull
tnull marked this pull request as ready for review April 28, 2025 14:57
wpaulino
wpaulino reviewed Apr 28, 2025
View reviewed changes
Comment threadlightning-transaction-sync/Cargo.toml
@TheBlueMatt

TheBlueMatt commented Apr 28, 2025

Copy link
Copy Markdown
Collaborator

Discussed with @tnull and we concluded its probably fine to just do a patch for ldk-node release and we can skip backporting this (which technically breaks compat).

@tnull

tnull commented Apr 29, 2025

Copy link
Copy Markdown
ContributorAuthor

Discussed with @tnull and we concluded its probably fine to just do a patch for ldk-node release and we can skip backporting this (which technically breaks compat).

Right, I can probably do without the backport for now. Would still be great to land this PR though.

@tnulltnull mentioned this pull request May 5, 2025
Merged
valentinewallace
valentinewallace approved these changes May 5, 2025
View reviewed changes
Comment threadlightning-transaction-sync/tests/integration_tests.rs
Comment threadlightning-transaction-sync/Cargo.toml
@tnull
Allow users to the rustls crypto provider used by electrum-client
d84a776
Recently `rustls` introduced a new default crypto provider, `aws-lc-rc`.
While it generally works fine, it doesn't seem to build properly on
`aarch64-apple-ios`. Here, we therefore allow the users to decide which
crypto provider the `electrum-client` client should use for its `rustls`
TLS backend. To this end, we disable default features while keeping the
`proxy` enabled, as it's a prerequisite for the
`electrum_client::Client` to be exposed / usable (also see
bitcoindevkit/rust-electrum-client#91).
@tnull
tnullforce-pushed the 2025-02-bump-electrum-client branch from c641af3 to d84a776CompareMay 7, 2025 12:00
@tnull

tnull commented May 7, 2025

Copy link
Copy Markdown
ContributorAuthor

Going ahead landing this as it has two ACKs, and only the commit message of the last commit was adjusted since.

@tnull
tnull merged commit 78fee88 into lightningdevkit:mainMay 7, 2025
This was referenced Jun 5, 2025
Closed
Closed
@tnulltnull mentioned this pull request Aug 12, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wpaulinowpaulinowpaulino approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tnull@TheBlueMatt@wpaulino@valentinewallace