From 4dafeede5c81d1ba7626480083ad4bb6716a5f70 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 25 Jan 2022 14:24:31 -0600 Subject: [PATCH] coin moves: notify when we make deposits to external accounts The blockheight is zero though, since these aren't included in a block yet. We also don't issue an 'external' deposit event if we can tell that the address you're sending to actually belongs to our wallet (we'll issue a deposit event when it gets included in a block) --- common/psbt_open.c | 17 +++++++++++ common/psbt_open.h | 12 ++++++++ doc/PLUGINS.md | 4 ++- plugins/spender/multiwithdraw.c | 16 +++++++--- plugins/txprepare.c | 12 ++++++++ tests/test_misc.py | 12 +++++++- tests/test_wallet.py | 28 ++++++++++++++++-- tests/utils.py | 2 +- wallet/walletrpc.c | 52 ++++++++++++++++++++++++++++++++- 9 files changed, 144 insertions(+), 11 deletions(-) diff --git a/common/psbt_open.c b/common/psbt_open.c index b7a6f330c..646b4ea02 100644 --- a/common/psbt_open.c +++ b/common/psbt_open.c @@ -491,6 +491,23 @@ bool psbt_has_our_input(const struct wally_psbt *psbt) return false; } +void psbt_output_mark_as_external(const tal_t *ctx, + struct wally_psbt_output *output) +{ + u8 *key = psbt_make_key(tmpctx, PSBT_TYPE_OUTPUT_EXTERNAL, NULL); + beint16_t bev = cpu_to_be16(1); + + psbt_output_set_unknown(ctx, output, key, &bev, sizeof(bev)); +} + +bool psbt_output_to_external(const struct wally_psbt_output *output) +{ + size_t unused; + void *result = psbt_get_lightning(&output->unknowns, + PSBT_TYPE_OUTPUT_EXTERNAL, &unused); + return !(!result); +} + bool psbt_contribs_changed(struct wally_psbt *orig, struct wally_psbt *new) { diff --git a/common/psbt_open.h b/common/psbt_open.h index 8e41a4e13..134a5da65 100644 --- a/common/psbt_open.h +++ b/common/psbt_open.h @@ -37,6 +37,7 @@ struct psbt_changeset { #define PSBT_TYPE_SERIAL_ID 0x01 #define PSBT_TYPE_INPUT_MARKER 0x02 +#define PSBT_TYPE_OUTPUT_EXTERNAL 0x04 /* psbt_get_serial_id - Returns the serial_id from an unknowns map * @@ -177,6 +178,17 @@ bool psbt_input_is_ours(const struct wally_psbt_input *input); */ bool psbt_has_our_input(const struct wally_psbt *psbt); +/* psbt_output_mark_external - Marks an output as a deposit to + * an external address. + * Used when withdrawing from internal + * wallet */ +void psbt_output_mark_as_external(const tal_t *ctx, + struct wally_psbt_output *output); + +/* psbt_output_to_external - Is this an output we're paying to an external + * party? */ +bool psbt_output_to_external(const struct wally_psbt_output *output); + /* psbt_contribs_changed - Returns true if the psbt's inputs/outputs * have changed. * diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index 32ce9229a..c6e1af1ac 100644 --- a/doc/PLUGINS.md +++ b/doc/PLUGINS.md @@ -788,7 +788,9 @@ both the debit/credit contain fees. Technically routed debits are the - `lease_fee`: amount paid as lease fee - `leased`: tags channel_open, channel contains leased funds -`blockheight` is the block the txid is included in. +`blockheight` is the block the txid is included in. `channel_mvt`s will be null, +so will the blockheight for withdrawals to external parties (we issue these events +when we send the tx containing them, before they're included in the chain). The `timestamp` is seconds since Unix epoch of the node's machine time at the time lightningd broadcasts the notification. diff --git a/plugins/spender/multiwithdraw.c b/plugins/spender/multiwithdraw.c index afe7a3534..b89638c68 100644 --- a/plugins/spender/multiwithdraw.c +++ b/plugins/spender/multiwithdraw.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -44,6 +45,8 @@ struct multiwithdraw_destination { struct amount_sat amount; /* Whether the amount was "all". */ bool all; + /* Whether this is to an external addr (all passed in are assumed) */ + bool is_to_external; }; struct multiwithdraw_command { @@ -93,6 +96,7 @@ param_outputs_array(struct command *cmd, enum address_parse_result res; dest = &(*outputs)[i]; + dest->is_to_external = true; if (e->type != JSMN_OBJECT) goto err; @@ -537,6 +541,7 @@ mw_after_newaddr(struct command *cmd, change.script = script; change.amount = mw->change_amount; change.all = false; + change.is_to_external = false; tal_arr_expand(&mw->outputs, change); @@ -560,15 +565,18 @@ mw_load_outputs(struct multiwithdraw_command *mw) { /* Insert outputs at random locations. */ for (size_t i = 0; i < tal_count(mw->outputs); ++i) { + struct wally_psbt_output *out; /* There are already `i` outputs at this point, * select from 0 to `i` inclusive, with 0 meaning * "before first output" and `i` meaning "after * last output". */ size_t point = pseudorand(i + 1); - psbt_insert_output(mw->psbt, - mw->outputs[i].script, - mw->outputs[i].amount, - point); + out = psbt_insert_output(mw->psbt, + mw->outputs[i].script, + mw->outputs[i].amount, + point); + if (mw->outputs[i].is_to_external) + psbt_output_mark_as_external(mw->psbt, out); } if (chainparams->is_elements) { diff --git a/plugins/txprepare.c b/plugins/txprepare.c index 82776adc0..4de70d44f 100644 --- a/plugins/txprepare.c +++ b/plugins/txprepare.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -13,6 +14,7 @@ struct tx_output { struct amount_sat amount; const u8 *script; + bool is_to_external; }; struct txprepare { @@ -79,6 +81,9 @@ static struct command_result *param_outputs(struct command *cmd, enum address_parse_result res; struct tx_output *out = &txp->outputs[i]; + /* We assume these are accounted for elsewhere */ + out->is_to_external = false; + /* output format: {destination: amount} */ if (t->type != JSMN_OBJECT) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, @@ -190,6 +195,11 @@ static struct command_result *finish_txprepare(struct command *cmd, struct amount_sat, &txp->outputs[i].amount)); psbt_add_output(txp->psbt, out, i); + + if (txp->outputs[i].is_to_external) + psbt_output_mark_as_external(txp->psbt, + &txp->psbt->outputs[i]); + wally_tx_output_free(out); } @@ -239,6 +249,7 @@ static struct command_result *newaddr_done(struct command *cmd, sizeof(txp->outputs[0]) * (num - pos)); txp->outputs[pos].amount = txp->change_amount; + txp->outputs[pos].is_to_external = false; if (json_to_address_scriptpubkey(txp, chainparams, buf, addr, &txp->outputs[pos].script) != ADDRESS_PARSE_SUCCESS) { @@ -516,6 +527,7 @@ static struct command_result *json_withdraw(struct command *cmd, } txp->outputs[0].amount = *amount; txp->outputs[0].script = scriptpubkey; + txp->outputs[0].is_to_external = true; txp->weight = bitcoin_tx_core_weight(1, tal_count(txp->outputs)) + bitcoin_tx_output_weight(tal_bytelen(scriptpubkey)); diff --git a/tests/test_misc.py b/tests/test_misc.py index 3cde1fad7..3f13b1cad 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -10,7 +10,7 @@ from pyln.testing.utils import ( wait_for, TailableProc, env ) from utils import ( - account_balance, scriptpubkey_addr + account_balance, scriptpubkey_addr, check_coin_moves ) from ephemeral_port_reserve import reserve from utils import EXPERIMENTAL_FEATURES @@ -623,6 +623,16 @@ def test_withdraw_misc(node_factory, bitcoind, chainparams): sync_blockheight(bitcoind, [l1]) 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']}, + ] + + check_coin_moves(l1, 'external', external_moves, chainparams) + def test_io_logging(node_factory, executor): l1 = node_factory.get_node(options={'log-level': 'io'}) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 4aaf7557d..f7c601b26 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -7,6 +7,7 @@ from pyln.client import RpcError, Millisatoshi from utils import ( only_one, wait_for, sync_blockheight, EXPERIMENTAL_FEATURES, VALGRIND, check_coin_moves, TailableProc, scriptpubkey_addr, + check_utxos_channel ) import os @@ -223,7 +224,8 @@ def test_addfunds_from_block(node_factory, bitcoind): """Send funds to the daemon without telling it explicitly """ # Previous runs with same bitcoind can leave funds! - l1 = node_factory.get_node(random_hsm=True) + coin_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') + l1 = node_factory.get_node(random_hsm=True, options={'plugin': coin_plugin}) addr = l1.rpc.newaddr()['bech32'] bitcoind.rpc.sendtoaddress(addr, 0.1) @@ -248,6 +250,15 @@ def test_addfunds_from_block(node_factory, bitcoind): output = only_one(l1.rpc.listfunds()['outputs']) assert output['address'] == addr + # We don't print a 'external deposit' event + # for funds that come back to our own wallet + expected_utxos = { + '0': [('wallet', ['deposit'], ['withdrawal'], 'A')], + 'A': [('wallet', ['deposit'], None, None)], + } + + check_utxos_channel(l1, [], expected_utxos) + def test_txprepare_multi(node_factory, bitcoind): amount = 10000000 @@ -1298,11 +1309,13 @@ def test_withdraw_nlocktime_fuzz(node_factory, bitcoind): raise Exception("No transaction with fuzzed nLockTime !") -def test_multiwithdraw_simple(node_factory, bitcoind): +def test_multiwithdraw_simple(node_factory, bitcoind, chainparams): """ Test simple multiwithdraw usage. """ - l1, l2, l3 = node_factory.get_nodes(3) + coin_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') + l1, l2, l3 = node_factory.get_nodes(3, opts=[{'plugin': coin_plugin}, + {}, {}]) l1.fundwallet(10**8) addr2 = l2.rpc.newaddr()['bech32'] @@ -1329,6 +1342,15 @@ def test_multiwithdraw_simple(node_factory, bitcoind): assert only_one(funds3)["status"] == "confirmed" assert only_one(funds3)["amount_msat"] == amount3 + expected_utxos = { + '0': [('wallet', ['deposit'], ['withdrawal'], 'A')], + 'A': [('wallet', ['deposit'], None, None), + ('external', ['deposit'], None, None), + ('external', ['deposit'], None, None)], + } + + check_utxos_channel(l1, [], expected_utxos) + @unittest.skipIf( TEST_NETWORK == 'liquid-regtest', diff --git a/tests/utils.py b/tests/utils.py index d10fdada3..1d417a2da 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -136,7 +136,7 @@ def check_coin_moves(n, account_id, expected_moves, chainparams): assert mv['timestamp'] > 0 assert mv['coin_type'] == chainparams['bip173_prefix'] # chain moves should have blockheights - if mv['type'] == 'chain_mvt': + if mv['type'] == 'chain_mvt' and mv['account_id'] != 'external': assert mv['blockheight'] is not None for num, m in enumerate(expected_moves): diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 5aac2c19d..584dfa815 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -10,11 +10,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -753,9 +755,48 @@ struct sending_psbt { struct command *cmd; struct utxo **utxos; struct wally_tx *wtx; + /* Hold onto b/c has data about + * which are to external addresses */ + struct wally_psbt *psbt; u32 reserve_blocks; }; +static void maybe_notify_new_external_send(struct lightningd *ld, + struct bitcoin_txid *txid, + u32 outnum, + struct wally_psbt *psbt) +{ + struct chain_coin_mvt *mvt; + struct bitcoin_outpoint outpoint; + struct amount_sat amount; + u32 index; + bool is_p2sh; + const u8 *script; + + /* If it's not going to an external address, ignore */ + if (!psbt_output_to_external(&psbt->outputs[outnum])) + return; + + /* If it's going to our wallet, ignore */ + script = wally_tx_output_get_script(tmpctx, + &psbt->tx->outputs[outnum]); + if (wallet_can_spend(ld->wallet, script, &index, &is_p2sh)) + return; + + outpoint.txid = *txid; + outpoint.n = outnum; + amount = psbt_output_get_amount(psbt, outnum); + + mvt = new_coin_external_deposit(NULL, &outpoint, + 0, amount, + DEPOSIT); + + mvt->originating_acct = WALLET; + notify_chain_mvt(ld, mvt); + tal_free(mvt); +} + + static void sendpsbt_done(struct bitcoind *bitcoind UNUSED, bool success, const char *msg, struct sending_psbt *sending) @@ -787,9 +828,12 @@ static void sendpsbt_done(struct bitcoind *bitcoind UNUSED, /* Extract the change output and add it to the DB */ wallet_extract_owned_outputs(ld->wallet, sending->wtx, NULL, &change); + wally_txid(sending->wtx, &txid); + + for (size_t i = 0; i < sending->psbt->num_outputs; i++) + maybe_notify_new_external_send(ld, &txid, i, sending->psbt); response = json_stream_success(sending->cmd); - wally_txid(sending->wtx, &txid); json_add_hex_talarr(response, "tx", linearize_wtx(tmpctx, sending->wtx)); json_add_txid(response, "txid", &txid); was_pending(command_success(sending->cmd, response)); @@ -818,6 +862,12 @@ static struct command_result *json_sendpsbt(struct command *cmd, psbt_finalize(psbt); sending->wtx = psbt_final_tx(sending, psbt); + + /* psbt contains info about which outputs are to external, + * and thus need a coin_move issued for them. We only + * notify if the transaction broadcasts */ + sending->psbt = tal_steal(sending, psbt); + if (!sending->wtx) return command_fail(cmd, LIGHTNINGD, "PSBT not finalizeable %s",