Make payment_secret mandatory and ASSUMED by erickcestari · Pull Request #1 · erickcestari/bolts

erickcestari · 2025-03-28T19:45:11Z

Background

The BOLT 11 payment secret (s field) was previously required for writers but not for readers (as implemented in lightning#887), likely to maintain backward compatibility with unupgraded nodes during the initial rollout period. Now that several years have passed and implementations have widely adopted this requirement, this PR updates the spec to make the payment secret field mandatory for both writer and reader, and updates the payment_secret feature to ASSUMED status in BOLT 9.

Changes

Added requirement for readers to fail payment if the s field is missing
Remove requirement to skip 's' fields with incorrect length
Added a test vector demonstrating an invalid invoice that's missing the s field
Changed the payment_secret feature to ASSUMED in BOLT 9, since all nodes are now required to support it

Motivation

This change improves protocol privacy by ensuring that all Lightning invoices contain a payment secret. The payment secret prevents intermediate nodes in the payment path from probing for the destination by generating their own payment onions. Since implementations like LDK have already implemented this requirement (refusing to pay invoices missing a payment secret) and it's been ~4 years since the writer-side requirement was introduced, formalizing it in the specification makes sense. At this point, all implementations now always write payment secrets, and other implementations should follow LDK's example in refusing to process invoices without them.

Making the payment_secret feature ASSUMED in BOLT 9 reflects the reality that this feature has become standard across the Lightning Network and should be considered a core part of the protocol rather than an optional feature.

morehouse · 2025-03-28T23:11:56Z

+  - MUST require a valid `s` field to be present
+    - MUST fail the payment if no valid `s` field is found
+    - MUST use the `s` field value as [`payment_secret`](04-onion-routing.md#tlv_payload-payload-format)


For consistency, I think we should use wording that's similar to the other requirements. Something like:

Suggested change

- MUST require a valid `s` field to be present

- MUST fail the payment if no valid `s` field is found

- MUST use the `s` field value as [`payment_secret`](04-onion-routing.md#tlv_payload-payload-format)

- if a valid `s` field is not provided:

- MUST fail the payment.

- otherwise:

- MUST use the `s` field as [`payment_secret`](04-onion-routing.md#tlv_payload-payload-format)

Question: "MUST fail the payment if no valid s field is found". What would be an invalid s field? it should fail anyway, no? It seems like it should be "MUST fail the payment if no s field is found"?

An s field that's not len=52 would be invalid. Your suggested wording would mean the reader should not fail if such a field is found.

Yes, you're right, thanks

morehouse · 2025-03-28T23:41:54Z

Background

The payment secret (s field) was previously required for writers but not for readers, likely to maintain backward compatibility with unupgraded nodes during the initial rollout period. Now that sufficient time has passed and implementations have widely adopted this requirement, this PR updates the spec to make the payment secret field mandatory for both sides of the protocol.

Suggestion: "The BOLT 11 payment secret (s field)..."

Motivation

This change improves protocol consistency by ensuring that all Lightning invoices contain a payment secret, which enhances security and payment reliability. Since implementations like LDK have already implemented this requirement, formalizing it in the specification makes sense.

I think this is more of a privacy enhancement than a security/reliability change. The idea is to prevent intermediate nodes in the payment path from probing for the destination by generating their own payment onions.

Also I think it would be helpful to point back to the PR that made the change on the writer side, and then explain that it's been X years since then and all implementations now always write payment secrets. Additionally, LDK already refuses to pay invoices that are missing a payment secret and other implementations should do the same

Potential BOLT 9 change

If we make the current change, it might also make sense to change the payment_secret feature to ASSUMED in BOLT 9, since all nodes are now required to use the feature. And if we do that, we might want to reword the PR title and description a bit to focus on that part of the PR.

brunoerg · 2025-03-30T22:42:19Z

Background

The payment secret (s field) was previously required for writers but not for readers, likely to maintain backward compatibility with unupgraded nodes during the initial rollout period. Now that sufficient time has passed and implementations have widely adopted this requirement, this PR updates the spec to make the payment secret field mandatory for both sides of the protocol.

nit: I think "for both sides of the protocol" sounds a little bit weird. Could be "for both writer and reader".

morehouse

Basically LGTM. The PR description is great!

morehouse · 2025-03-31T17:44:27Z

+  - if a valid `s` field is not provided:
+    - MUST fail the payment.
+  - otherwise:
+    - MUST use the `s` field as [`payment_secret`](04-onion-routing.md#tlv_payload-payload-format)


It might also make sense to remove the requirement further up that incorrect-length s fields be skipped. WDYT?

Yes, I agree!

brunoerg

ACK ce17d45

brunoerg · 2025-03-31T18:37:04Z

 A reader:
-  - MUST skip over unknown fields, OR an `f` field with unknown `version`, OR  `p`, `h`, `s` or
+  - MUST skip over unknown fields, OR an `f` field with unknown `version`, OR  `p`, `h` or
  `n` fields that do NOT have `data_length`s of 52, 52, 52 or 53, respectively.


Since you removed the s you have to update here as well?

Suggested change

`n` fields that do NOT have `data_length`s of 52, 52, 52 or 53, respectively.

`n` fields that do NOT have `data_length`s of 52, 52, or 53, respectively.

Yes, good catch. I completely forgot about this. Thanks for pointing it out.

Make the payment secret ('s' field) mandatory for readers in addition to writers, and update the payment_secret feature to ASSUMED status in BOLT 9. This formalizes the expectation that all Lightning invoices must include a payment secret after ~4 years of writer-side requirements. The payment secret prevents intermediate nodes in the payment path from probing for the destination by generating their own payment onions, enhancing privacy in the Lightning Network. - Add requirement for readers to fail payment if 's' field is missing - Remove requirement to skip 's' fields with incorrect length - Add test vector demonstrating an invalid invoice missing 's' field - Change payment_secret feature to ASSUMED in BOLT 9 This aligns with existing implementations like LDK which already refuse to pay invoices missing a payment secret. Co-authored-by: morehouse <mattmorehouse@gmail.com> Co-authored-by: brunoerg <brunoely.gc@gmail.com>

brunoerg

reACK 7dbbe9c

morehouse

ACK 7dbbe9c

erickcestari force-pushed the require-payment-secret-for-readers branch from 79c9a63 to cae28f6 Compare March 28, 2025 19:49

morehouse reviewed Mar 28, 2025

View reviewed changes

erickcestari force-pushed the require-payment-secret-for-readers branch 2 times, most recently from 579c90c to d292135 Compare March 31, 2025 12:34

erickcestari requested review from brunoerg and morehouse March 31, 2025 12:44

erickcestari changed the title ~~Make payment secret required for readers as well as writers~~ Make payment_secret mandatory and ASSUMED Mar 31, 2025

erickcestari force-pushed the require-payment-secret-for-readers branch 2 times, most recently from c6c8819 to ce17d45 Compare March 31, 2025 12:48

morehouse approved these changes Mar 31, 2025

View reviewed changes

brunoerg approved these changes Mar 31, 2025

View reviewed changes

erickcestari force-pushed the require-payment-secret-for-readers branch from ce17d45 to 523f97d Compare March 31, 2025 18:22

brunoerg reviewed Mar 31, 2025

View reviewed changes

erickcestari force-pushed the require-payment-secret-for-readers branch from 523f97d to a8cdbd0 Compare March 31, 2025 18:44

erickcestari force-pushed the require-payment-secret-for-readers branch from a8cdbd0 to 7dbbe9c Compare March 31, 2025 18:46

brunoerg approved these changes Mar 31, 2025

View reviewed changes

erickcestari requested a review from morehouse March 31, 2025 18:48

morehouse approved these changes Mar 31, 2025

View reviewed changes

erickcestari closed this Apr 1, 2025

	`n` fields that do NOT have `data_length`s of 52, 52, 52 or 53, respectively.
	`n` fields that do NOT have `data_length`s of 52, 52, or 53, respectively.

Conversation

erickcestari commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Changes

Motivation

Uh oh!

morehouse Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

brunoerg Mar 30, 2025

Choose a reason for hiding this comment

Uh oh!

morehouse Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brunoerg Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

morehouse commented Mar 28, 2025

Background

Motivation

Potential BOLT 9 change

Uh oh!

brunoerg commented Mar 30, 2025

Background

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

morehouse Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

erickcestari Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

brunoerg left a comment

Choose a reason for hiding this comment

Uh oh!

brunoerg Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

erickcestari Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

brunoerg left a comment

Choose a reason for hiding this comment

Uh oh!

morehouse left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

erickcestari commented Mar 28, 2025 •

edited

Loading

morehouse Mar 31, 2025 •

edited

Loading