Make cltv_expiry for keysends slightly more conservative

[valentinewallace]

Hi all! I'm working on making Rust-Lightning compatible with CL's keysend.

Would you all be open to a change like this? Rust-Lightning is a bit more conservative than the default 18 for min_final_cltv_expiry, so I thought it might be reasonable to make CL use a bit more conservative value too. Let me know what you think.

[cdecker]

Thanks for the proposal @valentinewallace, what's the rationale for bumping this higher? Is it just to add a bit more slack to out-of-sync nodes? And is 22 blocks somehow special?

IIRC the final CLTV delta serves two purposes:

• Be far enough in the future to give the recipient some time to claim the incoming payment, even though the sender may be lagging slightly with its blockchain (resulting in lower absolute CLTV values, potentially causing timeouts earlier than the sender intended)
• Be a bit of a smudging factor for the destination, allowing larger values to pretend that there are further hops. This is mostly forgotten today, and unlikely to be impacted by a constant increment of 4.

[valentinewallace]

Thanks for the response @cdecker!

what's the rationale for bumping this higher? Is it just to add a bit more slack to out-of-sync nodes? And is 22 blocks somehow special?

I don't think 22 is particularly special, but yeah, I believe tl;dr the idea is to account for our peer being behind on blocks. Partly, the theory is that this may protect us from channel failures for low-power, slow peers due to HTLC expiration.

(More context: in RL, the extra fudge factor is named LATENCY_GRACE_PERIOD_BLOCKS. More docs here, i.e. we require the final CLTV delta to be greater than HTLC_FAIL_BACK_BUFFER).

[TheBlueMatt]

The final CLTV should be configurable (as it is in invoices). Lack of ability for the recipient to control their risk tolerance via the CLTV delta limit is a major shortcoming of keysend (and afaiu lnd actually requires senders to provide it). Obviously forcing it be provided from senders is a pretty terrible UX, but all the more reason to be conservative and provide a CLTV delta that a recipient is likely to support, maybe something more like the BOLT-suggested spec times two or so.

[rustyrussell]

Yes, keysend is a hack. However, wider interoperability is always good, so changing this to 22 is the Right Thing IMHO.

[rustyrussell]

Though I'd prefer a decent comment: it should literally be 22 with a comment saying that is the Rust lightning default and the highest we know of.

[valentinewallace]

Though I'd prefer a decent comment: it should literally be 22 with a comment saying that is the Rust lightning default and the highest we know of.

Thanks Rusty, implemented this!

[rustyrussell]

Oh, and can we have (in the commit message) a Changelog line:

Changelog-Changed: keysend now uses 22 for the final CTLV, making it rust-lightning compatible.

(These get auto-generated into our CHANGELOG.md at release).

Thanks!

[valentinewallace]

Oh, and can we have (in the commit message) a Changelog line:

Changelog-Changed: keysend now uses 22 for the final CTLV, making it rust-lightning compatible.

(These get auto-generated into our CHANGELOG.md at release).

Thanks!

Sorry for the delay, done!

[rustyrussell]

Ack 281f21b