-
Notifications
You must be signed in to change notification settings - Fork 404
Follow-ups for ChannelManager::create_bolt11_invoice
#3405
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
Follow-ups for ChannelManager::create_bolt11_invoice
#3405
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh heh, I wasn't sure it was worth doing a followup for this, but glad I got to clarify my concern.
lightning/src/ln/channelmanager.rs
Outdated
}; | ||
|
||
// This may be up to 2 hours in the future because of bitcoin's block time rule or about | ||
// 10-30 minutes in the past in case of chain reorgs. This should be fine as the payment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I mean cause like blocks just sometimes aren't found for 10-30 minutes :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. That makes more sense. 😛
lightning/src/ln/channelmanager.rs
Outdated
|
||
// This may be up to 2 hours in the future because of bitcoin's block time rule or about | ||
// 10-30 minutes in the past in case of chain reorgs. This should be fine as the payment | ||
// secret includes a two hour buffer to ensure payments aren't failed too early. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern was really more the payment actually timing out in the invoice, causing the sender to not pay even though the invoice is still valid. IIRC the invoice default expiry is still two hours, tho, so we have some time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated and also expand the user-facing docs.
035212f
to
20ef217
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is trivial just gonna land.
Addresses some minor comments from #3389.