From 88f8e63ab6343220ef9b393d4665856999dcbe00 Mon Sep 17 00:00:00 2001 From: niftynei Date: Thu, 5 Aug 2021 12:30:41 -0500 Subject: [PATCH 1/8] funder-rbf: correctly iniitalize lease params We weren't initializing lease params, which was leading to undefined behavior for rbfs --- doc/lightning-openchannel_bump.7 | 5 ++++- doc/lightning-openchannel_bump.7.md | 2 ++ plugins/funder.c | 13 ++++++++++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/doc/lightning-openchannel_bump.7 b/doc/lightning-openchannel_bump.7 index 1fd4497ac2e6..e4032cd38198 100644 --- a/doc/lightning-openchannel_bump.7 +++ b/doc/lightning-openchannel_bump.7 @@ -32,6 +32,9 @@ is not met\. funding transaction\. Defaults to 1/64th greater than the last feerate used for this channel\. + +Warning: bumping a leased channel will lose the lease\. + .SH RETURN VALUE On success, an object is returned, containing: @@ -94,4 +97,4 @@ lightning-fundchannel_\fBstart\fR(7), lightning-fundchannel_\fBcomplete\fR(7), Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:357923fcd85b832cc52f77858c0744cae6c06d7612aaebb87e65f2ef0a42f367 +\" SHA256STAMP:dc25396dde0592afdcf44e62ef8c897649b43910d9afd4c63327033994f8b9fa diff --git a/doc/lightning-openchannel_bump.7.md b/doc/lightning-openchannel_bump.7.md index 487533bebbbd..f66a227c79b0 100644 --- a/doc/lightning-openchannel_bump.7.md +++ b/doc/lightning-openchannel_bump.7.md @@ -30,6 +30,8 @@ is not met. funding transaction. Defaults to 1/64th greater than the last feerate used for this channel. +Warning: bumping a leased channel will lose the lease. + RETURN VALUE ------------ diff --git a/plugins/funder.c b/plugins/funder.c index 4a70f464481c..ac1c87556aa4 100644 --- a/plugins/funder.c +++ b/plugins/funder.c @@ -288,6 +288,17 @@ struct open_info { struct amount_sat requested_lease; }; +static struct open_info *new_open_info(const tal_t *ctx) +{ + struct open_info *info = tal(ctx, struct open_info); + + info->requested_lease = AMOUNT_SAT(0); + info->lease_blockheight = 0; + info->node_blockheight = 0; + + return info; +} + static struct command_result * psbt_funded(struct command *cmd, const char *buf, @@ -641,7 +652,7 @@ json_rbf_channel_call(struct command *cmd, const char *buf, const jsmntok_t *params) { - struct open_info *info = tal(cmd, struct open_info); + struct open_info *info = new_open_info(cmd); u64 feerate_our_max, feerate_our_min; const char *err; struct out_req *req; From 9df4234e8febde894e25e191aa5a01e77d25f0fc Mon Sep 17 00:00:00 2001 From: niftynei Date: Thu, 5 Aug 2021 12:31:49 -0500 Subject: [PATCH 2/8] funder: default to only funding leases Make the default to only lease out funds. Changelog-Changed: funder plugin defaults to leases-only --- external/lnprototest | 2 +- plugins/funder_policy.c | 4 +--- plugins/test/run-funder_policy.c | 2 +- tests/test_closing.py | 3 ++- tests/test_connection.py | 9 ++++++--- tests/test_opening.py | 10 +++++++--- 6 files changed, 18 insertions(+), 12 deletions(-) diff --git a/external/lnprototest b/external/lnprototest index ff3750b0ce88..2b99492ed054 160000 --- a/external/lnprototest +++ b/external/lnprototest @@ -1 +1 @@ -Subproject commit ff3750b0ce88130c09dcd02086340baaec2be016 +Subproject commit 2b99492ed0544a6c0475a79ca4ed484bc9191635 diff --git a/plugins/funder_policy.c b/plugins/funder_policy.c index 1aefdfef5221..50c45be3b755 100644 --- a/plugins/funder_policy.c +++ b/plugins/funder_policy.c @@ -95,9 +95,7 @@ default_funder_policy(const tal_t *ctx, 0, /* fuzz_factor */ AMOUNT_SAT(0), /* reserve_tank */ 100, - /* Defaults to true iif we're advertising - * offers */ - false, + true, /* Leases-only by default */ NULL); } diff --git a/plugins/test/run-funder_policy.c b/plugins/test/run-funder_policy.c index 09ca6a3a9ee8..c2eeee527e31 100644 --- a/plugins/test/run-funder_policy.c +++ b/plugins/test/run-funder_policy.c @@ -489,7 +489,7 @@ int main(int argc, const char *argv[]) AMOUNT_SAT(50000), AMOUNT_SAT(50000), AMOUNT_SAT(100000), - AMOUNT_SAT(0), + AMOUNT_SAT(100000), &our_funds); assert(amount_sat_eq(empty, our_funds)); assert(!err); diff --git a/tests/test_closing.py b/tests/test_closing.py index 737f21f396fc..d4adfecc1d7c 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -857,7 +857,8 @@ def test_channel_lease_unilat_closes(node_factory, bitcoind): l2-l3: l2 leases funds from l3; l3 goes to chain unilaterally ''' opts = {'funder-policy': 'match', 'funder-policy-mod': 100, - 'lease-fee-base-msat': '100sat', 'lease-fee-basis': 100} + 'lease-fee-base-msat': '100sat', 'lease-fee-basis': 100, + 'funder-lease-requests-only': False} l1, l2, l3 = node_factory.get_nodes(3, opts=opts) # Allow l2 some warnings diff --git a/tests/test_connection.py b/tests/test_connection.py index 86c7d7070a34..a3ba2fd379d5 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -410,7 +410,8 @@ def test_disconnect_fundee_v2(node_factory): l2 = node_factory.get_node(disconnect=disconnects, options={'funder-policy': 'match', 'funder-policy-mod': 100, - 'funder-fuzz-percent': 0}) + 'funder-fuzz-percent': 0, + 'funder-lease-requests-only': False}) l1.fundwallet(2000000) l2.fundwallet(2000000) @@ -1687,10 +1688,12 @@ def test_multifunding_v2_exclusive(node_factory, bitcoind): options = [{}, {'funder-policy': 'match', 'funder-policy-mod': 100, - 'funder-fuzz-percent': 0}, + 'funder-fuzz-percent': 0, + 'funder-lease-requests-only': False}, {'funder-policy': 'match', 'funder-policy-mod': 100, - 'funder-fuzz-percent': 0}, + 'funder-fuzz-percent': 0, + 'funder-lease-requests-only': False}, {}] l1, l2, l3, l4 = node_factory.get_nodes(4, opts=options) diff --git a/tests/test_opening.py b/tests/test_opening.py index a5ba5dedbd87..1d688b5a9d2a 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1047,6 +1047,7 @@ def test_funder_options(node_factory, bitcoind): assert funder_opts['reserve_tank_msat'] == Millisatoshi('0msat') assert funder_opts['fuzz_percent'] == 0 assert funder_opts['fund_probability'] == 100 + assert funder_opts['leases_only'] # l2 funds a chanenl with us. We don't contribute l2.rpc.connect(l1.info['id'], 'localhost', l1.port) @@ -1066,7 +1067,8 @@ def test_funder_options(node_factory, bitcoind): 'per_channel_max_msat': '10000000000msat', 'reserve_tank_msat': '3000000msat', 'fund_probability': 99, - 'fuzz_percent': 0}) + 'fuzz_percent': 0, + 'leases_only': False}) assert funder_opts['policy'] == 'available' assert funder_opts['policy_mod'] == 100 @@ -1126,7 +1128,8 @@ def test_funder_contribution_limits(node_factory, bitcoind): 'min_their_funding_msat': '1000msat', 'per_channel_min_msat': '1000000msat', 'fund_probability': 100, - 'fuzz_percent': 0}) + 'fuzz_percent': 0, + 'leases_only': False}) # Set our contribution to 50k sat, should only use 7 of 12 available utxos l3.rpc.call('funderupdate', @@ -1136,7 +1139,8 @@ def test_funder_contribution_limits(node_factory, bitcoind): 'per_channel_min_msat': '1000sat', 'per_channel_max_msat': '500000sat', 'fund_probability': 100, - 'fuzz_percent': 0}) + 'fuzz_percent': 0, + 'leases_only': False}) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.fundchannel(l2, 10**7) From a9de23f9939c24320fa8e933f716b329694f0bf7 Mon Sep 17 00:00:00 2001 From: niftynei Date: Thu, 5 Aug 2021 12:32:38 -0500 Subject: [PATCH 3/8] tests: promote method to utility file We'll reuse it later --- tests/test_closing.py | 10 +--------- tests/test_opening.py | 2 +- tests/utils.py | 7 +++++++ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/test_closing.py b/tests/test_closing.py index d4adfecc1d7c..9c17eebc127a 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -6,7 +6,7 @@ from utils import ( only_one, sync_blockheight, wait_for, TIMEOUT, account_balance, first_channel_id, closing_fee, TEST_NETWORK, - scriptpubkey_addr + scriptpubkey_addr, calc_lease_fee ) import os @@ -718,14 +718,6 @@ def test_penalty_outhtlc(node_factory, bitcoind, executor, chainparams): assert account_balance(l2, channel_id) == 0 -# check that the fee paid is correct -def calc_lease_fee(amt, feerate, rates): - fee = rates['lease_fee_base_msat'] - fee += amt * rates['lease_fee_basis'] // 10 - fee += rates['funding_weight'] * feerate - return fee - - @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') @pytest.mark.openchannel('v2') @pytest.mark.slow_test diff --git a/tests/test_opening.py b/tests/test_opening.py index 1d688b5a9d2a..cdd351e2dec8 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -2,7 +2,7 @@ from fixtures import TEST_NETWORK from pyln.client import RpcError, Millisatoshi from utils import ( - only_one, wait_for, sync_blockheight, first_channel_id + only_one, wait_for, sync_blockheight, first_channel_id, calc_lease_fee ) import pytest diff --git a/tests/utils.py b/tests/utils.py index 81f34f111b55..a21851b20d62 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -74,6 +74,13 @@ def move_matches(exp, mv): return True +def calc_lease_fee(amt, feerate, rates): + fee = rates['lease_fee_base_msat'] + fee += amt * rates['lease_fee_basis'] // 10 + fee += rates['funding_weight'] * feerate + return fee + + def check_coin_moves(n, account_id, expected_moves, chainparams): moves = n.rpc.call('listcoinmoves_plugin')['coin_moves'] node_id = n.info['id'] From f719343b4e93915a6a2b11e39eea3d3b1f6430f5 Mon Sep 17 00:00:00 2001 From: niftynei Date: Thu, 5 Aug 2021 12:32:58 -0500 Subject: [PATCH 4/8] tests: check that we don't re-up the leased funds when we RBF Check that the peer won't put funds into a RBF'd channel lease. FIXME: allow leases to pass through to RBFs Changelog-None. --- tests/test_opening.py | 79 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/test_opening.py b/tests/test_opening.py index cdd351e2dec8..15cdbef8c457 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -334,6 +334,85 @@ def test_v2_rbf_single(node_factory, bitcoind, chainparams): l1.daemon.wait_for_log('sendrawtx exit 0') +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') +@pytest.mark.openchannel('v2') +def test_v2_rbf_liquidity_ad(node_factory, bitcoind, chainparams): + + opts = {'funder-policy': 'match', 'funder-policy-mod': 100, + 'lease-fee-base-msat': '100sat', 'lease-fee-basis': 100, + 'may_reconnect': True} + l1, l2 = node_factory.get_nodes(2, opts=opts) + + # what happens when we RBF? + feerate = 2000 + amount = 500000 + l1.fundwallet(20000000) + l2.fundwallet(20000000) + + # l1 leases a channel from l2 + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + rates = l1.rpc.dev_queryrates(l2.info['id'], amount, amount) + l1.daemon.wait_for_log('disconnect') + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + chan_id = l1.rpc.fundchannel(l2.info['id'], amount, request_amt=amount, + feerate='{}perkw'.format(feerate), + compact_lease=rates['compact_lease'])['channel_id'] + + vins = [x for x in l1.rpc.listfunds()['outputs'] if x['reserved']] + assert only_one(vins) + prev_utxos = ["{}:{}".format(vins[0]['txid'], vins[0]['output'])] + + # Check that we're waiting for lockin + l1.daemon.wait_for_log(' to DUALOPEND_AWAITING_LOCKIN') + + est_fees = calc_lease_fee(amount, feerate, rates) + + # This should be the accepter's amount + fundings = only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['funding'] + assert Millisatoshi(est_fees + amount * 1000) == Millisatoshi(fundings['remote_msat']) + + # rbf the lease with a higher amount + rate = int(find_next_feerate(l1, l2)[:-5]) + # We 4x the feerate to beat the min-relay fee + next_feerate = '{}perkw'.format(rate * 4) + + # Initiate an RBF + startweight = 42 + 172 # base weight, funding output + initpsbt = l1.rpc.utxopsbt(amount, next_feerate, startweight, + prev_utxos, reservedok=True, + min_witness_weight=110, + excess_as_change=True)['psbt'] + + # do the bump + bump = l1.rpc.openchannel_bump(chan_id, amount, initpsbt, + funding_feerate=next_feerate) + update = l1.rpc.openchannel_update(chan_id, bump['psbt']) + assert update['commitments_secured'] + # Sign our inputs, and continue + signed_psbt = l1.rpc.signpsbt(update['psbt'])['signed_psbt'] + l1.rpc.openchannel_signed(chan_id, signed_psbt) + + # what happens when the channel opens? + bitcoind.generate_block(6) + l1.daemon.wait_for_log('to CHANNELD_NORMAL') + + # This should be the accepter's amount + fundings = only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['funding'] + # FIXME: The lease goes away :( + assert Millisatoshi(0) == Millisatoshi(fundings['remote_msat']) + + wait_for(lambda: [c['active'] for c in l1.rpc.listchannels(l1.get_channel_scid(l2))['channels']] == [True, True]) + + # send some payments, mine a block or two + inv = l2.rpc.invoice(10**4, '1', 'no_1') + l1.rpc.pay(inv['bolt11']) + + # l2 attempts to close a channel that it leased, should succeed + # (channel isnt leased) + l2.rpc.close(l1.get_channel_scid(l2)) + l1.daemon.wait_for_log('State changed from CLOSINGD_SIGEXCHANGE to CLOSINGD_COMPLETE') + + @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') @pytest.mark.openchannel('v2') def test_v2_rbf_multi(node_factory, bitcoind, chainparams): From 8157e378f45c07925c9a67d4208207a0d421e4a7 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 9 Aug 2021 08:52:23 +0200 Subject: [PATCH 5/8] wallet: correct CSV check in `deep_enough` Return false if the timelock didn't mature yet, not the other way around. Also, the check shouldn't be strict: if the CSV is 1 it is valid at utxo->blockheight + 1. Signed-off-by: Antoine Poinsot --- wallet/db_postgres_sqlgen.c | 2 +- wallet/db_sqlite3_sqlgen.c | 2 +- wallet/statements_gettextgen.po | 2 +- wallet/wallet.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/wallet/db_postgres_sqlgen.c b/wallet/db_postgres_sqlgen.c index 34e0b303c30c..539287837cf6 100644 --- a/wallet/db_postgres_sqlgen.c +++ b/wallet/db_postgres_sqlgen.c @@ -2026,4 +2026,4 @@ struct db_query db_postgres_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */ -// SHA256STAMP:6353b67b3e4f539da2d1f0c2564c4a8243f07d59cd0b73bc83d5552600bd67f7 +// SHA256STAMP:27a166e040e517422e91cf7ffbd12426b34337b8d75f82d7aa4c448beae5e821 diff --git a/wallet/db_sqlite3_sqlgen.c b/wallet/db_sqlite3_sqlgen.c index 0b7fd17eb1c8..c57cb377201d 100644 --- a/wallet/db_sqlite3_sqlgen.c +++ b/wallet/db_sqlite3_sqlgen.c @@ -2026,4 +2026,4 @@ struct db_query db_sqlite3_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */ -// SHA256STAMP:6353b67b3e4f539da2d1f0c2564c4a8243f07d59cd0b73bc83d5552600bd67f7 +// SHA256STAMP:27a166e040e517422e91cf7ffbd12426b34337b8d75f82d7aa4c448beae5e821 diff --git a/wallet/statements_gettextgen.po b/wallet/statements_gettextgen.po index 636c0bbc37ca..317b0406da54 100644 --- a/wallet/statements_gettextgen.po +++ b/wallet/statements_gettextgen.po @@ -1337,4 +1337,4 @@ msgstr "" #: wallet/test/run-wallet.c:1696 msgid "INSERT INTO channels (id) VALUES (1);" msgstr "" -# SHA256STAMP:e203d19d9f4192ad6b3aa1a6f257d4f7b267df56c5de9810b573f0affa0122fd +# SHA256STAMP:3652b5850f08383c0f0c01a7f11c1a22ec2b4ac16018152d2770a73674fb05ec diff --git a/wallet/wallet.c b/wallet/wallet.c index 67cdc4c3c1cf..02ba582d069f 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -530,7 +530,7 @@ static bool deep_enough(u32 maxheight, const struct utxo *utxo, if (utxo->close_info) { u32 csv_free = *utxo->blockheight + utxo->close_info->csv; assert(csv_free > *utxo->blockheight); - if (csv_free < current_blockheight) + if (current_blockheight < csv_free) return false; } return *utxo->blockheight <= maxheight; From f935107d01bbee115c572debe69adac96a91c7c4 Mon Sep 17 00:00:00 2001 From: niftynei Date: Mon, 9 Aug 2021 14:39:37 -0500 Subject: [PATCH 6/8] changelog: v0.10.1 release notes --- CHANGELOG.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dae926c9f16..5f364fd526f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,18 +4,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [0.10.1rc2] - 2021-07-30: "" +## [0.10.1] - 2021-08-09: "eltoo: Ethereum Layer Too" This release named by @nalinbhardwaj. NOTE ONE: Both the dual-funding and offers protocols have changed, and -are incompatible with older releses (they're both still draft) +are incompatible with older releses (they're both still draft) #reckless NOTE TWO: `rebalance` and `drain` plugins will need to be redownloaded as older versions will no longer work -- `payment_secret` is now compulsory. - ### Added - JSON-RPC: `invoice` now outputs explicit `payment_secret` as its own field. ([#4646]) @@ -1390,7 +1389,7 @@ There predate the BOLT specifications, and are only of vague historic interest: 6. [0.5.1] - 2016-10-21 7. [0.5.2] - 2016-11-21: "Bitcoin Savings & Trust Daily Interest II" -[0.10.1rc1]: https://github.com/ElementsProject/lightning/releases/tag/v0.10.1rc1 +[0.10.1]: https://github.com/ElementsProject/lightning/releases/tag/v0.10.1 [0.10.0]: https://github.com/ElementsProject/lightning/releases/tag/v0.10.0 [0.9.2]: https://github.com/ElementsProject/lightning/releases/tag/v0.9.2 [0.9.1]: https://github.com/ElementsProject/lightning/releases/tag/v0.9.1 From 76edaac1a9791385645f9e7af15c830aa95b92d7 Mon Sep 17 00:00:00 2001 From: LightningHelper <35897350+LightningHelper@users.noreply.github.com> Date: Sat, 24 Jul 2021 23:25:54 -0400 Subject: [PATCH 7/8] new line for default value, makes it more visable See here #4670 --- doc/lightning-close.7 | 6 ++++-- doc/lightning-close.7.md | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/lightning-close.7 b/doc/lightning-close.7 index 6f8c2837a20f..f18aabd7f402 100644 --- a/doc/lightning-close.7 +++ b/doc/lightning-close.7 @@ -53,10 +53,12 @@ insist on our fee as much as possible\. .IP \[bu] "100%": our next proposal will be 3000\. This is the most relaxed case when we quickly accept the peer's proposal\. -The default is "50%"\. .RE +The default is "50%"\. + + \fIwrong_funding_txid\fR can only be specified if both sides have offered the "shutdown_wrong_funding" feature (enabled by the \fBexperimental-shutdown-wrong-funding\fR option): it must be a @@ -128,4 +130,4 @@ ZmnSCPxj \fI is mainly responsible\. Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:c36a8ba48c3d2826344e23f880c21e0183942df8523da94394e5786dec874083 +\" SHA256STAMP:a28dd05f6b0a86e1f3d748759ae9bcaa0ffc95686fb904969f07def600c23168 diff --git a/doc/lightning-close.7.md b/doc/lightning-close.7.md index 4d9ab13225b4..0a68841e151f 100644 --- a/doc/lightning-close.7.md +++ b/doc/lightning-close.7.md @@ -45,6 +45,7 @@ proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000: insist on our fee as much as possible. * "100%": our next proposal will be 3000. This is the most relaxed case when we quickly accept the peer's proposal. + The default is "50%". *wrong_funding_txid* can only be specified if both sides have offered From 405b914dd9f74abc36e0ad6fe6adf5ee1191a1be Mon Sep 17 00:00:00 2001 From: niftynei Date: Mon, 9 Aug 2021 19:53:19 -0500 Subject: [PATCH 8/8] changelog: fix typo in release notes Suggested-By: @nalinbhardwaj --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f364fd526f6..7bab2d7478c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 This release named by @nalinbhardwaj. NOTE ONE: Both the dual-funding and offers protocols have changed, and -are incompatible with older releses (they're both still draft) #reckless +are incompatible with older releases (they're both still draft) #reckless NOTE TWO: `rebalance` and `drain` plugins will need to be redownloaded as older versions will no longer work -- `payment_secret` is now compulsory.