fix: add BalanceAt mock to prevent balance monitor race#22041
fix: add BalanceAt mock to prevent balance monitor race#22041
Conversation
The EVM balance monitor's background worker calls BalanceAt on the eth
client. When this mock helper is used without a BalanceAt expectation
and the balance monitor is enabled, the call hits testify's unexpected
method error path which formats arguments via fmt.Sprintf("%#v", ctx).
This reflect-based read races with concurrent context cancellation
during test teardown.
Adding a default BalanceAt expectation with .Maybe() prevents the
unexpected call error path from firing.
|
👋 Fletch153, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
|
Closing in favor of the upstream root-cause fix in chainlink-common: smartcontractkit/chainlink-common#1984 The catch-all BalanceAt mock in the shared helper would shadow test-specific BalanceAt expectations (testify matches in registration order). The chainlink-common fix eliminates the race at the source by replacing the CtxCancel goroutine with a read-only context — no test changes needed in this repo. |
|





Summary
Adds a default
BalanceAtmock expectation toNewEthMocksWithStartupAssertions, preventing a data race when the EVM balance monitor is enabled.Root Cause
NewEthMocksWithStartupAssertionssets up common mock expectations but omitsBalanceAt. When a test enables the balance monitor (default behavior), its backgroundSleeperTaskworker callsBalanceAton the mock client. With no matching expectation, testify hits its "unexpected method call" error path, which callscallString()→fmt.Sprintf("%#v", ctx)→ reflect-reads the context. This races with concurrent context cancellation during test teardown.Race stack trace (from CI)
Note:
mock.go:507is the "method called over N times" / unexpected call error path — this only fires because there's noBalanceAtexpectation at all.Fix
One line: add
BalanceAtwith.Maybe()to the mock helper. This gives the balance monitor a matching expectation with unlimited repeatability, socallString()is never invoked.Related