From 6b2a3f4dfbb70b38fe4fd5bd318e481da72b9b46 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 28 Aug 2020 12:07:57 +0930 Subject: [PATCH] txprepare: remove old code, switch to plugin. Some minor phrasing differences cause test changes. Signed-off-by: Rusty Russell Changelog-Changed: txprepare reservations stay across restarts: use fundpsbt/reservepsbt/unreservepsbt Changelog-Removed: txprepare `destination` `satoshi` argument form removed (deprecated v0.7.3) --- plugins/txprepare.c | 6 +- tests/test_connection.py | 6 +- tests/test_wallet.py | 24 +++++--- wallet/walletrpc.c | 120 --------------------------------------- 4 files changed, 22 insertions(+), 134 deletions(-) diff --git a/plugins/txprepare.c b/plugins/txprepare.c index 57e1d4964..7aa322ba5 100644 --- a/plugins/txprepare.c +++ b/plugins/txprepare.c @@ -434,21 +434,21 @@ static struct command_result *json_txsend(struct command *cmd, static const struct plugin_command commands[] = { { - "newtxprepare", + "txprepare", "bitcoin", "Create a transaction, with option to spend in future (either txsend and txdiscard)", "Create an unsigned transaction paying {outputs} with optional {feerate}, {minconf} and {utxos}", json_txprepare }, { - "newtxdiscard", + "txdiscard", "bitcoin", "Discard a transaction created by txprepare", "Discard a transcation by {txid}", json_txdiscard }, { - "newtxsend", + "txsend", "bitcoin", "Send a transaction created by txprepare", "Send a transacation by {txid}", diff --git a/tests/test_connection.py b/tests/test_connection.py index f7ab73ffb..1bdd72ac6 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -771,7 +771,7 @@ def test_funding_fail(node_factory, bitcoind): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) # We don't have enough left to cover fees if we try to spend it all. - with pytest.raises(RpcError, match=r'Cannot afford transaction'): + with pytest.raises(RpcError, match=r'Could not afford'): l1.rpc.fundchannel(l2.info['id'], funds) # Should still be connected. @@ -863,7 +863,7 @@ def test_funding_by_utxos(node_factory, bitcoind): utxos = [utxo["txid"] + ":" + str(utxo["output"]) for utxo in l1.rpc.listfunds()["outputs"]] # Fund with utxos we don't own - with pytest.raises(RpcError, match=r"No matching utxo was found from the wallet"): + with pytest.raises(RpcError, match=r"Unknown UTXO "): l3.rpc.fundchannel(l2.info["id"], int(0.01 * 10**8), utxos=utxos) # Fund with an empty array @@ -878,7 +878,7 @@ def test_funding_by_utxos(node_factory, bitcoind): l1.rpc.fundchannel(l3.info["id"], int(0.007 * 10**8), utxos=[utxos[2]]) # Fund another channel with already spent utxos - with pytest.raises(RpcError, match=r"No matching utxo was found from the wallet"): + with pytest.raises(RpcError, match=r"Already spent UTXO "): l1.rpc.fundchannel(l3.info["id"], int(0.01 * 10**8), utxos=utxos) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 3415f5832..53e947970 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -341,8 +341,7 @@ def test_txprepare(node_factory, bitcoind, chainparams): # Try passing unconfirmed utxos unconfirmed_utxo = l1.rpc.withdraw(l1.rpc.newaddr()["bech32"], 10**5) uutxos = [unconfirmed_utxo["txid"] + ":0"] - with pytest.raises(RpcError, match=r"Cannot afford transaction .* use " - "confirmed utxos."): + with pytest.raises(RpcError, match=r"Could not afford"): l1.rpc.txprepare([{addr: Millisatoshi(amount * 3.5 * 1000)}], utxos=uutxos) @@ -357,15 +356,24 @@ def test_txprepare(node_factory, bitcoind, chainparams): # We should have a change output, so this is exact assert len(decode['vout']) == 3 if chainparams['feeoutput'] else 2 - assert decode['vout'][1]['value'] == Decimal(amount * 3.5) / 10**8 - assert decode['vout'][1]['scriptPubKey']['type'] == 'witness_v0_keyhash' - assert decode['vout'][1]['scriptPubKey']['addresses'] == [addr] + # Change output pos is random. + for vout in decode['vout']: + if vout['scriptPubKey']['addresses'] == [addr]: + changeout = vout + + assert changeout['value'] == Decimal(amount * 3.5) / 10**8 + assert changeout['scriptPubKey']['type'] == 'witness_v0_keyhash' + assert changeout['scriptPubKey']['addresses'] == [addr] # Discard prep4 and get all funds again l1.rpc.txdiscard(prep5['txid']) - with pytest.raises(RpcError, match=r'this destination wants all satoshi. The count of outputs can\'t be more than 1'): - prep5 = l1.rpc.txprepare([{addr: Millisatoshi(amount * 3 * 1000)}, - {addr: 'all'}]) + # You can have one which is all, but not two. + prep5 = l1.rpc.txprepare([{addr: Millisatoshi(amount * 3 * 1000)}, + {addr: 'all'}]) + l1.rpc.txdiscard(prep5['txid']) + with pytest.raises(RpcError, match=r"'all'"): + prep5 = l1.rpc.txprepare([{addr: 'all'}, {addr: 'all'}]) + prep5 = l1.rpc.txprepare([{addr: Millisatoshi(amount * 3 * 500 + 100000)}, {addr: Millisatoshi(amount * 3 * 500 - 100000)}]) decode = bitcoind.rpc.decoderawtransaction(prep5['unsigned_tx']) diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 453df3a10..278e30ea4 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -470,126 +470,6 @@ create_tx: return NULL; } -static struct command_result *json_txprepare(struct command *cmd, - const char *buffer, - const jsmntok_t *obj UNNEEDED, - const jsmntok_t *params) -{ - struct unreleased_tx *utx; - struct command_result *res; - struct json_stream *response; - - res = json_prepare_tx(cmd, buffer, params, false, &utx, NULL); - if (res) - return res; - - /* utx will persist past this command. */ - tal_steal(cmd->ld->wallet, utx); - add_unreleased_tx(cmd->ld->wallet, utx); - - response = json_stream_success(cmd); - json_add_tx(response, "unsigned_tx", utx->tx); - json_add_txid(response, "txid", &utx->txid); - return command_success(cmd, response); -} -static const struct json_command txprepare_command = { - "txprepare", - "bitcoin", - json_txprepare, - "Create a transaction, with option to spend in future (either txsend and txdiscard)", - false -}; -AUTODATA(json_command, &txprepare_command); - -static struct command_result *param_unreleased_txid(struct command *cmd, - const char *name, - const char *buffer, - const jsmntok_t *tok, - struct unreleased_tx **utx) -{ - struct command_result *res; - struct bitcoin_txid *txid; - - res = param_txid(cmd, name, buffer, tok, &txid); - if (res) - return res; - - *utx = find_unreleased_tx(cmd->ld->wallet, txid); - if (!*utx) - return command_fail(cmd, LIGHTNINGD, - "%s not an unreleased txid", - type_to_string(cmd, struct bitcoin_txid, - txid)); - tal_free(txid); - return NULL; -} - -static struct command_result *json_txsend(struct command *cmd, - const char *buffer, - const jsmntok_t *obj UNNEEDED, - const jsmntok_t *params) -{ - struct unreleased_tx *utx; - - if (!param(cmd, buffer, params, - p_req("txid", param_unreleased_txid, &utx), - NULL)) - return command_param_failed(); - - /* We delete from list now, and this command owns it. */ - remove_unreleased_tx(utx); - tal_steal(cmd, utx); - - /* We're the owning cmd now. */ - utx->wtx->cmd = cmd; - - wallet_transaction_add(cmd->ld->wallet, utx->tx->wtx, 0, 0); - wallet_transaction_annotate(cmd->ld->wallet, &utx->txid, - TX_UNKNOWN, 0); - - return broadcast_and_wait(cmd, utx); -} - -static const struct json_command txsend_command = { - "txsend", - "bitcoin", - json_txsend, - "Sign and broadcast a transaction created by txprepare", - false -}; -AUTODATA(json_command, &txsend_command); - -static struct command_result *json_txdiscard(struct command *cmd, - const char *buffer, - const jsmntok_t *obj UNNEEDED, - const jsmntok_t *params) -{ - struct unreleased_tx *utx; - struct json_stream *response; - - if (!param(cmd, buffer, params, - p_req("txid", param_unreleased_txid, &utx), - NULL)) - return command_param_failed(); - - /* Free utx with this command */ - tal_steal(cmd, utx); - - response = json_stream_success(cmd); - json_add_tx(response, "unsigned_tx", utx->tx); - json_add_txid(response, "txid", &utx->txid); - return command_success(cmd, response); -} - -static const struct json_command txdiscard_command = { - "txdiscard", - "bitcoin", - json_txdiscard, - "Abandon a transaction created by txprepare", - false -}; -AUTODATA(json_command, &txdiscard_command); - /** * json_withdraw - Entrypoint for the withdrawal flow *