TestSweepBatcherCloseDuringAdding previously started Batcher.Run in a
detached goroutine and called test assertions from that goroutine. The
test only waited for the add/cancel workers, so a Run-side shutdown
error could be missed or reported unreliably.
Route the Run result through a channel and wait for it in the main test
goroutine. While waiting, keep draining spend registrations so shutdown
cannot deadlock on mock notifier traffic.
This makes the existing shutdown-race test cover both sides of the race:
AddSweep callers may exit with cancellation, and Batcher.Run must also
terminate with an expected shutdown error.
PresignSweepsGroup uses context-sensitive wallet and presigned-helper
calls, but it previously returned their raw wrapped errors even when the
caller context or batcher shutdown state had already become terminal.
That leaves backend/helper errors visible during normal cancellation.
Check for shutdown/cancellation before presigning and after fee lookup
or presigning failures, preferring context.Canceled or
ErrBatcherShuttingDown over lower-level errors.
Log the original presign-path error before returning the shutdown or
cancellation error so normal shutdown remains debuggable without
changing the returned error.
Add a regression test with a presigned helper that cancels the caller
context while returning driver.ErrBadConn from SignTx. The test asserts
PresignSweepsGroup reports context.Canceled and does not wrap the driver
error, and runs against both mock and SQL-backed stores.
AddSweep now masks cancellation races during its caller-side setup, but
a request can still be accepted by the batcher run loop before the run
context cancellation is selected. If handleSweeps then performs a
context-sensitive store call while shutdown is in progress, a
backend-specific error such as driver.ErrBadConn can still bubble out
through Batcher.Run.
Prefer the run context's terminal error when startup, handleSweeps, or
asynchronous errChan failures happen after the run context has been
canceled. This keeps normal shutdown reporting consistent and avoids
surfacing backend driver errors from the accepted-request and async
error paths.
Log the original run-loop error before returning the context error so
normal shutdown remains debuggable without changing the returned error.
Add regression tests for both covered run-loop races. One lets AddSweep
successfully hand a sweep request to Run, then cancels the run context
from the second GetSweepStatus call while returning driver.ErrBadConn.
The other queues an errChan error from the event loop while canceling
the run context. Both tests assert that Run returns context.Canceled and
does not wrap the driver error.
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.
