Skip to content

Conversation

joostjager
Copy link
Contributor

Backoff wasn't actually working and polling would happen without any delay at all.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Sep 19, 2025

👋 Thanks for assigning @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.

@joostjager joostjager requested a review from tnull September 19, 2025 12:43
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.

Thanks! Seems like wait_for_outpoint_spend might benefit from the same fix?

}

pub(crate) fn wait_for_tx<E: ElectrumApi>(electrs: &E, txid: Txid) {
let mut tx_res = electrs.transaction_get(&txid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's worth to keep the pattern to avoid the unnecessary ping if the result is immediately available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. What is the ping for actually?

Copy link
Collaborator

Choose a reason for hiding this comment

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

See https://docs.rs/electrum-client/latest/electrum_client/trait.ElectrumApi.html#tymethod.ping

ping might trigger electrs to process new data before we actually attempt the lookup.

Backoff wasn't actually working and polling would happen without any
delay at all.
@joostjager joostjager requested a review from tnull September 19, 2025 14:11
@joostjager joostjager changed the title Fix wait_for_tx exponential backoff Fix wait_for_tx/outpoint exponential backoff Sep 19, 2025
@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

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.

LGTM

@tnull tnull merged commit 65945f8 into lightningdevkit:develop Sep 22, 2025
10 of 15 checks passed
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