Use dedicated type for error codes

Before this patch we used `int` for error codes. The problem with
`int` is that we try to pass it to/from wire and the size of `int` is
not defined by the standard. So a sender with 4-byte `int` would write
4 bytes to the wire and a receiver with 2-byte `int` (for example) would
read just 2 bytes from the wire.

To resolve this:

* Introduce an error code type with a known size:
  `typedef s32 errcode_t`.

* Change all error code macros to constants of type `errcode_t`.
  Constants also play better with gdb - it would visualize the name of
  the constant instead of the numeric value.

* Change all functions that take error codes to take the new type
  `errcode_t` instead of `int`.

* Introduce towire / fromwire functions to send / receive the newly added
  type `errcode_t` and use it instead of `towire_int()`.

In addition:

* Remove the now unneeded `towire_int()`.

* Replace a hardcoded error code `-2` with a new constant
  `INVOICE_EXPIRED_DURING_WAIT` (903).

Changelog-Changed: The waitinvoice command would now return error code 903 to designate that the invoice expired during wait, instead of the previous -2

common/Makefile

@@ -68,7 +68,14 @@ COMMON_SRC_NOGEN :=				\
 
 COMMON_SRC_GEN := common/gen_status_wire.c common/gen_peer_status_wire.c
 
-COMMON_HEADERS_NOGEN := $(COMMON_SRC_NOGEN:.c=.h) common/overflows.h common/htlc.h common/status_levels.h common/json_command.h common/jsonrpc_errors.h common/gossip_constants.h
+COMMON_HEADERS_NOGEN := $(COMMON_SRC_NOGEN:.c=.h)	\
+	common/errcode.h				\
+	common/gossip_constants.h			\
+	common/htlc.h					\
+	common/json_command.h				\
+	common/jsonrpc_errors.h				\
+	common/overflows.h				\
+	common/status_levels.h
 COMMON_HEADERS_GEN := common/gen_htlc_state_names.h common/gen_status_wire.h common/gen_peer_status_wire.h
 
 COMMON_HEADERS := $(COMMON_HEADERS_GEN) $(COMMON_HEADERS_NOGEN)

common/errcode.h

@@ -0,0 +1,13 @@
+#ifndef LIGHTNING_COMMON_ERRCODE_H
+#define LIGHTNING_COMMON_ERRCODE_H
+
+#include "config.h"
+
+#include <ccan/short_types/short_types.h>
+#include <inttypes.h>
+
+typedef s32 errcode_t;
+
+#define PRIerrcode PRId32
+
+#endif /* LIGHTNING_COMMON_ERRCODE_H */

common/json.c

@@ -65,6 +65,30 @@ bool json_to_u64(const char *buffer, const jsmntok_t *tok,
 	return true;
 }
 
