From fd7b1c2d5e113319f593ef464cf309609428e27d Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:42 +0200 Subject: [PATCH 01/12] lntest/lnd test: make OpenChannel take channel param struct Also add option for setting min_htlc value on channel creation. --- lnd_test.go | 419 +++++++++++++++++++++++++++++++++++++--------- lntest/harness.go | 40 ++++- 2 files changed, 368 insertions(+), 91 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index 7e448474..48c2be93 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -149,11 +149,10 @@ func mineBlocks(t *harnessTest, net *lntest.NetworkHarness, num uint32) []*wire. // channel. func openChannelAndAssert(ctx context.Context, t *harnessTest, net *lntest.NetworkHarness, alice, bob *lntest.HarnessNode, - fundingAmt btcutil.Amount, pushAmt btcutil.Amount, - private bool) *lnrpc.ChannelPoint { + p lntest.OpenChannelParams) *lnrpc.ChannelPoint { chanOpenUpdate, err := net.OpenChannel( - ctx, alice, bob, fundingAmt, pushAmt, private, true, + ctx, alice, bob, p, ) if err != nil { t.Fatalf("unable to open channel: %v", err) @@ -648,7 +647,11 @@ func testBasicChannelFunding(net *lntest.NetworkHarness, t *harnessTest) { // successfully. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) ctxt, _ = context.WithTimeout(ctxb, time.Second*15) @@ -722,7 +725,12 @@ func testUnconfirmedChannelFunding(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanOpenUpdate, err := net.OpenChannel( - ctxt, carol, net.Alice, chanAmt, pushAmt, false, false, + ctxt, carol, net.Alice, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + SpendUnconfirmed: true, + }, ) if err != nil { t.Fatalf("unable to open channel between carol and alice: %v", @@ -916,7 +924,11 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { // Create a channel Alice->Bob. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) ctxt, _ = context.WithTimeout(ctxb, time.Second*15) @@ -947,7 +959,11 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ = context.WithTimeout(ctxb, timeout) chanPoint2 := openChannelAndAssert( - ctxt, t, net, net.Bob, carol, chanAmt, pushAmt, false, + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) ctxt, _ = context.WithTimeout(ctxb, time.Second*15) @@ -1043,7 +1059,11 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPoint3 := openChannelAndAssert( - ctxt, t, net, net.Alice, carol, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) ctxt, _ = context.WithTimeout(ctxb, time.Second*15) @@ -1607,7 +1627,10 @@ func testChannelBalance(net *lntest.NetworkHarness, t *harnessTest) { } chanPoint := openChannelAndAssert( - ctx, t, net, net.Alice, net.Bob, amount, 0, false, + ctx, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: amount, + }, ) // Wait for both Alice and Bob to recognize this new channel. @@ -1809,7 +1832,11 @@ func testChannelForceClosure(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, carol, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) // Wait for Alice and Carol to receive the channel edge from the @@ -2525,7 +2552,10 @@ func testSphinxReplayPersistence(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, 0, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) assertAmountSent := func(amt btcutil.Amount) { @@ -2663,7 +2693,10 @@ func testSingleHopInvoice(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ := context.WithTimeout(ctxb, timeout) chanAmt := btcutil.Amount(100000) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) assertAmountSent := func(amt btcutil.Amount) { @@ -2819,7 +2852,10 @@ func testListPayments(net *lntest.NetworkHarness, t *harnessTest) { chanAmt := btcutil.Amount(100000) ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Now that the channel is open, create an invoice for Bob which @@ -3043,7 +3079,10 @@ func testMultiHopPayments(net *lntest.NetworkHarness, t *harnessTest) { // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) networkChans = append(networkChans, chanPointAlice) @@ -3082,7 +3121,10 @@ func testMultiHopPayments(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointDave := openChannelAndAssert( - ctxt, t, net, dave, net.Alice, chanAmt, 0, false, + ctxt, t, net, dave, net.Alice, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) networkChans = append(networkChans, chanPointDave) txidHash, err = getChanPointFundingTxid(chanPointDave) @@ -3115,7 +3157,10 @@ func testMultiHopPayments(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, 0, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) networkChans = append(networkChans, chanPointCarol) @@ -3324,8 +3369,12 @@ func testSingleHopSendToRoute(net *lntest.NetworkHarness, t *harnessTest) { // Open a channel with 100k satoshis between Alice and Bob with Alice // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) - chanPointAlice := openChannelAndAssert(ctxt, t, net, net.Alice, - net.Bob, chanAmt, 0, false) + chanPointAlice := openChannelAndAssert( + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, + ) networkChans = append(networkChans, chanPointAlice) txidHash, err := getChanPointFundingTxid(chanPointAlice) @@ -3471,8 +3520,12 @@ func testMultiHopSendToRoute(net *lntest.NetworkHarness, t *harnessTest) { // Open a channel with 100k satoshis between Alice and Bob with Alice // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) - chanPointAlice := openChannelAndAssert(ctxt, t, net, net.Alice, - net.Bob, chanAmt, 0, false) + chanPointAlice := openChannelAndAssert( + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, + ) networkChans = append(networkChans, chanPointAlice) txidHash, err := getChanPointFundingTxid(chanPointAlice) @@ -3505,8 +3558,12 @@ func testMultiHopSendToRoute(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("unable to send coins to bob: %v", err) } ctxt, _ = context.WithTimeout(ctxb, timeout) - chanPointBob := openChannelAndAssert(ctxt, t, net, net.Bob, - carol, chanAmt, 0, false) + chanPointBob := openChannelAndAssert( + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, + ) networkChans = append(networkChans, chanPointBob) txidHash, err = getChanPointFundingTxid(chanPointBob) if err != nil { @@ -3659,8 +3716,12 @@ func testSendToRouteErrorPropagation(net *lntest.NetworkHarness, t *harnessTest) // Open a channel with 100k satoshis between Alice and Bob with Alice // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) - chanPointAlice := openChannelAndAssert(ctxt, t, net, net.Alice, - net.Bob, chanAmt, 0, false) + chanPointAlice := openChannelAndAssert( + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, + ) ctxt, _ = context.WithTimeout(ctxb, timeout) err := net.Alice.WaitForNetworkChannelOpen(ctxt, chanPointAlice) if err != nil { @@ -3701,8 +3762,12 @@ func testSendToRouteErrorPropagation(net *lntest.NetworkHarness, t *harnessTest) } ctxt, _ = context.WithTimeout(ctxb, timeout) - chanPointCarol := openChannelAndAssert(ctxt, t, net, carol, - charlie, chanAmt, 0, false) + chanPointCarol := openChannelAndAssert( + ctxt, t, net, carol, charlie, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, + ) ctxt, _ = context.WithTimeout(ctxb, timeout) err = carol.WaitForNetworkChannelOpen(ctxt, chanPointCarol) if err != nil { @@ -3790,7 +3855,10 @@ func testPrivateChannels(net *lntest.NetworkHarness, t *harnessTest) { // Open a channel with 200k satoshis between Alice and Bob. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt*2, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt * 2, + }, ) networkChans = append(networkChans, chanPointAlice) @@ -3823,7 +3891,10 @@ func testPrivateChannels(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointDave := openChannelAndAssert( - ctxt, t, net, dave, net.Alice, chanAmt, 0, false, + ctxt, t, net, dave, net.Alice, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) networkChans = append(networkChans, chanPointDave) txidHash, err = getChanPointFundingTxid(chanPointDave) @@ -3856,7 +3927,10 @@ func testPrivateChannels(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, 0, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) networkChans = append(networkChans, chanPointCarol) @@ -3907,7 +3981,11 @@ func testPrivateChannels(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("unable to connect dave to alice: %v", err) } chanOpenUpdate, err := net.OpenChannel( - ctxb, carol, net.Alice, chanAmt, 0, true, true, + ctxb, carol, net.Alice, + lntest.OpenChannelParams{ + Amt: chanAmt, + Private: true, + }, ) if err != nil { t.Fatalf("unable to open channel: %v", err) @@ -4134,7 +4212,12 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { // payment. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointBob := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, chanAmt/2, true, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: chanAmt / 2, + Private: true, + }, ) // Then, we'll create Carol's node and open a public channel between her @@ -4151,7 +4234,11 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, net.Alice, carol, chanAmt, chanAmt/2, false, + ctxt, t, net, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: chanAmt / 2, + }, ) // Then, we'll create Dave's node and open a private channel between him @@ -4169,7 +4256,11 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointDave := openChannelAndAssert( - ctxt, t, net, net.Alice, dave, chanAmt, 0, true, + ctxt, t, net, net.Alice, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + Private: true, + }, ) // Finally, we'll create Eve's node and open a private channel between @@ -4185,7 +4276,12 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointEve := openChannelAndAssert( - ctxt, t, net, net.Alice, eve, chanAmt, chanAmt/2, true, + ctxt, t, net, net.Alice, eve, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: chanAmt / 2, + Private: true, + }, ) // Make sure all the channels have been opened. @@ -4307,7 +4403,11 @@ func testMultiHopOverPrivateChannels(net *lntest.NetworkHarness, t *harnessTest) // being the funder. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, true, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + Private: true, + }, ) ctxt, _ = context.WithTimeout(ctxb, timeout) @@ -4350,7 +4450,10 @@ func testMultiHopOverPrivateChannels(net *lntest.NetworkHarness, t *harnessTest) } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointBob := openChannelAndAssert( - ctxt, t, net, net.Bob, carol, chanAmt, 0, false, + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) ctxt, _ = context.WithTimeout(ctxb, timeout) @@ -4403,7 +4506,11 @@ func testMultiHopOverPrivateChannels(net *lntest.NetworkHarness, t *harnessTest) } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, 0, true, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + Private: true, + }, ) ctxt, _ = context.WithTimeout(ctxb, timeout) @@ -4514,7 +4621,10 @@ func testInvoiceSubscriptions(net *lntest.NetworkHarness, t *harnessTest) { // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Next create a new invoice for Bob requesting 1k satoshis. @@ -4752,7 +4862,10 @@ func testBasicChannelCreation(net *lntest.NetworkHarness, t *harnessTest) { for i := 0; i < numChannels; i++ { ctx, _ := context.WithTimeout(context.Background(), timeout) chanPoints[i] = openChannelAndAssert( - ctx, t, net, net.Alice, net.Bob, amount, 0, false, + ctx, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: amount, + }, ) } @@ -4803,7 +4916,10 @@ func testMaxPendingChannels(net *lntest.NetworkHarness, t *harnessTest) { for i := 0; i < maxPendingChannels; i++ { ctx, _ = context.WithTimeout(context.Background(), timeout) stream, err := net.OpenChannel( - ctx, net.Alice, carol, amount, 0, false, true, + ctx, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: amount, + }, ) if err != nil { t.Fatalf("unable to open channel: %v", err) @@ -4815,8 +4931,12 @@ func testMaxPendingChannels(net *lntest.NetworkHarness, t *harnessTest) { // channel request should cause ErrorGeneric to be sent back to Alice. ctx, _ = context.WithTimeout(context.Background(), timeout) _, err = net.OpenChannel( - ctx, net.Alice, carol, amount, 0, false, true, + ctx, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: amount, + }, ) + if err == nil { t.Fatalf("error wasn't received") } else if grpc.Code(err) != lnwire.ErrMaxPendingChannels.ToGrpcCode() { @@ -4973,7 +5093,10 @@ func testFailingChannel(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, carol, chanAmt, 0, false, + ctxt, t, net, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // With the channel open, we'll create a invoice for Carol that Alice @@ -5146,7 +5269,10 @@ func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) { ctxb := context.Background() ctxt, _ := context.WithTimeout(ctxb, timeout) coopChanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Create Carol's node and connect Alice to her. @@ -5164,7 +5290,10 @@ func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) { // closed. ctxt, _ = context.WithTimeout(ctxb, timeout) forceCloseChanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, carol, chanAmt, 0, false, + ctxt, t, net, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Now, create Dave's a node and also open a channel between Alice and @@ -5180,7 +5309,10 @@ func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) persistentChanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, dave, chanAmt, 0, false, + ctxt, t, net, net.Alice, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // isConnected is a helper closure that checks if a peer is connected to @@ -5360,7 +5492,10 @@ func testRevokedCloseRetribution(net *lntest.NetworkHarness, t *harnessTest) { // 0.5 BTC value. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, carol, net.Bob, chanAmt, 0, false, + ctxt, t, net, carol, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // With the channel open, we'll create a few invoices for Bob that @@ -5639,7 +5774,10 @@ func testRevokedCloseRetributionZeroValueRemoteOutput(net *lntest.NetworkHarness // 0.5 BTC value. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, dave, carol, chanAmt, 0, false, + ctxt, t, net, dave, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // With the channel open, we'll create a few invoices for Carol that @@ -5904,7 +6042,11 @@ func testRevokedCloseRetributionRemoteHodl(net *lntest.NetworkHarness, // maxBtcFundingAmount (2^24) satoshis value. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, dave, carol, chanAmt, pushAmt, false, + ctxt, t, net, dave, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) // With the channel open, we'll create a few invoices for Carol that @@ -6361,7 +6503,10 @@ func testDataLossProtection(net *lntest.NetworkHarness, t *harnessTest) { // We'll first open up a channel between them with a 0.5 BTC value. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, 0, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // We a´make a note of the nodes' current on-chain balances, to make @@ -6650,7 +6795,10 @@ func testHtlcErrorPropagation(net *lntest.NetworkHarness, t *harnessTest) { // and Bob. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) ctxt, _ = context.WithTimeout(ctxb, timeout) if err := net.Alice.WaitForNetworkChannelOpen(ctxt, chanPointAlice); err != nil { @@ -6695,7 +6843,10 @@ func testHtlcErrorPropagation(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ = context.WithTimeout(ctxb, timeout) const bobChanAmt = maxBtcFundingAmount chanPointBob := openChannelAndAssert( - ctxt, t, net, net.Bob, carol, chanAmt, 0, false, + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Ensure that Alice has Carol in her routing table before proceeding. @@ -6992,7 +7143,10 @@ func testGraphTopologyNotifications(net *lntest.NetworkHarness, t *harnessTest) // Open a new channel between Alice and Bob. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // The channel opening above should have triggered a few notifications @@ -7117,7 +7271,10 @@ out: } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPoint = openChannelAndAssert( - ctxt, t, net, net.Bob, carol, chanAmt, 0, false, + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Reconnect Alice and Bob. This should result in the nodes syncing up @@ -7213,7 +7370,10 @@ func testNodeAnnouncement(net *lntest.NetworkHarness, t *harnessTest) { timeout := time.Duration(time.Second * 5) ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Bob, dave, 1000000, 0, false, + ctxt, t, net, net.Bob, dave, + lntest.OpenChannelParams{ + Amt: 1000000, + }, ) // When Alice now connects with Dave, Alice will get his node @@ -7274,7 +7434,11 @@ func testNodeSignVerify(net *lntest.NetworkHarness, t *harnessTest) { // Create a channel between alice and bob. ctxt, _ := context.WithTimeout(ctxb, timeout) aliceBobCh := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) aliceMsg := []byte("alice msg") @@ -7370,7 +7534,10 @@ func testAsyncPayments(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ := context.WithTimeout(ctxb, timeout) channelCapacity := btcutil.Amount(paymentAmt * 2000) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, channelCapacity, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: channelCapacity, + }, ) info, err := getChanInfo(net.Alice) @@ -7553,8 +7720,11 @@ func testBidirectionalAsyncPayments(net *lntest.NetworkHarness, t *harnessTest) // Alice should send all money from her side to Bob. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, paymentAmt*2000, - paymentAmt*1000, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: paymentAmt * 2000, + PushAmt: paymentAmt * 1000, + }, ) info, err := getChanInfo(net.Alice) @@ -7896,7 +8066,10 @@ func createThreeHopHodlNetwork(t *harnessTest, timeout := time.Duration(time.Second * 15) ctxt, _ := context.WithTimeout(ctxb, timeout) aliceChanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) ctxt, _ = context.WithTimeout(ctxb, time.Second*15) @@ -7926,7 +8099,10 @@ func createThreeHopHodlNetwork(t *harnessTest, // open, our topology looks like: A -> B -> C. ctxt, _ = context.WithTimeout(ctxb, timeout) bobChanPoint := openChannelAndAssert( - ctxt, t, net, net.Bob, carol, chanAmt, 0, false, + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) ctxt, _ = context.WithTimeout(ctxb, time.Second*15) err = net.Bob.WaitForNetworkChannelOpen(ctxt, bobChanPoint) @@ -9463,7 +9639,11 @@ func testSwitchCircuitPersistence(net *lntest.NetworkHarness, t *harnessTest) { // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointAlice) @@ -9502,7 +9682,11 @@ func testSwitchCircuitPersistence(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointDave := openChannelAndAssert( - ctxt, t, net, dave, net.Alice, chanAmt, pushAmt, false, + ctxt, t, net, dave, net.Alice, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointDave) txidHash, err = getChanPointFundingTxid(chanPointDave) @@ -9536,7 +9720,11 @@ func testSwitchCircuitPersistence(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, pushAmt, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointCarol) @@ -9793,7 +9981,11 @@ func testSwitchOfflineDelivery(net *lntest.NetworkHarness, t *harnessTest) { // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointAlice) @@ -9832,7 +10024,11 @@ func testSwitchOfflineDelivery(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointDave := openChannelAndAssert( - ctxt, t, net, dave, net.Alice, chanAmt, pushAmt, false, + ctxt, t, net, dave, net.Alice, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointDave) txidHash, err = getChanPointFundingTxid(chanPointDave) @@ -9866,7 +10062,11 @@ func testSwitchOfflineDelivery(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, pushAmt, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointCarol) @@ -10130,7 +10330,11 @@ func testSwitchOfflineDeliveryPersistence(net *lntest.NetworkHarness, t *harness // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointAlice) @@ -10169,8 +10373,13 @@ func testSwitchOfflineDeliveryPersistence(net *lntest.NetworkHarness, t *harness } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointDave := openChannelAndAssert( - ctxt, t, net, dave, net.Alice, chanAmt, pushAmt, false, + ctxt, t, net, dave, net.Alice, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) + networkChans = append(networkChans, chanPointDave) txidHash, err = getChanPointFundingTxid(chanPointDave) if err != nil { @@ -10203,7 +10412,11 @@ func testSwitchOfflineDeliveryPersistence(net *lntest.NetworkHarness, t *harness } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, pushAmt, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointCarol) @@ -10474,7 +10687,11 @@ func testSwitchOfflineDeliveryOutgoingOffline( // being the sole funder of the channel. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, pushAmt, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointAlice) @@ -10513,7 +10730,11 @@ func testSwitchOfflineDeliveryOutgoingOffline( } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointDave := openChannelAndAssert( - ctxt, t, net, dave, net.Alice, chanAmt, pushAmt, false, + ctxt, t, net, dave, net.Alice, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointDave) txidHash, err = getChanPointFundingTxid(chanPointDave) @@ -10545,7 +10766,11 @@ func testSwitchOfflineDeliveryOutgoingOffline( } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, pushAmt, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: pushAmt, + }, ) networkChans = append(networkChans, chanPointCarol) @@ -10764,7 +10989,10 @@ func testQueryRoutes(net *lntest.NetworkHarness, t *harnessTest) { // Open a channel between Alice and Bob. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAlice := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) networkChans = append(networkChans, chanPointAlice) @@ -10784,7 +11012,10 @@ func testQueryRoutes(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointBob := openChannelAndAssert( - ctxt, t, net, net.Bob, carol, chanAmt, 0, false, + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) networkChans = append(networkChans, chanPointBob) @@ -10804,7 +11035,10 @@ func testQueryRoutes(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarol := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, 0, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) networkChans = append(networkChans, chanPointCarol) @@ -10958,7 +11192,10 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { // Open a channel between Alice and Bob. ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAliceBob := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Create Carol's node and open a channel between her and Alice with @@ -10980,7 +11217,10 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointAliceCarol := openChannelAndAssert( - ctxt, t, net, net.Alice, carol, chanAmt, 0, false, + ctxt, t, net, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Create Dave's node and open a channel between him and Bob with Bob @@ -10997,7 +11237,10 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointBobDave := openChannelAndAssert( - ctxt, t, net, net.Bob, dave, chanAmt, 0, false, + ctxt, t, net, net.Bob, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Open a channel between Carol and Dave. @@ -11007,7 +11250,10 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointCarolDave := openChannelAndAssert( - ctxt, t, net, carol, dave, chanAmt, 0, false, + ctxt, t, net, carol, dave, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Now that all the channels were set up, we'll wait for all the nodes @@ -11218,7 +11464,10 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { ctxb := context.Background() ctxt, _ := context.WithTimeout(ctxb, timeout) chanPointAliceBob := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, chanAmt, 0, false, + ctxt, t, net, net.Alice, net.Bob, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) carol, err := net.NewNode("Carol", nil) @@ -11232,7 +11481,10 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { } ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointAliceCarol := openChannelAndAssert( - ctxt, t, net, net.Alice, carol, chanAmt, 0, false, + ctxt, t, net, net.Alice, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // We create a new node Eve that has an inactive channel timeout of @@ -11260,7 +11512,10 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ = context.WithTimeout(ctxb, timeout) chanPointEveCarol := openChannelAndAssert( - ctxt, t, net, eve, carol, chanAmt, 0, false, + ctxt, t, net, eve, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + }, ) // Launch a node for Dave which will connect to Bob in order to receive diff --git a/lntest/harness.go b/lntest/harness.go index 963fb827..681fa128 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -19,6 +19,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnwire" ) // NetworkHarness is an integration testing harness for the lightning network. @@ -687,15 +688,35 @@ func (n *NetworkHarness) WaitForTxBroadcast(ctx context.Context, txid chainhash. } } +// OpenChannelParams houses the params to specify when opening a new channel. +type OpenChannelParams struct { + // Amt is the local amount being put into the channel. + Amt btcutil.Amount + + // PushAmt is the amount that should be pushed to the remote when the + // channel is opened. + PushAmt btcutil.Amount + + // Private is a boolan indicating whether the opened channel should be + // private. + Private bool + + // SpendUnconfirmed is a boolean indicating whether we can utilize + // unconfirmed outputs to fund the channel. + SpendUnconfirmed bool + + // MinHtlc is the htlc_minumum_msat value set when opening the channel. + MinHtlc lnwire.MilliSatoshi +} + // OpenChannel attempts to open a channel between srcNode and destNode with the // passed channel funding parameters. If the passed context has a timeout, then // if the timeout is reached before the channel pending notification is // received, an error is returned. The confirmed boolean determines whether we // should fund the channel with confirmed outputs or not. func (n *NetworkHarness) OpenChannel(ctx context.Context, - srcNode, destNode *HarnessNode, amt btcutil.Amount, - pushAmt btcutil.Amount, - private, confirmed bool) (lnrpc.Lightning_OpenChannelClient, error) { + srcNode, destNode *HarnessNode, p OpenChannelParams) ( + lnrpc.Lightning_OpenChannelClient, error) { // Wait until srcNode and destNode have the latest chain synced. // Otherwise, we may run into a check within the funding manager that @@ -708,17 +729,18 @@ func (n *NetworkHarness) OpenChannel(ctx context.Context, return nil, fmt.Errorf("Unable to sync destNode chain: %v", err) } - minConfs := int32(0) - if confirmed { - minConfs = 1 + minConfs := int32(1) + if p.SpendUnconfirmed { + minConfs = 0 } openReq := &lnrpc.OpenChannelRequest{ NodePubkey: destNode.PubKey[:], - LocalFundingAmount: int64(amt), - PushSat: int64(pushAmt), - Private: private, + LocalFundingAmount: int64(p.Amt), + PushSat: int64(p.PushAmt), + Private: p.Private, MinConfs: minConfs, + MinHtlcMsat: int64(p.MinHtlc), } respStream, err := srcNode.OpenChannel(ctx, openReq) From 4c153531169250c21a8a34ea0a766472a00eabb7 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:42 +0200 Subject: [PATCH 02/12] lnd test: make waitForChannelUpdate take array of updates --- lnd_test.go | 203 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 117 insertions(+), 86 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index 48c2be93..62aa9c7f 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -789,48 +789,72 @@ func txStr(chanPoint *lnrpc.ChannelPoint) string { return cp.String() } -// waitForChannelUpdate waits for a node to receive updates from the advertising -// node for the specified channels. -func waitForChannelUpdate(t *harnessTest, graphUpdates chan *lnrpc.GraphTopologyUpdate, - advertisingNode string, expectedPolicy *lnrpc.RoutingPolicy, - chanPoints ...*lnrpc.ChannelPoint) { +// expectedChanUpdate houses params we expect a ChannelUpdate to advertise. +type expectedChanUpdate struct { + advertisingNode string + expectedPolicy *lnrpc.RoutingPolicy + chanPoint *lnrpc.ChannelPoint +} - // Create a set containing all the channel points we are awaiting - // updates for. - cps := make(map[string]struct{}) - for _, chanPoint := range chanPoints { - cps[txStr(chanPoint)] = struct{}{} - } +// waitForChannelUpdate waits for a node to receive the expected channel +// updates. +func waitForChannelUpdate(t *harnessTest, + graphUpdates chan *lnrpc.GraphTopologyUpdate, + expUpdates []expectedChanUpdate) { + + // Create an array indicating which expected channel updates we have + // received. + found := make([]bool, len(expUpdates)) out: for { select { case graphUpdate := <-graphUpdates: for _, update := range graphUpdate.ChannelUpdates { - fundingTxStr := txStr(update.ChanPoint) - if _, ok := cps[fundingTxStr]; !ok { - continue - } - if update.AdvertisingNode != advertisingNode { - continue - } + // For each expected update, check if it matches + // the update we just received. + for i, exp := range expUpdates { + fundingTxStr := txStr(update.ChanPoint) + if fundingTxStr != txStr(exp.chanPoint) { + continue + } - err := checkChannelPolicy( - update.RoutingPolicy, expectedPolicy, - ) - if err != nil { - continue - } + if update.AdvertisingNode != + exp.advertisingNode { + continue + } - // We got a policy update that matched the - // values and channel point of what we - // expected, delete it from the map. - delete(cps, fundingTxStr) + err := checkChannelPolicy( + update.RoutingPolicy, + exp.expectedPolicy, + ) + if err != nil { + continue + } - // If we have no more channel points we are - // waiting for, break out of the loop. - if len(cps) == 0 { - break out + // We got a policy update that matched + // the values and channel point of what + // we expected, mark it as found. + found[i] = true + + // If we have no more channel updates + // we are waiting for, break out of the + // loop. + rem := 0 + for _, f := range found { + if !f { + rem++ + } + } + + if rem == 0 { + break out + } + + // Since we found a match among the + // expected updates, break out of the + // inner loop. + break } } case <-time.After(20 * time.Second): @@ -931,6 +955,11 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { }, ) + // We add all the nodes' update channels to a slice, such that we can + // make sure they all receive the expected updates. + nodeUpdates := []chan *lnrpc.GraphTopologyUpdate{aliceUpdates, bobUpdates} + nodes := []*lntest.HarnessNode{net.Alice, net.Bob} + ctxt, _ = context.WithTimeout(ctxb, time.Second*15) err := net.Alice.WaitForNetworkChannelOpen(ctxt, chanPoint) if err != nil { @@ -953,6 +982,9 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { carolUpdates, cQuit := subscribeGraphNotifications(t, ctxb, carol) defer close(cQuit) + nodeUpdates = append(nodeUpdates, carolUpdates) + nodes = append(nodes, carol) + if err := net.ConnectNodes(ctxb, carol, net.Bob); err != nil { t.Fatalf("unable to connect dave to alice: %v", err) } @@ -1009,26 +1041,21 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { } // Wait for all nodes to have seen the policy update done by Bob. - waitForChannelUpdate( - t, aliceUpdates, net.Bob.PubKeyStr, expectedPolicy, chanPoint, - ) - waitForChannelUpdate( - t, bobUpdates, net.Bob.PubKeyStr, expectedPolicy, chanPoint, - ) - waitForChannelUpdate( - t, carolUpdates, net.Bob.PubKeyStr, expectedPolicy, chanPoint, - ) + for _, updates := range nodeUpdates { + waitForChannelUpdate( + t, updates, + []expectedChanUpdate{ + {net.Bob.PubKeyStr, expectedPolicy, chanPoint}, + }, + ) + } // Check that all nodes now know about Bob's updated policy. - assertChannelPolicy( - t, net.Alice, net.Bob.PubKeyStr, expectedPolicy, chanPoint, - ) - assertChannelPolicy( - t, net.Bob, net.Bob.PubKeyStr, expectedPolicy, chanPoint, - ) - assertChannelPolicy( - t, carol, net.Bob.PubKeyStr, expectedPolicy, chanPoint, - ) + for _, node := range nodes { + assertChannelPolicy( + t, node, net.Bob.PubKeyStr, expectedPolicy, chanPoint, + ) + } // Now that all nodes have received the new channel update, we'll try // to send a payment from Alice to Carol to ensure that Alice has @@ -1101,33 +1128,24 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { // Wait for all nodes to have seen the policy updates for both of // Alice's channels. - waitForChannelUpdate( - t, aliceUpdates, net.Alice.PubKeyStr, expectedPolicy, chanPoint, - chanPoint3, - ) - waitForChannelUpdate( - t, bobUpdates, net.Alice.PubKeyStr, expectedPolicy, chanPoint, - chanPoint3, - ) - waitForChannelUpdate( - t, carolUpdates, net.Alice.PubKeyStr, expectedPolicy, chanPoint, - chanPoint3, - ) + for _, updates := range nodeUpdates { + waitForChannelUpdate( + t, updates, + []expectedChanUpdate{ + {net.Alice.PubKeyStr, expectedPolicy, chanPoint}, + {net.Alice.PubKeyStr, expectedPolicy, chanPoint3}, + }, + ) + } // And finally check that all nodes remembers the policy update they // received. - assertChannelPolicy( - t, net.Alice, net.Alice.PubKeyStr, expectedPolicy, chanPoint, - chanPoint3, - ) - assertChannelPolicy( - t, net.Bob, net.Alice.PubKeyStr, expectedPolicy, chanPoint, - chanPoint3, - ) - assertChannelPolicy( - t, carol, net.Alice.PubKeyStr, expectedPolicy, chanPoint, - chanPoint3, - ) + for _, node := range nodes { + assertChannelPolicy( + t, node, net.Alice.PubKeyStr, expectedPolicy, + chanPoint, chanPoint3, + ) + } // Close the channels. ctxt, _ = context.WithTimeout(ctxb, timeout) @@ -3064,8 +3082,10 @@ func updateChannelPolicy(t *harnessTest, node *lntest.HarnessNode, defer close(aQuit) waitForChannelUpdate( - t, listenerUpdates, node.PubKeyStr, expectedPolicy, - chanPoint, + t, listenerUpdates, + []expectedChanUpdate{ + {node.PubKeyStr, expectedPolicy, chanPoint}, + }, ) } @@ -11322,8 +11342,10 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { aliceUpdates, aQuit := subscribeGraphNotifications(t, ctxt, net.Alice) defer close(aQuit) waitForChannelUpdate( - t, aliceUpdates, carol.PubKeyStr, expectedPolicy, - chanPointCarolDave, + t, aliceUpdates, + []expectedChanUpdate{ + {carol.PubKeyStr, expectedPolicy, chanPointCarolDave}, + }, ) // We'll also need the channel IDs for Bob's channels in order to @@ -11549,8 +11571,10 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("unable to suspend carol: %v", err) } waitForChannelUpdate( - t, daveUpdates, eve.PubKeyStr, expectedPolicy, - chanPointEveCarol, + t, daveUpdates, + []expectedChanUpdate{ + {eve.PubKeyStr, expectedPolicy, chanPointEveCarol}, + }, ) // We restart Carol. Since the channel now becomes active again, Eve @@ -11561,8 +11585,10 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { expectedPolicy.Disabled = false waitForChannelUpdate( - t, daveUpdates, eve.PubKeyStr, expectedPolicy, - chanPointEveCarol, + t, daveUpdates, + []expectedChanUpdate{ + {eve.PubKeyStr, expectedPolicy, chanPointEveCarol}, + }, ) // Close Alice's channels with Bob and Carol cooperatively and @@ -11583,8 +11609,11 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { // receive an update marking each as disabled. expectedPolicy.Disabled = true waitForChannelUpdate( - t, daveUpdates, net.Alice.PubKeyStr, expectedPolicy, - chanPointAliceBob, chanPointAliceCarol, + t, daveUpdates, + []expectedChanUpdate{ + {net.Alice.PubKeyStr, expectedPolicy, chanPointAliceBob}, + {net.Alice.PubKeyStr, expectedPolicy, chanPointAliceCarol}, + }, ) // Finally, close the channels by mining the closing transactions. @@ -11602,8 +11631,10 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { } waitForChannelUpdate( - t, daveUpdates, eve.PubKeyStr, expectedPolicy, - chanPointEveCarol, + t, daveUpdates, + []expectedChanUpdate{ + {eve.PubKeyStr, expectedPolicy, chanPointEveCarol}, + }, ) _, err = waitForNTxsInMempool(net.Miner.Node, 1, timeout) From 0c65719613901b1face3585130c9092066b82e44 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:42 +0200 Subject: [PATCH 03/12] fundingmgr: check quit channel when sending update Avoids deadlock during failing fundingmanager tests. --- fundingmanager.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index f1163fea..5bc42586 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -1617,7 +1617,7 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { // is over. // TODO(roasbeef): add abstraction over updates to accommodate // long-polling, or SSE, etc. - resCtx.updates <- &lnrpc.OpenStatusUpdate{ + upd := &lnrpc.OpenStatusUpdate{ Update: &lnrpc.OpenStatusUpdate_ChanPending{ ChanPending: &lnrpc.PendingUpdate{ Txid: fundingPoint.Hash[:], @@ -1626,6 +1626,12 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { }, } + select { + case resCtx.updates <- upd: + case <-f.quit: + return + } + // At this point we have broadcast the funding transaction and done all // necessary processing. f.wg.Add(1) @@ -1693,7 +1699,7 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { // Give the caller a final update notifying them that // the channel is now open. // TODO(roasbeef): only notify after recv of funding locked? - resCtx.updates <- &lnrpc.OpenStatusUpdate{ + upd := &lnrpc.OpenStatusUpdate{ Update: &lnrpc.OpenStatusUpdate_ChanOpen{ ChanOpen: &lnrpc.ChannelOpenUpdate{ ChannelPoint: &lnrpc.ChannelPoint{ @@ -1706,6 +1712,12 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { }, } + select { + case resCtx.updates <- upd: + case <-f.quit: + return + } + err = f.annAfterSixConfs(completeChan, shortChanID) if err != nil { fndgLog.Errorf("failed sending channel announcement: %v", From e174fbd75923994e14dd1fb807f0ebdfbd26b16d Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:43 +0200 Subject: [PATCH 04/12] fundingmanager: use value the _remote_ require us to use in ChannelUpdate This commit fixes a bug that would make us advertise the remote's min_htlc value in our channel update. The min_htlc value is set by a node Alice to limit its exposure to small HTLCs, and the channel counter party should not forward HTLCs of value smaller than this to Alice. This means that the value a node Bob should advertise in its ChannelUpdate, is the min_htlc value the counter party require all HTLCs to be above. Instead of populating the ChannelUpdate with the MinHtlc value found in the remote constraints, we now use the value from the local constraints. --- fundingmanager.go | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index 5bc42586..ce6cf46c 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -2072,16 +2072,17 @@ func (f *fundingManager) addToRouterGraph(completeChan *channeldb.OpenChannel, chanID := lnwire.NewChanIDFromOutPoint(&completeChan.FundingOutpoint) - // We'll obtain their min HTLC as we'll use this value within our - // ChannelUpdate. We use this value isn't of ours, as the remote party - // will be the one that's carrying the HTLC towards us. - remoteMinHTLC := completeChan.RemoteChanCfg.MinHTLC + // We'll obtain the min HTLC value we can forward in our direction, as + // we'll use this value within our ChannelUpdate. This constraint is + // originally set by the remote node, as it will be the one that will + // need to determine the smallest HTLC it deems economically relevant. + fwdMinHTLC := completeChan.LocalChanCfg.MinHTLC ann, err := f.newChanAnnouncement( f.cfg.IDKey, completeChan.IdentityPub, completeChan.LocalChanCfg.MultiSigKey.PubKey, completeChan.RemoteChanCfg.MultiSigKey.PubKey, *shortChanID, - chanID, remoteMinHTLC, + chanID, fwdMinHTLC, ) if err != nil { return fmt.Errorf("error generating channel "+ @@ -2192,11 +2193,12 @@ func (f *fundingManager) annAfterSixConfs(completeChan *channeldb.OpenChannel, fndgLog.Infof("Announcing ChannelPoint(%v), short_chan_id=%v", &fundingPoint, spew.Sdump(shortChanID)) - // We'll obtain their min HTLC as we'll use this value within - // our ChannelUpdate. We use this value isn't of ours, as the - // remote party will be the one that's carrying the HTLC towards - // us. - remoteMinHTLC := completeChan.RemoteChanCfg.MinHTLC + // We'll obtain the min HTLC value we can forward in our + // direction, as we'll use this value within our ChannelUpdate. + // This constraint is originally set by the remote node, as it + // will be the one that will need to determine the smallest + // HTLC it deems economically relevant. + fwdMinHTLC := completeChan.LocalChanCfg.MinHTLC // Create and broadcast the proofs required to make this channel // public and usable for other nodes for routing. @@ -2204,7 +2206,7 @@ func (f *fundingManager) annAfterSixConfs(completeChan *channeldb.OpenChannel, f.cfg.IDKey, completeChan.IdentityPub, completeChan.LocalChanCfg.MultiSigKey.PubKey, completeChan.RemoteChanCfg.MultiSigKey.PubKey, - *shortChanID, chanID, remoteMinHTLC, + *shortChanID, chanID, fwdMinHTLC, ) if err != nil { return fmt.Errorf("channel announcement failed: %v", err) @@ -2371,10 +2373,10 @@ type chanAnnouncement struct { // identity pub keys of both parties to the channel, and the second segment is // authenticated only by us and contains our directional routing policy for the // channel. -func (f *fundingManager) newChanAnnouncement(localPubKey, remotePubKey *btcec.PublicKey, +func (f *fundingManager) newChanAnnouncement(localPubKey, remotePubKey, localFundingKey, remoteFundingKey *btcec.PublicKey, shortChanID lnwire.ShortChannelID, chanID lnwire.ChannelID, - remoteMinHTLC lnwire.MilliSatoshi) (*chanAnnouncement, error) { + fwdMinHTLC lnwire.MilliSatoshi) (*chanAnnouncement, error) { chainHash := *f.cfg.Wallet.Cfg.NetParams.GenesisHash @@ -2428,9 +2430,10 @@ func (f *fundingManager) newChanAnnouncement(localPubKey, remotePubKey *btcec.Pu Flags: chanFlags, TimeLockDelta: uint16(f.cfg.DefaultRoutingPolicy.TimeLockDelta), - // We use the *remote* party's HtlcMinimumMsat, as they'll be - // the ones carrying the HTLC routed *towards* us. - HtlcMinimumMsat: remoteMinHTLC, + // We use the HtlcMinimumMsat that the remote party required us + // to use, as our ChannelUpdate will be used to carry HTLCs + // towards them. + HtlcMinimumMsat: fwdMinHTLC, BaseFee: uint32(f.cfg.DefaultRoutingPolicy.BaseFee), FeeRate: uint32(f.cfg.DefaultRoutingPolicy.FeeRate), @@ -2509,7 +2512,7 @@ func (f *fundingManager) newChanAnnouncement(localPubKey, remotePubKey *btcec.Pu // finish, either successfully or with an error. func (f *fundingManager) announceChannel(localIDKey, remoteIDKey, localFundingKey, remoteFundingKey *btcec.PublicKey, shortChanID lnwire.ShortChannelID, - chanID lnwire.ChannelID, remoteMinHTLC lnwire.MilliSatoshi) error { + chanID lnwire.ChannelID, fwdMinHTLC lnwire.MilliSatoshi) error { // First, we'll create the batch of announcements to be sent upon // initial channel creation. This includes the channel announcement @@ -2517,7 +2520,7 @@ func (f *fundingManager) announceChannel(localIDKey, remoteIDKey, localFundingKe // proof needed to fully authenticate the channel. ann, err := f.newChanAnnouncement(localIDKey, remoteIDKey, localFundingKey, remoteFundingKey, shortChanID, chanID, - remoteMinHTLC, + fwdMinHTLC, ) if err != nil { fndgLog.Errorf("can't generate channel announcement: %v", err) From e069dd7f04535ba102095fe3106e80b64553e3f6 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:43 +0200 Subject: [PATCH 05/12] fundingmanager test: test that custom MinHTLC is in ChannelUpdate --- fundingmanager_test.go | 72 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 3 deletions(-) diff --git a/fundingmanager_test.go b/fundingmanager_test.go index 224e1e6e..2c860169 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -428,6 +428,12 @@ func recreateAliceFundingManager(t *testing.T, alice *testNode) { }, TempChanIDSeed: oldCfg.TempChanIDSeed, FindChannel: oldCfg.FindChannel, + DefaultRoutingPolicy: htlcswitch.ForwardingPolicy{ + MinHTLC: 5, + BaseFee: 100, + FeeRate: 1000, + TimeLockDelta: 10, + }, PublishTransaction: func(txn *wire.MsgTx) error { publishChan <- txn return nil @@ -806,7 +812,16 @@ func assertAddedToRouterGraph(t *testing.T, alice, bob *testNode, assertDatabaseState(t, bob, fundingOutPoint, addedToRouterGraph) } -func assertChannelAnnouncements(t *testing.T, alice, bob *testNode) { +// assertChannelAnnouncements checks that alice and bob both sends the expected +// announcements (ChannelAnnouncement, ChannelUpdate) after the funding tx has +// confirmed. The last arguments can be set if we expect the nodes to advertise +// custom min_htlc values as part of their ChannelUpdate. We expect Alice to +// advertise the value required by Bob and vice versa. If they are not set the +// advertised value will be checked againts the other node's default min_htlc +// value. +func assertChannelAnnouncements(t *testing.T, alice, bob *testNode, + customMinHtlc ...lnwire.MilliSatoshi) { + // After the FundingLocked message is sent, Alice and Bob will each // send the following messages to their gossiper: // 1) ChannelAnnouncement @@ -814,7 +829,8 @@ func assertChannelAnnouncements(t *testing.T, alice, bob *testNode) { // The ChannelAnnouncement is kept locally, while the ChannelUpdate // is sent directly to the other peer, so the edge policies are // known to both peers. - for j, node := range []*testNode{alice, bob} { + nodes := []*testNode{alice, bob} + for j, node := range nodes { announcements := make([]lnwire.Message, 2) for i := 0; i < len(announcements); i++ { select { @@ -827,10 +843,35 @@ func assertChannelAnnouncements(t *testing.T, alice, bob *testNode) { gotChannelAnnouncement := false gotChannelUpdate := false for _, msg := range announcements { - switch msg.(type) { + switch m := msg.(type) { case *lnwire.ChannelAnnouncement: gotChannelAnnouncement = true case *lnwire.ChannelUpdate: + + // The channel update sent by the node should + // advertise the MinHTLC value required by the + // _other_ node. + other := (j + 1) % 2 + minHtlc := nodes[other].fundingMgr.cfg. + DefaultRoutingPolicy.MinHTLC + + // We might expect a custom MinHTLC value. + if len(customMinHtlc) > 0 { + if len(customMinHtlc) != 2 { + t.Fatalf("only 0 or 2 custom " + + "min htlc values " + + "currently supported") + } + + minHtlc = customMinHtlc[j] + } + + if m.HtlcMinimumMsat != minHtlc { + t.Fatalf("expected ChannelUpdate to "+ + "advertise min HTLC %v, had %v", + minHtlc, m.HtlcMinimumMsat) + } + gotChannelUpdate = true } } @@ -2201,6 +2242,31 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { case <-time.After(time.Second * 5): t.Fatalf("alice did not publish funding tx") } + + // Notify that transaction was mined. + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + + // After the funding transaction is mined, Alice will send + // fundingLocked to Bob. + _ = assertFundingMsgSent( + t, alice.msgChan, "FundingLocked", + ).(*lnwire.FundingLocked) + + // And similarly Bob will send funding locked to Alice. + _ = assertFundingMsgSent( + t, bob.msgChan, "FundingLocked", + ).(*lnwire.FundingLocked) + + // Make sure both fundingManagers send the expected channel + // announcements. Alice should advertise the default MinHTLC value of + // 5, while bob should advertise the value minHtlc, since Alice + // required him to use it. + assertChannelAnnouncements(t, alice, bob, 5, minHtlc) + + // The funding transaction is now confirmed, wait for the + // OpenStatusUpdate_ChanOpen update + waitForOpenUpdate(t, updateChan) } // TestFundingManagerMaxPendingChannels checks that trying to open another From 7eee09c454e0ba5becda394e4fdf053218b34f87 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:43 +0200 Subject: [PATCH 06/12] lnd_test: check ChannelUpdate for custom min_htlc --- lnd_test.go | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 4 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index 62aa9c7f..d27ea47c 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -921,6 +921,10 @@ func checkChannelPolicy(policy, expectedPolicy *lnrpc.RoutingPolicy) error { expectedPolicy.TimeLockDelta, policy.TimeLockDelta) } + if policy.MinHtlc != expectedPolicy.MinHtlc { + return fmt.Errorf("expected min htlc %v, got %v", + expectedPolicy.MinHtlc, policy.MinHtlc) + } if policy.Disabled != expectedPolicy.Disabled { return errors.New("edge should be disabled but isn't") } @@ -934,6 +938,13 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { timeout := time.Duration(time.Second * 15) ctxb := context.Background() + const ( + defaultFeeBase = 1000 + defaultFeeRate = 1 + defaultTimeLockDelta = 144 + defaultMinHtlc = 1000 + ) + // Launch notification clients for all nodes, such that we can // get notified when they discover new channels and updates in the // graph. @@ -943,7 +954,7 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { defer close(bQuit) chanAmt := maxBtcFundingAmount - pushAmt := btcutil.Amount(100000) + pushAmt := chanAmt / 2 // Create a channel Alice->Bob. ctxt, _ := context.WithTimeout(ctxb, timeout) @@ -960,6 +971,35 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { nodeUpdates := []chan *lnrpc.GraphTopologyUpdate{aliceUpdates, bobUpdates} nodes := []*lntest.HarnessNode{net.Alice, net.Bob} + // Alice and Bob should see each other's ChannelUpdates, advertising the + // default routing policies. + expectedPolicy := &lnrpc.RoutingPolicy{ + FeeBaseMsat: defaultFeeBase, + FeeRateMilliMsat: defaultFeeRate, + TimeLockDelta: defaultTimeLockDelta, + MinHtlc: defaultMinHtlc, + } + + for _, updates := range nodeUpdates { + waitForChannelUpdate( + t, updates, + []expectedChanUpdate{ + {net.Alice.PubKeyStr, expectedPolicy, chanPoint}, + {net.Bob.PubKeyStr, expectedPolicy, chanPoint}, + }, + ) + } + + // They should now know about the default policies. + for _, node := range nodes { + assertChannelPolicy( + t, node, net.Alice.PubKeyStr, expectedPolicy, chanPoint, + ) + assertChannelPolicy( + t, node, net.Bob.PubKeyStr, expectedPolicy, chanPoint, + ) + } + ctxt, _ = context.WithTimeout(ctxb, time.Second*15) err := net.Alice.WaitForNetworkChannelOpen(ctxt, chanPoint) if err != nil { @@ -985,19 +1025,68 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { nodeUpdates = append(nodeUpdates, carolUpdates) nodes = append(nodes, carol) + // Send some coins to Carol that can be used for channel funding. + ctxt, _ = context.WithTimeout(ctxb, time.Second*15) + err = net.SendCoins(ctxt, btcutil.SatoshiPerBitcoin, carol) + if err != nil { + t.Fatalf("unable to send coins to carol: %v", err) + } + if err := net.ConnectNodes(ctxb, carol, net.Bob); err != nil { t.Fatalf("unable to connect dave to alice: %v", err) } + // Open the channel Carol->Bob with a custom min_htlc value set. Since + // Carol is opening the channel, she will require Bob to not forward + // HTLCs smaller than this value, and hence he should advertise it as + // part of his ChannelUpdate. + const customMinHtlc = 5000 ctxt, _ = context.WithTimeout(ctxb, timeout) chanPoint2 := openChannelAndAssert( - ctxt, t, net, net.Bob, carol, + ctxt, t, net, carol, net.Bob, lntest.OpenChannelParams{ Amt: chanAmt, PushAmt: pushAmt, + MinHtlc: customMinHtlc, }, ) + expectedPolicyBob := &lnrpc.RoutingPolicy{ + FeeBaseMsat: defaultFeeBase, + FeeRateMilliMsat: defaultFeeRate, + TimeLockDelta: defaultTimeLockDelta, + MinHtlc: customMinHtlc, + } + + expectedPolicyCarol := &lnrpc.RoutingPolicy{ + FeeBaseMsat: defaultFeeBase, + FeeRateMilliMsat: defaultFeeRate, + TimeLockDelta: defaultTimeLockDelta, + MinHtlc: defaultMinHtlc, + } + + for _, updates := range nodeUpdates { + waitForChannelUpdate( + t, updates, + []expectedChanUpdate{ + {net.Bob.PubKeyStr, expectedPolicyBob, chanPoint2}, + {carol.PubKeyStr, expectedPolicyCarol, chanPoint2}, + }, + ) + } + + // Check that all nodes now know about the updated policies. + for _, node := range nodes { + assertChannelPolicy( + t, node, net.Bob.PubKeyStr, expectedPolicyBob, + chanPoint2, + ) + assertChannelPolicy( + t, node, carol.PubKeyStr, expectedPolicyCarol, + chanPoint2, + ) + } + ctxt, _ = context.WithTimeout(ctxb, time.Second*15) err = net.Alice.WaitForNetworkChannelOpen(ctxt, chanPoint2) if err != nil { @@ -1021,10 +1110,11 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { feeRate := int64(12) timeLockDelta := uint32(66) - expectedPolicy := &lnrpc.RoutingPolicy{ + expectedPolicy = &lnrpc.RoutingPolicy{ FeeBaseMsat: baseFee, FeeRateMilliMsat: testFeeBase * feeRate, TimeLockDelta: timeLockDelta, + MinHtlc: defaultMinHtlc, } req := &lnrpc.PolicyUpdateRequest{ @@ -1062,7 +1152,9 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { // internalized this fee update. This shouldn't affect the route that // Alice takes though: we updated the Alice -> Bob channel and she // doesn't pay for transit over that channel as it's direct. - payAmt := lnwire.MilliSatoshi(2000) + // Note that the payment amount is >= the min_htlc value for the + // channel Bob->Carol, so it should successfully be forwarded. + payAmt := btcutil.Amount(5) invoice := &lnrpc.Invoice{ Memo: "testing", Value: int64(payAmt), @@ -3059,6 +3151,7 @@ func updateChannelPolicy(t *harnessTest, node *lntest.HarnessNode, FeeBaseMsat: baseFee, FeeRateMilliMsat: feeRate, TimeLockDelta: timeLockDelta, + MinHtlc: 1000, // default value } updateFeeReq := &lnrpc.PolicyUpdateRequest{ @@ -11322,6 +11415,7 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { FeeBaseMsat: baseFee, FeeRateMilliMsat: testFeeBase * feeRate, TimeLockDelta: timeLockDelta, + MinHtlc: 1000, // default value } updateFeeReq := &lnrpc.PolicyUpdateRequest{ @@ -11341,6 +11435,7 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ = context.WithTimeout(ctxb, timeout) aliceUpdates, aQuit := subscribeGraphNotifications(t, ctxt, net.Alice) defer close(aQuit) + waitForChannelUpdate( t, aliceUpdates, []expectedChanUpdate{ @@ -11561,6 +11656,7 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { FeeBaseMsat: int64(defaultBitcoinBaseFeeMSat), FeeRateMilliMsat: int64(defaultBitcoinFeeRate), TimeLockDelta: defaultBitcoinTimeLockDelta, + MinHtlc: 1000, // default value Disabled: true, } From f4f7b64b2d3697dba6b55c0eb05180c4e6047657 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:43 +0200 Subject: [PATCH 07/12] lnwallet/channel: add FwdMinHtlc(), returning MinHTLC from localChanCfg --- lnwallet/channel.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index f654d1c6..bad130b1 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -6254,3 +6254,9 @@ func (lc *LightningChannel) RemoteCommitHeight() uint64 { return lc.channelState.RemoteCommitment.CommitHeight } + +// FwdMinHtlc returns the minimum HTLC value required by the remote node, i.e. +// the minimum value HTLC we can forward on this channel. +func (lc *LightningChannel) FwdMinHtlc() lnwire.MilliSatoshi { + return lc.localChanCfg.MinHTLC +} From 229fd882895a6f6c15a92da56cf397d78952d518 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:43 +0200 Subject: [PATCH 08/12] peer: use custom MinHTLC value at initial link add This commit fixes a bug within the peer, where we would always use the default forwarding policy when adding new links to the switch. Mostly this wasn't a problem, as we most often are using default values for new channels, but the min_htlc value is a value that is set by the remote, not us. If the remote specified a custom min_htlc during channel creation, we would still use our DefaultMinHtlc value when first adding the link, making us try to forward HTLCs that would be rejected by the remote. We fix this by getting the required min_htlc value from the channel state machine, and setting this for the link's forwarding policy. Note that on restarts we will query the database for our latest ChannelEdgePolicy, and use that to craft the forwardingPolicy. This means that the value will be set correctly if the policy is found in the database. --- peer.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/peer.go b/peer.go index f6366c80..9145a97c 100644 --- a/peer.go +++ b/peer.go @@ -1550,9 +1550,23 @@ out: continue } + // We'll query the localChanCfg of the new channel to + // determine the minimum HTLC value that can be + // forwarded. For fees we'll use the default values, as + // they currently are always set to the default values + // at initial channel creation. + fwdMinHtlc := newChan.FwdMinHtlc() + defaultPolicy := p.server.cc.routingPolicy + forwardingPolicy := &htlcswitch.ForwardingPolicy{ + MinHTLC: fwdMinHtlc, + BaseFee: defaultPolicy.BaseFee, + FeeRate: defaultPolicy.FeeRate, + TimeLockDelta: defaultPolicy.TimeLockDelta, + } + // Create the link and add it to the switch. err = p.addLink( - chanPoint, newChan, &p.server.cc.routingPolicy, + chanPoint, newChan, forwardingPolicy, chainEvents, currentHeight, false, ) if err != nil { From de5e9e139c014a54d38be4dd299e9fd82785ecc8 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:44 +0200 Subject: [PATCH 09/12] server: make extractChannelUpdate extract from set of potential policies --- peer.go | 2 +- server.go | 32 +++++++++++++++++++++----------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/peer.go b/peer.go index 9145a97c..db06a493 100644 --- a/peer.go +++ b/peer.go @@ -2139,6 +2139,6 @@ func fetchLastChanUpdate(s *server, local = edge1 } - return extractChannelUpdate(info, local) + return extractChannelUpdate(pubKey[:], info, local) } } diff --git a/server.go b/server.go index ae4745f2..b820c494 100644 --- a/server.go +++ b/server.go @@ -2953,6 +2953,18 @@ func (s *server) fetchLastChanUpdateByOutPoint(op wire.OutPoint) ( return nil, err } + pubKey := s.identityPriv.PubKey().SerializeCompressed() + return extractChannelUpdate(pubKey, info, edge1, edge2) +} + +// extractChannelUpdate attempts to retrieve a lnwire.ChannelUpdate message +// from an edge's info and a set of routing policies. +// NOTE: the passed policies can be nil. +func extractChannelUpdate(ownerPubKey []byte, + info *channeldb.ChannelEdgeInfo, + policies ...*channeldb.ChannelEdgePolicy) ( + *lnwire.ChannelUpdate, error) { + // Helper function to extract the owner of the given policy. owner := func(edge *channeldb.ChannelEdgePolicy) []byte { var pubKey *btcec.PublicKey @@ -2972,21 +2984,19 @@ func (s *server) fetchLastChanUpdateByOutPoint(op wire.OutPoint) ( } // Extract the channel update from the policy we own, if any. - ourPubKey := s.identityPriv.PubKey().SerializeCompressed() - if edge1 != nil && bytes.Equal(ourPubKey, owner(edge1)) { - return extractChannelUpdate(info, edge1) + for _, edge := range policies { + if edge != nil && bytes.Equal(ownerPubKey, owner(edge)) { + return createChannelUpdate(info, edge) + } } - if edge2 != nil && bytes.Equal(ourPubKey, owner(edge2)) { - return extractChannelUpdate(info, edge2) - } - - return nil, fmt.Errorf("unable to find channel(%v)", op) + return nil, fmt.Errorf("unable to extract ChannelUpdate for channel %v", + info.ChannelPoint) } -// extractChannelUpdate retrieves a lnwire.ChannelUpdate message from an edge's -// info and routing policy. -func extractChannelUpdate(info *channeldb.ChannelEdgeInfo, +// createChannelUpdate reconstructs a signed ChannelUpdate from the given edge +// info and policy. +func createChannelUpdate(info *channeldb.ChannelEdgeInfo, policy *channeldb.ChannelEdgePolicy) (*lnwire.ChannelUpdate, error) { update := &lnwire.ChannelUpdate{ From 19e3a194fad37a3d1e4c1453b87474c77841de8d Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:44 +0200 Subject: [PATCH 10/12] peer+server: make fetchLastChanUpdate _always_ fetch our own update This commit fixes a bug that would cause us to fetch our peer's ChannelUpdate in some cases, where we really wanted to fetch our own. The reason this happened was that we passed the peer's pubkey to fetchLastChanUpdate, making us match on their policy. This would lead to ChannelUpdates being sent during routing which would have no effect on the attempted path. We fix this by always use our own pubkey in fetchLastChanUpdate, and also uses the common methods within the server to be able to extract the update even when only one policy is known. --- peer.go | 60 +++++++++++++------------------------------------------ server.go | 17 +++++++++++++++- 2 files changed, 30 insertions(+), 47 deletions(-) diff --git a/peer.go b/peer.go index db06a493..df8529cf 100644 --- a/peer.go +++ b/peer.go @@ -539,22 +539,20 @@ func (p *peer) addLink(chanPoint *wire.OutPoint, } linkCfg := htlcswitch.ChannelLinkConfig{ - Peer: p, - DecodeHopIterators: p.server.sphinx.DecodeHopIterators, - ExtractErrorEncrypter: p.server.sphinx.ExtractErrorEncrypter, - FetchLastChannelUpdate: fetchLastChanUpdate( - p.server, p.PubKey(), - ), - DebugHTLC: cfg.DebugHTLC, - HodlMask: cfg.Hodl.Mask(), - Registry: p.server.invoices, - Switch: p.server.htlcSwitch, - Circuits: p.server.htlcSwitch.CircuitModifier(), - ForwardPackets: p.server.htlcSwitch.ForwardPackets, - FwrdingPolicy: *forwardingPolicy, - FeeEstimator: p.server.cc.feeEstimator, - PreimageCache: p.server.witnessBeacon, - ChainEvents: chainEvents, + Peer: p, + DecodeHopIterators: p.server.sphinx.DecodeHopIterators, + ExtractErrorEncrypter: p.server.sphinx.ExtractErrorEncrypter, + FetchLastChannelUpdate: p.server.fetchLastChanUpdate(), + DebugHTLC: cfg.DebugHTLC, + HodlMask: cfg.Hodl.Mask(), + Registry: p.server.invoices, + Switch: p.server.htlcSwitch, + Circuits: p.server.htlcSwitch.CircuitModifier(), + ForwardPackets: p.server.htlcSwitch.ForwardPackets, + FwrdingPolicy: *forwardingPolicy, + FeeEstimator: p.server.cc.feeEstimator, + PreimageCache: p.server.witnessBeacon, + ChainEvents: chainEvents, UpdateContractSignals: func(signals *contractcourt.ContractSignals) error { return p.server.chainArb.UpdateContractSignals( *chanPoint, signals, @@ -2112,33 +2110,3 @@ func (p *peer) StartTime() time.Time { } // TODO(roasbeef): make all start/stop mutexes a CAS - -// fetchLastChanUpdate returns a function which is able to retrieve the last -// channel update for a target channel. -func fetchLastChanUpdate(s *server, - pubKey [33]byte) func(lnwire.ShortChannelID) (*lnwire.ChannelUpdate, error) { - - return func(cid lnwire.ShortChannelID) (*lnwire.ChannelUpdate, error) { - info, edge1, edge2, err := s.chanRouter.GetChannelByID(cid) - if err != nil { - return nil, err - } - - if edge1 == nil || edge2 == nil { - return nil, fmt.Errorf("unable to find channel by "+ - "ShortChannelID(%v)", cid) - } - - // If we're the outgoing node on the first edge, then that - // means the second edge is our policy. Otherwise, the first - // edge is our policy. - var local *channeldb.ChannelEdgePolicy - if bytes.Equal(edge1.Node.PubKeyBytes[:], pubKey[:]) { - local = edge2 - } else { - local = edge1 - } - - return extractChannelUpdate(pubKey[:], info, local) - } -} diff --git a/server.go b/server.go index b820c494..9424eedc 100644 --- a/server.go +++ b/server.go @@ -336,7 +336,7 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB, cc *chainControl, FwdingLog: chanDB.ForwardingLog(), SwitchPackager: channeldb.NewSwitchPackager(), ExtractErrorEncrypter: s.sphinx.ExtractErrorEncrypter, - FetchLastChannelUpdate: fetchLastChanUpdate(s, serializedPubKey), + FetchLastChannelUpdate: s.fetchLastChanUpdate(), Notifier: s.cc.chainNotifier, FwdEventTicker: ticker.New( htlcswitch.DefaultFwdEventInterval), @@ -2957,6 +2957,21 @@ func (s *server) fetchLastChanUpdateByOutPoint(op wire.OutPoint) ( return extractChannelUpdate(pubKey, info, edge1, edge2) } +// fetchLastChanUpdate returns a function which is able to retrieve our latest +// channel update for a target channel. +func (s *server) fetchLastChanUpdate() func(lnwire.ShortChannelID) ( + *lnwire.ChannelUpdate, error) { + + ourPubKey := s.identityPriv.PubKey().SerializeCompressed() + return func(cid lnwire.ShortChannelID) (*lnwire.ChannelUpdate, error) { + info, edge1, edge2, err := s.chanRouter.GetChannelByID(cid) + if err != nil { + return nil, err + } + return extractChannelUpdate(ourPubKey[:], info, edge1, edge2) + } +} + // extractChannelUpdate attempts to retrieve a lnwire.ChannelUpdate message // from an edge's info and a set of routing policies. // NOTE: the passed policies can be nil. From 030eca2347c747bc12f7f6b228cda9d7334ef5a1 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:44 +0200 Subject: [PATCH 11/12] lnd_test: test payment failing because < min_htlc --- lnd_test.go | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 123 insertions(+), 3 deletions(-) diff --git a/lnd_test.go b/lnd_test.go index d27ea47c..892acbb0 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -1103,6 +1103,126 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("carol didn't report channel: %v", err) } + // First we'll try to send a payment from Alice to Carol with an amount + // less than the min_htlc value required by Carol. This payment should + // fail, as the channel Bob->Carol cannot carry HTLCs this small. + payAmt := btcutil.Amount(4) + invoice := &lnrpc.Invoice{ + Memo: "testing", + Value: int64(payAmt), + } + resp, err := carol.AddInvoice(ctxb, invoice) + if err != nil { + t.Fatalf("unable to add invoice: %v", err) + } + + ctxt, _ = context.WithTimeout(ctxb, timeout) + err = completePaymentRequests( + ctxt, net.Alice, []string{resp.PaymentRequest}, true, + ) + + // Alice knows about the channel policy of Carol and should therefore + // not be able to find a path during routing. + if err == nil || + !strings.Contains(err.Error(), "unable to find a path") { + t.Fatalf("expected payment to fail, instead got %v", err) + } + + // Now we try to send a payment over the channel with a value too low + // to be accepted. First we query for a route to route a payment of + // 5000 mSAT, as this is accepted. + payAmt = btcutil.Amount(5) + routesReq := &lnrpc.QueryRoutesRequest{ + PubKey: carol.PubKeyStr, + Amt: int64(payAmt), + NumRoutes: 1, + FinalCltvDelta: 144, + } + + ctxt, _ = context.WithTimeout(ctxb, timeout) + routes, err := net.Alice.QueryRoutes(ctxt, routesReq) + if err != nil { + t.Fatalf("unable to get route: %v", err) + } + + if len(routes.Routes) != 1 { + t.Fatalf("expected to find 1 route, got %v", len(routes.Routes)) + } + + // We change the route to carry a payment of 4000 mSAT instead of 5000 + // mSAT. + payAmt = btcutil.Amount(4) + amtSat := int64(payAmt) + amtMSat := int64(lnwire.NewMSatFromSatoshis(payAmt)) + routes.Routes[0].Hops[0].AmtToForward = amtSat + routes.Routes[0].Hops[0].AmtToForwardMsat = amtMSat + routes.Routes[0].Hops[1].AmtToForward = amtSat + routes.Routes[0].Hops[1].AmtToForwardMsat = amtMSat + + // Send the payment with the modified value. + ctxt, _ = context.WithTimeout(ctxb, timeout) + alicePayStream, err := net.Alice.SendToRoute(ctxt) + if err != nil { + t.Fatalf("unable to create payment stream for alice: %v", err) + } + sendReq := &lnrpc.SendToRouteRequest{ + PaymentHash: resp.RHash, + Routes: routes.Routes, + } + + err = alicePayStream.Send(sendReq) + if err != nil { + t.Fatalf("unable to send payment: %v", err) + } + + // We expect this payment to fail, and that the min_htlc value is + // communicated back to us, since the attempted HTLC value was too low. + sendResp, err := alicePayStream.Recv() + if err != nil { + t.Fatalf("unable to send payment: %v", err) + } + + // Expected as part of the error message. + substrs := []string{ + "AmountBelowMinimum", + "HtlcMinimumMsat: (lnwire.MilliSatoshi) 5000 mSAT", + } + for _, s := range substrs { + if !strings.Contains(sendResp.PaymentError, s) { + t.Fatalf("expected error to contain \"%v\", instead "+ + "got %v", sendResp.PaymentError) + } + } + + // Make sure sending using the original value succeeds. + payAmt = btcutil.Amount(5) + amtSat = int64(payAmt) + amtMSat = int64(lnwire.NewMSatFromSatoshis(payAmt)) + routes.Routes[0].Hops[0].AmtToForward = amtSat + routes.Routes[0].Hops[0].AmtToForwardMsat = amtMSat + routes.Routes[0].Hops[1].AmtToForward = amtSat + routes.Routes[0].Hops[1].AmtToForwardMsat = amtMSat + + sendReq = &lnrpc.SendToRouteRequest{ + PaymentHash: resp.RHash, + Routes: routes.Routes, + } + + err = alicePayStream.Send(sendReq) + if err != nil { + t.Fatalf("unable to send payment: %v", err) + } + + sendResp, err = alicePayStream.Recv() + if err != nil { + t.Fatalf("unable to send payment: %v", err) + } + + if sendResp.PaymentError != "" { + t.Fatalf("expected payment to succeed, instead got %v", + sendResp.PaymentError) + } + // With our little cluster set up, we'll update the fees for the // channel Bob side of the Alice->Bob channel, and make sure all nodes // learn about it. @@ -1154,12 +1274,12 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { // doesn't pay for transit over that channel as it's direct. // Note that the payment amount is >= the min_htlc value for the // channel Bob->Carol, so it should successfully be forwarded. - payAmt := btcutil.Amount(5) - invoice := &lnrpc.Invoice{ + payAmt = btcutil.Amount(5) + invoice = &lnrpc.Invoice{ Memo: "testing", Value: int64(payAmt), } - resp, err := carol.AddInvoice(ctxb, invoice) + resp, err = carol.AddInvoice(ctxb, invoice) if err != nil { t.Fatalf("unable to add invoice: %v", err) } From e9066c2d349a05e249bd375ea9258d31b1857189 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 22 Aug 2018 09:32:44 +0200 Subject: [PATCH 12/12] peer: print warning if selfPolicy not found --- peer.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/peer.go b/peer.go index df8529cf..a49a3cf7 100644 --- a/peer.go +++ b/peer.go @@ -396,10 +396,14 @@ func (p *peer) loadActiveChannels(chans []*channeldb.OpenChannel) error { TimeLockDelta: uint32(selfPolicy.TimeLockDelta), } } else { + peerLog.Warnf("Unable to find our forwarding policy "+ + "for channel %v, using default values", + chanPoint) forwardingPolicy = &p.server.cc.routingPolicy } - peerLog.Tracef("Using link policy of: %v", spew.Sdump(forwardingPolicy)) + peerLog.Tracef("Using link policy of: %v", + spew.Sdump(forwardingPolicy)) // Register this new channel link with the HTLC Switch. This is // necessary to properly route multi-hop payments, and forward