contributor-rewards: stop the keypair tests racing on a shared env var

[bgm-malbeclabs]

test_keypair_env_fallback in contributor-rewards fails intermittently in CI. It failed on two unrelated pull requests today, and a re-run of the identical commit passed both times.

It is a race between tests, not a fault in the code under test.

What happens

Three tests in offchain/crates/contributor-rewards/tests/test_sanity.rs set or clear the same process-wide variable:

• test_keypair_cli_takes_precedence sets REWARDER_KEYPAIR_PATH, then clears it
• test_keypair_env_fallback sets it, calls load_keypair(&None), then clears it
• test_keypair_not_provided_error clears it, then expects load_keypair(&None) to fail

Cargo runs tests in parallel threads of one process, and std::env is per process, not per thread. When one of the other two clears the variable between test_keypair_env_fallback's set_var and its load_keypair(&None), the fallback finds nothing and assert!(result.is_ok()) fails. The window is a few microseconds wide, which is why this shows up now and then rather than every run.

The other two keypair tests pass an explicit path, so the CLI argument wins and the variable is irrelevant to them. They need no change.

Summary of Changes

• The three tests that touch REWARDER_KEYPAIR_PATH now carry #[serial], so they run one at a time while the rest of the suite still runs in parallel.
• serial_test was already a workspace dependency, used by doublezero-cli-core, config, smartcontract/cli and smartcontract/sdk/rs. It is added to this crate's dev-dependencies only, so nothing new enters the build.

The production load_keypair is untouched. Reading the variable inside it is what makes the tests need real environment state, but the tests are verifying exactly that fallback, so serialising them is the fix rather than changing the signature its callers use.

Diff Breakdown

Category	Files	Lines (+/-)	Net
Tests	1	+13 / -0	+13
Config	2	+4 / -0	+4
Docs	2	+3 / -0	+3
Total	5	+20 / -0	+20

Testing Verification

• Reproduced the failure before fixing it: 60 runs of cargo test -p doublezero-contributor-rewards --test test_sanity on the unchanged code produced 1 failure, on run 26, panicking at test_sanity.rs:63 exactly as CI did.
• The same 60 runs after the change: 0 failures.
• Both loops ran the whole file, not the three tests alone, so the parallel scheduling that causes the race was in play.

[Copilot]

🔵 Needs a closer look

The root CHANGELOG.md update introduces a duplicated “Offchain” section header in the Unreleased list, which should be consolidated for correct release notes structure.

Pull request overview

This pull request addresses intermittent CI failures in the doublezero-contributor-rewards integration tests by preventing three keypair-related tests from racing on the process-wide REWARDER_KEYPAIR_PATH environment variable.

Changes:

• Serialize the three tests that set or clear REWARDER_KEYPAIR_PATH using serial_test to eliminate cross-test interference.
• Add serial_test as a dev-dependency for contributor-rewards (already present in the workspace dependency set).
• Document the test-flake fix in both the crate-level and root changelogs, and update the workspace lockfile.

File summaries

File	Description
offchain/crates/contributor-rewards/tests/test_sanity.rs	Adds #[serial] to the three env-var-mutating tests to prevent parallel execution races.
offchain/crates/contributor-rewards/CHANGELOG.md	Notes the test serialization change and the reason for it.
offchain/crates/contributor-rewards/Cargo.toml	Adds serial_test under dev-dependencies for this crate.
CHANGELOG.md	Adds an Unreleased note about the fix (but currently introduces a duplicated “Offchain” heading).
Cargo.lock	Records the new serial_test dependency resolution in the workspace lockfile.

Review details

Suppressed comments (1)

CHANGELOG.md:35

• Issue
  The root changelog has two "Offchain" headings in the same Unreleased list.

Context
A reader scans the Unreleased section by category. The duplicated heading splits one category into two blocks and makes the list harder to read.

Proposed Fix
Keep a single "Offchain" heading and list all Offchain bullets under it.

- Offchain
  - `contributor-rewards`' three keypair tests no longer race. All three set or clear the process-wide `REWARDER_KEYPAIR_PATH`, and cargo runs tests in parallel threads of one process, so one test clearing the variable between another's set and its load made that one fail. Reproduced at roughly 1 run in 60 and gone in 60 runs after. They now use `#[serial]`, which four other crates in this workspace already use.
- E2E/QA
  - New e2e coverage for RFC-27 proof enforcement with `require-ip-ownership-proof` set: the working path still reaches BGP, a client with no verifier to reach is rejected with `IpOwnershipProofRequired`, a wildcard (`0.0.0.0`) access pass binds `client_ip` only when a proof is attached, the sentinel authority stays exempt so the oracle path keeps working, and `connect` refuses a proof whose address disagrees with the one it provisions. (#4243)
  - Remove `TestQA_MulticastSettlement`. It funded a seat through `doublezero-solana shreds pay`, which is going away. The agent seat-pay RPC now returns Unimplemented if something still calls it. Unused settlement helpers go with the test. (#4248)
- Offchain
  - The scheduler refuses to boot when `SOLANA_RPC` is unset or empty, rather than handing `nil` or an empty string to the Rust NIF. All three workers read the one config key, so the check sits in `config/runtime.exs` where the single cause was. The test environment is exempt, since the tests set the key themselves. `DZ_LEDGER_RPC` no longer sets a `ledger_rpc` key that nothing in the application read. (#4240 follow-up)
  - The scheduler README closes its shell code block, so the Installation heading and everything after it stop rendering inside it.

• Files reviewed: 4/5 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bgm-malbeclabs]

Superseded by #4265, which landed on main as 6994a4f while this was in flight. Same fix: use serial_test::serial plus #[serial] on the same three tests — test_keypair_cli_takes_precedence, test_keypair_env_fallback and test_keypair_not_provided_error — with serial_test added as a dev-dependency and both changelogs updated. Closing this rather than resolving the conflicts, since there is nothing here the merged fix does not already do.

One thing worth recording, from before I saw #4265. I reproduced the race to confirm the diagnosis: 60 runs of cargo test -p doublezero-contributor-rewards --test test_sanity on the unfixed code failed once, on run 26, panicking at test_sanity.rs:63 exactly as CI did. The same 60 runs with #[serial] applied failed none. So roughly a 1-in-60 flake rate, and the merged fix removes it.