+bool json_to_s64(const char *buffer, const jsmntok_t *tok, s64 *num)
+{
+	char *end;
+	long long l;
+
+	l = strtoll(buffer + tok->start, &end, 0);
+	if (end != buffer + tok->end)
+		return false;
+
+	BUILD_ASSERT(sizeof(l) >= sizeof(*num));
+	*num = l;
+
+	/* Check for overflow/underflow */
+	if ((l == LONG_MAX || l == LONG_MIN) && errno == ERANGE)
+		return false;
+
+	/* Check if the number did not fit in `s64` (in case `long long`
+	is a bigger type). */
+	if (*num != l)
+		return false;
+
+	return true;
+}
+
 bool json_to_double(const char *buffer, const jsmntok_t *tok, double *num)
 {
 	char *end;
@@ -122,22 +146,29 @@ bool json_to_u32(const char *buffer, const jsmntok_t *tok,
 
 bool json_to_int(const char *buffer, const jsmntok_t *tok, int *num)
 {
-	char *end;
-	long l;
+	s64 tmp;
 
-	l = strtol(buffer + tok->start, &end, 0);
-	if (end != buffer + tok->end)
+	if (!json_to_s64(buffer, tok, &tmp))
 		return false;
+	*num = tmp;
 
-	BUILD_ASSERT(sizeof(l) >= sizeof(*num));
-	*num = l;
+	/* Just in case it doesn't fit. */
+	if (*num != tmp)
+		return false;
 
-	/* Check for overflow/underflow */
-	if ((l == LONG_MAX || l == LONG_MIN) && errno == ERANGE)
+	return true;
+}
+
+bool json_to_errcode(const char *buffer, const jsmntok_t *tok, errcode_t *errcode)
+{
+	s64 tmp;
+
+	if (!json_to_s64(buffer, tok, &tmp))
 		return false;
+	*errcode = tmp;
 
-	/* Check for truncation */
-	if (*num != l)
+	/* Just in case it doesn't fit. */
+	if (*errcode != tmp)
 		return false;
 
 	return true;

common/json.h

@@ -3,7 +3,9 @@
 #include "config.h"
 #include <bitcoin/preimage.h>
 #include <bitcoin/privkey.h>
+#include <ccan/short_types/short_types.h>
 #include <ccan/tal/tal.h>
+#include <common/errcode.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -37,6 +39,9 @@ bool json_to_number(const char *buffer, const jsmntok_t *tok,
 bool json_to_u64(const char *buffer, const jsmntok_t *tok,
 		 uint64_t *num);
 
+/* Extract signed 64 bit integer from this (may be a string, or a number literal) */
+bool json_to_s64(const char *buffer, const jsmntok_t *tok, s64 *num);
+
 /* Extract number from this (may be a string, or a number literal) */
 bool json_to_u32(const char *buffer, const jsmntok_t *tok,
                  uint32_t *num);
@@ -51,6 +56,9 @@ bool json_to_double(const char *buffer, const jsmntok_t *tok, double *num);
 /* Extract signed integer from this (may be a string, or a number literal) */
 bool json_to_int(const char *buffer, const jsmntok_t *tok, int *num);
 
+/* Extract an error code from this (may be a string, or a number literal) */
+bool json_to_errcode(const char *buffer, const jsmntok_t *tok, errcode_t *errcode);
+
 /* Extract boolean from this */
 bool json_to_bool(const char *buffer, const jsmntok_t *tok, bool *b);
 

common/json_command.h

@@ -4,13 +4,14 @@
 #define LIGHTNING_COMMON_JSON_COMMAND_H
 #include "config.h"
 #include <ccan/compiler/compiler.h>
+#include <common/errcode.h>
 #include <stdbool.h>
 
 struct command;
 struct command_result;
 
 /* Caller supplied this: param assumes it can call it. */
-struct command_result *command_fail(struct command *cmd, int code,
+struct command_result *command_fail(struct command *cmd, errcode_t code,
 				    const char *fmt, ...)
 	PRINTF_FMT(3, 4) WARN_UNUSED_RESULT;
 

common/jsonrpc_errors.h

@@ -3,55 +3,59 @@
  */
 #ifndef LIGHTNING_COMMON_JSONRPC_ERRORS_H
 #define LIGHTNING_COMMON_JSONRPC_ERRORS_H
+
 #include "config.h"
 
+#include <common/errcode.h>
+
 /* Standard errors defined by JSON-RPC 2.0 standard */
-#define JSONRPC2_INVALID_REQUEST	-32600
-#define JSONRPC2_METHOD_NOT_FOUND	-32601
-#define JSONRPC2_INVALID_PARAMS		-32602
+static const errcode_t JSONRPC2_INVALID_REQUEST = -32600;
+static const errcode_t JSONRPC2_METHOD_NOT_FOUND = -32601;
+static const errcode_t JSONRPC2_INVALID_PARAMS = -32602;
 
 /* Uncategorized error.
  * FIXME: This should be replaced in all places
  * with a specific error code, and then removed.
  */
-#define LIGHTNINGD                      -1
+static const errcode_t LIGHTNINGD = -1;
 
 /* Developer error in the parameters to param() call */
-#define PARAM_DEV_ERROR                 -2
+static const errcode_t PARAM_DEV_ERROR = -2;
 
 /* Plugin returned an error */
-#define PLUGIN_ERROR                    -3
+static const errcode_t PLUGIN_ERROR = -3;
 
 /* Errors from `pay`, `sendpay`, or `waitsendpay` commands */
-#define PAY_IN_PROGRESS			200
-#define PAY_RHASH_ALREADY_USED		201
-#define PAY_UNPARSEABLE_ONION		202
-#define PAY_DESTINATION_PERM_FAIL	203
-#define PAY_TRY_OTHER_ROUTE		204
-#define PAY_ROUTE_NOT_FOUND		205
-#define PAY_ROUTE_TOO_EXPENSIVE		206
-#define PAY_INVOICE_EXPIRED             207
-#define PAY_NO_SUCH_PAYMENT		208
-#define PAY_UNSPECIFIED_ERROR		209
-#define PAY_STOPPED_RETRYING		210
+static const errcode_t PAY_IN_PROGRESS = 200;
+static const errcode_t PAY_RHASH_ALREADY_USED = 201;
+static const errcode_t PAY_UNPARSEABLE_ONION = 202;
+static const errcode_t PAY_DESTINATION_PERM_FAIL = 203;
+static const errcode_t PAY_TRY_OTHER_ROUTE = 204;
+static const errcode_t PAY_ROUTE_NOT_FOUND = 205;
+static const errcode_t PAY_ROUTE_TOO_EXPENSIVE = 206;
+static const errcode_t PAY_INVOICE_EXPIRED = 207;
+static const errcode_t PAY_NO_SUCH_PAYMENT = 208;
+static const errcode_t PAY_UNSPECIFIED_ERROR = 209;
+static const errcode_t PAY_STOPPED_RETRYING = 210;
 
 /* `fundchannel` or `withdraw` errors */
-#define FUND_MAX_EXCEEDED               300
-#define FUND_CANNOT_AFFORD              301
-#define FUND_OUTPUT_IS_DUST             302
-#define FUNDING_BROADCAST_FAIL          303
-#define FUNDING_STILL_SYNCING_BITCOIN   304
-#define FUNDING_PEER_NOT_CONNECTED      305
-#define FUNDING_UNKNOWN_PEER            306
+static const errcode_t FUND_MAX_EXCEEDED = 300;
+static const errcode_t FUND_CANNOT_AFFORD = 301;
+static const errcode_t FUND_OUTPUT_IS_DUST = 302;
+static const errcode_t FUNDING_BROADCAST_FAIL = 303;
+static const errcode_t FUNDING_STILL_SYNCING_BITCOIN = 304;
+static const errcode_t FUNDING_PEER_NOT_CONNECTED = 305;
+static const errcode_t FUNDING_UNKNOWN_PEER = 306;
 
 /* `connect` errors */
-#define CONNECT_NO_KNOWN_ADDRESS	400
-#define CONNECT_ALL_ADDRESSES_FAILED	401
+static const errcode_t CONNECT_NO_KNOWN_ADDRESS = 400;
+static const errcode_t CONNECT_ALL_ADDRESSES_FAILED = 401;
 
 /* Errors from `invoice` command */
-#define INVOICE_LABEL_ALREADY_EXISTS	900
-#define INVOICE_PREIMAGE_ALREADY_EXISTS	901
-#define INVOICE_HINTS_GAVE_NO_ROUTES	902
-#define INVOICE_WAIT_TIMED_OUT		904
+static const errcode_t INVOICE_LABEL_ALREADY_EXISTS = 900;
+static const errcode_t INVOICE_PREIMAGE_ALREADY_EXISTS = 901;
+static const errcode_t INVOICE_HINTS_GAVE_NO_ROUTES = 902;
+static const errcode_t INVOICE_EXPIRED_DURING_WAIT = 903;
+static const errcode_t INVOICE_WAIT_TIMED_OUT = 904;
 
 #endif /* LIGHTNING_COMMON_JSONRPC_ERRORS_H */

common/test/run-param.c

@@ -5,6 +5,7 @@
 #include "../param.c"
 #include <ccan/array_size/array_size.h>
 #include <ccan/err/err.h>
+#include <common/errcode.h>
 #include <common/json.h>
 #include <setjmp.h>
 #include <signal.h>
@@ -28,7 +29,7 @@ struct command_result {
 static struct command_result cmd_failed;
 
 struct command_result *command_fail(struct command *cmd,
-				    int code, const char *fmt, ...)
+				    errcode_t code, const char *fmt, ...)
 {
 	failed = true;
 	va_list ap;

connectd/connect_wire.csv

@@ -44,7 +44,7 @@ msgdata,connectctl_connect_to_peer,addrhint,?wireaddr_internal,
 # Connectd->master: connect failed.
 msgtype,connectctl_connect_failed,2020
 msgdata,connectctl_connect_failed,id,node_id,
-msgdata,connectctl_connect_failed,failcode,int,
+msgdata,connectctl_connect_failed,failcode,errcode_t,
 msgdata,connectctl_connect_failed,failreason,wirestring,
 msgdata,connectctl_connect_failed,seconds_to_delay,u32,
 msgdata,connectctl_connect_failed,addrhint,?wireaddr_internal,

connectd/connectd.c

@@ -29,6 +29,7 @@
 #include <common/cryptomsg.h>
 #include <common/daemon_conn.h>
 #include <common/decode_array.h>
+#include <common/errcode.h>
 #include <common/features.h>
 #include <common/jsonrpc_errors.h>
 #include <common/memleak.h>
@@ -566,15 +567,15 @@ static void connect_failed(struct daemon *daemon,
 			   const struct node_id *id,
 			   u32 seconds_waited,
 			   const struct wireaddr_internal *addrhint,
-			   int errcode,
+			   errcode_t errcode,
 			   const char *errfmt, ...)
 	PRINTF_FMT(6,7);
 
 static void connect_failed(struct daemon *daemon,
 			   const struct node_id *id,
 			   u32 seconds_waited,
 			   const struct wireaddr_internal *addrhint,
-			   int errcode,
+			   errcode_t errcode,
 			   const char *errfmt, ...)
 {
 	u8 *msg;

doc/lightning-waitinvoice.7.md

@@ -18,12 +18,12 @@ RETURN VALUE
 On success, an invoice description will be returned as per
 lightning-listinvoice(7). The *status* field will be *paid*.
 
-If the invoice is deleted while unpaid, or the invoice does not exist,
-this command will return with an error with code -1.
+On error the returned object will contain `code` and `message` properties,
+with `code` being one of the following:
 
-If the invoice expires before being paid, or is already expired, this
-command will return with an error with code -2, with the data being the
-invoice data as per **listinvoice**.
+- -32602: If the given parameters are wrong.
+- -1: If the invoice is deleted while unpaid, or the invoice does not exist.
+- 903: If the invoice expires before being paid, or is already expired.
 
 AUTHOR
 ------

lightningd/connect_control.c

@@ -2,6 +2,7 @@
 #include <ccan/fdpass/fdpass.h>
 #include <ccan/list/list.h>
 #include <ccan/tal/str/str.h>
+#include <common/errcode.h>
 #include <common/features.h>
 #include <common/json_command.h>
 #include <common/json_helpers.h>
@@ -231,7 +232,7 @@ void delay_then_reconnect(struct channel *channel, u32 seconds_delay,
 static void connect_failed(struct lightningd *ld, const u8 *msg)
 {
 	struct node_id id;
-	int errcode;
+	errcode_t errcode;
 	char *errmsg;
 	struct connect *c;
 	u32 seconds_to_delay;

lightningd/invoice.c

@@ -82,8 +82,7 @@ static struct command_result *tell_waiter(struct command *cmd,
 		json_add_invoice(response, details);
 		return command_success(cmd, response);
 	} else {
-		/* FIXME: -2 should be a constant in jsonrpc_errors.h.  */
-		response = json_stream_fail(cmd, -2,
+		response = json_stream_fail(cmd, INVOICE_EXPIRED_DURING_WAIT,
 					    "invoice expired during wait");
 		json_add_invoice(response, details);
 		json_object_end(response);