Merge pull request #3339 from Crypt-iQ/sendpayment_block_pad_0723

routing: add block padding to sendpayment

lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go

@@ -125,8 +125,8 @@ func testMultiHopHtlcLocalChainClaim(net *lntest.NetworkHarness, t *harnessTest)
 
 	// We'll now mine enough blocks so Carol decides that she needs to go
 	// on-chain to claim the HTLC as Bob has been inactive.
-	numBlocks := uint32(invoiceReq.CltvExpiry -
-		lnd.DefaultIncomingBroadcastDelta)
+	numBlocks := padCLTV(uint32(invoiceReq.CltvExpiry -
+		lnd.DefaultIncomingBroadcastDelta))
 
 	if _, err := net.Miner.Node.Generate(numBlocks); err != nil {
 		t.Fatalf("unable to generate blocks")

lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go

@@ -114,9 +114,9 @@ func testMultiHopReceiverChainClaim(net *lntest.NetworkHarness, t *harnessTest)
 	// Now we'll mine enough blocks to prompt carol to actually go to the
 	// chain in order to sweep her HTLC since the value is high enough.
 	// TODO(roasbeef): modify once go to chain policy changes
-	numBlocks := uint32(
+	numBlocks := padCLTV(uint32(
 		invoiceReq.CltvExpiry - lnd.DefaultIncomingBroadcastDelta,
-	)
+	))
 	if _, err := net.Miner.Node.Generate(numBlocks); err != nil {
 		t.Fatalf("unable to generate blocks")
 	}

lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go

@@ -138,8 +138,8 @@ func testMultiHopHtlcRemoteChainClaim(net *lntest.NetworkHarness, t *harnessTest
 
 	// We'll now mine enough blocks so Carol decides that she needs to go
 	// on-chain to claim the HTLC as Bob has been inactive.
-	numBlocks := uint32(invoiceReq.CltvExpiry-
-		lnd.DefaultIncomingBroadcastDelta) - defaultCSV
+	numBlocks := padCLTV(uint32(invoiceReq.CltvExpiry-
+		lnd.DefaultIncomingBroadcastDelta) - defaultCSV)
 
 	if _, err := net.Miner.Node.Generate(numBlocks); err != nil {
 		t.Fatalf("unable to generate blocks")

lntest/itest/lnd_test.go

@@ -38,6 +38,7 @@ import (
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/lightningnetwork/lnd/routing"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
 )
@@ -2605,6 +2606,12 @@ func checkPendingHtlcStageAndMaturity(
 	return nil
 }
 
+// padCLTV is a small helper function that pads a cltv value with a block
+// padding.
+func padCLTV(cltv uint32) uint32 {
+	return cltv + uint32(routing.BlockPadding)
+}
+
 // testChannelForceClosure performs a test to exercise the behavior of "force"
 // closing a channel or unilaterally broadcasting the latest local commitment
 // state on-chain. The test creates a new channel between Alice and Carol, then
@@ -2733,8 +2740,8 @@ func testChannelForceClosure(net *lntest.NetworkHarness, t *harnessTest) {
 	var (
 		startHeight           = uint32(curHeight)
 		commCsvMaturityHeight = startHeight + 1 + defaultCSV
-		htlcExpiryHeight      = startHeight + defaultCLTV
-		htlcCsvMaturityHeight = startHeight + defaultCLTV + 1 + defaultCSV
+		htlcExpiryHeight      = padCLTV(startHeight + defaultCLTV)
+		htlcCsvMaturityHeight = padCLTV(startHeight + defaultCLTV + 1 + defaultCSV)
 	)
 
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
@@ -3055,8 +3062,8 @@ func testChannelForceClosure(net *lntest.NetworkHarness, t *harnessTest) {
 	// output spending from the commitment txn, so we must deduct the number
 	// of blocks we have generated since adding it to the nursery, and take
 	// an additional block off so that we end up one block shy of the expiry
-	// height.
-	cltvHeightDelta := defaultCLTV - defaultCSV - 2 - 1
+	// height, and add the block padding.
+	cltvHeightDelta := padCLTV(defaultCLTV - defaultCSV - 2 - 1)
 
 	// Advance the blockchain until just before the CLTV expires, nothing
 	// exciting should have happened during this time.
@@ -9951,7 +9958,9 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest) {
 	// commitment transaction due to the fact that the HTLC is about to
 	// timeout. With the default outgoing broadcast delta of zero, this will
 	// be the same height as the htlc expiry height.
-	numBlocks := uint32(finalCltvDelta - lnd.DefaultOutgoingBroadcastDelta)
+	numBlocks := padCLTV(
+		uint32(finalCltvDelta - lnd.DefaultOutgoingBroadcastDelta),
+	)
 	if _, err := net.Miner.Node.Generate(numBlocks); err != nil {
 		t.Fatalf("unable to generate blocks: %v", err)
 	}
@@ -10217,7 +10226,8 @@ func testMultiHopLocalForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness,
 
 	// We'll now mine enough blocks for the HTLC to expire. After this, Bob
 	// should hand off the now expired HTLC output to the utxo nursery.
-	if _, err := net.Miner.Node.Generate(finalCltvDelta - defaultCSV - 1); err != nil {
+	numBlocks := padCLTV(uint32(finalCltvDelta - defaultCSV - 1))
+	if _, err := net.Miner.Node.Generate(numBlocks); err != nil {
 		t.Fatalf("unable to generate blocks: %v", err)
 	}
 
@@ -10470,7 +10480,8 @@ func testMultiHopRemoteForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness,
 	// Next, we'll mine enough blocks for the HTLC to expire. At this
 	// point, Bob should hand off the output to his internal utxo nursery,
 	// which will broadcast a sweep transaction.
-	if _, err := net.Miner.Node.Generate(finalCltvDelta - 1); err != nil {
+	numBlocks := padCLTV(finalCltvDelta - 1)
+	if _, err := net.Miner.Node.Generate(numBlocks); err != nil {
 		t.Fatalf("unable to generate blocks: %v", err)
 	}
 

routing/payment_session.go

@@ -8,6 +8,10 @@ import (
 	"github.com/lightningnetwork/lnd/routing/route"
 )
 
+// BlockPadding is used to increment the finalCltvDelta value for the last hop
+// to prevent an HTLC being failed if some blocks are mined while it's in-flight.
+const BlockPadding uint16 = 3
+
 // PaymentSession is used during SendPayment attempts to provide routes to
 // attempt. It also defines methods to give the PaymentSession additional
 // information learned during the previous attempts.
@@ -65,6 +69,10 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment,
 		return nil, fmt.Errorf("pre-built route already tried")
 	}
 
+	// Add BlockPadding to the finalCltvDelta so that the receiving node
+	// does not reject the HTLC if some blocks are mined while it's in-flight.
+	finalCltvDelta += BlockPadding
+
 	// If a route cltv limit was specified, we need to subtract the final
 	// delta before passing it into path finding. The optimal path is
 	// independent of the final cltv delta and the path finding algorithm is

routing/payment_session_test.go

@@ -19,8 +19,8 @@ func TestRequestRoute(t *testing.T) {
 		error) {
 
 		// We expect find path to receive a cltv limit excluding the
-		// final cltv delta.
-		if *r.CltvLimit != 22 {
+		// final cltv delta (including the block padding).
+		if *r.CltvLimit != 22-uint32(BlockPadding) {
 			t.Fatal("wrong cltv limit")
 		}
 
@@ -59,7 +59,8 @@ func TestRequestRoute(t *testing.T) {
 	}
 
 	// We expect an absolute route lock value of height + finalCltvDelta
-	if route.TotalTimeLock != 18 {
+	// + BlockPadding.
+	if route.TotalTimeLock != 18+uint32(BlockPadding) {
 		t.Fatalf("unexpected total time lock of %v",
 			route.TotalTimeLock)
 	}