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.

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

Rephrase last node payload requirements #615

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@OrfeasLitos
Copy link
Contributor

commented May 27, 2019

Mention that outgoing_cltv_value has to be equal to min_final_cltv_expiry and amt_to_forward has to be equal to amount, as seen in the invoice

@t-bast
Copy link
Collaborator

left a comment

Actually thinking about this more, I prefer the previous "vague" wording.
You don't necessarily need to use a Bolt 11 invoice to pay someone / receive a payment.
The current wording is generally correct and less likely to change when new payment techniques are added.

@t-bast t-bast dismissed their stale review Jul 8, 2019

Dismissing my review as after some thoughts, I prefer the original sentence.

@OrfeasLitos

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

The old wording however leaves it unclear where these numbers come from. It definitely took me a while to understand what it meant, but a spec should try to help the reader. What do you think about something like this:

  • outgoing_cltv_value: set to the final expiry specified by the recipient (e.g. min_final_cltv_expiry in invoice)
  • amt_to_forward: set to the final amount specified by the recipient (e.g. amount in invoice)

or any other version that mentions min_final_cltv_expiry and amount from the invoice, but also allows for other ways of defining outgoing_cltv_value and amt_to_forward?

@t-bast

This comment has been minimized.

Copy link
Collaborator

commented Jul 8, 2019

That sounds good!
I would replace in invoice by from a [BOLT #11](11-payment-encoding.md) payment invoice but that's nit.

@OrfeasLitos

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

Great, I'll change it now

Rephrase last node payload requirements
Mention that `outgoing_cltv_value` has to be equal to
`min_final_cltv_expiry` and `amt_to_forward` has to be equal to
`amount` if the [BOLT #11](11-payment-encoding.md) invoice is used

@OrfeasLitos OrfeasLitos force-pushed the OrfeasLitos:final-onion branch from 78238f4 to ae0146f Jul 8, 2019

@t-bast

This comment has been minimized.

Copy link
Collaborator

commented Jul 9, 2019

@cdecker could we get your opinion/ack on that small change?

@t-bast

t-bast approved these changes Jul 9, 2019

Copy link
Collaborator

left a comment

ACK ae0146f

@t-bast t-bast added the spelling label Jul 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.