fix: add Maybe BalanceAt expectations to tests missing them#22042
Merged
fix: add Maybe BalanceAt expectations to tests missing them#22042
Conversation
…flect
Tests using mock eth clients that don't set a BalanceAt expectation are
vulnerable to a data race: the balance monitor's background worker may
call BalanceAt during shutdown, hitting testify's unexpected-call error
path which does fmt.Sprintf("%#v", ctx) — an unsynchronized reflect read
that races with concurrent context cancellation.
Adding .Maybe().Return(big.NewInt(0), nil) registers the expectation so
testify takes the happy path (match and return) instead of the error
path (reflect into arguments). The race cannot fire on the happy path.
product-security-plaid-production
Bot
requested review from
Atrax1,
dhaidashenko and
j-nowak
Contributor
|
👋 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! |
Contributor
|
✅ No conflicts with other open PRs targeting |
jmank88
previously approved these changes
Apr 16, 2026
jmank88
approved these changes
Apr 16, 2026
pavel-raykov
approved these changes
Apr 16, 2026
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
.Maybe()BalanceAt mock expectations to 8 tests that use mock eth clients without their own BalanceAt expectation. This prevents a data race ingo_core_race_tests.Problem
Tests that create chains with mock clients but complete quickly may never need a
BalanceAtexpectation. However, the balance monitor's background worker can fire during shutdown, callingBalanceAton the mock. With no matching expectation, testify hits its "unexpected method call" error path, which callscallString()→fmt.Sprintf("%#v", ctx)— an unsynchronized reflect read that races with concurrent context cancellation.testify's
callString()reflecting into arguments is fundamentally broken (unsynchronized), but an upstream fix was rejected. The workaround: make the call expected so testify takes the happy path (match → return, no reflect).Fix
Added to each vulnerable test:
.Maybe()means "this call might happen, I don't care either way." If the balance monitor callsBalanceAt, testify matches it and returns — no reflect, no race. If it doesn't call it,AssertExpectationsdoesn't complain.Tests Modified
core/internal/features/features_test.goTestIntegration_ExternalInitiatorV2core/services/job/runner_integration_test.goTestRunner_Success_Callback_AsyncJobcore/services/job/runner_integration_test.goTestRunner_Error_Callback_AsyncJobcore/services/pipeline/task.eth_call_test.goTestETHCallTaskcore/services/relay/evm/write_target_test.goTestEvmWritecore/services/vrf/v2/integration_v2_test.goTestStartingCountsV1core/web/pipeline_runs_controller_test.goTestPipelineRunsController_CreateWithBody_HappyPathcore/web/pipeline_runs_controller_test.goTestPipelineRunsController_CreateNoBody_HappyPathRelated