From 3c49d7c852bc55d03833ca10d3e611676ecb5b26 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Sun, 5 Apr 2026 17:05:14 -0500 Subject: [PATCH] staticaddr: deflake deposit manager test The sqlite and postgres race jobs were both hanging in deposit.TestManager. The test was observing the manager through implementation details that were not safe to share with the manager itself: - it replaced the manager's internal finalizedDepositChan and then waited on the same channel the manager consumes - it reused package-level block and confirmation channels across runs - it treated confirmationHeight+expiry as the last pre-expiry block even though the production IsExpired check uses >= - it relied on scheduler timing when asserting that no sign request had happened yet Make the test assert on stable effects instead of internal channel ownership: - create per-test notifier channels in the test context - run the manager from a cancellable t.Context-derived context and assert clean shutdown - send the actual last pre-expiry height, then the expiry height - wait for the expiry sign and publish steps with bounded timeouts - verify finalization by waiting for the manager to remove the deposit from activeDeposits instead of racing its private finalization channel This keeps the test aligned with the production expiry semantics and removes the race that only showed up reliably under -race. --- staticaddr/deposit/manager_test.go | 86 ++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 23 deletions(-) diff --git a/staticaddr/deposit/manager_test.go b/staticaddr/deposit/manager_test.go index 659238ac9..78b05fbe5 100644 --- a/staticaddr/deposit/manager_test.go +++ b/staticaddr/deposit/manager_test.go @@ -33,16 +33,6 @@ var ( defaultExpiry = uint32(100) defaultDepositConfirmations = uint32(3) - - confChan = make(chan *chainntnfs.TxConfirmation) - - confErrChan = make(chan error) - - blockChan = make(chan int32) - - blockErrChan = make(chan error) - - finalizedDepositChan = make(chan wire.OutPoint) ) type mockStaticAddressClient struct { @@ -229,48 +219,87 @@ func (m *MockChainNotifier) RegisterSpendNtfn(ctx context.Context, // TestManager checks that the manager processes the right channel notifications // while a deposit is expiring. func TestManager(t *testing.T) { - ctx := context.Background() + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() // Create the test context with required mocks. testContext := newManagerTestContext(t) // Start the deposit manager. initChan := make(chan struct{}) + runErrChan := make(chan error, 1) go func() { - require.NoError(t, testContext.manager.Run(ctx, initChan)) + runErrChan <- testContext.manager.Run(ctx, initChan) }() // Ensure that the manager has been initialized. - <-initChan + select { + case <-initChan: + case err := <-runErrChan: + require.NoError(t, err, "manager failed to start") + + case <-time.After(test.Timeout): + t.Fatal("manager timed out starting") + } // Notify about the last block before the expiry. - blockChan <- int32(defaultDepositConfirmations + defaultExpiry) + testContext.blockChan <- int32( + defaultDepositConfirmations + defaultExpiry - 1, + ) // Ensure that the deposit state machine didn't sign for the expiry tx. select { case <-testContext.mockLnd.SignOutputRawChannel: t.Fatal("received unexpected sign request") - default: + case <-time.After(test.Timeout / 10): } // Mine the expiry tx height. - blockChan <- int32(defaultDepositConfirmations + defaultExpiry) + testContext.blockChan <- int32( + defaultDepositConfirmations + defaultExpiry, + ) - // Ensure that the deposit state machine didn't sign for the expiry tx. - <-testContext.mockLnd.SignOutputRawChannel + // Ensure that the deposit state machine signed the expiry tx. + select { + case <-testContext.mockLnd.SignOutputRawChannel: + + case <-time.After(test.Timeout): + t.Fatal("did not receive sign request") + } // Ensure that the signed expiry transaction is published. - expiryTx := <-testContext.mockLnd.TxPublishChannel + var expiryTx *wire.MsgTx + select { + case expiryTx = <-testContext.mockLnd.TxPublishChannel: + + case <-time.After(test.Timeout): + t.Fatal("did not receive published expiry tx") + } // Ensure that the deposit is waiting for a confirmation notification. - confChan <- &chainntnfs.TxConfirmation{ + testContext.confChan <- &chainntnfs.TxConfirmation{ BlockHeight: defaultDepositConfirmations + defaultExpiry + 3, Tx: expiryTx, } - // Ensure that the deposit is finalized. - <-finalizedDepositChan + // Ensure that the manager observed the finalization and removed the + // deposit from its active set. + require.Eventually(t, func() bool { + testContext.manager.mu.Lock() + defer testContext.manager.mu.Unlock() + + return len(testContext.manager.activeDeposits) == 0 + }, test.Timeout, 10*time.Millisecond) + + cancel() + select { + case err := <-runErrChan: + require.ErrorIs(t, err, context.Canceled) + + case <-time.After(test.Timeout): + t.Fatal("manager did not stop") + } } // ManagerTestContext is a helper struct that contains all the necessary @@ -281,6 +310,10 @@ type ManagerTestContext struct { mockLnd *test.LndMockServices mockStaticAddressClient *mockStaticAddressClient mockAddressManager *mockAddressManager + confChan chan *chainntnfs.TxConfirmation + confErrChan chan error + blockChan chan int32 + blockErrChan chan error } // newManagerTestContext creates a new test context for the reservation manager. @@ -292,6 +325,10 @@ func newManagerTestContext(t *testing.T) *ManagerTestContext { mockAddressManager := new(mockAddressManager) mockStore := new(mockStore) mockChainNotifier := new(MockChainNotifier) + confChan := make(chan *chainntnfs.TxConfirmation) + confErrChan := make(chan error) + blockChan := make(chan int32) + blockErrChan := make(chan error) ID, err := GetRandomDepositID() utxo := &lnwallet.Utxo{ @@ -353,7 +390,6 @@ func newManagerTestContext(t *testing.T) *ManagerTestContext { } manager := NewManager(cfg) - manager.finalizedDepositChan = finalizedDepositChan testContext := &ManagerTestContext{ manager: manager, @@ -361,6 +397,10 @@ func newManagerTestContext(t *testing.T) *ManagerTestContext { mockLnd: mockLnd, mockStaticAddressClient: mockStaticAddressClient, mockAddressManager: mockAddressManager, + confChan: confChan, + confErrChan: confErrChan, + blockChan: blockChan, + blockErrChan: blockErrChan, } staticAddress := generateStaticAddress(