Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add payment metadata to payment request (feature 48) #912

Merged
merged 1 commit into from
Jan 3, 2022
Merged

Add payment metadata to payment request (feature 48) #912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions 04-onion-routing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,9 @@ It is formatted according to the Type-Length-Value format defined in [BOLT #1](0
2. data:
* [`32*byte`:`payment_secret`]
* [`tu64`:`total_msat`]
1. type: 16 (`payment_metadata`)
2. data:
* [`...*byte`:`payment_metadata`]

### Requirements

Expand All @@ -280,6 +283,9 @@ The writer:
- MUST include `payment_data`
- MUST set `payment_secret` to the one provided
- MUST set `total_msat` to the total amount it will send
- if the recipient provided `payment_metadata`:
- MUST include `payment_metadata` with every HTLC
- MUST not apply any limits to the size of payment_metadata except the limits implied by the fixed onion size

The reader:
- MUST return an error if `amt_to_forward` or `outgoing_cltv_value` are not present.
Expand All @@ -302,6 +308,9 @@ ultimate sender that the rest of the payment will follow in succeeding
HTLCs; we call these outstanding HTLCs which have the same preimage,
an "HTLC set".

`payment_metadata` is to be included in every payment part, so that
invalid payment details can be detected as early as possible.

#### Requirements

The writer:
Expand Down Expand Up @@ -948,9 +957,9 @@ handling by the processing node.
* [`u32`:`height`]

The `payment_hash` is unknown to the final node, the `payment_secret` doesn't
match the `payment_hash`, the amount for that `payment_hash` is incorrect or
match the `payment_hash`, the amount for that `payment_hash` is incorrect,
the CLTV expiry of the htlc is too close to the current block height for safe
handling.
handling or `payment_metadata` isn't present while it should be.

