From 1161d87eec810b84933c4d46bfbe78794c65e64c Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 4 Jun 2019 10:13:33 +0200 Subject: [PATCH 1/2] routing/router: correct SendToRoute's amount record in DB Previously we would mistakenly use the payment value from the dummy LightningPayment struct, which would obviously be 0 always. Now we instead calculate the value from the given route. --- routing/router.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/routing/router.go b/routing/router.go index 37443b2d..2081bac8 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1637,26 +1637,31 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( // Create a payment session for just this route. paySession := r.cfg.MissionControl.NewPaymentSessionForRoute(route) - // Create a (mostly) dummy payment, as the created payment session is - // not going to do path finding. - payment := &LightningPayment{ - PaymentHash: hash, - } + // Calculate amount paid to receiver. + amt := route.TotalAmount - route.TotalFees() // Record this payment hash with the ControlTower, ensuring it is not // already in-flight. info := &channeldb.PaymentCreationInfo{ - PaymentHash: payment.PaymentHash, - Value: payment.Amount, + PaymentHash: hash, + Value: amt, CreationDate: time.Now(), PaymentRequest: nil, } - err := r.cfg.Control.InitPayment(payment.PaymentHash, info) + err := r.cfg.Control.InitPayment(hash, info) if err != nil { return [32]byte{}, err } + // Create a (mostly) dummy payment, as the created payment session is + // not going to do path finding. + // TODO(halseth): sendPayment doesn't relly need LightningPayment, make + // it take just needed fields instead. + payment := &LightningPayment{ + PaymentHash: hash, + } + // Since this is the first time this payment is being made, we pass nil // for the existing attempt. preimage, _, err := r.sendPayment(nil, payment, paySession) From 6d52128da0f46a9964c9dababc095426c8a0a6ab Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 4 Jun 2019 10:34:59 +0200 Subject: [PATCH 2/2] routing/router test: assert SendToRoute init values This commit adds an assertion to the SendToRoute test that the payment value stored to the DB during SendToRoute execution is the correct one. This assertion would fail before the previous commit that fixed a missing value initialization. --- routing/router_test.go | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/routing/router_test.go b/routing/router_test.go index f90dbd8c..54ee4708 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -3175,27 +3175,31 @@ func TestSendToRouteStructuredError(t *testing.T) { } defer cleanUp() + // Set up an init channel for the control tower, such that we can make + // sure the payment is initiated correctly. + init := make(chan initArgs, 1) + ctx.router.cfg.Control.(*mockControlTower).init = init + // Setup a route from source a to destination c. The route will be used // in a call to SendToRoute. SendToRoute also applies channel updates, // but it saves us from including RequestRoute in the test scope too. + const payAmt = lnwire.MilliSatoshi(10000) hop1 := ctx.aliases["b"] hop2 := ctx.aliases["c"] - hops := []*route.Hop{ { - ChannelID: 1, - PubKeyBytes: hop1, + ChannelID: 1, + PubKeyBytes: hop1, + AmtToForward: payAmt, }, { - ChannelID: 2, - PubKeyBytes: hop2, + ChannelID: 2, + PubKeyBytes: hop2, + AmtToForward: payAmt, }, } - rt, err := route.NewRouteFromHops( - lnwire.MilliSatoshi(10000), 100, - ctx.aliases["a"], hops, - ) + rt, err := route.NewRouteFromHops(payAmt, 100, ctx.aliases["a"], hops) if err != nil { t.Fatalf("unable to create route: %v", err) } @@ -3239,4 +3243,14 @@ func TestSendToRouteStructuredError(t *testing.T) { if _, ok := fErr.FailureMessage.(*lnwire.FailFeeInsufficient); !ok { t.Fatalf("expected fee insufficient error") } + + // Check that the correct values were used when initiating the payment. + select { + case initVal := <-init: + if initVal.c.Value != payAmt { + t.Fatalf("expected %v, got %v", payAmt, initVal.c.Value) + } + case <-time.After(100 * time.Millisecond): + t.Fatalf("initPayment not called") + } }