fundchannel: cap 'all' at 2^24-1 satoshi.

The easiest way to do this is to play with the 'wallet_tx' semantics
and have 'amount' have meaning even when 'all_funds' is set.

Note that we change the string 'Cannot afford funding transaction' to
'Cannot afford transaction' as this code is also used for withdrawls.

Inspired-by: molz on #c-lightning
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

CHANGELOG.md

@@ -46,6 +46,7 @@ changes.
 - Protocol: `node_announcement` multiple addresses are correctly ordered and uniquified.
 - JSON API: `listnodes`: now displays node aliases and colors even if they
   don't advertise a network address
+- JSON API: `fundchannel all`: now restricts to 2^24-1 satoshis rather than failing.
 - When we reconnect and have to retransmit failing HTLCs, the errors weren't
   encrypted by us.
 - `lightningd_config` man page is now installed by `make install`.

common/wallet_tx.c

@@ -12,17 +12,17 @@ void wtx_init(struct command *cmd, struct wallet_tx * wtx)
 	wtx->all_funds = false;
 }
 
-static bool check_amount(const struct wallet_tx *tx)
+static bool check_amount(const struct wallet_tx *tx, u64 amount)
 {
 	if (!tx->utxos) {
 		command_fail(tx->cmd, FUND_CANNOT_AFFORD,
-			     "Cannot afford funding transaction");
+			     "Cannot afford transaction");
 		return false;
 	}
-	if (tx->amount < 546) {
+	if (amount < 546) {
 		command_fail(tx->cmd, FUND_OUTPUT_IS_DUST,
 			     "Output %"PRIu64" satoshis would be dust",
-			     tx->amount);
+			     amount);
 		return false;
 	}
 	return true;
