Repository navigation
Backport interactive tx output caps and block parse hardening - #73
Merged
Merged
Conversation
pull_bitcoin_tx_only() returns NULL once the cursor runs out, and the next line writes through it, so a block that ends mid-transaction segfaults lightningd instead of being rejected. The block comes from the chain backend via getrawblockbyheight, so this needs a bitcoind serving a malformed or unparseable block rather than anything a peer can send. Test walks every truncation of the existing test block; it segfaults at 162 bytes without the fix. (cherry picked from commit 7d0c184)
linear_tx hangs off ctx, so the failure paths free b and leave it behind. (cherry picked from commit 71fa672)
(cherry picked from commit 5b2dff7)
(cherry picked from commit f323458)
(cherry picked from commit 1466f9c)
During the negotiation of a dualopen channel a peer can send a tx_add_output with a very large amount. dualopend should handle it with tx_abort. We add a test for splice as well since they share the same interactive transaction logic. Changelog-None Signed-off-by: Lagrang3 <lagrang3@protonmail.com> Suggeded-by: @erickcestari (cherry picked from commit fd84826)
Harden the handling of tx_add_output messages. Fail the negotiation if the sum of the outputs exceeds 21M BTC. Changelog-Fixed: dual-fund/splice: fail the interactive channel negotiation if the sum of the outputs exceed 21M BTC (satisfy BOLT-02 cap of 21M BTC per output) Signed-off-by: Lagrang3 <lagrang3@protonmail.com> (cherry picked from commit f0cf91e)
Make the MAX_MONEY checks of tx_add_output into an internal API that we can re-use in interactivetx (used by splices) and dualopend. Changelog-None Signed-off-by: Lagrang3 <lagrang3@protonmail.com> (cherry picked from commit 574b15d)
l3 was watching its own (censored) commitment tx, so it can take a while before it notices l2's commitment is the one onchain and tries to fulfill the HTLC. If we released the censored txs before that, we only broadcast l3's own commitment txs, which conflict with l2's already-mined commitment and are rejected, leaving the mempool empty and making the final generate_block(wait_for_mempool=1) time out. Wait for l3's fulfill attempt (a tx spending an output of l2's commitment) before releasing. Also release only those txs, deduped by txid and in capture order, so duplicate or RBF fulfill attempts are not broadcast as stale versions. Changelog-None (cherry picked from commit 356cdf1)
The graceful command notifies watchers about the closest-expiry HTLC, including its state, whenever that message changes. But nothing re-evaluated the message when an HTLC changed state: only HTLC removal, peer disconnect and another graceful invocation re-ran the check. If graceful was invoked while a commitment dance was in flight, the initial notification named a transient state (e.g. RCVD_ADD_REVOCATION) and no follow-up ever announced the settled state. test_graceful_htlc waits for exactly that follow-up (since 6994681 "flake: Fix test_graceful_htlc to be flexible for notifs"), so it times out whenever graceful catches the dance mid-flight, which valgrind CI runs make likely. In one CI failure graceful caught the outgoing HTLC in RCVD_ADD_REVOCATION, the dance completed 600ms later, and no notification followed for the remaining 180 seconds. Re-check graceful progress in the handlers that advance HTLC states (peer_sending_commitsig, peer_got_commitsig, peer_got_revoke). The check is a no-op unless a graceful command is outstanding, and identical messages are already deduplicated. The dance now generates transient-state notifications, so rewrite the test to match expected notifications as an ordered subsequence instead of by exact index. That also removes two accidents the old indexing depended on: the RCVD_ADD_REVOCATION special case (the settled state now always notifies), and the wait for a notification after l1's disconnect, which was really satisfied by the graceful(1) call's own initial notification arriving on the shared rpc socket -- the disconnect itself never notifies, since the message text describes the still-connected peer l3 and does not change. Changelog-Fixed: JSON-RPC: `graceful` notifications now update when a pending HTLC changes state. (cherry picked from commit 2e5bc4f)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backports from upstream Core Lightning master, cherry-picked with
-x.tx_add_outputamount, or the running total of outputs, exceeds 21M BTC, in both dual-open and splice. The check lives ininteractive_tx_add_output_check_max_money, with a new unit testcommon/test/run-interactivetx.cand pytest coverage for dual-open and splice.graceful: re-check progress when an HTLC changes state, so its notifications follow the commitment exchange instead of stalling on a transient state.test_htlc_no_force_close: wait for the fulfill attempt before releasing the censored transactions.Fork-specific follow-ups: the raw peer test helper now offers the unified signer bit in its
channel_type, which this fork requires for new channels, andtest_closing.pydrops an import it no longer uses.Changelog-None