Skip to content

Move Electrum fee rate estimation into a free function - #1164

Open
randomlogin wants to merge 1 commit into
lightningdevkit:mainfrom
randomlogin:cbf-prep-electrum-fee-fn
Open

randomlogin wants to merge 1 commit into
lightningdevkit:mainfrom
randomlogin:cbf-prep-electrum-fee-fn

Conversation

@randomlogin

Copy link
Copy Markdown
Contributor

Electrum fee estimation is used both by cbf chain and electrum chain sources, thus we make the function a free one.

ElectrumRuntimeClient::get_fee_rate_cache_update only needs the runtime, the Electrum client, the network, the update timeout and a logger. Turn it into the free function
get_electrum_fee_rate_cache_update so the estimation can be run without an ElectrumRuntimeClient.

The client no longer reads its config field after this, so drop the field and the argument threaded through start/new.

No functional change.

Electrum fee estimation is used both by cbf chain and electrum chain
sources, thus we make the function a free one.

`ElectrumRuntimeClient::get_fee_rate_cache_update` only needs the
runtime, the Electrum client, the network, the update timeout and a
logger. Turn it into the free function
`get_electrum_fee_rate_cache_update` so the estimation can be run
without an `ElectrumRuntimeClient`.

The client no longer reads its `config` field after this, so drop the
field and the argument threaded through `start`/`new`.

No functional change.

Co-authored-by: febyeji <yeji.han@sf.snu.ac.kr>
@ldk-reviews-bot

ldk-reviews-bot commented Oct 13, 2026 •

Copy link
Copy Markdown

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@tnull tnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm, two questions:
a) wouldn't Esplora be the preferred chain source for quickly getting just fee rates
b) if we want to use these Electrum methods, shouldn't they also get moved to mod.rs like we did in #1162 ?

@randomlogin

randomlogin commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

a) It is the preferred way, and is present: https://github.com/lightningdevkit/ldk-node/pull/828/changes#diff-57e92d1ef2766422873e6720acf66e43a40699b73b9b0ea98071815e8d931bc5R160.
Esplora didn't require any code changes.
While Electrum is not a preferred one, it is still present for the sake of completeness.

b) Yeah, makes sense, I will update the PR

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