From 7b23db280cd645b03e51ab90f20c017b7f063b0b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 14 Apr 2020 11:36:00 +0930 Subject: [PATCH] plugins: simplify htlc_accepted hook payload-setting API. As discussed with Christian, prepending the length to the payload returned is awkward, but it's the only way to set a legacy payload. As this will be soon deprecated, simplify the external API. Signed-off-by: Rusty Russell --- doc/PLUGINS.md | 6 ++++-- lightningd/peer_htlcs.c | 21 ++++++++++++++++++++- plugins/keysend.c | 8 ++------ tests/plugins/replace_payload.py | 9 +++++++-- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index 05905ac3f..3d36ca1af 100644 --- a/doc/PLUGINS.md +++ b/doc/PLUGINS.md @@ -804,8 +804,10 @@ if we're the recipient, or attempt to forward it otherwise. Notice that the usual checks such as sufficient fees and CLTV deltas are still enforced. It can also replace the `onion.payload` by specifying a `payload` in -the response. This will be re-parsed; it's useful for removing onion -fields which a plugin doesn't want lightningd to consider. +the response. Note that this is always a TLV-style payload, so unlike +`onion.payload` there is no length prefix (and it must be at least 4 +hex digits long). This will be re-parsed; it's useful for removing +onion fields which a plugin doesn't want lightningd to consider. ```json diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 2a1912fd5..99638a781 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -870,6 +870,11 @@ htlc_accepted_hook_deserialize(const tal_t *ctx, " hook: %.*s", payloadtok->end - payloadtok->start, buffer + payloadtok->start); + if (tal_bytelen(*payload) < 2) + fatal("Too short payload for htlc_accepted" + " hook: %.*s", + payloadtok->end - payloadtok->start, + buffer + payloadtok->start); } else *payload = NULL; @@ -1011,6 +1016,18 @@ htlc_accepted_hook_try_resolve(struct htlc_accepted_hook_payload *request, } } +static u8 *prepend_length(const tal_t *ctx, const u8 *payload) +{ + u8 buf[BIGSIZE_MAX_LEN], *ret; + size_t len; + + len = bigsize_put(buf, tal_bytelen(payload)); + ret = tal_arr(ctx, u8, len + tal_bytelen(payload)); + memcpy(ret, buf, len); + memcpy(ret + len, payload, tal_bytelen(payload)); + return ret; +} + /** * Callback when a plugin answers to the htlc_accepted hook */ @@ -1033,9 +1050,11 @@ htlc_accepted_hook_callback(struct htlc_accepted_hook_payload *request, /* If we have a replacement payload, parse it now. */ if (payload) { + /* To distinguish legacy and non-legacy, we always prepend length + * to tlv-style payloads */ tal_free(request->payload); tal_free(rs->raw_payload); - rs->raw_payload = tal_steal(rs, payload); + rs->raw_payload = prepend_length(rs, payload); request->payload = onion_decode(request, rs, hin->blinding, &hin->blinding_ss, &request->failtlvtype, diff --git a/plugins/keysend.c b/plugins/keysend.c index c047526ab..4506d0742 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -18,17 +18,13 @@ static struct command_result * htlc_accepted_continue(struct command *cmd, struct tlv_tlv_payload *payload) { struct json_stream *response; - u8 *binpayload, *rawpayload; response = jsonrpc_stream_success(cmd); json_add_string(response, "result", "continue"); if (payload) { - binpayload = tal_arr(cmd, u8, 0); + u8 *binpayload = tal_arr(cmd, u8, 0); towire_tlvstream_raw(&binpayload, payload->fields); - rawpayload = tal_arr(cmd, u8, 0); - towire_bigsize(&rawpayload, tal_bytelen(binpayload)); - towire(&rawpayload, binpayload, tal_bytelen(binpayload)); - json_add_string(response, "payload", tal_hex(cmd, rawpayload)); + json_add_string(response, "payload", tal_hex(cmd, binpayload)); } return command_finished(cmd, response); } diff --git a/tests/plugins/replace_payload.py b/tests/plugins/replace_payload.py index 0c55c2c41..122899653 100755 --- a/tests/plugins/replace_payload.py +++ b/tests/plugins/replace_payload.py @@ -13,11 +13,16 @@ plugin = Plugin() def on_htlc_accepted(htlc, onion, plugin, **kwargs): # eg. '2902017b04016d0821fff5b6bd5018c8731aa0496c3698ef49f132ef9a3000c94436f4957e79a2f8827b' # (but values change depending on pay's randomness!) + print("payload was:{}".format(onion['payload'])) + assert onion['payload'][0:2] == '29' + if plugin.replace_payload == 'corrupt_secret': + # Note: we don't include length prefix in returned payload, since it doesn't + # support the pre-TLV legacy form. if onion['payload'][18] == '0': - newpayload = onion['payload'][:18] + '1' + onion['payload'][19:] + newpayload = onion['payload'][2:18] + '1' + onion['payload'][19:] else: - newpayload = onion['payload'][:18] + '0' + onion['payload'][19:] + newpayload = onion['payload'][2:18] + '0' + onion['payload'][19:] else: newpayload = plugin.replace_payload print("payload was:{}".format(onion['payload']))