wire: remove towire_double()

Before this patch we used to send `double`s over the wire by just
copying them. This is not portable because the internal represenation
of a `double` is implementation specific.

Instead of this, multiply any floating-point numbers that come from
the outside (e.g. JSONs) by 1 million and round them to integers when
handling them.

* Introduce a new param_millionths() that expects a floating-point
  number and returns it multipled by 1000000 as an integer.

* Replace param_double() and param_percent() with param_millionths()

* Previously the riskfactor would be allowed to be negative, which must
  have been unintentional. This patch changes that to require a
  non-negative number.

Changelog-None

common/json_tok.c

@@ -35,17 +35,21 @@ struct command_result *param_bool(struct command *cmd, const char *name,
 			    name, tok->end - tok->start, buffer + tok->start);
 }
 
-struct command_result *param_double(struct command *cmd, const char *name,
-				    const char *buffer, const jsmntok_t *tok,
-				    double **num)
+struct command_result *param_millionths(struct command *cmd, const char *name,
+					const char *buffer,
+					const jsmntok_t *tok, uint64_t **num)
 {
-	*num = tal(cmd, double);
-	if (json_to_double(buffer, tok, *num))
+	double d;
+	if (json_to_double(buffer, tok, &d) && d >= 0.0) {
+		*num = tal(cmd, uint64_t);
+		**num = (uint64_t)(d * 1000000);
 		return NULL;
+	}
 
-	return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
-			    "'%s' should be a double, not '%.*s'",
-			    name, tok->end - tok->start, buffer + tok->start);
+	return command_fail(
+	    cmd, JSONRPC2_INVALID_PARAMS,
+	    "'%s' should be a non-negative floating-point number, not '%.*s'",
+	    name, tok->end - tok->start, buffer + tok->start);
 }
 
 struct command_result *param_escaped_string(struct command *cmd,
@@ -128,19 +132,6 @@ struct command_result *param_sha256(struct command *cmd, const char *name,
 			    name, tok->end - tok->start, buffer + tok->start);
 }
 
-struct command_result *param_percent(struct command *cmd, const char *name,
-				     const char *buffer, const jsmntok_t *tok,
-				     double **num)
-{
-	*num = tal(cmd, double);
-	if (json_to_double(buffer, tok, *num) && **num >= 0.0)
-		return NULL;
-
-	return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
-			    "'%s' should be a positive double, not '%.*s'",
-			    name, tok->end - tok->start, buffer + tok->start);
-}
-
 struct command_result *param_u64(struct command *cmd, const char *name,
 				 const char *buffer, const jsmntok_t *tok,
 				 uint64_t **num)

common/json_tok.h

@@ -25,10 +25,14 @@ struct command_result *param_bool(struct command *cmd, const char *name,
 				  const char *buffer, const jsmntok_t *tok,
 				  bool **b);
 
