Trying to put all the disconnect logic into the same path was a dumb
idea. If you asked to reconnect but passed in an 'unsaved' channel, we
would not call the 'reconnect' code.
Instead, we make a differentiation between "unsaved" channels
(ones that we haven't received commitment tx for) and handle the
disconnect for these separate from where we want to do a reconnect.
We were freeing the payload, which is then subsequently freed by the
plugin_hook caller. Whoops.
Now we pass through to the callback function and just clean up neatly.
------------------------------- Valgrind errors --------------------------------
Valgrind error file: valgrind-errors.406602
==406602== Invalid read of size 8
==406602== at 0x12AC93: openchannel2_hook_cb (dual_open_control.c:669)
==406602== by 0x12AF0A: openchannel2_hook_deserialize (dual_open_control.c:721)
==406602== by 0x16EF0E: plugin_hook_callback (plugin_hook.c:186)
==406602== by 0x169746: plugin_response_handle (plugin.c:514)
==406602== by 0x169959: plugin_read_json_one (plugin.c:620)
==406602== by 0x169B23: plugin_read_json (plugin.c:665)
==406602== by 0x1F4076: next_plan (io.c:59)
==406602== by 0x1F4C5B: do_plan (io.c:407)
==406602== by 0x1F4C9D: io_ready (io.c:417)
==406602== by 0x1F6F35: io_loop (poll.c:445)
==406602== by 0x13D48D: io_loop_with_timers (io_loop_with_timers.c:24)
==406602== by 0x143388: main (lightningd.c:1111)
==406602== Address 0x75e7418 is 56 bytes inside a block of size 3,520 free'd
==406602== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==406602== by 0x204FB0: del_tree (tal.c:421)
==406602== by 0x20527E: tal_free (tal.c:486)
==406602== by 0x122D68: delete_channel (channel.c:124)
==406602== by 0x129291: channel_disconnect (dual_open_control.c:63)
==406602== by 0x129364: channel_close_conn (dual_open_control.c:82)
==406602== by 0x131CF6: peer_please_disconnect (connect_control.c:304)
==406602== by 0x131DEB: connectd_msg (connect_control.c:326)
==406602== by 0x172023: sd_msg_read (subd.c:509)
==406602== by 0x1F4076: next_plan (io.c:59)
==406602== by 0x1F4C5B: do_plan (io.c:407)
==406602== by 0x1F4C9D: io_ready (io.c:417)
==406602== Block was alloc'd at
==406602== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==406602== by 0x204A39: allocate (tal.c:250)
==406602== by 0x204FFA: tal_alloc_ (tal.c:428)
==406602== by 0x123165: new_unsaved_channel (channel.c:209)
==406602== by 0x130D34: peer_start_dualopend (dual_open_control.c:2985)
==406602== by 0x15BD2A: peer_connected_hook_final (peer_control.c:1105)
==406602== by 0x16F2E5: plugin_hook_call_ (plugin_hook.c:275)
==406602== by 0x15BF5C: plugin_hook_call_peer_connected (peer_control.c:1155)
==406602== by 0x15C16C: peer_connected (peer_control.c:1208)
==406602== by 0x131E3B: connectd_msg (connect_control.c:332)
==406602== by 0x172023: sd_msg_read (subd.c:509)
==406602== by 0x171842: read_fds (subd.c:310)
The main change which affects us is that 2016 blocks to forget a channel
is a fixed number in the spec; we make this clear by renaming the
(developer-only) max_funding_unconfirmed to dev_max_funding_unconfirmed
and making it compile DEVELOPER only.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
You can now activate dual-funded channels using the
`--experimental-dual-fund` flag
Changelog-Changed: Config: `--experimental-dual-fund` runtime flag will enable dual-funded protocol on this node
There are perfectly valid reasons for us to not have a command on return
(something went boom while sending them our sigs and we've now gotten
their sigs during a reconnect and subsequently broadcast the tx)
This can result in us logging a warning if we've 1) dropped their sigs
response, 2) only us (the opener) added inputs, 3) and we broadcast on
their reconnect (when they retransmit their sigs)
Allows us to clean up an in-progress open that we won't be completing
Changelog-Added: EXPERIMENTAL JSON-RPC: Permit user-initiated aborting of in-progress opens. Only valid for not-yet-committed opens and RBF-attempts
The `rbf_channel` hook uses `our_funding_msat`, which is a nicer
and more easily understood than the `openchannel2`
`accepter_funding_msat`.
This updates the `openchannel2` hook to use the same nomenclature as
`rbf_channel`.
The receiving node: ...
- MUST fail the channel if:
- the `witness_stack` weight lowers the effective `feerate`
below the agreed upon transaction `feerate`
We consolidate to the latest/singular RFC patch for dual-funding, so
there's just a single patchfile for the change. Plus we move back to the
opener setting the desired feerate, the accepter merely declines to
participate if they disagree with the set rate.
We were getting a memleak error that the open_attempt isnt' being
cleaned up in test_rbf_reconnect_tx_construct. I had some trouble
reproducing it, so I removed the reliance on using `tmpctx` to clean it
up and was more surgical about cleaning it up inline.
We move over to the new "warning" paradigm, instead of using
an "rbf_fail" message.
Every failure is either a warning or an error; on warnings we
hang up and reconnect later, effectively resetting the state.
We're *mostly* set up for both sides doing RBF, except that it reverses
the callback flow (using the plugin vs RPC calls) and we're not
currently smart enough to flip between them gracefully
niftynei
Rusty Russell