From 7767b68ee9a48985c01d51b7bc29c21b639fea40 Mon Sep 17 00:00:00 2001 From: Mark Beckwith Date: Thu, 5 Apr 2018 14:19:47 -0500 Subject: [PATCH] Removed redundancies in withdraw and fundchannel. No new functionality, just a continuation of my work toward completing #665. I removed the common members of `struct withdrawal` and `struct fund_channel` and placed them in a new `struct wallet_tx`. Then it was fairly straightforward to reimplement the existing code in terms of `wallet_tx`. Since I made some structural changes I wanted to get this approved before I go any farther. Added 'all' to fundchannel help message. --- common/Makefile | 1 + common/wallet_tx.c | 48 +++++++++++++++ common/wallet_tx.h | 25 ++++++++ lightningd/Makefile | 2 +- lightningd/build_utxos.c | 34 ----------- lightningd/build_utxos.h | 15 ----- lightningd/jsonrpc.c | 13 ++++ lightningd/jsonrpc.h | 5 ++ lightningd/opening_control.c | 115 ++++++++++++----------------------- lightningd/peer_control.c | 1 - wallet/walletrpc.c | 89 ++++++++------------------- 11 files changed, 157 insertions(+), 191 deletions(-) create mode 100644 common/wallet_tx.c create mode 100644 common/wallet_tx.h delete mode 100644 lightningd/build_utxos.c delete mode 100644 lightningd/build_utxos.h diff --git a/common/Makefile b/common/Makefile index 17f043b23..94d4aa2db 100644 --- a/common/Makefile +++ b/common/Makefile @@ -42,6 +42,7 @@ COMMON_SRC_NOGEN := \ common/utils.c \ common/utxo.c \ common/version.c \ + common/wallet_tx.c \ common/wireaddr.c \ common/wire_error.c \ common/withdraw_tx.c diff --git a/common/wallet_tx.c b/common/wallet_tx.c new file mode 100644 index 000000000..bc0ca3af6 --- /dev/null +++ b/common/wallet_tx.c @@ -0,0 +1,48 @@ +#include +#include +#include +#include + +void wtx_init(struct command *cmd, struct wallet_tx * wtx) +{ + wtx->cmd = cmd; + wtx->amount = 0; + wtx->change_key_index = 0; + wtx->utxos = NULL; + wtx->all_funds = false; +} + +bool wtx_select_utxos(struct wallet_tx * tx, u32 fee_rate_per_kw, + size_t out_len) +{ + u64 fee_estimate; + if (tx->all_funds) { + tx->utxos = wallet_select_all(tx->cmd, tx->cmd->ld->wallet, + fee_rate_per_kw, out_len, + &tx->amount, + &fee_estimate); + if (!tx->utxos || tx->amount < 546) { + command_fail(tx->cmd, "Cannot afford fee %"PRIu64, + fee_estimate); + return false; + } + tx->change = 0; + } else { + tx->utxos = wallet_select_coins(tx->cmd, tx->cmd->ld->wallet, + tx->amount, + fee_rate_per_kw, out_len, + &fee_estimate, &tx->change); + if (!tx->utxos || tx->amount < 546) { + command_fail(tx->cmd, + "Cannot afford funding transaction"); + return false; + } + if (tx->change < 546) { + tx->change = 0; + tx->change_key_index = 0; + } else { + tx->change_key_index = wallet_get_newindex(tx->cmd->ld); + } + } + return true; +} diff --git a/common/wallet_tx.h b/common/wallet_tx.h new file mode 100644 index 000000000..716fac36a --- /dev/null +++ b/common/wallet_tx.h @@ -0,0 +1,25 @@ +#ifndef LIGHTNING_COMMON_WALLET_TX_H +#define LIGHTNING_COMMON_WALLET_TX_H +#include "config.h" +#include +#include +#include +#include + +/* A specification of funds in the wallet used for funding channels and + * withdrawal. + */ +struct wallet_tx { + struct command *cmd; + u64 amount; + u64 change; + u32 change_key_index; + const struct utxo **utxos; + + bool all_funds; +}; + +void wtx_init(struct command *cmd, struct wallet_tx *wtx); +bool wtx_select_utxos(struct wallet_tx * tx, u32 fee_rate_per_kw, + size_t out_len); +#endif /* LIGHTNING_COMMON_WALLET_TX_H */ diff --git a/lightningd/Makefile b/lightningd/Makefile index d93ea956b..0615a70a3 100644 --- a/lightningd/Makefile +++ b/lightningd/Makefile @@ -44,13 +44,13 @@ LIGHTNINGD_COMMON_OBJS := \ common/utils.o \ common/utxo.o \ common/version.o \ + common/wallet_tx.o \ common/wire_error.o \ common/wireaddr.o \ common/withdraw_tx.o LIGHTNINGD_SRC := \ lightningd/bitcoind.c \ - lightningd/build_utxos.c \ lightningd/chaintopology.c \ lightningd/channel.c \ lightningd/channel_control.c \ diff --git a/lightningd/build_utxos.c b/lightningd/build_utxos.c deleted file mode 100644 index e52d56943..000000000 --- a/lightningd/build_utxos.c +++ /dev/null @@ -1,34 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include - - -const struct utxo **build_utxos(const tal_t *ctx, - struct lightningd *ld, u64 satoshi_out, - u32 feerate_per_kw, u64 dust_limit, - size_t outputscriptlen, - u64 *change_satoshis, u32 *change_keyindex) -{ - u64 fee_estimate = 0; - const struct utxo **utxos = - wallet_select_coins(ctx, ld->wallet, satoshi_out, feerate_per_kw, - outputscriptlen, - &fee_estimate, change_satoshis); - - /* Oops, didn't have enough coins available */ - if (!utxos) - return NULL; - - /* Do we need a change output? */ - if (*change_satoshis < dust_limit) { - *change_satoshis = 0; - *change_keyindex = 0; - } else { - *change_keyindex = wallet_get_newindex(ld); - } - return utxos; -} diff --git a/lightningd/build_utxos.h b/lightningd/build_utxos.h deleted file mode 100644 index 305d89067..000000000 --- a/lightningd/build_utxos.h +++ /dev/null @@ -1,15 +0,0 @@ -#ifndef LIGHTNING_LIGHTNINGD_BUILD_UTXOS_H -#define LIGHTNING_LIGHTNINGD_BUILD_UTXOS_H -#include "config.h" -#include -#include - -/* Reserves UTXOs to build tx which pays this amount; returns NULL if - * impossible. */ -const struct utxo **build_utxos(const tal_t *ctx, - struct lightningd *ld, u64 satoshi_out, - u32 feerate_per_kw, u64 dust_limit, - size_t outputscriptlen, - u64 *change_satoshis, u32 *change_keyindex); - -#endif /* LIGHTNING_LIGHTNINGD_BUILD_UTXOS_H */ diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 1ffc00160..02a109377 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -852,3 +853,15 @@ json_tok_address_scriptpubkey(const tal_t *cxt, return ADDRESS_PARSE_UNRECOGNIZED; } + +bool json_tok_wtx(struct wallet_tx * tx, const char * buffer, + const jsmntok_t *sattok) +{ + if (json_tok_streq(buffer, sattok, "all")) { + tx->all_funds = true; + } else if (!json_tok_u64(buffer, sattok, &tx->amount)) { + command_fail(tx->cmd, "Invalid satoshis"); + return false; + } + return true; +} diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h index 33f7344d7..0cb8ff867 100644 --- a/lightningd/jsonrpc.h +++ b/lightningd/jsonrpc.h @@ -8,6 +8,7 @@ struct bitcoin_txid; struct wireaddr; +struct wallet_tx; /* Context for a command (from JSON, but might outlive the connection!) * You can allocate off this for temporary objects. */ @@ -101,5 +102,9 @@ json_tok_address_scriptpubkey(const tal_t *ctx, const char *buffer, const jsmntok_t *tok, const u8 **scriptpubkey); +/* Parse the satoshi token in wallet_tx. */ +bool json_tok_wtx(struct wallet_tx * tx, const char * buffer, + const jsmntok_t * sattok); + AUTODATA_TYPE(json_command, struct json_command); #endif /* LIGHTNING_LIGHTNINGD_JSONRPC_H */ diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 4f12e7661..46aea9725 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -4,11 +4,11 @@ #include #include #include +#include #include #include #include #include -#include #include #include #include @@ -60,22 +60,19 @@ struct uncommitted_channel { struct channel_config our_config; }; + struct funding_channel { /* In lightningd->fundchannels while waiting for gossipd reply. */ struct list_node list; struct command *cmd; /* Which also owns us. */ + struct wallet_tx wtx; + u64 push_msat; + u8 channel_flags; /* Peer we're trying to reach. */ struct pubkey peerid; - /* Details of how to make funding. */ - const struct utxo **utxomap; - u64 change; - u32 change_keyindex; - u64 funding_satoshi, push_msat; - u8 channel_flags; - /* Channel. */ struct uncommitted_channel *uc; }; @@ -159,7 +156,7 @@ void json_add_uncommitted_channel(struct json_result *response, if (uc->fc) { u64 msatoshi_total, our_msatoshi; - msatoshi_total = uc->fc->funding_satoshi * 1000; + msatoshi_total = uc->fc->wtx.amount * 1000; our_msatoshi = msatoshi_total - uc->fc->push_msat; json_add_u64(response, "msatoshi_to_us", our_msatoshi); json_add_u64(response, "msatoshi_total", msatoshi_total); @@ -324,18 +321,18 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, &channel_info.remote_per_commit)); /* Generate the funding tx. */ - if (fc->change + if (fc->wtx.change && !bip32_pubkey(ld->wallet->bip32_base, - &changekey, fc->change_keyindex)) - fatal("Error deriving change key %u", fc->change_keyindex); + &changekey, fc->wtx.change_key_index)) + fatal("Error deriving change key %u", fc->wtx.change_key_index); derive_basepoints(&fc->uc->seed, &local_fundingkey, NULL, NULL, NULL); fundingtx = funding_tx(tmpctx, &funding_outnum, - fc->utxomap, fc->funding_satoshi, + fc->wtx.utxos, fc->wtx.amount, &local_fundingkey, &channel_info.remote_fundingkey, - fc->change, &changekey, + fc->wtx.change, &changekey, ld->wallet->bip32_base); log_debug(fc->uc->log, "Funding tx has %zi inputs, %zu outputs:", @@ -344,8 +341,8 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, for (size_t i = 0; i < tal_count(fundingtx->input); i++) { log_debug(fc->uc->log, "%zi: %"PRIu64" satoshi (%s) %s\n", - i, fc->utxomap[i]->amount, - fc->utxomap[i]->is_p2sh ? "P2SH" : "SEGWIT", + i, fc->wtx.utxos[i]->amount, + fc->wtx.utxos[i]->is_p2sh ? "P2SH" : "SEGWIT", type_to_string(tmpctx, struct bitcoin_txid, &fundingtx->input[i].txid)); } @@ -358,8 +355,8 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, " satoshi %"PRIu64" change %"PRIu64 " changeidx %u" " localkey %s remotekey %s", - fc->funding_satoshi, - fc->change, fc->change_keyindex, + fc->wtx.amount, + fc->wtx.change, fc->wtx.change_key_index, type_to_string(fc, struct pubkey, &local_fundingkey), type_to_string(fc, struct pubkey, @@ -369,8 +366,8 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, " satoshi %"PRIu64" change %"PRIu64 " changeidx %u" " localkey %s remotekey %s", - fc->funding_satoshi, - fc->change, fc->change_keyindex, + fc->wtx.amount, + fc->wtx.change, fc->wtx.change_key_index, type_to_string(fc, struct pubkey, &local_fundingkey), type_to_string(fc, struct pubkey, @@ -384,7 +381,7 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, &remote_commit_sig, &funding_txid, funding_outnum, - fc->funding_satoshi, + fc->wtx.amount, fc->push_msat, fc->channel_flags, &channel_info, @@ -398,10 +395,10 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, log_debug(channel->log, "Getting HSM to sign funding tx"); msg = towire_hsm_sign_funding(tmpctx, channel->funding_satoshi, - fc->change, fc->change_keyindex, + fc->wtx.change, fc->wtx.change_key_index, &local_fundingkey, &channel_info.remote_fundingkey, - fc->utxomap); + fc->wtx.utxos); if (!wire_sync_write(ld->hsm_fd, take(msg))) fatal("Could not write to HSM: %s", strerror(errno)); @@ -430,7 +427,7 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, /* Start normal channel daemon. */ peer_start_channeld(channel, &cs, fds[0], fds[1], NULL, false); - wallet_confirm_utxos(ld->wallet, fc->utxomap); + wallet_confirm_utxos(ld->wallet, fc->wtx.utxos); response = new_json_result(fc->cmd); json_object_start(response, NULL); @@ -814,13 +811,13 @@ static void peer_offer_channel(struct lightningd *ld, cs, &fc->uc->seed); subd_send_msg(fc->uc->openingd, take(msg)); - msg = towire_opening_funder(fc, fc->funding_satoshi, + msg = towire_opening_funder(fc, fc->wtx.amount, fc->push_msat, get_feerate(ld->topology, FEERATE_NORMAL), max_minimum_depth, - fc->change, fc->change_keyindex, + fc->wtx.change, fc->wtx.change_key_index, fc->channel_flags, - fc->utxomap, + fc->wtx.utxos, ld->wallet->bip32_base); subd_req(fc, fc->uc->openingd, @@ -901,67 +898,35 @@ static void json_fund_channel(struct command *cmd, const char *buffer, const jsmntok_t *params) { jsmntok_t *desttok, *sattok; - bool all_funds = false; - struct funding_channel * fc; + struct funding_channel * fc = tal(cmd, struct funding_channel); u32 feerate_per_kw = get_feerate(cmd->ld->topology, FEERATE_NORMAL); - u64 fee_estimate; u8 *msg; - if (!json_get_params(cmd, buffer, params, - "id", &desttok, - "satoshi", &sattok, - NULL)) { - return; - } - - fc = tal(cmd, struct funding_channel); - fc->uc = NULL; fc->cmd = cmd; - fc->change_keyindex = 0; - fc->funding_satoshi = 0; - - if (json_tok_streq(buffer, sattok, "all")) { - all_funds = true; - } else if (!json_tok_u64(buffer, sattok, &fc->funding_satoshi)) { - command_fail(cmd, "Invalid satoshis"); + fc->uc = NULL; + wtx_init(cmd, &fc->wtx); + if (!json_get_params(fc->cmd, buffer, params, + "id", &desttok, + "satoshi", &sattok, NULL)) + return; + if (!json_tok_wtx(&fc->wtx, buffer, sattok)) return; - } - if (!pubkey_from_hexstr(buffer + desttok->start, - desttok->end - desttok->start, &fc->peerid)) { + desttok->end - desttok->start, + &fc->peerid)) { command_fail(cmd, "Could not parse id"); return; } + /* FIXME: Support push_msat? */ fc->push_msat = 0; fc->channel_flags = OUR_CHANNEL_FLAGS; - /* Try to do this now, so we know if insufficient funds. */ - /* FIXME: dustlimit */ - if (all_funds) { - fc->utxomap = wallet_select_all(cmd, cmd->ld->wallet, - feerate_per_kw, - BITCOIN_SCRIPTPUBKEY_P2WSH_LEN, - &fc->funding_satoshi, - &fee_estimate); - if (!fc->utxomap || fc->funding_satoshi < 546) { - command_fail(cmd, "Cannot afford fee %"PRIu64, - fee_estimate); - return; - } - fc->change = 0; - } else { - fc->utxomap = build_utxos(fc, cmd->ld, fc->funding_satoshi, - feerate_per_kw, - 600, BITCOIN_SCRIPTPUBKEY_P2WSH_LEN, - &fc->change, &fc->change_keyindex); - if (!fc->utxomap) { - command_fail(cmd, "Cannot afford funding transaction"); - return; - } - } + if (!wtx_select_utxos(&fc->wtx, feerate_per_kw, + BITCOIN_SCRIPTPUBKEY_P2WSH_LEN)) + return; - if (fc->funding_satoshi > MAX_FUNDING_SATOSHI) { + if (fc->wtx.amount > MAX_FUNDING_SATOSHI) { command_fail(cmd, "Funding satoshi must be <= %d", MAX_FUNDING_SATOSHI); return; @@ -978,6 +943,6 @@ static void json_fund_channel(struct command *cmd, static const struct json_command fund_channel_command = { "fundchannel", json_fund_channel, - "Fund channel with {id} using {satoshi} satoshis" + "Fund channel with {id} using {satoshi} (or 'all') satoshis" }; AUTODATA(json_command, &fund_channel_command); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index d91f8011f..c068b044b 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 123a5d0a9..d35bde6ee 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -23,11 +24,9 @@ #include struct withdrawal { - u64 amount, changesatoshi; - u8 *destination; - const struct utxo **utxos; - u64 change_key_index; struct command *cmd; + struct wallet_tx wtx; + u8 *destination; const char *hextx; }; @@ -52,7 +51,7 @@ static void wallet_withdrawal_broadcast(struct bitcoind *bitcoind UNUSED, char *output = tal_strjoin(cmd, tal_strsplit(cmd, msg, "\n", STR_NO_EMPTY), " ", STR_NO_TRAIL); if (exitstatus == 0) { /* Mark used outputs as spent */ - wallet_confirm_utxos(ld->wallet, withdraw->utxos); + wallet_confirm_utxos(ld->wallet, withdraw->wtx.utxos); /* Parse the tx and extract the change output. We * generated the hex tx, so this should always work */ @@ -60,9 +59,9 @@ static void wallet_withdrawal_broadcast(struct bitcoind *bitcoind UNUSED, assert(tx != NULL); wallet_extract_owned_outputs(ld->wallet, tx, NULL, &change_satoshi); - /* Note normally, change_satoshi == withdraw->changesatoshi, but + /* Note normally, change_satoshi == withdraw->wtx.change, but * not if we're actually making a payment to ourselves! */ - assert(change_satoshi >= withdraw->changesatoshi); + assert(change_satoshi >= withdraw->wtx.change); struct json_result *response = new_json_result(cmd); json_object_start(response, NULL); @@ -86,29 +85,21 @@ static void json_withdraw(struct command *cmd, const char *buffer, const jsmntok_t *params) { jsmntok_t *desttok, *sattok; - struct withdrawal *withdraw; + struct withdrawal *withdraw = tal(cmd, struct withdrawal); + u32 feerate_per_kw = get_feerate(cmd->ld->topology, FEERATE_NORMAL); - u64 fee_estimate; struct bitcoin_tx *tx; - bool all_funds = false; + enum address_parse_result addr_parse; - if (!json_get_params(cmd, buffer, params, - "destination", &desttok, - "satoshi", &sattok, - NULL)) { - return; - } - - withdraw = tal(cmd, struct withdrawal); withdraw->cmd = cmd; - - if (json_tok_streq(buffer, sattok, "all")) - all_funds = true; - else if (!json_tok_u64(buffer, sattok, &withdraw->amount)) { - command_fail(cmd, "Invalid satoshis"); + wtx_init(cmd, &withdraw->wtx); + if (!json_get_params(withdraw->cmd, buffer, params, + "destination", &desttok, + "satoshi", &sattok, NULL)) + return; + if (!json_tok_wtx(&withdraw->wtx, buffer, sattok)) return; - } /* Parse address. */ addr_parse = json_tok_address_scriptpubkey(cmd, @@ -125,53 +116,21 @@ static void json_withdraw(struct command *cmd, /* Check address given is compatible with the chain we are on. */ if (addr_parse == ADDRESS_PARSE_WRONG_NETWORK) { command_fail(cmd, - "Destination address is not on network %s", - get_chainparams(cmd->ld)->network_name); + "Destination address is not on network %s", + get_chainparams(cmd->ld)->network_name); return; } - /* Select the coins */ - if (all_funds) { - withdraw->utxos = wallet_select_all(cmd, cmd->ld->wallet, - feerate_per_kw, - tal_len(withdraw->destination), - &withdraw->amount, - &fee_estimate); - /* FIXME Pull dust amount from the daemon config */ - if (!withdraw->utxos || withdraw->amount < 546) { - command_fail(cmd, "Cannot afford fee %"PRIu64, - fee_estimate); - return; - } - withdraw->changesatoshi = 0; - } else { - withdraw->utxos = wallet_select_coins(cmd, cmd->ld->wallet, - withdraw->amount, - feerate_per_kw, - tal_len(withdraw->destination), - &fee_estimate, - &withdraw->changesatoshi); - if (!withdraw->utxos) { - command_fail(cmd, "Not enough funds available"); - return; - } - } - - /* FIXME(cdecker) Pull this from the daemon config */ - if (withdraw->changesatoshi <= 546) - withdraw->changesatoshi = 0; - - if (withdraw->changesatoshi) - withdraw->change_key_index = wallet_get_newindex(cmd->ld); - else - withdraw->change_key_index = 0; + if (!wtx_select_utxos(&withdraw->wtx, feerate_per_kw, + tal_len(withdraw->destination))) + return; u8 *msg = towire_hsm_sign_withdrawal(cmd, - withdraw->amount, - withdraw->changesatoshi, - withdraw->change_key_index, + withdraw->wtx.amount, + withdraw->wtx.change, + withdraw->wtx.change_key_index, withdraw->destination, - withdraw->utxos); + withdraw->wtx.utxos); if (!wire_sync_write(cmd->ld->hsm_fd, take(msg))) fatal("Could not write sign_withdrawal to HSM: %s",