@@ -33,28 +33,37 @@ bool wtx_select_utxos(struct wallet_tx * tx, u32 fee_rate_per_kw,
 {
 	u64 fee_estimate;
 	if (tx->all_funds) {
+		u64 amount;
 		tx->utxos = wallet_select_all(tx->cmd, tx->cmd->ld->wallet,
 					      fee_rate_per_kw, out_len,
-					      &tx->amount,
+					      &amount,
 					      &fee_estimate);
-		if (!check_amount(tx))
+		if (!check_amount(tx, amount))
 			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 (!check_amount(tx))
-			return false;
-
-		if (tx->change < 546) {
+		if (amount <= tx->amount) {
 			tx->change = 0;
-			tx->change_key_index = 0;
-		} else {
-			tx->change_key_index = wallet_get_newindex(tx->cmd->ld);
+			tx->amount = amount;
+			return true;
 		}
+
+		/* Too much?  Try again, but ask for limit instead. */
+		tx->all_funds = false;
+		tx->utxos = tal_free(tx->utxos);
+	}
+
+	tx->utxos = wallet_select_coins(tx->cmd, tx->cmd->ld->wallet,
+					tx->amount,
+					fee_rate_per_kw, out_len,
+					&fee_estimate, &tx->change);
+	if (!check_amount(tx, tx->amount))
+		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

@@ -16,7 +16,7 @@ struct wallet_tx {
 	u32 change_key_index;
 	const struct utxo **utxos;
 
-	bool all_funds;
+	bool all_funds; /* In this case, amount is a maximum. */
 };
 
 void wtx_init(struct command *cmd, struct wallet_tx *wtx);

doc/lightning-fundchannel.7

@@ -2,12 +2,12 @@
 .\"     Title: lightning-fundchannel
 .\"    Author: [FIXME: author] [see http://docbook.sf.net/el/author]
 .\" Generator: DocBook XSL Stylesheets v1.79.1 <http://docbook.sf.net/>
-.\"      Date: 06/17/2018
+.\"      Date: 07/29/2018
 .\"    Manual: \ \&
 .\"    Source: \ \&
 .\"  Language: English
 .\"
-.TH "LIGHTNING\-FUNDCHANN" "7" "06/17/2018" "\ \&" "\ \&"
+.TH "LIGHTNING\-FUNDCHANN" "7" "07/29/2018" "\ \&" "\ \&"
 .\" -----------------------------------------------------------------
 .\" * Define some portability stuff
 .\" -----------------------------------------------------------------
@@ -38,7 +38,7 @@ The \fBfundchannel\fR RPC command opens a payment channel with a peer by commiti
 .sp
 \fIid\fR is the peer id obtained from \fBconnect\fR\&.
 .sp
-\fIsatoshi\fR is the amount in satoshis taken from the internal wallet to fund the channel\&. The string \fIall\fR can be used to specify all available funds\&. This value cannot be less than the dust limit, currently set to 546\&. And it must be less than 1<<24 (approximately 0\&.16778 BTC)\&.
+\fIsatoshi\fR is the amount in satoshis taken from the internal wallet to fund the channel\&. The string \fIall\fR can be used to specify all available funds (or 16777215 satoshi if more is available)\&. The value cannot be less than the dust limit, currently set to 546, nor more than 16777215 satoshi\&.
 .SH "RETURN VALUE"
 .sp
 On success, the \fItx\fR and \fItxid\fR of the transaction is returned, as well as the \fIchannel_id\fR of the newly created channel\&. On failure, an error is reported and the channel is not funded\&.

doc/lightning-fundchannel.7.txt

@@ -23,9 +23,9 @@ for the channel.
 'id' is the peer id obtained from *connect*.
 
 'satoshi' is the amount in satoshis taken from the internal wallet to fund the channel.
-The string 'all' can be used to specify all available funds.
-This value cannot be less than the dust limit, currently set to 546.
-And it must be less than 1<<24 (approximately 0.16778 BTC).
+The string 'all' can be used to specify all available funds (or 16777215 satoshi if more is available).
+The value cannot be less than the dust limit, currently set to 546, nor more
+than 16777215 satoshi.
 
 RETURN VALUE
 ------------

lightningd/jsonrpc.c

@@ -775,14 +775,19 @@ bool json_tok_newaddr(const char *buffer, const jsmntok_t *tok, bool *is_p2wpkh)
 }
 
 bool json_tok_wtx(struct wallet_tx * tx, const char * buffer,
-                  const jsmntok_t *sattok)
+                  const jsmntok_t *sattok, u64 max)
 {
         if (json_tok_streq(buffer, sattok, "all")) {
                 tx->all_funds = true;
+		tx->amount = max;
         } else if (!json_tok_u64(buffer, sattok, &tx->amount)) {
                 command_fail(tx->cmd, JSONRPC2_INVALID_PARAMS,
 			     "Invalid satoshis");
                 return false;
-        }
+	} else if (tx->amount > max) {
+                command_fail(tx->cmd, FUND_MAX_EXCEEDED,
+			     "Amount exceeded %"PRIu64, max);
+                return false;
+	}
         return true;
 }

lightningd/jsonrpc.h

@@ -99,7 +99,7 @@ bool json_tok_newaddr(const char *buffer, const jsmntok_t *tok, bool *is_p2wpkh)
 
 /* Parse the satoshi token in wallet_tx. */
 bool json_tok_wtx(struct wallet_tx * tx, const char * buffer,
-		  const jsmntok_t * sattok);
+		  const jsmntok_t * sattok, u64 max);
 
 AUTODATA_TYPE(json_command, struct json_command);
 #endif /* LIGHTNING_LIGHTNINGD_JSONRPC_H */

lightningd/opening_control.c

@@ -932,7 +932,7 @@ static void json_fund_channel(struct command *cmd,
 		   NULL))
 		return;
 