The `htlc_msat` parameter is superfluous, but left in for backwards
compatibility. The value of `htlc_msat` always matches the amount specified in
Expand Down
1 change: 1 addition & 0 deletions 09-features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ The Context column decodes as follows:
| 22/23 | `option_anchors_zero_fee_htlc_tx` | Anchor commitment type with zero fee HTLC transactions | IN | | [BOLT #3][bolt03-htlc-tx], [lightning-dev][ml-sighash-single-harmful]|
| 26/27 | `option_shutdown_anysegwit` | Future segwit versions allowed in `shutdown` | IN | | [BOLT #2][bolt02-shutdown] |
| 44/45 | `option_channel_type` | Node supports the `channel_type` field in open/accept | IN | | [BOLT #2](02-peer-protocol.md#the-open_channel-message) |
| 48/49 | `option_payment_metadata` | Payment metadata in tlv record | 9 | | [BOLT #11](11-payment-encoding.md#tagged-fields)

## Definitions

Expand Down
35 changes: 34 additions & 1 deletion 11-payment-encoding.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ Currently defined tagged fields are:
* `p` (1): `data_length` 52. 256-bit SHA256 payment_hash. Preimage of this provides proof of payment.
* `s` (16): `data_length` 52. This 256-bit secret prevents forwarding nodes from probing the payment recipient.
* `d` (13): `data_length` variable. Short description of purpose of payment (UTF-8), e.g. '1 cup of coffee' or 'ナンセンス 1杯'
* `m` (27): `data_length` variable. Additional metadata to attach to the
payment. Note that the size of this field is limited by the maximum hop payload size. Long metadata fields reduce the maximum route length.
* `n` (19): `data_length` 53. 33-byte public key of the payee node
* `h` (23): `data_length` 52. 256-bit description of purpose of payment (SHA256). This is used to commit to an associated description that is over 639 bytes, but the transport mechanism for the description in that case is transport specific and not defined here.
* `x` (6): `data_length` variable. `expiry` time in seconds (big-endian). Default is 3600 (1 hour) if not specified.
Expand Down Expand Up @@ -216,7 +218,8 @@ A reader:
- MUST use that as [`payment_secret`](04-onion-routing.md#tlv_payload-payload-format)
- if the `c` field (`min_final_cltv_expiry`) is not provided:
- MUST use an expiry delta of at least 18 when making the payment

- if an `m` field is provided:
- MUST use that as [`payment_metadata`](04-onion-routing.md#tlv_payload-payload-format)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: please add an empty line between this and ### Rationale

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the payer does not use this as payment_metadata in the tlv_payload?

In BOLT 04 we have the onion failure message 1. type: PERM|15 (incorrect_or_unknown_payment_details)

https://github.com/joostjager/lightning-rfc/blob/custom-invoice-data/04-onion-routing.md#L953 currently reads:

The payment_hash is unknown to the final node, the payment_secret doesn't
match the payment_hash, the amount for that payment_hash is incorrect or
the CLTV expiry of the htlc is too close to the current block height for safe
handling.

Maybe we should also include that this error is sent if the payment_metadata was not included?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can use invalid_onion_payload?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit confused right now. Which of the two error codes protects the privacy of the recipient better?

I guess if an intermediate malicious node interrupts routing and sends onions to various candidate recipients without including payment_metadata those wrong candidates would regularly send incorrect_or_unknown_payment_details back to the intermediate node. However following your suggestion the real recipient would send back invalid_onion_payload as they expected the payment_metadata field. Thus my feeling is that incorrect_or_unknown_payment_details will be more secure as it prevents such an attack. Following #627 I am under the impression that invalid_onion_payload is intended to be used if something goes wrong for a routing node when decoding the onion. But maybe I missed something and @t-bast can clarify?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thus my feeling is that incorrect_or_unknown_payment_details will be more secure as it prevents such an attack.

I agree. If you make payment_metadata mandatory and if it wasn't included in the onion you receive, I would send back incorrect_or_unknown_payment_details. You mandated the payment to include a payment_metadata and it didn't, so it's really an incorrect payment (and gives the best anonymity set as it's the error used whenever something is invalid with the payment at the recipient's end).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated PR to return incorrect_or_unknown_payment_details.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear @joostjager I am very pleased that you found my review & suggestion useful. Yet I am a bit surprised that you did not accept the patch that I provided 5 days ago with my suggestion and for your convenience directly as a pull request to your branch at joostjager#1. I hope the reason is that you have simply overseen my pull request and patch. As you did not credit my contribution in your recent commit may I ask you kindly to revert the most recent addition and accept the provided patch instead? Thank you very much!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lightning-developer, No I didn't see your patch on my personal repo.

Generally speaking and also in this particular case, lots of people contribute ideas. I think it will get complicated to list all of them as co-authors. They may not all have contributed text literally, but an idea often deserves credit too.

### Rationale

The type-and-length format allows future extensions to be backward
Expand All @@ -235,6 +238,9 @@ by which the description is served is as-yet unspecified and will
probably be transport dependent. The `h` format could change in the future,
by changing the length, so readers ignore it if it's not 256 bits.

The `m` field allows metadata to be attached to the payment. This supports
joostjager marked this conversation as resolved.
Show resolved Hide resolved
applications where the recipient doesn't keep any context for the payment.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more a rationale for how this field could be used, but i can see other use cases as well (split PoS and recipient, ...). I'd personally prefer having these separate from the prescriptive text.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this part is already inside the rationale section, isn't it? So it should be ok? Or do you mean that you don't want something as concrete here, and would rather have a separate doc (a spark?) explain how the payment_metadata field can be used for invoices not stored in a local DB?


The `n` field can be used to explicitly specify the destination node ID,
instead of requiring signature recovery.

Expand Down Expand Up @@ -704,6 +710,33 @@ Breakdown:
* `z599y53s3ujmcfjp5xrdap68qxymkqphwsexhmhr8wdz5usdzkzrse33chw6dlp3jhuhge9ley7j2ayx36kawe7kmgg8sv5ugdyusdcq`: signature
* `zn8z9x`: Bech32 checksum

> ### Please send 0.01 BTC with payment metadata 0x01fafaf0
> lnbc10m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdp9wpshjmt9de6zqmt9w3skgct5vysxjmnnd9jx2mq8q8a04uqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q2gqqqqqqsgq7hf8he7ecf7n4ffphs6awl9t6676rrclv9ckg3d3ncn7fct63p6s365duk5wrk202cfy3aj5xnnp5gs3vrdvruverwwq7yzhkf5a3xqpd05wjc

Breakdown:

* `lnbc`: prefix, Lightning on Bitcoin mainnet
* `10m`: amount (10 milli-bitcoin)
* `1`: Bech32 separator
* `pvjluez`: timestamp (1496314658)
* `p`: payment hash
* `p5`: `data_length` (`p` = 1, `5` = 20; 1 * 32 + 20 == 52)
* `qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypq`: payment hash 0001020304050607080900010203040506070809000102030405060708090102
* `d`: short description
* `p9`: `data_length` (`p` = 1, `9` = 5; 1 * 32 + 5 == 37)
* `wpshjmt9de6zqmt9w3skgct5vysxjmnnd9jx2`: 'payment metadata inside'
* `m`: metadata
* `q8`: `data_length` (`q` = 0, `8` = 7; 0 * 32 + 7 == 7)
* `q8a04uq`: 0x01fafaf0
* `s`: payment secret
* `p5`: `data_length` (`p` = 1, `5` = 20; 1 * 32 + 20 == 52)
* `zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs`: 0x1111111111111111111111111111111111111111111111111111111111111111
* `9`: features
* `q2`: `data_length` (`q` = 0, `2` = 10; 0 * 32 + 10 == 10)
* `gqqqqqqsgq`: [b01000000000000000000000000000000000100000100000000] = 8 + 14 + 48
* `7hf8he7ecf7n4ffphs6awl9t6676rrclv9ckg3d3ncn7fct63p6s365duk5wrk202cfy3aj5xnnp5gs3vrdvruverwwq7yzhkf5a3xqp`: signature
* `d05wjc`: Bech32 checksum

# Examples of Invalid Invoices

> # Same, but adding invalid unknown feature 100
Expand Down