Skip to content

Update fee rate cache on startup and make sync interval configurable - #102

Merged
tnull merged 7 commits into
lightningdevkit:mainfrom
tnull:2023-05-fee-fix
Jun 5, 2023
Merged

Update fee rate cache on startup and make sync interval configurable#102
tnull merged 7 commits into
lightningdevkit:mainfrom
tnull:2023-05-fee-fix

Conversation

@tnull

@tnulltnull commented May 19, 2023

Copy link
Copy Markdown
Collaborator

Based on #93.
Based on #108.

So far we'd immediately start background syncing upon start() which would work fine, except when these initial syncs failed, in which case we'd use our fallback fee. This might lead to being disconnected from peers as our fallback values are likely out of current fee ranges.

To mitigate this we here simply enforce that an initial update of our fee rate cache in start() is successful and hence we'd at least once got a recent fee estimation update.

Moreover, we make the background syncing intervals configurable, however enforcing a floor of 10 seconds.

@tnulltnull added this to the 0.1 milestone May 19, 2023
@tnulltnull mentioned this pull request May 19, 2023
47 tasks
@tnull
tnullforce-pushed the 2023-05-fee-fix branch 4 times, most recently from 65bb071 to af9028cCompareMay 23, 2023 11:22
@tnull

Copy link
Copy Markdown
CollaboratorAuthor

Rebased on #108.

@tnull
tnullforce-pushed the 2023-05-fee-fix branch 11 times, most recently from 984d936 to 6b1f2ffCompareMay 25, 2023 09:57
@tnull

tnull commented May 25, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Rebased on #108 after #101 landed.

@tnull
tnullforce-pushed the 2023-05-fee-fix branch 2 times, most recently from db655e8 to 5053096CompareMay 27, 2023 06:57
@tnull

Copy link
Copy Markdown
CollaboratorAuthor

Rebased on #108 after #88 landed.

@tnull
tnullforce-pushed the 2023-05-fee-fix branch 5 times, most recently from c4f3fe4 to 575c9f7CompareMay 31, 2023 18:03
@tnulltnull changed the title Enforce successful initial wallet sync and make sync interval configurableEnforce successful initial fee rate cache update and make sync interval configurableMay 31, 2023
@tnulltnull changed the title Enforce successful initial fee rate cache update and make sync interval configurableUpdate fee rate cache on startup and make sync interval configurableMay 31, 2023
@tnull
tnullforce-pushed the 2023-05-fee-fix branch 2 times, most recently from 0393a8f to d218da2CompareJune 1, 2023 09:53
@jkczyz
jkczyz self-requested a review June 1, 2023 13:10
@tnull

tnull commented Jun 1, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Rebased on main.

Comment threadsrc/types.rs
Comment threadsrc/lib.rs
Comment threadsrc/lib.rs Outdated
Comment threadsrc/lib.rs
Comment threadsrc/lib.rs Outdated
Comment threadsrc/lib.rs
We improve the docs, in particular mentioning now that users should
always prefer background syncing over blocking sync if possible.
Moreover, as we already have a current thread runtime setup for the BDK
wallet, we now also use it to sync the LDK wallet. This is a good idea
as we want to avoid calling `block_on` on our main runtime where
possible, which is always in danger of (temporarily or permanently)
blocking the world if something goes wrong.

@jkczyzjkczyz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Please squash.

tnull added 2 commits June 3, 2023 01:21
So far we'd immediately start background syncing wallets and the fee
cache upon `start()` which
would work fine, except when these initial syncs failed, in which case
we'd use our fallback fee. This might lead to being disconnected
from peers as our fallback values are likely out of current fee ranges.
To mitigate this we here simply enforce that an initial fee rate update in
`start()` is successful.
Additionally, we make the fee rate cache update
interval configurable.
@tnull

tnull commented Jun 2, 2023

Copy link
Copy Markdown
CollaboratorAuthor

LGTM. Please squash.

Squashed without further changes.

@tnull
tnull merged commit e95f04e into lightningdevkit:mainJun 5, 2023
joostjager pushed a commit to joostjager/ldk-node that referenced this pull request Jan 28, 2026
Sign up for freeto 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.

2 participants

@tnull@jkczyz