wire: remove towire_double()

[vasild]

Before this patch we used to send doubles 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.

[rustyrussell]

OK, I prefer to attack the root cause here, which is to remove json_to_double (and the unused json_add_double) and all users.

1. Implement param_millionths which does what param_double does, except as a u64.
2. Use that to replace the two param_double users.
3. Clean up by removing double functions.

[vasild]

Done, except that I left json_to_double() in order to still be able to parse floating-point numbers from JSONs.

[rustyrussell]

My discomfort with json_to_double is that it uses strtod, which tries to accept just about anything. This means what actually happens is that it's machine-specific.

I'd be more comfortable with a json_to_millionths (which, actually was surprisingly nontrivial to code up):

diff --git a/common/json.c b/common/json.c
index e13bea6a3..e17b07a02 100644
--- a/common/json.c
+++ b/common/json.c
@@ -13,6 +13,7 @@
 #include <common/json.h>
 #include <common/json_stream.h>
 #include <common/node_id.h>
+#include <common/overflows.h>
 #include <common/utils.h>
 #include <common/wireaddr.h>
 #include <errno.h>
@@ -107,6 +108,47 @@ bool json_to_double(const char *buffer, const jsmntok_t *tok, double *num)
 	return true;
 }
 
+bool json_to_millionths(const char *buffer, const jsmntok_t *tok, u64 *millionths)
+{
+	int decimal_places = -1;
+	bool has_digits = 0;
+
+	*millionths = 0;
+	for (size_t i = tok->start; i < tok->end; i++) {
+		if (isdigit(buffer[i])) {
+			has_digits = true;
+			/* Ignore too much precision */
+			if (decimal_places >= 0 && ++decimal_places > 6)
+				continue;
+			if (mul_overflows_u64(*millionths, 10))
+				return false;
+			*millionths *= 10;
+			if (add_overflows_u64(*millionths, buffer[i] - '0'))
+				return false;
+			*millionths += buffer[i] - '0';
+		} else if (buffer[i] == '.') {
+			if (decimal_places != -1)
+				return false;
+			decimal_places = 0;
+		} else
+			return false;
+	}
+
+	if (!has_digits)
+		return false;
+
+	if (decimal_places == -1)
+		decimal_places = 0;
+
+	while (decimal_places < 6) {
+		if (mul_overflows_u64(*millionths, 10))
+			return false;
+		*millionths *= 10;
+		decimal_places++;
+	}
+	return true;
+}
+
 bool json_to_number(const char *buffer, const jsmntok_t *tok,
 		    unsigned int *num)
 {
diff --git a/common/test/run-json.c b/common/test/run-json.c
index ed24dfd96..78c33f022 100644
--- a/common/test/run-json.c
+++ b/common/test/run-json.c
@@ -46,6 +46,63 @@ static int test_json_tok_bitcoin_amount(void)
 	return 0;
 }
 
+static void do_json_tok_millionths(const char *val, bool ok,
+				   uint64_t expected)
+{
+	uint64_t amount;
+	jsmntok_t tok;
+
+	tok.start = 0;
+	tok.end = strlen(val);
+
+	assert(json_to_millionths(val, &tok, &amount) == ok);
+	if (ok)
+		assert(amount == expected);
+}
+
+static int test_json_tok_millionths(void)
+{
+	do_json_tok_millionths("", false, 0);
+	do_json_tok_millionths("0..0", false, 0);
+	do_json_tok_millionths("0.0.", false, 0);
+	do_json_tok_millionths(".", false, 0);
+	do_json_tok_millionths("..", false, 0);
+
+	do_json_tok_millionths("0", true, 0);
+	do_json_tok_millionths(".0", true, 0);
+	do_json_tok_millionths("0.", true, 0);
+	do_json_tok_millionths("100", true, 100 * 1000000);
+	do_json_tok_millionths("100.0", true, 100 * 1000000);
+	do_json_tok_millionths("100.", true, 100 * 1000000);
+	do_json_tok_millionths("100.000001", true, 100 * 1000000 + 1);
+	do_json_tok_millionths("100.0000001", true, 100 * 1000000);
+	do_json_tok_millionths(".000009", true, 9);
+	do_json_tok_millionths(".0000099", true, 9);
+	do_json_tok_millionths("18446744073709.551615", true,
+			       18446744073709551615ULL);
+	do_json_tok_millionths("18446744073709.551616", false, 0);
+	do_json_tok_millionths("18446744073709.551625", false, 0);
+	do_json_tok_millionths("18446744073709.551715", false, 0);
+	do_json_tok_millionths("18446744073709.552615", false, 0);
+	do_json_tok_millionths("18446744073709.561615", false, 0);
+	do_json_tok_millionths("18446744073709.651615", false, 0);
+	do_json_tok_millionths("18446744073710.551615", false, 0);
+	do_json_tok_millionths("18446744073809.551615", false, 0);
+	do_json_tok_millionths("18446744074709.551615", false, 0);
+	do_json_tok_millionths("18446744083709.551615", false, 0);
+	do_json_tok_millionths("18446744173709.551615", false, 0);
+	do_json_tok_millionths("18446745073709.551615", false, 0);
+	do_json_tok_millionths("18446754073709.551615", false, 0);
+	do_json_tok_millionths("18446844073709.551615", false, 0);
+	do_json_tok_millionths("18447744073709.551615", false, 0);
+	do_json_tok_millionths("18456744073709.551615", false, 0);
+	do_json_tok_millionths("18546744073709.551615", false, 0);
+	do_json_tok_millionths("19446744073709.551615", false, 0);
+	do_json_tok_millionths("28446744073709.551615", false, 0);
+
+	return 0;
+}
+
 static void test_json_tok_size(void)
 {
 	const jsmntok_t *toks;
@@ -190,6 +247,7 @@ int main(void)
 
 	test_json_tok_size();
 	test_json_tok_bitcoin_amount();
+	test_json_tok_millionths();
 	test_json_delve();
 	assert(!taken_any());
 	take_cleanup();

[ZmnSCPxj]

JSON supports "E" syntax, e.g. 1E2 == 100: https://www.json.org/img/number.png

[ZmnSCPxj]

Reading only the JSON syntax, it would even accept 10E2 == 1000.

[vasild]

I added the above to this PR. I like that it specifically parses floating point numbers that will fit in u64 after being multiplied by 1 million. I don't like that it adds more code and now this PR is +201/-130 lines, whereas it was +103/-123 before.

It would accept both .123 and 456. neither of which is a valid JSON number.

It would not accept 1e2 which is a valid JSON number and was accepted before by strtod(3).

[rustyrussell]

JSON supports "E" syntax, e.g. 1E2 == 100: https://www.json.org/img/number.png

Yes, I deliberately excluded that. It's not really a useful for these cases, and I prefer to be explicit. I'll add patches to stop the leading or trailing dot cases, too.

[rustyrussell]

Ack 25c2858

[vasild]

Should it ever be needed to pass double in a portable way, http://beej.us/guide/bgnet/html/#serialization may be a good starting point.