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

@@ -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

common/wallet_tx.c

@@ -0,0 +1,48 @@
+#include <common/wallet_tx.h>
+#include <inttypes.h>
+#include <lightningd/jsonrpc_errors.h>
+#include <wallet/wallet.h>
+
+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;
+}

common/wallet_tx.h

@@ -0,0 +1,25 @@
+#ifndef LIGHTNING_COMMON_WALLET_TX_H
+#define LIGHTNING_COMMON_WALLET_TX_H
+#include "config.h"
+#include <ccan/short_types/short_types.h>
+#include <lightningd/json.h>
+#include <lightningd/jsonrpc.h>
+#include <lightningd/lightningd.h>
+
+/* 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 */

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		\

lightningd/build_utxos.c

@@ -1,34 +0,0 @@
-#include <bitcoin/base58.h>
-#include <bitcoin/script.h>
-#include <ccan/structeq/structeq.h>
-#include <common/utils.h>
-#include <lightningd/build_utxos.h>
-#include <lightningd/jsonrpc.h>
-#include <wally_bip32.h>
-
-
-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;
-}

lightningd/build_utxos.h

@@ -1,15 +0,0 @@
-#ifndef LIGHTNING_LIGHTNINGD_BUILD_UTXOS_H
-#define LIGHTNING_LIGHTNINGD_BUILD_UTXOS_H
-#include "config.h"
-#include <common/utxo.h>
-#include <lightningd/lightningd.h>
-
-/* 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 */

lightningd/jsonrpc.c

@@ -13,6 +13,7 @@
 #include <common/json_escaped.h>
 #include <common/memleak.h>
 #include <common/version.h>
+#include <common/wallet_tx.h>
 #include <common/wireaddr.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -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;
+}

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 */

lightningd/opening_control.c

@@ -4,11 +4,11 @@
 #include <common/channel_config.h>
 #include <common/funding_tx.h>
 #include <common/key_derive.h>
+#include <common/wallet_tx.h>
 #include <common/wire_error.h>
 #include <errno.h>
 #include <gossipd/gen_gossip_wire.h>
 #include <hsmd/gen_hsm_client_wire.h>
-#include <lightningd/build_utxos.h>
 #include <lightningd/chaintopology.h>
 #include <lightningd/channel_control.h>
 #include <lightningd/closing_control.h>
@@ -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);

lightningd/peer_control.c

@@ -26,7 +26,6 @@
 #include <hsmd/gen_hsm_client_wire.h>
 #include <inttypes.h>
 #include <lightningd/bitcoind.h>
-#include <lightningd/build_utxos.h>
 #include <lightningd/chaintopology.h>
 #include <lightningd/channel_control.h>
 #include <lightningd/closing_control.h>

wallet/walletrpc.c

@@ -6,6 +6,7 @@
 #include <common/key_derive.h>
 #include <common/status.h>
 #include <common/utxo.h>
+#include <common/wallet_tx.h>
 #include <common/withdraw_tx.h>
 #include <errno.h>
 #include <hsmd/gen_hsm_client_wire.h>
@@ -23,11 +24,9 @@
 #include <wire/wire_sync.h>
 
 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",