From 30aa1d79fb48a2f28c04e6af191e31b6fd686165 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 19 Jul 2022 14:35:56 -0500 Subject: [PATCH] bkpr: for zerconfs, we still wanna know you're opening a channel We need a record of the channel account before you start sending payments through it. Normally we don't start allowing payments to be sent until after the channel has locked in but zeroconf does away with this assumption. Instead we push out a "channel_proposed" event, which should only show up for zeroconfs. --- common/coin_mvt.c | 28 ++++++++++++++++++++ common/coin_mvt.h | 13 ++++++++- lightningd/channel_control.c | 48 ++++++++++++++++++++++------------ lightningd/channel_control.h | 2 +- lightningd/dual_open_control.c | 3 ++- lightningd/onchain_control.c | 5 ++-- plugins/bkpr/recorder.c | 1 + tests/test_opening.py | 20 +++++++++++--- 8 files changed, 94 insertions(+), 26 deletions(-) diff --git a/common/coin_mvt.c b/common/coin_mvt.c index bb2caa14f..4b087a73a 100644 --- a/common/coin_mvt.c +++ b/common/coin_mvt.c @@ -40,6 +40,7 @@ static const char *mvt_tags[] = { "lease_fee", "leased", "stealable", + "channel_proposed", }; const char *mvt_tag_str(enum mvt_tag tag) @@ -196,6 +197,33 @@ struct chain_coin_mvt *new_coin_channel_close(const tal_t *ctx, output_count); } +struct chain_coin_mvt *new_coin_channel_open_proposed(const tal_t *ctx, + const struct channel_id *chan_id, + const struct bitcoin_outpoint *out, + const struct node_id *peer_id, + const struct amount_msat amount, + const struct amount_sat output_val, + bool is_opener, + bool is_leased) +{ + struct chain_coin_mvt *mvt; + + mvt = new_chain_coin_mvt(ctx, NULL, NULL, out, NULL, 0, + take(new_tag_arr(NULL, CHANNEL_PROPOSED)), + amount, true, output_val, 0); + mvt->account_name = type_to_string(mvt, struct channel_id, chan_id); + mvt->peer_id = tal_dup(mvt, struct node_id, peer_id); + + /* If we're the opener, add to the tag list */ + if (is_opener) + tal_arr_expand(&mvt->tags, OPENER); + + if (is_leased) + tal_arr_expand(&mvt->tags, LEASED); + + return mvt; +} + struct chain_coin_mvt *new_coin_channel_open(const tal_t *ctx, const struct channel_id *chan_id, const struct bitcoin_outpoint *out, diff --git a/common/coin_mvt.h b/common/coin_mvt.h index 1f75641cb..d35340073 100644 --- a/common/coin_mvt.h +++ b/common/coin_mvt.h @@ -14,7 +14,7 @@ enum mvt_type { CHANNEL_MVT = 1, }; -#define NUM_MVT_TAGS (STEALABLE + 1) +#define NUM_MVT_TAGS (CHANNEL_PROPOSED + 1) enum mvt_tag { DEPOSIT = 0, WITHDRAWAL = 1, @@ -39,6 +39,7 @@ enum mvt_tag { LEASE_FEE = 20, LEASED = 21, STEALABLE = 22, + CHANNEL_PROPOSED = 23, }; struct channel_coin_mvt { @@ -188,6 +189,16 @@ struct chain_coin_mvt *new_coin_channel_close(const tal_t *ctx, u32 output_count) NON_NULL_ARGS(2, 3); +struct chain_coin_mvt *new_coin_channel_open_proposed(const tal_t *ctx, + const struct channel_id *chan_id, + const struct bitcoin_outpoint *out, + const struct node_id *peer_id, + const struct amount_msat amount, + const struct amount_sat output_val, + bool is_opener, + bool is_leased) + NON_NULL_ARGS(2, 3); + struct chain_coin_mvt *new_coin_channel_open(const tal_t *ctx, const struct channel_id *chan_id, const struct bitcoin_outpoint *out, diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 67a56b853..8d2cbb244 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -127,7 +127,7 @@ void notify_feerate_change(struct lightningd *ld) * peer. We *could* do so, however. */ } -void channel_record_open(struct channel *channel, u32 blockheight) +void channel_record_open(struct channel *channel, u32 blockheight, bool record_push) { struct chain_coin_mvt *mvt; struct amount_msat start_balance; @@ -153,20 +153,31 @@ void channel_record_open(struct channel *channel, u32 blockheight) &channel->push)); } - mvt = new_coin_channel_open(tmpctx, - &channel->cid, - &channel->funding, - &channel->peer->id, - blockheight, - start_balance, - channel->funding_sats, - channel->opener == LOCAL, - is_leased); + /* If it's not in a block yet, send a proposal */ + if (blockheight > 0) + mvt = new_coin_channel_open(tmpctx, + &channel->cid, + &channel->funding, + &channel->peer->id, + blockheight, + start_balance, + channel->funding_sats, + channel->opener == LOCAL, + is_leased); + else + mvt = new_coin_channel_open_proposed(tmpctx, + &channel->cid, + &channel->funding, + &channel->peer->id, + start_balance, + channel->funding_sats, + channel->opener == LOCAL, + is_leased); notify_chain_mvt(channel->peer->ld, mvt); /* If we pushed sats, *now* record them */ - if (is_pushed) + if (is_pushed && record_push) notify_channel_mvt(channel->peer->ld, new_coin_channel_push(tmpctx, &channel->cid, channel->push, @@ -206,10 +217,12 @@ static void lockin_complete(struct channel *channel) try_update_blockheight(channel->peer->ld, channel, get_block_height(channel->peer->ld->topology)); - /* Only record this once we get a real confirmation. */ - if (channel->scid) - channel_record_open(channel, - short_channel_id_blocknum(channel->scid)); + /* Emit an event for the channel open (or channel proposal if blockheight + * is zero) */ + channel_record_open(channel, + channel->scid ? + short_channel_id_blocknum(channel->scid) : 0, + true); } bool channel_on_funding_locked(struct channel *channel, @@ -867,9 +880,10 @@ bool channel_tell_depth(struct lightningd *ld, channel->peer->ld, channel, get_block_height(channel->peer->ld->topology)); - /* Only record this once we get a real confirmation. */ + /* Emit channel_open event */ channel_record_open(channel, - short_channel_id_blocknum(channel->scid)); + short_channel_id_blocknum(channel->scid), + false); } return true; } diff --git a/lightningd/channel_control.h b/lightningd/channel_control.h index 420754706..9008bed1d 100644 --- a/lightningd/channel_control.h +++ b/lightningd/channel_control.h @@ -35,7 +35,7 @@ bool channel_on_funding_locked(struct channel *channel, struct pubkey *next_per_commitment_point); /* Record channel open (coin movement notifications) */ -void channel_record_open(struct channel *channel, u32 blockheight); +void channel_record_open(struct channel *channel, u32 blockheight, bool record_push); /* A channel has unrecoverably fallen behind */ void channel_fallen_behind(struct channel *channel, const u8 *msg); diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index fba6ff1a4..71133b8fb 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -1750,7 +1750,8 @@ static void handle_channel_locked(struct subd *dualopend, REASON_UNKNOWN, "Lockin complete"); channel_record_open(channel, - short_channel_id_blocknum(channel->scid)); + short_channel_id_blocknum(channel->scid), + true); /* Empty out the inflights */ wallet_channel_clear_inflights(dualopend->ld->wallet, channel); diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 322487cd3..27ceafe4e 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -627,11 +627,10 @@ enum watch_result onchaind_funding_spent(struct channel *channel, /* If we haven't posted the open event yet, post an open */ if (!channel->scid || !channel->remote_funding_locked) { u32 blkh; - /* Note that blockheight will be zero if it's not in chain - * yet */ + /* Blockheight will be zero if it's not in chain */ blkh = wallet_transaction_height(channel->peer->ld->wallet, &channel->funding.txid); - channel_record_open(channel, blkh); + channel_record_open(channel, blkh, true); } diff --git a/plugins/bkpr/recorder.c b/plugins/bkpr/recorder.c index 9b1ab449f..2eb500c5e 100644 --- a/plugins/bkpr/recorder.c +++ b/plugins/bkpr/recorder.c @@ -1219,6 +1219,7 @@ void maybe_update_account(struct db *db, for (size_t i = 0; i < tal_count(tags); i++) { switch (tags[i]) { + case CHANNEL_PROPOSED: case CHANNEL_OPEN: updated = true; acct->open_event_db_id = tal(acct, u64); diff --git a/tests/test_opening.py b/tests/test_opening.py index af632cdda..f90a18a1b 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -2,7 +2,7 @@ from fixtures import * # noqa: F401,F403 from fixtures import TEST_NETWORK from pyln.client import RpcError, Millisatoshi from utils import ( - only_one, wait_for, sync_blockheight, first_channel_id, calc_lease_fee + only_one, wait_for, sync_blockheight, first_channel_id, calc_lease_fee, check_coin_moves ) from pathlib import Path @@ -1312,7 +1312,7 @@ def test_zeroconf_open(bitcoind, node_factory): l2.rpc.pay(inv) -def test_zeroconf_public(bitcoind, node_factory): +def test_zeroconf_public(bitcoind, node_factory, chainparams): """Test that we transition correctly from zeroconf to public The differences being that a public channel MUST use the public @@ -1321,9 +1321,10 @@ def test_zeroconf_public(bitcoind, node_factory): """ plugin_path = Path(__file__).parent / "plugins" / "zeroconf-selective.py" + coin_mvt_plugin = Path(__file__).parent / "plugins" / "coin_movements.py" l1, l2, l3 = node_factory.get_nodes(3, opts=[ - {}, + {'plugin': str(coin_mvt_plugin)}, { 'plugin': str(plugin_path), 'zeroconf-allow': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518' @@ -1346,6 +1347,13 @@ def test_zeroconf_public(bitcoind, node_factory): assert('short_channel_id' not in l1chan) assert('short_channel_id' not in l2chan) + # Channel is "proposed" + chan_val = 993198000 if chainparams['elements'] else 995673000 + l1_mvts = [ + {'type': 'chain_mvt', 'credit_msat': chan_val, 'debit_msat': 0, 'tags': ['channel_proposed', 'opener']}, + ] + check_coin_moves(l1, l1chan['channel_id'], l1_mvts, chainparams) + # Now add 1 confirmation, we should get a `short_channel_id` bitcoind.generate_block(1) l1.daemon.wait_for_log(r'Funding tx [a-f0-9]{64} depth 1 of 0') @@ -1356,6 +1364,12 @@ def test_zeroconf_public(bitcoind, node_factory): assert('short_channel_id' in l1chan) assert('short_channel_id' in l2chan) + # We also now have an 'open' event + l1_mvts += [ + {'type': 'chain_mvt', 'credit_msat': chan_val, 'debit_msat': 0, 'tags': ['channel_open', 'opener']}, + ] + check_coin_moves(l1, l1chan['channel_id'], l1_mvts, chainparams) + # Now make it public, we should be switching over to the real # scid. bitcoind.generate_block(5)