From 0a4f062e22b15b4102c1b30028d0ea070effb3bb Mon Sep 17 00:00:00 2001 From: positiveblue Date: Tue, 17 Jan 2023 14:15:39 -0800 Subject: [PATCH 1/2] routing: fix race condition in `TestUpdatePaymentState` The test cases in `TestUpdatePaymentState` run in parallel. One of the parameters is a pointer and the value to the struct it points to gets modified during the test. The race condition was introduced in 8d49dfb07ea00352effc9ed99ce995db270b78e6 To test the fix run from the main folder `go test ./routing/. -race` before this fix and after. --- routing/payment_lifecycle_test.go | 73 ++++++++++++++++--------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go index 102aa4c01..16d368d23 100644 --- a/routing/payment_lifecycle_test.go +++ b/routing/payment_lifecycle_test.go @@ -938,40 +938,14 @@ func TestUpdatePaymentState(t *testing.T) { // paymentHash is the identifier on paymentLifecycle. paymentHash := lntypes.Hash{} + preimage := lntypes.Preimage{} + failureReasonError := channeldb.FailureReasonError // TODO(yy): make MPPayment into an interface so we can mock it. The // current design implicitly tests the methods SendAmt, TerminalInfo, // and InFlightHTLCs on channeldb.MPPayment, which is not good. Once // MPPayment becomes an interface, we can then mock these methods here. - // SentAmt returns 90, 10 - // TerminalInfo returns non-nil, nil - // InFlightHTLCs returns 0 - var preimage lntypes.Preimage - paymentSettled := &channeldb.MPPayment{ - HTLCs: []channeldb.HTLCAttempt{ - makeSettledAttempt(100, 10, preimage), - }, - } - - // SentAmt returns 0, 0 - // TerminalInfo returns nil, non-nil - // InFlightHTLCs returns 0 - reason := channeldb.FailureReasonError - paymentFailed := &channeldb.MPPayment{ - FailureReason: &reason, - } - - // SentAmt returns 90, 10 - // TerminalInfo returns nil, nil - // InFlightHTLCs returns 1 - paymentActive := &channeldb.MPPayment{ - HTLCs: []channeldb.HTLCAttempt{ - makeActiveAttempt(100, 10), - makeFailedAttempt(100, 10), - }, - } - testCases := []struct { name string payment *channeldb.MPPayment @@ -992,16 +966,31 @@ func TestUpdatePaymentState(t *testing.T) { { // Test that when the sentAmt exceeds totalAmount, the // error is returned. - name: "amount exceeded error", - payment: paymentSettled, + name: "amount exceeded error", + // SentAmt returns 90, 10 + // TerminalInfo returns non-nil, nil + // InFlightHTLCs returns 0 + payment: &channeldb.MPPayment{ + HTLCs: []channeldb.HTLCAttempt{ + makeSettledAttempt(100, 10, preimage), + }, + }, totalAmt: 1, shouldReturnError: true, }, { // Test that when the fee budget is reached, the // remaining fee should be zero. - name: "fee budget reached", - payment: paymentActive, + name: "fee budget reached", + payment: &channeldb.MPPayment{ + // SentAmt returns 90, 10 + // TerminalInfo returns nil, nil + // InFlightHTLCs returns 1 + HTLCs: []channeldb.HTLCAttempt{ + makeActiveAttempt(100, 10), + makeFailedAttempt(100, 10), + }, + }, totalAmt: 1000, feeLimit: 1, expectedState: &paymentState{ @@ -1014,8 +1003,15 @@ func TestUpdatePaymentState(t *testing.T) { { // Test when the payment is settled, the state should // be marked as terminated. - name: "payment settled", - payment: paymentSettled, + name: "payment settled", + // SentAmt returns 90, 10 + // TerminalInfo returns non-nil, nil + // InFlightHTLCs returns 0 + payment: &channeldb.MPPayment{ + HTLCs: []channeldb.HTLCAttempt{ + makeSettledAttempt(100, 10, preimage), + }, + }, totalAmt: 1000, feeLimit: 100, expectedState: &paymentState{ @@ -1028,8 +1024,13 @@ func TestUpdatePaymentState(t *testing.T) { { // Test when the payment is failed, the state should be // marked as terminated. - name: "payment failed", - payment: paymentFailed, + name: "payment failed", + // SentAmt returns 0, 0 + // TerminalInfo returns nil, non-nil + // InFlightHTLCs returns 0 + payment: &channeldb.MPPayment{ + FailureReason: &failureReasonError, + }, totalAmt: 1000, feeLimit: 100, expectedState: &paymentState{ From 057d38e2fce4dd6a39f7d876a7633adf07be6ae5 Mon Sep 17 00:00:00 2001 From: positiveblue Date: Tue, 17 Jan 2023 14:24:54 -0800 Subject: [PATCH 2/2] docs: update release notes --- docs/release-notes/release-notes-0.16.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index 1146051de..16dea30a9 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/docs/release-notes/release-notes-0.16.0.md @@ -219,6 +219,9 @@ data. * [Fix gomnd linter error](https://github.com/lightningnetwork/lnd/pull/7325) +* [Fix race condition in +`TestUpdatePaymentState`](https://github.com/lightningnetwork/lnd/pull/7336) + ## `lncli` * [Add an `insecure` flag to skip tls auth as well as a `metadata` string slice