From acc30c0b3f2064c92024b969d79d5fa407873e83 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 2 Oct 2023 09:29:51 +1030 Subject: [PATCH] lightningd: split DUALOPEND_OPEN_INIT into DUALOPEND_OPEN_INIT and DUALOPEND_OPEN_COMMITTED. The latter is used when we're put in the db, the former is the uncommitted state. Currently dbid == 0 is used in addition to the state, which is unwieldy. Signed-off-by: Rusty Russell Changelog-Experimental: JSON-RPC: added new dual-funding state `DUALOPEND_OPEN_COMMITTED` --- lightningd/channel.h | 22 +++++++----- lightningd/channel_state.h | 13 +++++--- lightningd/closing_control.c | 2 ++ lightningd/coin_mvts.c | 1 + lightningd/dual_open_control.c | 20 +++++++++-- lightningd/peer_control.c | 9 ++++- tests/test_connection.py | 2 +- tests/test_opening.py | 4 +-- tests/test_plugin.py | 61 ++++++++++++++++++++++++---------- wallet/wallet.h | 7 ++-- 10 files changed, 102 insertions(+), 39 deletions(-) diff --git a/lightningd/channel.h b/lightningd/channel.h index bcda5a547..0e326723e 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -415,6 +415,7 @@ static inline bool channel_can_add_htlc(const struct channel *channel) case ONCHAIN: case CLOSED: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: return false; case CHANNELD_NORMAL: @@ -436,6 +437,7 @@ static inline bool channel_can_remove_htlc(const struct channel *channel) case ONCHAIN: case CLOSED: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: return false; case CHANNELD_SHUTTING_DOWN: @@ -452,6 +454,7 @@ static inline bool channel_state_closing(enum channel_state state) case CHANNELD_AWAITING_LOCKIN: case CHANNELD_NORMAL: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: case CHANNELD_AWAITING_SPLICE: return false; @@ -478,6 +481,7 @@ static inline bool channel_state_fees_can_change(enum channel_state state) case ONCHAIN: case CLOSED: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: return false; case CHANNELD_NORMAL: @@ -499,6 +503,7 @@ static inline bool channel_state_failing_onchain(enum channel_state state) case CLOSINGD_COMPLETE: case CLOSED: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: return false; case AWAITING_UNILATERAL: @@ -514,6 +519,7 @@ static inline bool channel_state_pre_open(enum channel_state state) switch (state) { case CHANNELD_AWAITING_LOCKIN: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: return true; case CHANNELD_NORMAL: @@ -535,6 +541,7 @@ static inline bool channel_state_closed(enum channel_state state) switch (state) { case CHANNELD_AWAITING_LOCKIN: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: case CHANNELD_NORMAL: case CHANNELD_AWAITING_SPLICE: @@ -556,7 +563,8 @@ static inline bool channel_state_uncommitted(const struct channel *channel) { switch (channel->state) { case DUALOPEND_OPEN_INIT: - return channel->dbid == 0; + return true; + case DUALOPEND_OPEN_COMMITTED: case CHANNELD_AWAITING_LOCKIN: case DUALOPEND_AWAITING_LOCKIN: case CHANNELD_NORMAL: @@ -576,18 +584,16 @@ static inline bool channel_state_uncommitted(const struct channel *channel) /* Established enough, that we could reach out to peer to discuss */ static inline bool channel_wants_peercomms(const struct channel *channel) { - if (channel_state_uncommitted(channel)) - return false; - switch (channel->state) { case CHANNELD_AWAITING_LOCKIN: - case DUALOPEND_OPEN_INIT: case DUALOPEND_AWAITING_LOCKIN: + case DUALOPEND_OPEN_COMMITTED: case CHANNELD_NORMAL: case CHANNELD_AWAITING_SPLICE: case CLOSINGD_SIGEXCHANGE: case CHANNELD_SHUTTING_DOWN: return true; + case DUALOPEND_OPEN_INIT: case CLOSINGD_COMPLETE: case AWAITING_UNILATERAL: case FUNDING_SPEND_SEEN: @@ -601,18 +607,16 @@ static inline bool channel_wants_peercomms(const struct channel *channel) /* Established enough, that we have to fail onto chain */ static inline bool channel_wants_onchain_fail(const struct channel *channel) { - if (channel_state_uncommitted(channel)) - return false; - switch (channel->state) { case CHANNELD_AWAITING_LOCKIN: - case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: case CHANNELD_NORMAL: case CHANNELD_AWAITING_SPLICE: case CLOSINGD_SIGEXCHANGE: case CHANNELD_SHUTTING_DOWN: return true; + case DUALOPEND_OPEN_INIT: case CLOSINGD_COMPLETE: case AWAITING_UNILATERAL: case FUNDING_SPEND_SEEN: diff --git a/lightningd/channel_state.h b/lightningd/channel_state.h index 8ba616fbe..147c33015 100644 --- a/lightningd/channel_state.h +++ b/lightningd/channel_state.h @@ -6,8 +6,12 @@ /* These are in the database, so don't renumber them! */ enum channel_state { + /* For dual-funded channels: goes to DUALOPEND_OPEN_COMMITTED + * after sigs have been exchanged */ + DUALOPEND_OPEN_INIT = 1, + /* In channeld, still waiting for lockin. */ - CHANNELD_AWAITING_LOCKIN = 2, + CHANNELD_AWAITING_LOCKIN, /* Normal operating state. */ CHANNELD_NORMAL, @@ -33,16 +37,15 @@ enum channel_state { /* Final state after we have fully settled on-chain */ CLOSED, - /* For dual-funded channels, we start at a different state. - * We transition to 'awaiting lockin' after sigs have - * been exchanged */ - DUALOPEND_OPEN_INIT, + /* Dual-funded initialized and committed. */ + DUALOPEND_OPEN_COMMITTED, /* Dual-funded channel, waiting for lock-in */ DUALOPEND_AWAITING_LOCKIN, /* Channel has started splice and is awaiting lock-in */ CHANNELD_AWAITING_SPLICE, + }; #define CHANNEL_STATE_MAX CHANNELD_AWAITING_SPLICE diff --git a/lightningd/closing_control.c b/lightningd/closing_control.c index ae95e807b..9065a2a19 100644 --- a/lightningd/closing_control.c +++ b/lightningd/closing_control.c @@ -570,6 +570,7 @@ static bool channel_state_can_close(const struct channel *channel) case CHANNELD_AWAITING_LOCKIN: case DUALOPEND_AWAITING_LOCKIN: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case CLOSINGD_SIGEXCHANGE: case CHANNELD_SHUTTING_DOWN: return true; @@ -898,6 +899,7 @@ static struct command_result *json_close(struct command *cmd, break; case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case CLOSINGD_COMPLETE: case AWAITING_UNILATERAL: case FUNDING_SPEND_SEEN: diff --git a/lightningd/coin_mvts.c b/lightningd/coin_mvts.c index 67294ca42..c64f206ae 100644 --- a/lightningd/coin_mvts.c +++ b/lightningd/coin_mvts.c @@ -85,6 +85,7 @@ static bool report_chan_balance(const struct channel *chan) switch (chan->state) { case CHANNELD_AWAITING_LOCKIN: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: case CLOSINGD_COMPLETE: case AWAITING_UNILATERAL: diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index 4b74125ad..56a8e6ba4 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -1248,6 +1248,22 @@ wallet_commit_channel(struct lightningd *ld, assert(channel->unsaved_dbid != 0); channel->dbid = channel->unsaved_dbid; channel->unsaved_dbid = 0; + /* We can't call channel_set_state here: channel isn't in db, so + * really this is a "channel creation" event. */ + assert(channel->state == DUALOPEND_OPEN_INIT); + log_info(channel->log, "State changed from %s to %s", + channel_state_name(channel), + channel_state_str(DUALOPEND_OPEN_COMMITTED)); + channel->state = DUALOPEND_OPEN_COMMITTED; + notify_channel_state_changed(channel->peer->ld, + &channel->peer->id, + &channel->cid, + channel->scid, + time_now(), + DUALOPEND_OPEN_INIT, + DUALOPEND_OPEN_COMMITTED, + REASON_REMOTE, + "Commitment transaction committed"); channel->funding = *funding; channel->funding_sats = total_funding; @@ -1675,7 +1691,7 @@ static void handle_peer_tx_sigs_sent(struct subd *dualopend, send_funding_tx(channel, take(wtx)); /* Must be in an "init" state */ - assert(channel->state == DUALOPEND_OPEN_INIT + assert(channel->state == DUALOPEND_OPEN_COMMITTED || channel->state == DUALOPEND_AWAITING_LOCKIN); channel_set_state(channel, channel->state, @@ -2060,7 +2076,7 @@ static void handle_peer_tx_sigs_msg(struct subd *dualopend, send_funding_tx(channel, take(wtx)); - assert(channel->state == DUALOPEND_OPEN_INIT + assert(channel->state == DUALOPEND_OPEN_COMMITTED /* We might be reconnecting */ || channel->state == DUALOPEND_AWAITING_LOCKIN); channel_set_state(channel, channel->state, diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 7574a23d1..118718113 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -367,6 +367,7 @@ void resend_closing_transactions(struct lightningd *ld) case CHANNELD_AWAITING_LOCKIN: case CHANNELD_NORMAL: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: case CHANNELD_AWAITING_SPLICE: case CHANNELD_SHUTTING_DOWN: @@ -1168,6 +1169,7 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel case FUNDING_SPEND_SEEN: case CLOSINGD_COMPLETE: case CLOSED: + case DUALOPEND_OPEN_INIT: /* Channel is active */ abort(); case AWAITING_UNILATERAL: @@ -1177,7 +1179,7 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel "Awaiting unilateral close"); goto send_error; - case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: assert(!channel->owner); if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) { @@ -1859,6 +1861,7 @@ static void subd_tell_depth(struct channel *channel, case ONCHAIN: case CLOSED: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: return; case CHANNELD_NORMAL: @@ -1911,6 +1914,7 @@ static enum watch_result funding_depth_cb(struct lightningd *ld, case DUALOPEND_AWAITING_LOCKIN: case CHANNELD_AWAITING_LOCKIN: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: log_debug(channel->log, "Funding tx %s reorganized out!", type_to_string(tmpctx, struct bitcoin_txid, txid)); channel->scid = tal_free(channel->scid); @@ -2022,6 +2026,7 @@ static enum watch_result funding_depth_cb(struct lightningd *ld, return DELETE_WATCH; case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: /* You cannot be watching yet */ abort(); @@ -2576,6 +2581,7 @@ static struct command_result *json_getinfo(struct command *cmd, switch (channel->state) { case CHANNELD_AWAITING_LOCKIN: case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case DUALOPEND_AWAITING_LOCKIN: pending_channels++; continue; @@ -2797,6 +2803,7 @@ static bool channel_state_can_setchannel(enum channel_state state) case DUALOPEND_AWAITING_LOCKIN: return true; case DUALOPEND_OPEN_INIT: + case DUALOPEND_OPEN_COMMITTED: case CLOSINGD_SIGEXCHANGE: case CHANNELD_SHUTTING_DOWN: case CLOSINGD_COMPLETE: diff --git a/tests/test_connection.py b/tests/test_connection.py index efb14094a..001b844b8 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -682,7 +682,7 @@ def test_reconnect_signed(node_factory): # Technically, this is async to fundchannel (and could reconnect first) if EXPERIMENTAL_DUAL_FUND: l1.daemon.wait_for_logs(['sendrawtx exit 0', - 'Peer has reconnected, state DUALOPEND_OPEN_INIT']) + 'Peer has reconnected, state DUALOPEND_OPEN_COMMITTED']) else: l1.daemon.wait_for_logs(['sendrawtx exit 0', 'Peer has reconnected, state CHANNELD_AWAITING_LOCKIN']) diff --git a/tests/test_opening.py b/tests/test_opening.py index 78037121d..8a10fa846 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -166,7 +166,7 @@ def test_v2_open_sigs_restart(node_factory, bitcoind): assert log psbt = re.search("psbt (.*)", log).group(1) - l1.daemon.wait_for_log('Peer has reconnected, state DUALOPEND_OPEN_INIT') + l1.daemon.wait_for_log('Peer has reconnected, state DUALOPEND_OPEN_COMMITTED') try: # FIXME: why do we need to retry signed? l1.rpc.openchannel_signed(chan_id, psbt) @@ -254,7 +254,7 @@ def test_v2_open_sigs_restart_while_dead(node_factory, bitcoind): assert log psbt = re.search("psbt (.*)", log).group(1) - l1.daemon.wait_for_log('Peer has reconnected, state DUALOPEND_OPEN_INIT') + l1.daemon.wait_for_log('Peer has reconnected, state DUALOPEND_OPEN_COMMITTED') try: # FIXME: why do we need to retry signed? l1.rpc.openchannel_signed(chan_id, psbt) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 9e65a4047..b20836c77 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -799,25 +799,47 @@ def test_channel_state_changed_bilateral(node_factory, bitcoind): assert 'closer' not in l1.rpc.listpeerchannels()['channels'][0] assert 'closer' not in l2.rpc.listpeerchannels()['channels'][0] - event1 = wait_for_event(l1) - event2 = wait_for_event(l2) - assert(event1['peer_id'] == l2_id) # we only test these IDs the first time - assert(event1['channel_id'] == cid) - assert(event1['short_channel_id'] is None) # None until locked in - assert(event1['cause'] == "user") + if l1.config('experimental-dual-fund'): + # Dual funded channels go through two state transitions. + event1a, event1b = wait_for_event(l1), wait_for_event(l1) + event2a, event2b = wait_for_event(l2), wait_for_event(l2) - assert(event2['peer_id'] == l1_id) # we only test these IDs the first time - assert(event2['channel_id'] == cid) - assert(event2['short_channel_id'] is None) # None until locked in - assert(event2['cause'] == "remote") + for ev in [event1a, event1b]: + assert(ev['peer_id'] == l2_id) # we only test these IDs the first time + assert(ev['channel_id'] == cid) + assert(ev['short_channel_id'] is None) # None until locked in + assert(event1a['cause'] == "remote") + assert(event1b['cause'] == "user") - for ev in [event1, event2]: - # Dual funded channels - if l1.config('experimental-dual-fund'): + for ev in [event2a, event2b]: + assert(ev['peer_id'] == l1_id) # we only test these IDs the first time + assert(ev['channel_id'] == cid) + assert(ev['short_channel_id'] is None) # None until locked in + assert(ev['cause'] == "remote") + + for ev in [event1a, event2a]: assert(ev['old_state'] == "DUALOPEND_OPEN_INIT") + assert(ev['new_state'] == "DUALOPEND_OPEN_COMMITTED") + assert(ev['message'] == "Commitment transaction committed") + + for ev in [event1b, event2b]: + assert(ev['old_state'] == "DUALOPEND_OPEN_COMMITTED") assert(ev['new_state'] == "DUALOPEND_AWAITING_LOCKIN") assert(ev['message'] == "Sigs exchanged, waiting for lock-in") - else: + else: + event1 = wait_for_event(l1) + event2 = wait_for_event(l2) + assert(event1['peer_id'] == l2_id) # we only test these IDs the first time + assert(event1['channel_id'] == cid) + assert(event1['short_channel_id'] is None) # None until locked in + assert(event1['cause'] == "user") + + assert(event2['peer_id'] == l1_id) # we only test these IDs the first time + assert(event2['channel_id'] == cid) + assert(event2['short_channel_id'] is None) # None until locked in + assert(event2['cause'] == "remote") + + for ev in [event1, event2]: assert(ev['old_state'] == "unknown") assert(ev['new_state'] == "CHANNELD_AWAITING_LOCKIN") assert(ev['message'] == "new channel opened") @@ -942,8 +964,13 @@ def test_channel_state_changed_unilateral(node_factory, bitcoind): if l2.config('experimental-dual-fund'): assert(event2['old_state'] == "DUALOPEND_OPEN_INIT") - assert(event2['new_state'] == "DUALOPEND_AWAITING_LOCKIN") - assert(event2['message'] == "Sigs exchanged, waiting for lock-in") + assert(event2['new_state'] == "DUALOPEND_OPEN_COMMITTED") + assert(event2['message'] == "Commitment transaction committed") + + event2 = wait_for_event(l2) + assert event2['old_state'] == "DUALOPEND_OPEN_COMMITTED" + assert event2['new_state'] == "DUALOPEND_AWAITING_LOCKIN" + assert event2['message'] == "Sigs exchanged, waiting for lock-in" else: assert(event2['old_state'] == "unknown") assert(event2['new_state'] == "CHANNELD_AWAITING_LOCKIN") @@ -1037,7 +1064,7 @@ def test_channel_state_change_history(node_factory, bitcoind): history = l1.rpc.listpeerchannels()['channels'][0]['state_changes'] if l1.config('experimental-dual-fund'): assert(history[0]['cause'] == "user") - assert(history[0]['old_state'] == "DUALOPEND_OPEN_INIT") + assert(history[0]['old_state'] == "DUALOPEND_OPEN_COMMITTED") assert(history[0]['new_state'] == "DUALOPEND_AWAITING_LOCKIN") assert(history[1]['cause'] == "user") assert(history[1]['old_state'] == "DUALOPEND_AWAITING_LOCKIN") diff --git a/wallet/wallet.h b/wallet/wallet.h index ff4b5db44..a7512cad9 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -303,8 +303,8 @@ static inline enum channel_state channel_state_in_db(enum channel_state s) case CLOSED: BUILD_ASSERT(CLOSED == 10); return s; - case DUALOPEND_OPEN_INIT: - BUILD_ASSERT(DUALOPEND_OPEN_INIT == 11); + case DUALOPEND_OPEN_COMMITTED: + BUILD_ASSERT(DUALOPEND_OPEN_COMMITTED == 11); return s; case DUALOPEND_AWAITING_LOCKIN: BUILD_ASSERT(DUALOPEND_AWAITING_LOCKIN == 12); @@ -312,6 +312,9 @@ static inline enum channel_state channel_state_in_db(enum channel_state s) case CHANNELD_AWAITING_SPLICE: BUILD_ASSERT(CHANNELD_AWAITING_SPLICE == 13); return s; + case DUALOPEND_OPEN_INIT: + /* Never appears in db! */ + break; } fatal("%s: %u is invalid", __func__, s); }