lightningd: handle tlv-style payloads.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-changed: JSON API: `htlc_accepted` hook has `type` (currently `legacy` or `tlv`) and other fields directly inside `onion`.
Changelog-deprecated: JSON API: `htlc_accepted` hook `per_hop_v0` object deprecated, as is `short_channel_id` for the final hop.

common/sphinx.c

@@ -826,10 +826,10 @@ bool route_step_decode_end(const struct route_step *rs,
 	case SPHINX_TLV_PAYLOAD:
 		if (!rs->payload.tlv->amt_to_forward)
 			return false;
-		amt_forward->millisatoshis /* Raw: tu64 -> millisatoshis */
-			= rs->payload.tlv->amt_to_forward->amt_to_forward;
 		if (!rs->payload.tlv->outgoing_cltv_value)
 			return false;
+		amt_forward->millisatoshis /* Raw: tu64 -> millisatoshis */
+			= rs->payload.tlv->amt_to_forward->amt_to_forward;
 		*outgoing_cltv = rs->payload.tlv->outgoing_cltv_value->outgoing_cltv_value;
 		return true;
 	case SPHINX_INVALID_PAYLOAD:

doc/PLUGINS.md

@@ -588,8 +588,7 @@ The payload of the hook call has the following format:
 {
   "onion": {
     "payload": "",
-    "per_hop_v0": {
+    "type": "legacy",
-      "realm": "00",
     "short_channel_id": "1x2x3",
     "forward_amount": "42msat",
     "outgoing_cltv_value": 500014
@@ -606,18 +605,13 @@ The payload of the hook call has the following format:
 }
 ```
 
-The `per_hop_v0` will only be present if the per hop payload has format `0x00`
+For detailed information about each field please refer to [BOLT 04 of the specification][bolt4], the following is just a brief summary:
-as defined by the specification. If not present an object representing the
-type-length-vale (TLV) payload will be added (pending specification). For detailed information about each field please refer to [BOLT 04 of the specification][bolt4], the following is just a brief summary:
 
  - `onion.payload` contains the unparsed payload that was sent to us from the
    sender of the payment.
- - `onion.per_hop_v0`:
+ - `onion.type` is `legacy` for realm 0 payments, `tlv` for realm > 1.
-   - `realm` will always be `00` since that value determines that we are using
-     the `per_hop_v0` format.
  - `short_channel_id` determines the channel that the sender is hinting
-     should be used next (set to `0x0x0` if we are the recipient of the
+     should be used next.  Not present if we're the final destination.
-     payment).
  - `forward_amount` is the amount we should be forwarding to the next hop,
      and should match the incoming funds in case we are the recipient.
  - `outgoing_cltv_value` determines what the CLTV value for the HTLC that we

lightningd/peer_htlcs.c

@@ -636,6 +636,10 @@ struct htlc_accepted_hook_payload {
 	struct htlc_in *hin;
 	struct channel *channel;
 	struct lightningd *ld;
+	struct amount_msat amt_to_forward;
+	u32 outgoing_cltv_value;
+	/* NULL if this is node is final */
+	struct short_channel_id *short_channel_id;
 	u8 *next_onion;
 };
 
@@ -726,6 +730,7 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p,
 
 	json_add_hex_talarr (s, "payload", rs->raw_payload);
 	if (rs->type == SPHINX_V0_PAYLOAD) {
+		if (deprecated_apis) {
 			json_object_start(s, "per_hop_v0");
 			json_add_string(s, "realm", "00");
 			json_add_short_channel_id(s, "short_channel_id", &rs->payload.v0.channel_id);
@@ -733,7 +738,16 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p,
 			json_add_u64(s, "outgoing_cltv_value", rs->payload.v0.outgoing_cltv);
 			json_object_end(s);
 		}
+		json_add_string(s, "type", "legacy");
+	} else if (rs->type == SPHINX_TLV_PAYLOAD) {
+		json_add_string(s, "type", "tlv");
+	}
 
+	if (p->short_channel_id)
+		json_add_short_channel_id(s, "short_channel_id",
+					  p->short_channel_id);
+	json_add_amount_msat_only(s, "forward_amount", p->amt_to_forward);
+	json_add_u32(s, "outgoing_cltv_value", p->outgoing_cltv_value);
 	json_add_hex_talarr(s, "next_onion", p->next_onion);
 	json_add_secret(s, "shared_secret", hin->shared_secret);
 	json_object_end(s);
@@ -762,19 +776,17 @@ htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request,
 	enum htlc_accepted_result result;
 	enum onion_type failure_code;
 	u8 *channel_update;
-	struct hop_data_legacy *hop_data;
 	result = htlc_accepted_hook_deserialize(buffer, toks, &payment_preimage, &failure_code, &channel_update);
 
-	hop_data = &rs->payload.v0;
 	switch (result) {
 	case htlc_accepted_continue:
 		if (rs->nextcase == ONION_FORWARD) {
 			struct gossip_resolve *gr = tal(ld, struct gossip_resolve);
 
 			gr->next_onion = serialize_onionpacket(gr, rs->next);
-			gr->next_channel = hop_data->channel_id;
+			gr->next_channel = *request->short_channel_id;
-			gr->amt_to_forward = hop_data->amt_forward;
+			gr->amt_to_forward = request->amt_to_forward;
-			gr->outgoing_cltv_value = hop_data->outgoing_cltv;
+			gr->outgoing_cltv_value = request->outgoing_cltv_value;
 			gr->hin = hin;
 
 			req = towire_gossip_get_channel_peer(tmpctx, &gr->next_channel);
@@ -785,13 +797,19 @@ htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request,
 				 channel_resolve_reply, gr);
 		} else
 			handle_localpay(hin, hin->cltv_expiry, &hin->payment_hash,
-					hop_data->amt_forward,
+					request->amt_to_forward,
-					hop_data->outgoing_cltv);
+					request->outgoing_cltv_value);
 		break;
 	case htlc_accepted_fail:
 		log_debug(channel->log,
 			  "Failing incoming HTLC as instructed by plugin hook");
-		fail_in_htlc(hin, failure_code, NULL, &hop_data->channel_id);
+		if ((failure_code & UPDATE) && rs->nextcase == ONION_END) {
+			log_broken(channel->log,
+				   "htlc_acccepted hook: Can't return failure %u on last hop!",
+				   failure_code);
+			failure_code = WIRE_TEMPORARY_NODE_FAILURE;
+		}
+		fail_in_htlc(hin, failure_code, NULL, request->short_channel_id);
 		break;
 	case htlc_accepted_resolve:
 		fulfill_htlc(hin, &payment_preimage);
@@ -897,16 +915,32 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id,
 	}
 
 	/* Unknown realm isn't a bad onion, it's a normal failure. */
-	/* FIXME: if we want hooks to handle foreign realms we should
+	if (rs->type == SPHINX_INVALID_PAYLOAD) {
-	 * move this check to the hook callback. */
-	if (rs->type != SPHINX_V0_PAYLOAD) {
 		*failcode = WIRE_INVALID_REALM;
 		goto out;
 	}
 
-	/* It's time to package up all the information and call the
-	 * hook so plugins can interject if they want */
 	hook_payload = tal(hin, struct htlc_accepted_hook_payload);
+
+	if (rs->nextcase == ONION_END) {
+		if (!route_step_decode_end(rs, &hook_payload->amt_to_forward,
+					   &hook_payload->outgoing_cltv_value)) {
+			*failcode = WIRE_INVALID_ONION_PAYLOAD;
+			goto out;
+		}
+		hook_payload->short_channel_id = NULL;
+	} else {
+		hook_payload->short_channel_id
+			= tal(hook_payload, struct short_channel_id);
+		if (!route_step_decode_forward(rs,
+					       &hook_payload->amt_to_forward,
+					       &hook_payload->outgoing_cltv_value,
+					       hook_payload->short_channel_id)) {
+			*failcode = WIRE_INVALID_ONION_PAYLOAD;
+			goto out;
+		}
+	}
+
 	hook_payload->route_step = tal_steal(hook_payload, rs);
 	hook_payload->ld = ld;
 	hook_payload->hin = hin;

tests/test_plugin.py

@@ -537,8 +537,8 @@ def test_htlc_accepted_hook_forward_restart(node_factory, executor):
     assert re.match(r'^00006700000.000100000000000003e8000000..000000000000000000000000$', onion['payload'])
     assert len(onion['payload']) == 64
     assert len(onion['shared_secret']) == 64
-    assert onion['per_hop_v0']['realm'] == "00"
+    assert onion['type'] == 'legacy'
-    assert onion['per_hop_v0']['forward_amount'] == '1000msat'
+    assert onion['forward_amount'] == '1000msat'
     assert len(onion['next_onion']) == 2 * (1300 + 32 + 33 + 1)
 
     f1.result()