Skip to content
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

Require best block timestamp within ChannelManager::new #2372

Conversation

wpaulino
Copy link
Contributor

This ensures freshly initialized nodes can proceed to create unexpired invoices without a call to best_block_updated, since an invoice's expiration delta is applied to highest_seen_timestamp.

Fixes #2345.

@TheBlueMatt
Copy link
Collaborator

LGTM modulo the test failure.

@TheBlueMatt TheBlueMatt added this to the 0.0.116 milestone Jun 27, 2023
@wpaulino wpaulino force-pushed the channelmanager-new-highest-seen-timestamp branch 2 times, most recently from aef6ac2 to 554d48f Compare June 27, 2023 18:55
@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2023

Codecov Report

Patch coverage: 85.71% and project coverage change: +0.01 🎉

Comparison is base (15b1c9b) 90.30% compared to head (aef6ac2) 90.32%.

❗ Current head aef6ac2 differs from pull request most recent head 82e0df5. Consider uploading reports for the commit 82e0df5 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2372      +/-   ##
==========================================
+ Coverage   90.30%   90.32%   +0.01%     
==========================================
  Files         106      106              
  Lines       54747    54954     +207     
  Branches    54747    54954     +207     
==========================================
+ Hits        49441    49635     +194     
- Misses       5306     5319      +13     
Impacted Files Coverage Δ
lightning/src/ln/functional_test_utils.rs 87.85% <ø> (+0.21%) ⬆️
lightning/src/ln/channelmanager.rs 86.24% <75.00%> (-0.06%) ⬇️
lightning-background-processor/src/lib.rs 82.61% <100.00%> (+0.02%) ⬆️
lightning-invoice/src/utils.rs 97.48% <100.00%> (+<0.01%) ⬆️

... and 19 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wpaulino wpaulino force-pushed the channelmanager-new-highest-seen-timestamp branch from 554d48f to 2eff97c Compare June 27, 2023 19:17
@valentinewallace
Copy link
Contributor

CI sad. Could this be useful on restart as well, if the node was down for a while?

@wpaulino
Copy link
Contributor Author

That should be covered by syncing to the current chain tip.

@wpaulino wpaulino force-pushed the channelmanager-new-highest-seen-timestamp branch from 2eff97c to 1731b8f Compare June 27, 2023 19:41
TheBlueMatt
TheBlueMatt previously approved these changes Jun 27, 2023
This ensures freshly initialized nodes can proceed to create unexpired
invoices without a call to `best_block_updated`, since an invoice's
expiration delta is applied to `highest_seen_timestamp`.
@wpaulino wpaulino force-pushed the channelmanager-new-highest-seen-timestamp branch from 1731b8f to 82e0df5 Compare June 27, 2023 20:43
@TheBlueMatt TheBlueMatt merged commit bd12067 into lightningdevkit:main Jun 29, 2023
14 checks passed
@wpaulino wpaulino deleted the channelmanager-new-highest-seen-timestamp branch June 29, 2023 04:25
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.

Failing HTLC with payment_hash: expired payment
5 participants