-/* Extract double from this (must be a number literal) */
-struct command_result *param_double(struct command *cmd, const char *name,
-				    const char *buffer, const jsmntok_t *tok,
-				    double **num);
+/*
+ * Extract a non-negative (either 0 or positive) floating-point number from this
+ * (must be a number literal), multiply it by 1 million and return it as an
+ * integer.
+ */
+struct command_result *param_millionths(struct command *cmd, const char *name,
+					const char *buffer,
+					const jsmntok_t *tok, uint64_t **num);
 
 /* Extract an escaped string (and unescape it) */
 struct command_result *param_escaped_string(struct command *cmd,
@@ -57,11 +61,6 @@ struct command_result *param_sha256(struct command *cmd, const char *name,
 				    const char *buffer, const jsmntok_t *tok,
 				    struct sha256 **hash);
 
-/* Extract double in range [0.0, 100.0] */
-struct command_result *param_percent(struct command *cmd, const char *name,
-				     const char *buffer, const jsmntok_t *tok,
-				     double **num);
-
 /* Extract number from this (may be a string, or a number literal) */
 struct command_result *param_u64(struct command *cmd, const char *name,
 				 const char *buffer, const jsmntok_t *tok,

common/test/run-param.c

@@ -131,17 +131,17 @@ struct sanity {
 	char *str;
 	bool failed;
 	int ival;
-	double dval;
+	u64 fpval; /* floating-point, multiplied by 1000000 */
 	char *fail_str;
 };
 
 struct sanity buffers[] = {
 	// pass
-	{"['42', '3.15']", false, 42, 3.15, NULL},
-	{"{ 'u64' : '42', 'double' : '3.15' }", false, 42, 3.15, NULL},
+	{"['42', '3.15']", false, 42, 3150000, NULL},
+	{"{ 'u64' : '42', 'fp' : '3.15' }", false, 42, 3150000, NULL},
 
 	// fail
-	{"{'u64':'42', 'double':'3.15', 'extra':'stuff'}", true, 0, 0,
+	{"{'u64':'42', 'fp':'3.15', 'extra':'stuff'}", true, 0, 0,
 	 "unknown parameter"},
 	{"['42', '3.15', 'stuff']", true, 0, 0, "too many"},
 	{"['42', '3.15', 'null']", true, 0, 0, "too many"},
@@ -151,17 +151,17 @@ struct sanity buffers[] = {
 	{"['42']", true, 0, 0, "missing required"},
 
 	// fail wrong type
-	{"{'u64':'hello', 'double':'3.15'}", true, 0, 0, "be an unsigned 64"},
+	{"{'u64':'hello', 'fp':'3.15'}", true, 0, 0, "be an unsigned 64"},
 	{"['3.15', '3.15', 'stuff']", true, 0, 0, "integer"},
 };
 
 static void stest(const struct json *j, struct sanity *b)
 {
 	u64 *ival;
-	double *dval;
+	u64 *fpval;
 	if (!param(cmd, j->buffer, j->toks,
 		   p_req("u64", param_u64, &ival),
-		   p_req("double", param_double, &dval), NULL)) {
+		   p_req("fp", param_millionths, &fpval), NULL)) {
 		assert(check_fail());
 		assert(b->failed == true);
 		if (!strstr(fail_msg, b->fail_str)) {
@@ -172,7 +172,7 @@ static void stest(const struct json *j, struct sanity *b)
 		assert(!check_fail());
 		assert(b->failed == false);
 		assert(*ival == 42);
-		assert(*dval > 3.1499 && b->dval < 3.1501);
+		assert(*fpval > 3149900 && b->fpval < 3150100);
 	}
 }
 
@@ -222,13 +222,13 @@ static void dup_names(void)
 {
 	struct json *j =
 		json_parse(cmd,
-			   "{ 'u64' : '42', 'u64' : '43', 'double' : '3.15' }");
+			   "{ 'u64' : '42', 'u64' : '43', 'fp' : '3.15' }");
 
 	u64 *i;
-	double *d;
+	u64 *fp;
 	assert(!param(cmd, j->buffer, j->toks,
 		      p_req("u64", param_u64, &i),
-		      p_req("double", param_double, &d), NULL));
+		      p_req("fp", param_millionths, &fp), NULL));
 }
 
 static void null_params(void)
@@ -290,28 +290,28 @@ static void bad_programmer(void)
 {
 	u64 *ival;
 	u64 *ival2;
-	double *dval;
+	u64 *fpval;
 	struct json *j = json_parse(cmd, "[ '25', '546', '26' ]");
 
 	/* check for repeated names */
 	assert(!param(cmd, j->buffer, j->toks,
 		      p_req("repeat", param_u64, &ival),
-		      p_req("double", param_double, &dval),
+		      p_req("fp", param_millionths, &fpval),
 		      p_req("repeat", param_u64, &ival2), NULL));
 	assert(check_fail());
 	assert(strstr(fail_msg, "developer error"));
 
 	assert(!param(cmd, j->buffer, j->toks,
 		      p_req("repeat", param_u64, &ival),
-		      p_req("double", param_double, &dval),
+		      p_req("fp", param_millionths, &fpval),
 		      p_req("repeat", param_u64, &ival), NULL));
 	assert(check_fail());
 	assert(strstr(fail_msg, "developer error"));
 
 	assert(!param(cmd, j->buffer, j->toks,
 		      p_req("u64", param_u64, &ival),
-		      p_req("repeat", param_double, &dval),
-		      p_req("repeat", param_double, &dval), NULL));
+		      p_req("repeat", param_millionths, &fpval),
+		      p_req("repeat", param_millionths, &fpval), NULL));
 	assert(check_fail());
 	assert(strstr(fail_msg, "developer error"));
 
@@ -330,12 +330,13 @@ static void bad_programmer(void)
 	/* Add required param after optional */
 	j = json_parse(cmd, "[ '25', '546', '26', '1.1' ]");
 	unsigned int *msatoshi;
-	double *riskfactor;
-	assert(!param(cmd, j->buffer, j->toks,
-		      p_req("u64", param_u64, &ival),
-		      p_req("double", param_double, &dval),
-		      p_opt_def("msatoshi", param_number, &msatoshi, 100),
-		      p_req("riskfactor", param_double, &riskfactor), NULL));
+	u64 *riskfactor_millionths;
+	assert(!param(
+	    cmd, j->buffer, j->toks, p_req("u64", param_u64, &ival),
+	    p_req("fp", param_millionths, &fpval),
+	    p_opt_def("msatoshi", param_number, &msatoshi, 100),
+	    p_req("riskfactor", param_millionths, &riskfactor_millionths),
+	    NULL));
 	assert(*msatoshi);
 	assert(*msatoshi == 100);
 	assert(check_fail());
