Skip to content

Commit

Permalink
accounts: safe check of pendingPayments in tests
Browse files Browse the repository at this point in the history
In this commit, we avoid race conditions that can happen for tests that
directly access the service's `pendingPayment` map.
  • Loading branch information
ellemouton committed Jan 21, 2025
1 parent 4d8d4e1 commit a4506c9
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
12 changes: 12 additions & 0 deletions accounts/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,18 @@ func (s *InterceptorService) removePayment(ctx context.Context,
return nil
}

// hasPayment returns true if the payment is currently being tracked by the
// service.
//
// NOTE: this is currently used only for tests.
func (s *InterceptorService) hasPayment(hash lntypes.Hash) bool {
s.RLock()
defer s.RUnlock()

_, ok := s.pendingPayments[hash]
return ok
}

// successState returns true if a payment was completed successfully.
func successState(status lnrpc.Payment_PaymentStatus) bool {
return status == lnrpc.Payment_SUCCEEDED
Expand Down
19 changes: 11 additions & 8 deletions accounts/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,13 +383,14 @@ func TestAccountService(t *testing.T) {
// Assert that the invoice subscription succeeded.
require.Contains(t, s.invoiceToAccount, testHash)

// But setting up the payment tracking should have failed.
// But setting up the payment tracking should have
// failed.
require.False(t, s.IsRunning())

// Finally let's assert that we didn't successfully add the
// payment to pending payment, and that lnd isn't awaiting
// the payment request.
require.NotContains(t, s.pendingPayments, testHash)
// Finally let's assert that we didn't successfully add
// the payment to pending payment, and that lnd isn't
// awaiting the payment request.
require.False(t, s.hasPayment(testHash))
r.assertNoPaymentRequest(t)
},
}, {
Expand Down Expand Up @@ -426,7 +427,9 @@ func TestAccountService(t *testing.T) {
// This will cause an error send an update over
// the payment channel, which should disable the
// service.
s.pendingPayments = make(map[lntypes.Hash]*trackedPayment)
s.pendingPayments = make(
map[lntypes.Hash]*trackedPayment,
)

// Send an invalid payment over the payment chan
// which should error and disable the service
Expand Down Expand Up @@ -568,7 +571,7 @@ func TestAccountService(t *testing.T) {
return p.Status == lnrpc.Payment_FAILED
})

require.NotContains(t, s.pendingPayments, testHash2)
require.False(t, s.hasPayment(testHash2))

// Finally, if an unknown payment turns out to be
// a non-initiated payment, we should stop the tracking
Expand Down Expand Up @@ -616,7 +619,7 @@ func TestAccountService(t *testing.T) {

// Ensure that the payment was removed from the pending
// payments.
require.NotContains(t, s.pendingPayments, testHash3)
require.False(t, s.hasPayment(testHash3))
},
}, {
name: "keep track of invoice indexes",
Expand Down

0 comments on commit a4506c9

Please sign in to comment.