Skip to content

Conversation

@erickcestari
Copy link
Contributor

@erickcestari erickcestari commented Nov 26, 2025

Final nodes should reject payments that include short_channel_id in their payload, as this field is only meaningful for forwarding nodes. While BOLT 4 specifies that writers MUST NOT include short_channel_id for the final node, it doesn't explicitly require final nodes to return an error when this field is present.

Current spec (writer requirements):

For the final node:
  - MUST NOT include `short_channel_id`

Current spec (reader requirements):

If it is the final node:
  - MUST treat `total_msat` as if it were equal to `amt_to_forward` if it is not present.
  - MUST return an error if:
    - incoming `amount_msat` < `amt_to_forward`.
    - incoming `cltv_expiry` < `outgoing_cltv_value`.
    - incoming `cltv_expiry` < `current_block_height` + `min_final_cltv_expiry_delta`

This ambiguity has led to implementation inconsistencies:

This inconsistency was discovered through differential fuzzing (bitcoinfuzz), where the same onion was rejected by LND and rust-lightning but accepted by Core Lightning.

This PR adds an explicit requirement for final nodes to return an error if short_channel_id is present in the payload, making the validation requirements consistent with the writer requirements.

…nel_id

Final nodes should reject payments where short_channel_id is present
in their payload, as this field is only meaningful for forwarding nodes.

This requirement was implicit but not explicitly documented in the
validation rules for final nodes.
@t-bast
Copy link
Collaborator

t-bast commented Dec 15, 2025

It does create undefined behavior, but I'm not sure it's much of an issue since this case should never happen with a BOLT-compliant writer and it doesn't hurt the reader?

As a general rule of thumb, we must be strict in what we write, but we can be lenient in what we read when it's harmless, which avoids unnecessarily listing every harmless case in the spec to keep it somewhat compact. We have a lot of similar cases for various messages, and it would add a lot of requirements to the BOLTs to "fix" them all, so I'm not really sure we should do it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants