feerate: use suffix, not separate argument.

And, reluctantly, default to bitcoind style. "It's wrong to be right too soon." Suggested-by: @cdecker Signed-off-by: Rusty Russell <[email protected]>
jarret · Aug 30, 2018 · e0952ce · e0952ce
1 parent b270ab0
commit e0952ce
Show file tree

Hide file tree

Showing 14 changed files with 101 additions and 87 deletions.
diff --git a/contrib/pylightning/lightning/lightning.py b/contrib/pylightning/lightning/lightning.py
@@ -331,15 +331,14 @@ def listpeers(self, peerid=None, level=None):
         }
         return self.call("listpeers", payload)
 
-    def fundchannel(self, node_id, satoshi, feerate=None, feeratestyle=None):
+    def fundchannel(self, node_id, satoshi, feerate=None):
         """
         Fund channel with {id} using {satoshi} satoshis"
         """
         payload = {
             "id": node_id,
             "satoshi": satoshi,
-            "feerate": feerate,
-            "feeratestyle": feeratestyle
+            "feerate": feerate
         }
         return self.call("fundchannel", payload)
 
@@ -406,16 +405,15 @@ def dev_memleak(self):
         """
         return self.call("dev-memleak")
 
-    def withdraw(self, destination, satoshi, feerate=None, feeratestyle=None):
+    def withdraw(self, destination, satoshi, feerate=None):
         """
         Send to {destination} address {satoshi} (or "all")
         amount via Bitcoin transaction
         """
         payload = {
             "destination": destination,
             "satoshi": satoshi,
-            "feerate": feerate,
-            "feeratestyle": feeratestyle
+            "feerate": feerate
         }
         return self.call("withdraw", payload)
 

diff --git a/doc/lightning-fundchannel.7 b/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: 08/27/2018
+.\"      Date: 08/29/2018
 .\"    Manual: \ \&
 .\"    Source: \ \&
 .\"  Language: English
 .\"
-.TH "LIGHTNING\-FUNDCHANN" "7" "08/27/2018" "\ \&" "\ \&"
+.TH "LIGHTNING\-FUNDCHANN" "7" "08/29/2018" "\ \&" "\ \&"
 .\" -----------------------------------------------------------------
 .\" * Define some portability stuff
 .\" -----------------------------------------------------------------
@@ -31,7 +31,7 @@
 lightning-fundchannel \- Command for establishing a lightning channel\&.
 .SH "SYNOPSIS"
 .sp
-\fBfundchannel\fR \fIid\fR \fIsatoshi\fR [\fIfeerate\fR \fIfeeratestyle\fR]
+\fBfundchannel\fR \fIid\fR \fIsatoshi\fR [\fIfeerate\fR]
 .SH "DESCRIPTION"
 .sp
 The \fBfundchannel\fR RPC command opens a payment channel with a peer by commiting a funding transaction to the blockchain as defined in BOLT #2\&. \fBfundchannel\fR by itself does not attempt to open a connection\&. A connection must first be established using \fBconnect\fR\&. Once the transaction is confirmed, normal channel operations may begin\&. Readiness is indicated by \fBlistpeers\fR reporting a \fIstate\fR of CHANNELD_NORMAL for the channel\&.
@@ -40,7 +40,7 @@ The \fBfundchannel\fR RPC command opens a payment channel with a peer by commiti
 .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 (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\&.
 .sp
-\fIfeerate\fR is an optional feerate to use, overriding lightningd\(cqs internal estimate\&. If specified, \fIfeeratestyle\fR must be either \fI"perkw"\fR for if \fIfeerate\fR is in satoshi\-per\-kilosipa (weight), or \fI"perkb"\fR for if \fIfeerate\fR is in bitcoind\-style satoshi\-per\-kilobyte\&.
+\fIfeerate\fR is an optional feerate to use, overriding lightningd\(cqs internal estimate\&. \fIfeerate\fR is a number, with an optional suffix: \fIperkw\fR means the number is interpreted as satoshi\-per\-kilosipa (weight), and \fIperkb\fR means it is interpreted bitcoind\-style as satoshi\-per\-kilobyte\&. Omitting the suffix is equivalent to \fIperkb\fR\&.
 .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\&.

diff --git a/doc/lightning-fundchannel.7.txt b/doc/lightning-fundchannel.7.txt
@@ -8,7 +8,7 @@ lightning-fundchannel - Command for establishing a lightning channel.
 
 SYNOPSIS
 --------
-*fundchannel* 'id' 'satoshi' ['feerate' 'feeratestyle']
+*fundchannel* 'id' 'satoshi' ['feerate']
 
 DESCRIPTION
 -----------
@@ -28,9 +28,10 @@ The value cannot be less than the dust limit, currently set to 546, nor more
 than 16777215 satoshi.
 
 'feerate' is an optional feerate to use, overriding lightningd's
-internal estimate.  If specified, 'feeratestyle' must be either
-'"perkw"' for if 'feerate' is in satoshi-per-kilosipa (weight),
-or '"perkb"' for if 'feerate' is in bitcoind-style satoshi-per-kilobyte.
+internal estimate.  'feerate' is a number, with an optional suffix:
+'perkw' means the number is interpreted as satoshi-per-kilosipa
+(weight), and 'perkb' means it is interpreted bitcoind-style as
+satoshi-per-kilobyte.  Omitting the suffix is equivalent to 'perkb'.
 
 RETURN VALUE
 ------------

diff --git a/doc/lightning-withdraw.7 b/doc/lightning-withdraw.7
@@ -2,12 +2,12 @@
 .\"     Title: lightning-withdraw
 .\"    Author: [see the "AUTHOR" section]
 .\" Generator: DocBook XSL Stylesheets v1.79.1 <http://docbook.sf.net/>
-.\"      Date: 08/27/2018
+.\"      Date: 08/29/2018
 .\"    Manual: \ \&
 .\"    Source: \ \&
 .\"  Language: English
 .\"
-.TH "LIGHTNING\-WITHDRAW" "7" "08/27/2018" "\ \&" "\ \&"
+.TH "LIGHTNING\-WITHDRAW" "7" "08/29/2018" "\ \&" "\ \&"
 .\" -----------------------------------------------------------------
 .\" * Define some portability stuff
 .\" -----------------------------------------------------------------
@@ -31,7 +31,7 @@
 lightning-withdraw \- Command for withdrawing funds from the internal wallet\&.
 .SH "SYNOPSIS"
 .sp
-\fBwithdraw\fR \fIdestination\fR \fIsatoshi\fR [\fIfeerate\fR \fIfeeratestyle\fR]
+\fBwithdraw\fR \fIdestination\fR \fIsatoshi\fR [\fIfeerate\fR]
 .SH "DESCRIPTION"
 .sp
 The \fBwithdraw\fR RPC command sends funds from c\-lightning\(cqs internal wallet to the address specified in \fIdestination\fR\&.
@@ -40,7 +40,7 @@ The address can be of any Bitcoin accepted type, including bech32\&.
 .sp
 \fIsatoshi\fR is the amount to be withdrawn from the internal wallet (expressed, as name suggests, in satoshi)\&. The string \fIall\fR can be used to specify withdrawal of all available funds\&.
 .sp
-\fIfeerate\fR is an optional feerate to use, overriding lightningd\(cqs internal estimate\&. If specified, \fIfeeratestyle\fR must be either \fI"perkw"\fR for if \fIfeerate\fR is in satoshi\-per\-kilosipa (weight), or \fI"perkb"\fR for if \fIfeerate\fR is in bitcoind\-style satoshi\-per\-kilobyte\&.
+\fIfeerate\fR is an optional feerate to use, overriding lightningd\(cqs internal estimate\&. \fIfeerate\fR is a number, with an optional suffix: \fIperkw\fR means the number is interpreted as satoshi\-per\-kilosipa (weight), and \fIperkb\fR means it is interpreted bitcoind\-style as satoshi\-per\-kilobyte\&. Omitting the suffix is equivalent to \fIperkb\fR\&.
 .SH "RETURN VALUE"
 .sp
 On success, an object with attributes \fItx\fR and \fItxid\fR will be returned\&.

diff --git a/doc/lightning-withdraw.7.txt b/doc/lightning-withdraw.7.txt
@@ -9,7 +9,7 @@ internal wallet.
 
 SYNOPSIS
 --------
-*withdraw* 'destination' 'satoshi' ['feerate' 'feeratestyle']
+*withdraw* 'destination' 'satoshi' ['feerate']
 
 DESCRIPTION
 -----------
@@ -25,9 +25,10 @@ The string 'all' can be used to specify withdrawal of all
 available funds.
 
 'feerate' is an optional feerate to use, overriding lightningd's
-internal estimate.  If specified, 'feeratestyle' must be either
-'"perkw"' for if 'feerate' is in satoshi-per-kilosipa (weight),
-or '"perkb"' for if 'feerate' is in bitcoind-style satoshi-per-kilobyte.
+internal estimate.  'feerate' is a number, with an optional suffix:
+'perkw' means the number is interpreted as satoshi-per-kilosipa
+(weight), and 'perkb' means it is interpreted bitcoind-style as
+satoshi-per-kilobyte.  Omitting the suffix is equivalent to 'perkb'.
 
 RETURN VALUE
 ------------

diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c
@@ -447,36 +447,6 @@ u32 feerate_to_style(u32 feerate_perkw, enum feerate_style style)
 	abort();
 }
 
-/* If we have both feerate and style, use that, otherwise use inbuilt if avail.
- * Return false if we failed command, otherwise fills in feerate_perkw. */
-bool json_feerate_and_style(struct command *cmd,
-			    const u32 *feerate, enum feerate_style *style,
-			    u32 fallback_feerate_per_kw,
-			    u32 *feerate_per_kw)
-{
-	if (feerate) {
-		if (!style) {
-			command_fail(cmd, JSONRPC2_INVALID_PARAMS,
-				     "'feerate' requires 'feeratestyle'");
-			return false;
-		}
-		*feerate_per_kw = feerate_from_style(*feerate, *style);
-		return true;
-	} else {
-		if (style) {
-			command_fail(cmd, JSONRPC2_INVALID_PARAMS,
-				     "'feeratestyle' requires 'feerate'");
-			return false;
-		}
-		*feerate_per_kw = fallback_feerate_per_kw;
-		if (!*feerate_per_kw) {
-			command_fail(cmd, LIGHTNINGD, "Cannot estimate fees");
-			return false;
-		}
-		return true;
-	}
-}
-
 static void json_feerates(struct command *cmd,
 			    const char *buffer, const jsmntok_t *params)
 {

diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h
@@ -150,14 +150,6 @@ u32 unilateral_feerate(struct chain_topology *topo);
 u32 feerate_from_style(u32 feerate, enum feerate_style style);
 u32 feerate_to_style(u32 feerate_perkw, enum feerate_style style);
 
-/* If we have both feerate and style, use that, otherwise use fallback
- * if nonzero.  Return false if we failed command, otherwise fills in
- * feerate_per_kw. */
-bool json_feerate_and_style(struct command *cmd,
-			    const u32 *feerate, enum feerate_style *style,
-			    u32 fallback_feerate_per_kw,
-			    u32 *feerate_per_kw);
-
 /* Broadcast a single tx, and rebroadcast as reqd (copies tx).
  * If failed is non-NULL, call that and don't rebroadcast. */
 void broadcast_tx(struct chain_topology *topo,

diff --git a/lightningd/json.c b/lightningd/json.c
@@ -258,6 +258,50 @@ bool json_tok_feerate_style(struct command *cmd, const char *name,
 	return false;
 }
 
+bool json_tok_feerate(struct command *cmd, const char *name,
+		      const char *buffer, const jsmntok_t *tok,
+		      u32 **feerate)
+{
+	jsmntok_t base = *tok, suffix = *tok;
+	enum feerate_style style;
+	unsigned int num;
+
+	/* We have to split the number and suffix. */
+	suffix.start = suffix.end;
+	while (suffix.start > base.start && !isdigit(buffer[suffix.start-1])) {
+		suffix.start--;
+		base.end--;
+	}
+
+	if (!json_to_number(buffer, &base, &num)) {
+		command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+			     "'%s' prefix should be an integer, not '%.*s'",
+			     name, base.end - base.start, buffer + base.start);
+		return false;
+	}
+
+	if (json_tok_streq(buffer, &suffix, "")
+	    || json_tok_streq(buffer, &suffix,
+			      json_feerate_style_name(FEERATE_PER_KBYTE))) {
+		style = FEERATE_PER_KBYTE;
+	} else if (json_tok_streq(buffer, &suffix,
+				json_feerate_style_name(FEERATE_PER_KSIPA))) {
+		style = FEERATE_PER_KSIPA;
+	} else {
+		command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+			     "'%s' suffix should be '%s' or '%s', not '%.*s'",
+			     name,
+			     json_feerate_style_name(FEERATE_PER_KSIPA),
+			     json_feerate_style_name(FEERATE_PER_KBYTE),
+			     suffix.end - suffix.start, buffer + suffix.start);
+		return false;
+	}
+
+	*feerate = tal(cmd, u32);
+	**feerate = feerate_from_style(num, style);
+	return true;
+}
+
 bool
 json_tok_channel_id(const char *buffer, const jsmntok_t *tok,
 		    struct channel_id *cid)

diff --git a/lightningd/json.h b/lightningd/json.h
@@ -5,6 +5,7 @@
 #ifndef LIGHTNING_LIGHTNINGD_JSON_H
 #define LIGHTNING_LIGHTNINGD_JSON_H
 #include "config.h"
+#include <ccan/short_types/short_types.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
@@ -95,6 +96,11 @@ bool json_tok_feerate_style(struct command *cmd, const char *name,
 
 const char *json_feerate_style_name(enum feerate_style style);
 
+/* Extract a feerate with optional style suffix. */
+bool json_tok_feerate(struct command *cmd, const char *name,
+		      const char *buffer, const jsmntok_t *tok,
+		      u32 **feerate);
+
 /* '"fieldname" : "1234:5:6"' */
 void json_add_short_channel_id(struct json_result *response,
 			       const char *fieldname,

diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c
@@ -764,9 +764,7 @@ static void json_fund_channel(struct command *cmd,
 	struct pubkey *id;
 	struct peer *peer;
 	struct channel *channel;
-	unsigned int *feerate;
-	enum feerate_style *style;
-	u32 feerate_per_kw;
+	u32 *feerate_per_kw;
 	u8 *msg;
 
 	fc->cmd = cmd;
@@ -775,18 +773,21 @@ static void json_fund_channel(struct command *cmd,
 	if (!param(fc->cmd, buffer, params,
 		   p_req("id", json_tok_pubkey, &id),
 		   p_req("satoshi", json_tok_tok, &sattok),
-		   p_opt("feerate", json_tok_number, &feerate),
-		   p_opt("feeratestyle", json_tok_feerate_style, &style),
+		   p_opt("feerate", json_tok_feerate, &feerate_per_kw),
 		   NULL))
 		return;
 
 	if (!json_tok_wtx(&fc->wtx, buffer, sattok, MAX_FUNDING_SATOSHI))
 		return;
 
-	if (!json_feerate_and_style(cmd, feerate, style,
-				    opening_feerate(cmd->ld->topology),
-				    &feerate_per_kw))
-		return;
+	if (!feerate_per_kw) {
+		feerate_per_kw = tal(cmd, u32);
+		*feerate_per_kw = opening_feerate(cmd->ld->topology);
+		if (!*feerate_per_kw) {
+			command_fail(cmd, LIGHTNINGD, "Cannot estimate fees");
+			return;
+		}
+	}
 
 	peer = peer_by_id(cmd->ld, id);
 	if (!peer) {
@@ -815,7 +816,7 @@ static void json_fund_channel(struct command *cmd,
 	fc->push_msat = 0;
 	fc->channel_flags = OUR_CHANNEL_FLAGS;
 
-	if (!wtx_select_utxos(&fc->wtx, feerate_per_kw,
+	if (!wtx_select_utxos(&fc->wtx, *feerate_per_kw,
 			      BITCOIN_SCRIPTPUBKEY_P2WSH_LEN))
 		return;
 
@@ -827,7 +828,7 @@ static void json_fund_channel(struct command *cmd,
 	msg = towire_opening_funder(NULL,
 				    fc->wtx.amount,
 				    fc->push_msat,
-				    feerate_per_kw,
+				    *feerate_per_kw,
 				    fc->wtx.change,
 				    fc->wtx.change_key_index,
 				    fc->channel_flags,

diff --git a/lightningd/test/run-param.c b/lightningd/test/run-param.c
@@ -45,6 +45,9 @@ void command_fail_detailed(struct command *cmd, int code,
 }
 
 /* AUTOGENERATED MOCKS START */
+/* Generated stub for feerate_from_style */
+u32 feerate_from_style(u32 feerate UNNEEDED, enum feerate_style style UNNEEDED)
+{ fprintf(stderr, "feerate_from_style called!\n"); abort(); }
 /* Generated stub for fmt_wireaddr_without_port */
 char *fmt_wireaddr_without_port(const tal_t *ctx UNNEEDED, const struct wireaddr *a UNNEEDED)
 { fprintf(stderr, "fmt_wireaddr_without_port called!\n"); abort(); }

diff --git a/tests/test_connection.py b/tests/test_connection.py
@@ -1125,8 +1125,8 @@ def test_no_fee_estimate(node_factory, bitcoind, executor):
         l1.rpc.withdraw(l2.rpc.newaddr()['address'], 'all')
 
     # Can with manual feerate.
-    l1.rpc.withdraw(l2.rpc.newaddr()['address'], 10000, 1500, 'perkb')
-    l1.rpc.fundchannel(l2.info['id'], 10**6, 2000, 'perkw')
+    l1.rpc.withdraw(l2.rpc.newaddr()['address'], 10000, '1500perkb')
+    l1.rpc.fundchannel(l2.info['id'], 10**6, '2000perkw')
 
     # Make sure we clean up cahnnel for later attempt.
     l1.daemon.wait_for_log('sendrawtx exit 0')

diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
@@ -354,9 +354,6 @@ u8 *towire_errorfmt(const tal_t *ctx UNNEEDED,
 /* Generated stub for towire_hsm_sign_commitment_tx */
 u8 *towire_hsm_sign_commitment_tx(const tal_t *ctx UNNEEDED, const struct pubkey *peer_id UNNEEDED, u64 channel_dbid UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const struct pubkey *remote_funding_key UNNEEDED, u64 funding_amount UNNEEDED)
 { fprintf(stderr, "towire_hsm_sign_commitment_tx called!\n"); abort(); }
-/* Generated stub for try_get_feerate */
-u32 try_get_feerate(const struct chain_topology *topo UNNEEDED, enum feerate feerate UNNEEDED)
-{ fprintf(stderr, "try_get_feerate called!\n"); abort(); }
 /* Generated stub for watch_txid */
 struct txwatch *watch_txid(const tal_t *ctx UNNEEDED,
 			   struct chain_topology *topo UNNEEDED,