Skip to content

Commit 93f8a11

Browse files
committed
dual_funding: don't steal inflight field in update_channel_from_inflight.
If we call update_channel_from_inflight *twice* with the same inflight, we will get bad results. Using tal_steal() here was a premature optimization: ``` Valgrind error file: valgrind-errors.496395 ==496395== Invalid read of size 8 ==496395== at 0x22A9D3: to_tal_hdr (tal.c:174) ==496395== by 0x22B4B5: tal_steal_ (tal.c:498) ==496395== by 0x16A13D: update_channel_from_inflight (peer_control.c:1225) ==496395== by 0x16A4C7: funding_depth_cb (peer_control.c:1299) ==496395== by 0x182807: txw_fire (watch.c:232) ==496395== by 0x182AA9: watch_topology_changed (watch.c:300) ==496395== by 0x1290ED: updates_complete (chaintopology.c:624) ==496395== by 0x129BF4: get_new_block (chaintopology.c:835) ==496395== by 0x125EEF: getrawblockbyheight_callback (bitcoind.c:362) ==496395== by 0x176ECC: plugin_response_handle (plugin.c:584) ==496395== by 0x1770F5: plugin_read_json_one (plugin.c:690) ==496395== by 0x1772D9: plugin_read_json (plugin.c:735) ==496395== Address 0x89fbb08 is 24 bytes inside a block of size 104 free'd ==496395== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==496395== by 0x22B193: del_tree (tal.c:421) ==496395== by 0x22B461: tal_free (tal.c:486) ==496395== by 0x16A123: update_channel_from_inflight (peer_control.c:1223) ==496395== by 0x16A4C7: funding_depth_cb (peer_control.c:1299) ==496395== by 0x182807: txw_fire (watch.c:232) ==496395== by 0x182AA9: watch_topology_changed (watch.c:300) ==496395== by 0x1290ED: updates_complete (chaintopology.c:624) ==496395== by 0x129BF4: get_new_block (chaintopology.c:835) ==496395== by 0x125EEF: getrawblockbyheight_callback (bitcoind.c:362) ==496395== by 0x176ECC: plugin_response_handle (plugin.c:584) ==496395== by 0x1770F5: plugin_read_json_one (plugin.c:690) ==496395== Block was alloc'd at ==496395== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==496395== by 0x22AC1C: allocate (tal.c:250) ==496395== by 0x22B1DD: tal_alloc_ (tal.c:428) ==496395== by 0x22B3A6: tal_alloc_arr_ (tal.c:471) ==496395== by 0x22C094: tal_dup_ (tal.c:805) ==496395== by 0x12B274: new_inflight (channel.c:187) ==496395== by 0x136D4C: wallet_commit_channel (dual_open_control.c:1260) ==496395== by 0x13B084: handle_commit_received (dual_open_control.c:2839) ==496395== by 0x13B6AF: dual_opend_msg (dual_open_control.c:2976) ==496395== by 0x1809FF: sd_msg_read (subd.c:553) ==496395== by 0x218F5D: next_plan (io.c:59) ==496395== by 0x219B65: do_plan (io.c:407) ``` Signed-off-by: Rusty Russell <[email protected]>
1 parent dfb78eb commit 93f8a11

File tree

2 files changed

+3
-3
lines changed

2 files changed

+3
-3
lines changed

lightningd/peer_control.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,7 @@ static bool check_funding_tx(const struct bitcoin_tx *tx,
12091209

12101210
static void update_channel_from_inflight(struct lightningd *ld,
12111211
struct channel *channel,
1212-
struct channel_inflight *inflight)
1212+
const struct channel_inflight *inflight)
12131213
{
12141214
struct wally_psbt *psbt_copy;
12151215

@@ -1222,7 +1222,7 @@ static void update_channel_from_inflight(struct lightningd *ld,
12221222
channel->push = inflight->lease_fee;
12231223
tal_free(channel->lease_commit_sig);
12241224
channel->lease_commit_sig
1225-
= tal_steal(channel, inflight->lease_commit_sig);
1225+
= tal_dup_or_null(channel, secp256k1_ecdsa_signature, inflight->lease_commit_sig);
12261226
channel->lease_chan_max_msat = inflight->lease_chan_max_msat;
12271227
channel->lease_chan_max_ppt = inflight->lease_chan_max_ppt;
12281228

tests/test_closing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ def test_channel_lease_falls_behind(node_factory, bitcoind):
810810
compact_lease=rates['compact_lease'])
811811

812812
# sink the funding transaction
813-
bitcoind.generate_block(1)
813+
bitcoind.generate_block(1, wait_for_mempool=1)
814814

815815
# stop l1
816816
l1.stop()

0 commit comments

Comments
 (0)