Skip to content

BOLT4: don't allow a "fee" for the final node(failure messages)#718

Merged
t-bast merged 1 commit into
lightning:masterfrom
nueno4:bolt4_finalcheck
Jan 8, 2020
Merged

BOLT4: don't allow a "fee" for the final node(failure messages)#718
t-bast merged 1 commit into
lightning:masterfrom
nueno4:bolt4_finalcheck

Conversation

@nueno4
Copy link
Copy Markdown

@nueno4 nueno4 commented Dec 16, 2019

#711

For the final node, this value MUST be exactly equal to the incoming htlc amount, otherwise the HTLC should be rejected.

> For the final node, this value MUST be exactly equal to the incoming htlc amount, otherwise the HTLC should be rejected.
Comment thread 04-onion-routing.md
the final node's HTLC:
- MUST return `final_incorrect_cltv_expiry` error.
- if the `amt_to_forward` is greater than the `incoming_htlc_amt` from the
- if the `amt_to_forward` does NOT correspond with the `incoming_htlc_amt` from the
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what's the rationale for having these match? as long as it the amt_to_forward does not exceed the incoming_htlc_amt there's no problem.

i'm not 100% sure about this, but i believe a few use cases currently take advantage of this to allow for paying nodes via fees rather than invoice amounts.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for your comment.
I read the following sentence in BOLT4:

For the final node, this value MUST be exactly equal to the incoming htlc amount, otherwise the HTLC should be rejected.

and understood as follow:

non final node:
  incoming_htlc_amt - fee >= amt_to_forward

final node:
  incoming_htlc_amt == amt_to_forward

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I understood it like you @nayuta-ueno, IIUC the change you propose should have been made at the same time as #711 but was overlooked?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

oh i see the context now. thanks.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh I missed the link to #711, thanks for the clarification.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@t-bast

the change you propose should have been made at the same time as #711 but was overlooked?

I think so too.

Copy link
Copy Markdown
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

ACK 83f77bf

Comment thread 04-onion-routing.md
the final node's HTLC:
- MUST return `final_incorrect_cltv_expiry` error.
- if the `amt_to_forward` is greater than the `incoming_htlc_amt` from the
- if the `amt_to_forward` does NOT correspond with the `incoming_htlc_amt` from the
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I understood it like you @nayuta-ueno, IIUC the change you propose should have been made at the same time as #711 but was overlooked?

@niftynei
Copy link
Copy Markdown
Collaborator

niftynei commented Jan 7, 2020

ACK 83f77bf

@t-bast t-bast merged commit f219ee0 into lightning:master Jan 8, 2020
niftynei pushed a commit to niftynei/lightning-rfc that referenced this pull request Jan 28, 2020
niftynei pushed a commit to niftynei/lightning-rfc that referenced this pull request Feb 17, 2020
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.

3 participants