-	if (!json_tok_wtx(&fc->wtx, buffer, sattok))
+	if (!json_tok_wtx(&fc->wtx, buffer, sattok, MAX_FUNDING_SATOSHI))
 		return;
 	if (!pubkey_from_hexstr(buffer + desttok->start,
 				desttok->end - desttok->start,
@@ -949,12 +949,7 @@ static void json_fund_channel(struct command *cmd,
 			      BITCOIN_SCRIPTPUBKEY_P2WSH_LEN))
 		return;
 
-	if (fc->wtx.amount > MAX_FUNDING_SATOSHI) {
-		command_fail(cmd, FUND_MAX_EXCEEDED,
-			     "Funding satoshi must be <= %d",
-			     MAX_FUNDING_SATOSHI);
-		return;
-	}
+	assert(fc->wtx.amount <= MAX_FUNDING_SATOSHI);
 
 	list_add(&cmd->ld->fundchannels, &fc->list);
 	tal_add_destructor(fc, remove_funding_channel_from_list);

lightningd/test/run-param.c

@@ -53,7 +53,7 @@ bool json_tok_newaddr(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED
 { fprintf(stderr, "json_tok_newaddr called!\n"); abort(); }
 /* Generated stub for json_tok_wtx */
 bool json_tok_wtx(struct wallet_tx * tx UNNEEDED, const char * buffer UNNEEDED,
-		  const jsmntok_t * sattok UNNEEDED)
+		  const jsmntok_t * sattok UNNEEDED, u64 max UNNEEDED)
 { fprintf(stderr, "json_tok_wtx called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 

tests/test_lightningd.py

@@ -3949,7 +3949,7 @@ class LightningDTests(BaseLightningDTests):
 
         # This should fail, can't even afford fee.
         self.assertRaises(RpcError, l1.rpc.withdraw, waddr, 'all')
-        l1.daemon.wait_for_log('Cannot afford funding transaction')
+        l1.daemon.wait_for_log('Cannot afford transaction')
 
     def test_funding_change(self):
         """Add some funds, fund a channel, and make sure we remember the change
@@ -3984,6 +3984,17 @@ class LightningDTests(BaseLightningDTests):
         outputs = l1.db_query('SELECT value FROM outputs WHERE status=0;')
         assert len(outputs) == 0
 
+    def test_funding_all_too_much(self):
+        """Add more than max possible funds, fund a channel using all funds we can.
+        """
+        l1, l2 = self.connect()
+
+        self.give_funds(l1, 2**24 + 10000)
+        l1.rpc.fundchannel(l2.info['id'], "all")
+
+        assert only_one(l1.rpc.listfunds()['outputs'])['status'] == 'unconfirmed'
+        assert only_one(l1.rpc.listfunds()['channels'])['channel_total_sat'] == 2**24 - 1
+
     def test_funding_fail(self):
         """Add some funds, fund a channel without enough funds"""
         # Previous runs with same bitcoind can leave funds!
@@ -4013,7 +4024,7 @@ class LightningDTests(BaseLightningDTests):
         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.
-        self.assertRaisesRegex(RpcError, r'Cannot afford funding transaction',
+        self.assertRaisesRegex(RpcError, r'Cannot afford transaction',
                                l1.rpc.fundchannel, l2.info['id'], funds)
 
         # Should still be connected.
@@ -4042,7 +4053,7 @@ class LightningDTests(BaseLightningDTests):
             l1.rpc.fundchannel(l2.info['id'], amount)
             self.fail('Expected fundchannel to fail!')
         except RpcError as err:
-            assert 'Funding satoshi must be <= 16777215' in str(err)
+            assert 'Amount exceeded 16777215' in str(err)
 
         # This should work.
         amount = amount - 1

wallet/walletrpc.c

@@ -104,7 +104,7 @@ static void json_withdraw(struct command *cmd,
 		   NULL))
 		return;
 
-	if (!json_tok_wtx(&withdraw->wtx, buffer, sattok))
+	if (!json_tok_wtx(&withdraw->wtx, buffer, sattok, -1ULL))
 		return;
 
 	/* Parse address. */