htlc: Add a checker function tellung us whether we can continue

This function ensures we have all the infos we need to continue if the
htlc_accepted hook tells us to. It also enforces well-formedness of the TLV
payload if we have a TLV payload.

Suggested-by: List Neigut <@niftynei>
Signed-off-by: Christian Decker <@cdecker>

lightningd/peer_htlcs.c

@@ -31,6 +31,10 @@
 #include <wallet/wallet.h>
 #include <wire/gen_onion_wire.h>
 
+#ifndef SUPERVERBOSE
+#define SUPERVERBOSE(...)
+#endif
+
 static bool state_update_ok(struct channel *channel,
 			    enum htlc_state oldstate, enum htlc_state newstate,
 			    u64 htlc_id, const char *dir)
@@ -763,6 +767,44 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p,
 	json_object_end(s);
 }
 
+/* Make sure that we can continue with a default action if the htlc_accepted
+ * hook tells us to. This means enforcing that we have the necessary
+ * information to forward, fail or accept, and that the TVL payload is encoded
+ * correctly. */
+static bool htlc_accepted_can_continue(struct route_step *rs)
+{
+	if (rs->type == SPHINX_TLV_PAYLOAD && !tlv_payload_is_valid(rs->payload.tlv)) {
+		SUPERVERBOSE("Encoding of TLV payload is invalid");
+		return false;
+	}
+
+	/* BOLT #4:
+	 *
+	 * The writer:
+	 *  - MUST include `amt_to_forward` and `outgoing_cltv_value` for every node.
+	 *  - MUST include `short_channel_id` for every non-final node.
+	 *  - MUST NOT include `short_channel_id` for the final node.
+	 *
+	 * The reader:
+	 *  - MUST return an error if `amt_to_forward` or `outgoing_cltv_value` are not present.
+	 */
+	if (rs->amt_to_forward == NULL) {
+		SUPERVERBOSE("Missing amt_to_forward in payload");
+		return false;
+	}
+
+	if (rs->outgoing_cltv == NULL) {
+		SUPERVERBOSE("Missing outgoing_cltv_value in payload");
+		return false;
+	}
+
+	if (rs->nextcase && rs->forward_channel == NULL) {
+		SUPERVERBOSE("Missing short_channel_id in payload");
+		return false;
+	}
+	return true;
+}
+
 /**
  * Callback when a plugin answers to the htlc_accepted hook
  */
@@ -779,32 +821,11 @@ htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request,
 	enum htlc_accepted_result result;
 	enum onion_type failure_code;
 	u8 *channel_update;
-	bool valid;
 	result = htlc_accepted_hook_deserialize(buffer, toks, &payment_preimage, &failure_code, &channel_update);
 
-        /* BOLT #4:
-	 *
-	 * The writer:
-	 *  - MUST include `amt_to_forward` and `outgoing_cltv_value` for every node.
-	 *  - MUST include `short_channel_id` for every non-final node.
-	 *  - MUST NOT include `short_channel_id` for the final node.
-	 *
-	 * The reader:
-	 *  - MUST return an error if `amt_to_forward` or `outgoing_cltv_value` are not present.
-	 */
-	valid = rs->amt_to_forward != NULL && rs->outgoing_cltv != NULL &&
-		(rs->nextcase == ONION_END ||
-		 (rs->nextcase == ONION_FORWARD && rs->forward_channel != NULL));
-
-	/* In addition we also enforce the TLV validity rules:
-	 *  - No unknown even types
-	 *  - Types in monotonical non-repeating order
-	 */
-	valid = valid && (rs->type == SPHINX_V0_PAYLOAD || tlv_payload_is_valid(rs->payload.tlv));
-
 	switch (result) {
 	case htlc_accepted_continue:
-		if (!valid) {
+		if (!htlc_accepted_can_continue(rs)) {
 			log_debug(channel->log, "Failing HTLC because of an invalid payload");
 			failure_code = WIRE_INVALID_ONION_PAYLOAD;
 			fail_in_htlc(hin, failure_code, NULL, NULL);