0.2 backports #4221
Open
0.2 backports #4221
+1,190
−244
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In c084767 we a `ChainMonitor::new_async_beta` and a corresponding `MonitorUpdatingPersisterAsync` struct. To avoid circular references, the `ChainMonitor` owns the `MonitorUpdatingPersisterAsync` which uses a `FutureSpawner` to let async writes happen in the background after it returns control flow ownership to the `ChainMonitor`. This is great, except that because `MonitorUpdatingPersisterAsync` thus doesn't have a reference to the `ChainMonitor`, we have to poll for completion of the futures. We do so in the new `Persist::get_and_clear_completed_updates` that was added in the above-referenced commit. But on async monitor write completion we're supposed to call `ChainMonitor`'s `event_notifier.notify()` (ultimately waking up the background processor which `await`s the corresponding update future). This didn't happen in the new async flow. Here we move `ChainMonitor`'s `event_notifier` into an `Arc` and pass a reference to it through to the `MonitorUpdatingPersisterAsync` which can then directly `notify()` it and wake the background processor. Backport of 092a50a
Previously, we would also prune any pending `GetInfo` or expired `Buy` requests before we `persist`. This could have lead to races where we drop a pending request and even remove the peer when calling `persist`. Here, we simply split the pruning logic for the `pending_requests` and expired JIT channel state, and only prune the latter before persisting. This generally makes sense, as `pending_requests` isn't currently persisted, so there is no need to prune before repersisting. Both are however still pruned on peer disconnection. Backport of 5f8c54a
The details of these errors aren't particularly relevant, and its not worth the effort of exporting them, so we just mark them no-export here. Backport of 16aabce
As move semantics do not map to bindings, the BOLT 12 object builders are marked no-export. In the new `OffersMesageFlow`, here, we also mark the methods which return these builders as no-export. Backport of 7fb0f2a Trivial conflicts resolved in: * lightning/src/offers/flow.rs
Methods like `StaticInvoice::sign` have their own signing-function traits, so there's no actual need to use `SignFn` directly, its just a useful generic wrapper. Sadly, because it uses `AsRef` bounds, which aren't really practical to map in bindings, we can't really expose it directly in bindings. Because there's an alternative and its tricky to expose, we simply mark it no-export here. Backport of 2bf737c
We do not currently support async traits or methods in our home-grown bindings logic, so here mark them no-export. Backport of 3f82fd6
Rather than importing `ChannelTransactionParameters` via the `use` in `lightning::sign`, import it via its real path in `lightning::sign::ecdsa`. This makes reading the code (incredibly marginally) simpler, but also makes the bindings generator happy. Backport of b07111e
In 0.2 we added new `LengthLimitedRead` and `LengthReadable` traits, but forgot to copy the no-export tags from the existing `Read` and `Readable` traits. Here we add the missing tags. Backport of 8eabcc1
This avoids confusing the bindings generator. Backport of 126ac50
When we added the async traits and wrapper structs for the transaction-bumping logic, we didn't bother copying the documentation to the sync wrappers as we figured the links sufficed. Sadly, however, this means that our bindings logic will have no docs but a broken link to an async object that doesn't exist. Instead, here, we copy the docs from the async objects to the sync ones, at least leaving behind a comment noting that both need updating whenever one gets updated. We also fix one bogus link in `Wallet`'s docs. Backport of c311447
When we added the async trait for `KVStore`, we didn't bother copying the documentation to the sync wrappers as we figured the links sufficed. Sadly, however, this means that our bindings logic will have no docs but a broken link to an async object that doesn't exist. Instead, here, we copy the docs from the async objects to the sync ones, at least leaving behind a comment noting that both need updating whenever one gets updated. Backport of 9516400
When we added the async wrapper for `OutputSweeper`, we didn't bother copying the documentation to the sync wrappers as we figured the links sufficed. Sadly, however, this means that our bindings logic will have no docs but a broken link to an async object that doesn't exist. Instead, here, we copy the docs from the async objects to the sync ones, at least leaving behind a comment noting that both need updating whenever one gets updated. Backport of 4bc993c
When estimating signature weight, 73 WU was used in some places while 72 WU was used in others. Consistently use 72 WU and replace hardcoded values with constants. 73 WU signatures are non-standard and won't be produced by LDK. Additionally, using 73 WU along with grind_signatures adjustment is nonsensical. Backport of eb341de
When estimating the splice funding transaction fees, adjust for grind_signatures. Since LDK supplies one of the signatures, only adjust by 1 WU even though spending the shared input requires two signatures. Backport of 4698026
The interactive-tx construction protocol uses an agreed upon fee rate. Since the bitcoind coin selection algorithm may underpay fees when no change output is needed, providing a tolerance when checking if the remote's fee contribution could avoid some unexpected failures. This commit introduces a 95% tolerance similar to Eclair. Backport of 3aa4574
The interactive-tx construction protocol needs to make sure the constructed transaction does not exceed MAX_STANDARD_TX_WEIGHT. A naive estimate of the transaction weight after signing was used, but was not accurate. Specifically, it double-counted EMPTY_SCRIPT_SIG_WEIGHT and didn't include SEGWIT_MARKER_FLAG_WEIGHT. Backport of b4803c7
Useful for re-using code producing a FundingTxInput set to implement CoinSelectionSource. Backport of 2330ed9
Upon processing the counterparty's initial `commitment_signed` for a splice, we queue a monitor update with the new commitment transactions spending the new funding transaction. Once handled, funding transaction signatures can be exchanged and we must start monitoring the chain for a possible commitment transaction broadcast spending the new funding transaction. Aborting the splice negotiation then would therefore be unsafe, hence why we currently force close in such cases. However, there is no reason to force close prior to receiving their initial `commitment_signed`, as this would imply the funding transaction signatures have yet to be exchanged, thus making a commitment broadcast spending said transaction impossible allowing us to abort the splice negotiation safely. Backport of d282a47
When we propose quiescence due to a user initiated action, such as a splice, it's possible that the `stfu` message is not ready to be sent out yet due to the state machine pending a change. This would result in an error communicated back to the caller of `ChannelManager::splice_channel`, but this is unnecessary and confusing, the splice is still pending and will be carried out once quiescence is eventually negotiated. Backport of c2b15c2
We incorrectly assumed that both commitments must be at the same height, but this is certainly possible whenever updates are proposed in both directions at the same time. Backport of a6ed667
This method was originally intended to DRY the logic between `FundedChannel` and `PendingChannelV2` when handling `funding_tx_constructed`, though it's not very useful anymore. Backport of 25264d7
|
I've assigned @valentinewallace as a reviewer! |
This was referenced Nov 12, 2025
This was referenced Nov 12, 2025
Splicing should also have fuzz coverage, and it depends on the quiescence protocol. Backport of 1e78628
TheBlueMatt
force-pushed
the
2025-11-0.2-backports-release
branch
from
cd0ef9d to
063947f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 0.2 #4221 +/- ##
========================================
Coverage 88.86% 88.87%
========================================
Files 180 180
Lines 137802 138000 +198
Branches 137802 138000 +198
========================================
+ Hits 122456 122645 +189
+ Misses 12534 12533 -1
- Partials 2812 2822 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
TheBlueMatt
force-pushed
the
2025-11-0.2-backports-release
branch
from
c91b14c to
03eea88
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hopefully the final set. This backports #4200, #4201, #4203, #4208, #4204, and #4219. Plus an MSRV pin.