From 3dde7bc4e2a1e218ce8b3e725a57f0c9fd0ae0f2 Mon Sep 17 00:00:00 2001 From: nsa Date: Tue, 23 Jul 2019 20:59:31 -0400 Subject: [PATCH 1/2] routing: adding the BlockPadding value to sendpayment This commit adds the BlockPadding value (currently 3) to sendpayment calls so that if some blocks are mined while the htlc is in-flight, the exit hop won't reject it. --- routing/payment_session.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/routing/payment_session.go b/routing/payment_session.go index 117523e9..b1d8a9b0 100644 --- a/routing/payment_session.go +++ b/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 From 7762d55e80c81fe6bfe89619a427066237269cf5 Mon Sep 17 00:00:00 2001 From: nsa Date: Tue, 23 Jul 2019 21:00:30 -0400 Subject: [PATCH 2/2] itest+routing: updating tests to account for BlockPadding --- ...d_multi-hop_htlc_local_chain_claim_test.go | 4 +-- ...ulti-hop_htlc_receiver_chain_claim_test.go | 4 +-- ..._multi-hop_htlc_remote_chain_claim_test.go | 4 +-- lntest/itest/lnd_test.go | 25 +++++++++++++------ routing/payment_session_test.go | 7 +++--- 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go index e8b0b96f..7b478b00 100644 --- a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go +++ b/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") diff --git a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go index 25ce011e..4f9eff93 100644 --- a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go +++ b/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") } diff --git a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go index c4cc203c..a5cfbb06 100644 --- a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go +++ b/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") diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index c3026bc7..cbb796a1 100644 --- a/lntest/itest/lnd_test.go +++ b/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) } diff --git a/routing/payment_session_test.go b/routing/payment_session_test.go index 200f98ab..627f4f85 100644 --- a/routing/payment_session_test.go +++ b/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) }