From ca69e293d1ea1ac60305481c36d57855fa61f91c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 19 Jun 2022 16:49:11 +0930 Subject: [PATCH] coinmvt: don't use msats in fields not called "_msat". The new msat fields are turned into Millisatoshi, so handle that correctly too in tests too. Signed-off-by: Rusty Russell Changelog-Deprecated: Plugins: `coin_movement` notification: `balance`, `credit`, `debit` and `fees` (use `balance_msat`, `credit_msat`, `debit_msat` and `fees_msat`) --- lightningd/notification.c | 23 +++++++++++++++++----- tests/test_closing.py | 22 ++++++++++----------- tests/test_connection.py | 8 ++++---- tests/test_misc.py | 10 +++++----- tests/test_plugin.py | 28 +++++++++++++-------------- tests/test_wallet.py | 40 +++++++++++++++++++-------------------- tests/utils.py | 30 ++++++++++++++--------------- 7 files changed, 87 insertions(+), 74 deletions(-) diff --git a/lightningd/notification.c b/lightningd/notification.c index 00b29cf71..403ae1187 100644 --- a/lightningd/notification.c +++ b/lightningd/notification.c @@ -1,4 +1,5 @@ #include "config.h" +#include #include #include #include @@ -478,8 +479,13 @@ static void coin_movement_notification_serialize(struct json_stream *stream, json_add_string(stream, "originating_account", mvt->originating_acct); json_mvt_id(stream, mvt->type, &mvt->id); - json_add_amount_msat_only(stream, "credit", mvt->credit); - json_add_amount_msat_only(stream, "debit", mvt->debit); + if (deprecated_apis) { + json_add_amount_msat_only(stream, "credit", mvt->credit); + json_add_amount_msat_only(stream, "debit", mvt->debit); + } + json_add_amount_msat_only(stream, "credit_msat", mvt->credit); + json_add_amount_msat_only(stream, "debit_msat", mvt->debit); + /* Only chain movements */ if (mvt->output_val) json_add_amount_sats_deprecated(stream, "output_value", @@ -489,9 +495,13 @@ static void coin_movement_notification_serialize(struct json_stream *stream, json_add_num(stream, "output_count", mvt->output_count); - if (mvt->fees) - json_add_amount_msat_only(stream, "fees", + if (mvt->fees) { + if (deprecated_apis) + json_add_amount_msat_only(stream, "fees", + *mvt->fees); + json_add_amount_msat_only(stream, "fees_msat", *mvt->fees); + } json_array_start(stream, "tags"); for (size_t i = 0; i < tal_count(mvt->tags); i++) @@ -535,7 +545,10 @@ static void balance_snapshot_notification_serialize(struct json_stream *stream, json_object_start(stream, NULL); json_add_string(stream, "account_id", snap->accts[i]->acct_id); - json_add_amount_msat_only(stream, "balance", + if (deprecated_apis) + json_add_amount_msat_only(stream, "balance", + snap->accts[i]->balance); + json_add_amount_msat_only(stream, "balance_msat", snap->accts[i]->balance); json_add_string(stream, "coin_type", snap->accts[i]->bip173_name); json_object_end(stream); diff --git a/tests/test_closing.py b/tests/test_closing.py index 36c510b63..776201392 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -852,17 +852,17 @@ def test_channel_lease_post_expiry(node_factory, bitcoind, chainparams): l2.daemon.wait_for_log('Resolved FUNDING_TRANSACTION/FUNDING_OUTPUT by MUTUAL_CLOSE') channel_mvts_1 = [ - {'type': 'chain_mvt', 'credit': 506432000, 'debit': 0, 'tags': ['channel_open', 'opener', 'leased']}, - {'type': 'channel_mvt', 'credit': 0, 'debit': 6432000, 'tags': ['lease_fee'], 'fees': '0msat'}, - {'type': 'channel_mvt', 'credit': 0, 'debit': 10000, 'tags': ['invoice'], 'fees': '0msat'}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 499990000, 'tags': ['channel_close']}, + {'type': 'chain_mvt', 'credit_msat': 506432000, 'debit_msat': 0, 'tags': ['channel_open', 'opener', 'leased']}, + {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 6432000, 'tags': ['lease_fee'], 'fees_msat': '0msat'}, + {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 10000, 'tags': ['invoice'], 'fees_msat': '0msat'}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 499990000, 'tags': ['channel_close']}, ] channel_mvts_2 = [ - {'type': 'chain_mvt', 'credit': 500000000, 'debit': 0, 'tags': ['channel_open', 'leased']}, - {'type': 'channel_mvt', 'credit': 6432000, 'debit': 0, 'tags': ['lease_fee'], 'fees': '0msat'}, - {'type': 'channel_mvt', 'credit': 10000, 'debit': 0, 'tags': ['invoice'], 'fees': '0msat'}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 506442000, 'tags': ['channel_close']}, + {'type': 'chain_mvt', 'credit_msat': 500000000, 'debit_msat': 0, 'tags': ['channel_open', 'leased']}, + {'type': 'channel_mvt', 'credit_msat': 6432000, 'debit_msat': 0, 'tags': ['lease_fee'], 'fees_msat': '0msat'}, + {'type': 'channel_mvt', 'credit_msat': 10000, 'debit_msat': 0, 'tags': ['invoice'], 'fees_msat': '0msat'}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 506442000, 'tags': ['channel_close']}, ] check_coin_moves(l1, channel_id, channel_mvts_1, chainparams) @@ -1285,11 +1285,11 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams): if not chainparams['elements']: # Also check snapshots expected_bals_2 = [ - {'blockheight': 101, 'accounts': [{'balance': '0msat'}]}, - {'blockheight': 108, 'accounts': [{'balance': '995433000msat'}, {'balance': '500000000msat'}, {'balance': '499994999msat'}]}, + {'blockheight': 101, 'accounts': [{'balance_msat': '0msat'}]}, + {'blockheight': 108, 'accounts': [{'balance_msat': '995433000msat'}, {'balance_msat': '500000000msat'}, {'balance_msat': '499994999msat'}]}, # There's a duplicate because we stop and restart l2 twice # (both times at block 108) - {'blockheight': 108, 'accounts': [{'balance': '995433000msat'}, {'balance': '500000000msat'}, {'balance': '499994999msat'}]}, + {'blockheight': 108, 'accounts': [{'balance_msat': '995433000msat'}, {'balance_msat': '500000000msat'}, {'balance_msat': '499994999msat'}]}, ] check_balance_snaps(l2, expected_bals_2) diff --git a/tests/test_connection.py b/tests/test_connection.py index 308ca2805..5a643993f 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1157,12 +1157,12 @@ def test_funding_push(node_factory, bitcoind, chainparams): chanid = first_channel_id(l2, l1) channel_mvts_1 = [ - {'type': 'chain_mvt', 'credit': 16777215000, 'debit': 0, 'tags': ['channel_open', 'opener']}, - {'type': 'channel_mvt', 'credit': 0, 'debit': 20000000, 'tags': ['pushed'], 'fees': '0msat'}, + {'type': 'chain_mvt', 'credit_msat': 16777215000, 'debit_msat': 0, 'tags': ['channel_open', 'opener']}, + {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 20000000, 'tags': ['pushed'], 'fees_msat': '0msat'}, ] channel_mvts_2 = [ - {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tags': ['channel_open']}, - {'type': 'channel_mvt', 'credit': 20000000, 'debit': 0, 'tags': ['pushed'], 'fees': '0msat'}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 0, 'tags': ['channel_open']}, + {'type': 'channel_mvt', 'credit_msat': 20000000, 'debit_msat': 0, 'tags': ['pushed'], 'fees_msat': '0msat'}, ] check_coin_moves(l1, chanid, channel_mvts_1, chainparams) check_coin_moves(l2, chanid, channel_mvts_2, chainparams) diff --git a/tests/test_misc.py b/tests/test_misc.py index 37adca971..41b2b0b2b 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -626,11 +626,11 @@ def test_withdraw_misc(node_factory, bitcoind, chainparams): assert account_balance(l1, 'wallet') == 0 external_moves = [ - {'type': 'chain_mvt', 'credit': 2000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 2000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 2000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 2000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 11957603000, 'debit': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 11957603000, 'debit_msat': 0, 'tags': ['deposit']}, ] check_coin_moves(l1, 'external', external_moves, chainparams) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 40a4250ee..ebbf30e19 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -1921,26 +1921,26 @@ def test_coin_movement_notices(node_factory, bitcoind, chainparams): """Verify that channel coin movements are triggered correctly. """ l1_l2_mvts = [ - {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tags': ['channel_open']}, - {'type': 'channel_mvt', 'credit': 100001001, 'debit': 0, 'tags': ['routed'], 'fees': '1001msat'}, - {'type': 'channel_mvt', 'credit': 0, 'debit': 50000000, 'tags': ['routed'], 'fees': '501msat'}, - {'type': 'channel_mvt', 'credit': 100000000, 'debit': 0, 'tags': ['invoice'], 'fees': '0msat'}, - {'type': 'channel_mvt', 'credit': 0, 'debit': 50000000, 'tags': ['invoice'], 'fees': '0msat'}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 100001001, 'tags': ['channel_close']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 0, 'tags': ['channel_open']}, + {'type': 'channel_mvt', 'credit_msat': 100001001, 'debit_msat': 0, 'tags': ['routed'], 'fees_msat': '1001msat'}, + {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 50000000, 'tags': ['routed'], 'fees_msat': '501msat'}, + {'type': 'channel_mvt', 'credit_msat': 100000000, 'debit_msat': 0, 'tags': ['invoice'], 'fees_msat': '0msat'}, + {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 50000000, 'tags': ['invoice'], 'fees_msat': '0msat'}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 100001001, 'tags': ['channel_close']}, ] l2_l3_mvts = [ - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['channel_open', 'opener']}, - {'type': 'channel_mvt', 'credit': 0, 'debit': 100000000, 'tags': ['routed'], 'fees': '1001msat'}, - {'type': 'channel_mvt', 'credit': 50000501, 'debit': 0, 'tags': ['routed'], 'fees': '501msat'}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 950000501, 'tags': ['channel_close']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['channel_open', 'opener']}, + {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 100000000, 'tags': ['routed'], 'fees_msat': '1001msat'}, + {'type': 'channel_mvt', 'credit_msat': 50000501, 'debit_msat': 0, 'tags': ['routed'], 'fees_msat': '501msat'}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 950000501, 'tags': ['channel_close']}, ] l3_l2_mvts = [ - {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tags': ['channel_open']}, - {'type': 'channel_mvt', 'credit': 100000000, 'debit': 0, 'tags': ['invoice'], 'fees': '0msat'}, - {'type': 'channel_mvt', 'credit': 0, 'debit': 50000501, 'tags': ['invoice'], 'fees': '501msat'}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 49999499, 'tags': ['channel_close']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 0, 'tags': ['channel_open']}, + {'type': 'channel_mvt', 'credit_msat': 100000000, 'debit_msat': 0, 'tags': ['invoice'], 'fees_msat': '0msat'}, + {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 50000501, 'tags': ['invoice'], 'fees_msat': '501msat'}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 49999499, 'tags': ['channel_close']}, ] coin_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 8cab032ac..2c6ed0072 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -864,26 +864,26 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): l1.rpc.signpsbt(invalid_psbt) wallet_coin_mvts = [ - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tags': ['deposit']}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tags': ['withdrawal']}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tags': ['withdrawal']}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tags': ['withdrawal']}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tags': ['withdrawal']}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tags': ['withdrawal']}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tags': ['withdrawal']}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tags': ['withdrawal']}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tags': ['withdrawal']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 1000000000, 'debit_msat': 0, 'tags': ['deposit']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 1000000000, 'tags': ['withdrawal']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 1000000000, 'tags': ['withdrawal']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 1000000000, 'tags': ['withdrawal']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 1000000000, 'tags': ['withdrawal']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 1000000000, 'tags': ['withdrawal']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 1000000000, 'tags': ['withdrawal']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 1000000000, 'tags': ['withdrawal']}, + {'type': 'chain_mvt', 'credit_msat': 0, 'debit_msat': 1000000000, 'tags': ['withdrawal']}, ] check_coin_moves(l1, 'wallet', wallet_coin_mvts, chainparams) diff --git a/tests/utils.py b/tests/utils.py index 682dc5350..d06292ec0 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -74,18 +74,18 @@ def expected_channel_features(wumbo_channels=False, extra=[]): def move_matches(exp, mv): if mv['type'] != exp['type']: return False - if mv['credit'] != "{}msat".format(exp['credit']): + if Millisatoshi(mv['credit_msat']) != Millisatoshi(exp['credit_msat']): return False - if mv['debit'] != "{}msat".format(exp['debit']): + if Millisatoshi(mv['debit_msat']) != Millisatoshi(exp['debit_msat']): return False if mv['tags'] != exp['tags']: return False - if 'fees' in exp: - if 'fees' not in mv: + if 'fees_msat' in exp: + if 'fees_msat' not in mv: return False - if mv['fees'] != exp['fees']: + if Millisatoshi(mv['fees_msat']) != Millisatoshi(exp['fees_msat']): return False - elif 'fees' in mv: + elif 'fees_msat' in mv: return False return True @@ -103,8 +103,8 @@ def check_balance_snaps(n, expected_bals): assert snap['blockheight'] == exp['blockheight'] for acct, exp_acct in zip(snap['accounts'], exp['accounts']): # FIXME: also check 'account_id's (these change every run) - for item in ['balance']: - assert acct[item] == exp_acct[item] + for item in ['balance_msat']: + assert Millisatoshi(acct[item]) == Millisatoshi(exp_acct[item]) def check_coin_moves(n, account_id, expected_moves, chainparams): @@ -125,12 +125,12 @@ def check_coin_moves(n, account_id, expected_moves, chainparams): node_id = n.info['id'] acct_moves = [m for m in moves if m['account_id'] == account_id] for mv in acct_moves: - print("{{'type': '{}', 'credit': {}, 'debit': {}, 'tags': '{}' , ['fees'?: '{}']}}," + print("{{'type': '{}', 'credit_msat': {}, 'debit_msat': {}, 'tags': '{}' , ['fees_msat'?: '{}']}}," .format(mv['type'], - Millisatoshi(mv['credit']).millisatoshis, - Millisatoshi(mv['debit']).millisatoshis, + Millisatoshi(mv['credit_msat']).millisatoshis, + Millisatoshi(mv['debit_msat']).millisatoshis, mv['tags'], - mv['fees'] if 'fees' in mv else '')) + mv['fees_msat'] if 'fees_msat' in mv else '')) assert mv['version'] == 2 assert mv['node_id'] == node_id assert mv['timestamp'] > 0 @@ -165,10 +165,10 @@ def account_balance(n, account_id): moves = dedupe_moves(n.rpc.call('listcoinmoves_plugin')['coin_moves']) chan_moves = [m for m in moves if m['account_id'] == account_id] assert len(chan_moves) > 0 - m_sum = 0 + m_sum = Millisatoshi(0) for m in chan_moves: - m_sum += int(m['credit'][:-4]) - m_sum -= int(m['debit'][:-4]) + m_sum += Millisatoshi(m['credit_msat']) + m_sum -= Millisatoshi(m['debit_msat']) return m_sum