sphinx: Return the error in parse_onionpacket

As suggested by @niftynei here: ElementsProject#3260 (comment)

Suggested-by: Lisa Neigut <@niftynei>
Suggested-by: Rusty Russell <@rustyrussell>
Signed-off-by: Christian Decker <@cdecker>

channeld/channeld.c

@@ -584,25 +584,24 @@ static struct secret *get_shared_secret(const tal_t *ctx,
 					enum onion_type *why_bad,
 					struct sha256 *next_onion_sha)
 {
-	struct onionpacket *op;
+	struct onionpacket op;
 	struct secret *secret = tal(ctx, struct secret);
 	const u8 *msg;
 	struct route_step *rs;
 
 	/* We unwrap the onion now. */
-	op = parse_onionpacket(tmpctx, htlc->routing, TOTAL_PACKET_SIZE,
-			       why_bad);
-	if (!op)
+	*why_bad = parse_onionpacket(htlc->routing, TOTAL_PACKET_SIZE, &op);
+	if (*why_bad != 0)
 		return tal_free(secret);
 
 	/* Because wire takes struct pubkey. */
-	msg = hsm_req(tmpctx, towire_hsm_ecdh_req(tmpctx, &op->ephemeralkey));
+	msg = hsm_req(tmpctx, towire_hsm_ecdh_req(tmpctx, &op.ephemeralkey));
 	if (!fromwire_hsm_ecdh_resp(msg, secret))
 		status_failed(STATUS_FAIL_HSM_IO, "Reading ecdh response");
 
 	/* We make sure we can parse onion packet, so we know if shared secret
 	 * is actually valid (this checks hmac). */
-	rs = process_onionpacket(tmpctx, op, secret->data,
+	rs = process_onionpacket(tmpctx, &op, secret->data,
 				 htlc->rhash.u.u8,
 				 sizeof(htlc->rhash));
 	if (!rs) {

common/sphinx.c

@@ -272,36 +272,30 @@ u8 *serialize_onionpacket(
 	return dst;
 }
 
-struct onionpacket *parse_onionpacket(const tal_t *ctx,
-				      const void *src,
-				      const size_t srclen,
-				      enum onion_type *why_bad)
+enum onion_type parse_onionpacket(const u8 *src,
+				  const size_t srclen,
+				  struct onionpacket *dest)
 {
-	struct onionpacket *m;
 	int p = 0;
 	u8 rawEphemeralkey[PUBKEY_CMPR_LEN];
 
 	assert(srclen == TOTAL_PACKET_SIZE);
 
-	m = talz(ctx, struct onionpacket);
-
-	read_buffer(&m->version, src, 1, &p);
-	if (m->version != 0x00) {
+	read_buffer(&dest->version, src, 1, &p);
+	if (dest->version != 0x00) {
 		// FIXME add logging
-		*why_bad = WIRE_INVALID_ONION_VERSION;
-		return tal_free(m);
+		return WIRE_INVALID_ONION_VERSION;
 	}
 	read_buffer(rawEphemeralkey, src, sizeof(rawEphemeralkey), &p);
 
 	if (!pubkey_from_der(rawEphemeralkey, sizeof(rawEphemeralkey),
-			     &m->ephemeralkey)) {
-		*why_bad = WIRE_INVALID_ONION_KEY;
-		return tal_free(m);
+			     &dest->ephemeralkey)) {
+		return WIRE_INVALID_ONION_KEY;
 	}
 
-	read_buffer(&m->routinginfo, src, ROUTING_INFO_SIZE, &p);
-	read_buffer(&m->mac, src, HMAC_SIZE, &p);
-	return m;
+	read_buffer(&dest->routinginfo, src, ROUTING_INFO_SIZE, &p);
+	read_buffer(&dest->mac, src, HMAC_SIZE, &p);
+	return 0;
 }
 
 static void xorbytes(uint8_t *d, const uint8_t *a, const uint8_t *b, size_t len)

common/sphinx.h

@@ -168,15 +168,13 @@ u8 *serialize_onionpacket(
 /**
  * parse_onionpacket - Parse an onionpacket from a buffer.
  *
- * @ctx: tal context to allocate from
  * @src: buffer to read the packet from
  * @srclen: length of the @src (must be TOTAL_PACKET_SIZE)
- * @why_bad: if NULL return, this is what was wrong with the packet.
+ * @dest: the destination into which we should parse the packet
  */
-struct onionpacket *parse_onionpacket(const tal_t *ctx,
-				      const void *src,
-				      const size_t srclen,
-				      enum onion_type *why_bad);
+enum onion_type parse_onionpacket(const u8 *src,
+				  const size_t srclen,
+				  struct onionpacket *dest);
 
 struct onionreply {
 	/* Node index in the path that is replying */

devtools/onion.c

@@ -99,21 +99,21 @@ static struct route_step *decode_with_privkey(const tal_t *ctx, const u8 *onion,
 {
 	struct privkey seckey;
 	struct route_step *step;
-	struct onionpacket *packet;
+	struct onionpacket packet;
 	enum onion_type why_bad;
 	u8 shared_secret[32];
 	if (!hex_decode(hexprivkey, strlen(hexprivkey), &seckey, sizeof(seckey)))
 		errx(1, "Invalid private key hex '%s'", hexprivkey);
 
-	packet = parse_onionpacket(ctx, onion, TOTAL_PACKET_SIZE, &why_bad);
+	why_bad = parse_onionpacket(onion, TOTAL_PACKET_SIZE, &packet);
 
-	if (!packet)
+	if (why_bad != 0)
 		errx(1, "Error parsing message: %s", onion_type_name(why_bad));
 
-	if (!onion_shared_secret(shared_secret, packet, &seckey))
+	if (!onion_shared_secret(shared_secret, &packet, &seckey))
 		errx(1, "Error creating shared secret.");
 
-	step = process_onionpacket(ctx, packet, shared_secret, assocdata,
+	step = process_onionpacket(ctx, &packet, shared_secret, assocdata,
 				   tal_bytelen(assocdata));
 	return step;
 

lightningd/peer_htlcs.c

@@ -911,7 +911,7 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id,
 {
 	struct htlc_in *hin;
 	struct route_step *rs;
-	struct onionpacket *op;
+	struct onionpacket op;
 	struct lightningd *ld = channel->peer->ld;
 	struct htlc_accepted_hook_payload *hook_payload;
 
@@ -963,10 +963,10 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id,
 	/* FIXME: Have channeld hand through just the route_step! */
 
 	/* channeld tests this, so it should pass. */
-	op = parse_onionpacket(tmpctx, hin->onion_routing_packet,
-			       sizeof(hin->onion_routing_packet),
-			       failcode);
-	if (!op) {
+	*failcode = parse_onionpacket(hin->onion_routing_packet,
+				      sizeof(hin->onion_routing_packet),
+				      &op);
+	if (*failcode != 0) {
 		channel_internal_error(channel,
 				       "bad onion in got_revoke: %s",
 				       tal_hexstr(channel, hin->onion_routing_packet,
@@ -975,7 +975,7 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id,
 	}
 
 	/* If it's crap, not channeld's fault, just fail it */
-	rs = process_onionpacket(tmpctx, op, hin->shared_secret->data,
+	rs = process_onionpacket(tmpctx, &op, hin->shared_secret->data,
 				 hin->payment_hash.u.u8,
 				 sizeof(hin->payment_hash));
 	if (!rs) {

wallet/test/run-wallet.c

@@ -473,10 +473,9 @@ struct command_result *param_tok(struct command *cmd UNNEEDED, const char *name
 				 const jsmntok_t **out UNNEEDED)
 { fprintf(stderr, "param_tok called!\n"); abort(); }
 /* Generated stub for parse_onionpacket */
-struct onionpacket *parse_onionpacket(const tal_t *ctx UNNEEDED,
-				      const void *src UNNEEDED,
-				      const size_t srclen UNNEEDED,
-				      enum onion_type *why_bad UNNEEDED)
+enum onion_type parse_onionpacket(const u8 *src UNNEEDED,
+				  const size_t srclen UNNEEDED,
+				  struct onionpacket *dest UNNEEDED)
 { fprintf(stderr, "parse_onionpacket called!\n"); abort(); }
 /* Generated stub for payment_failed */
 void payment_failed(struct lightningd *ld UNNEEDED, const struct htlc_out *hout UNNEEDED,