queryroutes uses unrouteable final_cltv_delta value by default

[Crypt-iQ]

Background

The queryroutes command uses the zpay32.DefaultFinalCltvDelta value to extend to the final hop if none is specified with the request:

lnd/routing/router.go

Lines 1398 to 1403 in 4068e78

	var finalCLTVDelta uint16
	if len(finalExpiry) == 0 {
	finalCLTVDelta = zpay32.DefaultFinalCLTVDelta
	} else {
	finalCLTVDelta = finalExpiry[0]
	}

However, using sendtoroute will fail on the exit hop with FinalExpiryTooSoon because of the check here:

lnd/invoices/invoiceregistry.go

Lines 534 to 540 in 4068e78

	if htlcExpiry < uint32(currentHeight+i.finalCltvRejectDelta) {
	return ErrInvoiceExpiryTooSoon
	}
	if htlcExpiry < uint32(currentHeight)+expiry {
	return ErrInvoiceExpiryTooSoon
	}

The finalCltvRejectDelta is 13 which is greater than the final cltv default of 9, so this will fail. Additionally, the default --bitcoin.timelockdelta config option is 40, so the final cltv should be at least 40 or else the second check will probably fail given a user's configuration. This isn't a problem with the addinvoice command since that uses the --bitcoin.timelockdelta value. This is a quick fix, so I can fix it once we decide on a fix.

Your environment

• version of lnd: master

[joostjager]

Additionally, the default --bitcoin.timelockdelta config option is 40, so the final cltv should be at least 40 or else the second check will probably fail given a user's configuration.

I don't think this is true. The second check compares the htlc expiry with the invoice cltv delta requirement.

I indeed think the fix is to raise zpay32.DefaultFinalCLTVDelta to a larger number.

[Crypt-iQ]

The only way the invoice cltv delta of an invoice can be configured with the cli with the addinvoice command is through the --bitcoin.timelockdelta flag since the CltvExpiry field doesn't appear enabled. I tested raising zpay.DefaultFinalCLTVDelta above 13 and the sendtoroute call still failed on the exit hop because the default value of 40 was used as the invoice cltv delta when creating an invoice. Am I missing something?

This doesn't happen with sendpayment since that command knows the invoice cltv delta as it decodes it from the passed-in payment request whereas the queryroutes call does not know the cltv delta for a payment hash.

[joostjager]

If the goal is for QueryRoutes to come up with a valid route assuming the default lnd invoice expiry, it should indeed be raised to 40. But invoices could also have been created (using a direct rpc call) with a different expiry. And QueryRoutes can also be used to pay to other implementations that have different defaults.

Thinking about it now, maybe the problem really is that the default doesn't make sense? That it should be a required parameter to force callers to think about it?

[Crypt-iQ]

Didn't even think about other implementations, I think it should be a required parameter. Also, is there a reason why the addinvoice cli command doesn't allow a user to directly specify the final cltv delta?

[joostjager]

Not that I am aware of. I think it would be useful to add as a parameter. And for AddHoldInvoice too, as it is especially important for hodl invoices.

I have seen myself patch lncli more than a few times during testing to force a different cltv delta.