@@ -525,16 +526,16 @@ static void param_tests(void)
 	test_cb(param_bool, bool, "[ tru ]", false, false);
 	test_cb(param_bool, bool, "[ 1 ]", false, false);
 
-	test_cb(param_percent, double, "[ -0.01 ]", 0, false);
-	test_cb(param_percent, double, "[ 0.00 ]", 0, true);
-	test_cb(param_percent, double, "[ 1 ]", 1, true);
-	test_cb(param_percent, double, "[ 1.1 ]", 1.1, true);
-	test_cb(param_percent, double, "[ 1.01 ]", 1.01, true);
-	test_cb(param_percent, double, "[ 99.99 ]", 99.99, true);
-	test_cb(param_percent, double, "[ 100.0 ]", 100, true);
-	test_cb(param_percent, double, "[ 100.001 ]", 100.001, true);
-	test_cb(param_percent, double, "[ 1000 ]", 1000, true);
-	test_cb(param_percent, double, "[ 'wow' ]", 0, false);
+	test_cb(param_millionths, u64, "[ -0.01 ]", 0, false);
+	test_cb(param_millionths, u64, "[ 0.00 ]", 0, true);
+	test_cb(param_millionths, u64, "[ 1 ]", 1000000, true);
+	test_cb(param_millionths, u64, "[ 1.1 ]", 1100000, true);
+	test_cb(param_millionths, u64, "[ 1.01 ]", 1010000, true);
+	test_cb(param_millionths, u64, "[ 99.99 ]", 99990000, true);
+	test_cb(param_millionths, u64, "[ 100.0 ]", 100000000, true);
+	test_cb(param_millionths, u64, "[ 100.001 ]", 100001000, true);
+	test_cb(param_millionths, u64, "[ 1000 ]", 1000000000, true);
+	test_cb(param_millionths, u64, "[ 'wow' ]", 0, false);
 }
 
 static void test_invoice(struct command *cmd,

doc/lightning-getroute.7.md

@@ -21,9 +21,9 @@ in *btc*.
 
 There are two considerations for how good a route is: how low the fees
 are, and how long your payment will get stuck in a delayed output if a
-node goes down during the process. The *riskfactor* floating-point field
-controls this tradeoff; it is the annual cost of your funds being stuck
-(as a percentage).
+node goes down during the process. The *riskfactor* non-negative
+floating-point field controls this tradeoff; it is the annual cost of
+your funds being stuck (as a percentage).
 
 For example, if you thought the convenience of keeping your funds liquid
 (not stuck) was worth 20% per annum interest, *riskfactor* would be 20.
@@ -32,7 +32,7 @@ If you didn’t care about risk, *riskfactor* would be zero.
 
 *fromid* is the node to start the route from: default is this node.
 
-The *fuzzpercent* is a positive floating-point number, representing a
+The *fuzzpercent* is a non-negative floating-point number, representing a
 percentage of the actual fee. The *fuzzpercent* is used to distort
 computed fees along each channel, to provide some randomization to the
 route generated. 0.0 means the exact fee of that channel is used, while

gossipd/gossip_wire.csv

@@ -35,10 +35,9 @@ msgtype,gossip_getroute_request,3006
 msgdata,gossip_getroute_request,source,?node_id,
 msgdata,gossip_getroute_request,destination,node_id,
 msgdata,gossip_getroute_request,msatoshi,amount_msat,
-# We don't pass doubles, so pass riskfactor  1000000.
-msgdata,gossip_getroute_request,riskfactor_by_million,u64,
+msgdata,gossip_getroute_request,riskfactor_millionths,u64,
 msgdata,gossip_getroute_request,final_cltv,u32,
-msgdata,gossip_getroute_request,fuzz,double,
+msgdata,gossip_getroute_request,fuzz_millionths,u64,
 msgdata,gossip_getroute_request,num_excluded,u16,
 msgdata,gossip_getroute_request,excluded,exclude_entry,num_excluded
 msgdata,gossip_getroute_request,max_hops,u32,

gossipd/gossipd.c

@@ -883,11 +883,13 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon,
 	struct node_id *source, destination;
 	struct amount_msat msat;
 	u32 final_cltv;
-	u64 riskfactor_by_million;
+	/* risk factor 12.345% -> riskfactor_millionths = 12345000 */
+	u64 riskfactor_millionths;
 	u32 max_hops;
 	u8 *out;
 	struct route_hop *hops;
-	double fuzz;
+	/* fuzz 12.345% -> fuzz_millionths = 12345000 */
+	u64 fuzz_millionths;
 	struct exclude_entry **excluded;
 
 	/* To choose between variations, we need to know how much we're
@@ -901,12 +903,9 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon,
 	 * for a route from ourselves (the usual case): in that case,
 	 * we don't have to consider fees on our own outgoing channels.
 	 */
-	if (!fromwire_gossip_getroute_request(msg, msg,
-					      &source, &destination,
-					      &msat, &riskfactor_by_million,
-					      &final_cltv, &fuzz,
-					      &excluded,
-					      &max_hops))
+	if (!fromwire_gossip_getroute_request(
+		msg, msg, &source, &destination, &msat, &riskfactor_millionths,
+		&final_cltv, &fuzz_millionths, &excluded, &max_hops))
 		master_badmsg(WIRE_GOSSIP_GETROUTE_REQUEST, msg);
 
 	status_debug("Trying to find a route from %s to %s for %s",
@@ -916,9 +915,10 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon,
 		     type_to_string(tmpctx, struct amount_msat, &msat));
 
 	/* routing.c does all the hard work; can return NULL. */
-	hops = get_route(tmpctx, daemon->rstate, source, &destination,
-			 msat, riskfactor_by_million / 1000000.0, final_cltv,
-			 fuzz, pseudorand_u64(), excluded, max_hops);
+	hops = get_route(tmpctx, daemon->rstate, source, &destination, msat,
+			 riskfactor_millionths / 1000000.0, final_cltv,
+			 fuzz_millionths / 1000000.0, pseudorand_u64(),
+			 excluded, max_hops);
 
 	out = towire_gossip_getroute_reply(NULL, hops);
 	daemon_conn_send(daemon->master, take(out));

lightningd/gossip_control.c

@@ -381,7 +381,8 @@ static struct command_result *json_getroute(struct command *cmd,
 	const jsmntok_t *excludetok;
 	struct amount_msat *msat;
 	u32 *cltv;
-	double *riskfactor;
+	/* risk factor 12.345% -> riskfactor_millionths = 12345000 */
+	u64 *riskfactor_millionths;
 	const struct exclude_entry **excluded;
 	u32 *max_hops;
 
@@ -390,23 +391,23 @@ static struct command_result *json_getroute(struct command *cmd,
 	 * randomization (the higher-fee paths become more likely to
 	 * be selected) at the cost of increasing the probability of
 	 * selecting the higher-fee paths. */
-	double *fuzz;
-
-	if (!param(cmd, buffer, params,
-		   p_req("id", param_node_id, &destination),
-		   p_req("msatoshi", param_msat, &msat),
-		   p_req("riskfactor", param_double, &riskfactor),
-		   p_opt_def("cltv", param_number, &cltv, 9),
-		   p_opt("fromid", param_node_id, &source),
-		   p_opt_def("fuzzpercent", param_percent, &fuzz, 5.0),
-		   p_opt("exclude", param_array, &excludetok),
-		   p_opt_def("maxhops", param_number, &max_hops,
-			     ROUTING_MAX_HOPS),
-		   NULL))
+	u64 *fuzz_millionths; /* fuzz 12.345% -> fuzz_millionths = 12345000 */
+
+	if (!param(
+		cmd, buffer, params, p_req("id", param_node_id, &destination),
+		p_req("msatoshi", param_msat, &msat),
+		p_req("riskfactor", param_millionths, &riskfactor_millionths),
+		p_opt_def("cltv", param_number, &cltv, 9),
+		p_opt("fromid", param_node_id, &source),
+		p_opt_def("fuzzpercent", param_millionths, &fuzz_millionths,
+			  5000000),
+		p_opt("exclude", param_array, &excludetok),
+		p_opt_def("maxhops", param_number, &max_hops, ROUTING_MAX_HOPS),
+		NULL))
 		return command_param_failed();
 
 	/* Convert from percentage */
-	*fuzz = *fuzz / 100.0;
+	*fuzz_millionths /= 100;
 
 	if (excludetok) {
 		const jsmntok_t *t;
@@ -442,12 +443,9 @@ static struct command_result *json_getroute(struct command *cmd,
 		excluded = NULL;
 	}
 
-	u8 *req = towire_gossip_getroute_request(cmd, source, destination,
-						 *msat,
-						 *riskfactor * 1000000.0,
-						 *cltv, fuzz,
-						 excluded,
-						 *max_hops);
+	u8 *req = towire_gossip_getroute_request(
+	    cmd, source, destination, *msat, *riskfactor_millionths, *cltv,
+	    *fuzz_millionths, excluded, *max_hops);
 	subd_req(ld->gossip, ld->gossip, req, -1, 0, json_getroute_reply, cmd);
 	return command_still_pending(cmd);
 }