Flaky fuzz: testFlowBasicValidateMultipleSignedContexts can pass two colliding bound private keys (over-constrained vm.assume)

[thedavidmeister]

Summary

testFlowBasicValidateMultipleSignedContexts in test/src/concrete/Flow.signedContext.t.sol is a latent/flaky red on rainix-sol-test. It passes at its configured forge-config: default.fuzz.runs = 100 under the repo's pinned seed = "0xdeadbeef", but fails deterministically once enough fuzz runs execute the colliding input. CI fuzzes with a random seed, so this surfaces intermittently.

This is a BROKEN / over-constrained TEST, not a bug in the signed-context validation code (LibContext.build is correct). It was introduced by the boundPrivateKey refactor (#420 / #458).

Deterministic repro

The per-test runs = 100 override hides it at the default seed. Temporarily bump that line to a higher count and run under the pinned seed:

# in test/src/concrete/Flow.signedContext.t.sol bump:
#   forge-config: default.fuzz.runs = 100  ->  = 100000
nix develop -c forge test \
  --match-test testFlowBasicValidateMultipleSignedContexts \
  --fuzz-seed 0xdeadbeef -vvvv

Result (run 19):

[FAIL: next call did not revert as expected; counterexample: ...
 args=[[...], [...], 0, 1]]
Suite result: FAILED. 0 passed; 1 failed; 0 skipped
└─ ← [Revert] next call did not revert as expected

The load-bearing part of the counterexample is the last two scalars: fuzzedKeyAlice = 0, fuzzedKeyBob = 1.

Root cause

The test does:

vm.assume(fuzzedKeyBob != fuzzedKeyAlice);          // constrains RAW fuzz inputs
uint256 aliceKey = boundPrivateKey(fuzzedKeyAlice);  // folds into [1, n-1]
uint256 bobKey   = boundPrivateKey(fuzzedKeyBob);
...
// "bad" second context: signer = alice, signature = bob
signedContexts1[1] = vm.signContext(aliceKey, bobKey, context1);
vm.expectRevert(abi.encodeWithSelector(InvalidSignature.selector, 1));
flow.flow(evaluable, new uint256[](0), signedContexts1);

forge-std's boundPrivateKey is _bound(x, 1, SECP256K1_ORDER - 1), which is not injective over the full uint256 domain — in particular the wrap region collides. boundPrivateKey(0) == boundPrivateKey(1) == 1 (verified directly).

So for fuzzedKeyAlice = 0, fuzzedKeyBob = 1: the raw values differ (so vm.assume passes), but aliceKey == bobKey == 1. The "bad" signature is then signed by the same key it claims as signer, so LibContext.build's SignatureChecker.isValidSignatureNow(signer, digest, signature) correctly recovers signer and the signature is valid → no revert → vm.expectRevert(InvalidSignature(1)) fails with "next call did not revert as expected".

The assertion is only valid when the bounded keys differ; the test guards the raw keys instead.

Real-bug-vs-broken-test diagnosis

Broken test (over-constrained fuzz assertion). The code under test is correct: a signature whose ECDSA-recovered address matches the declared signer should validate, even if a separate "expected key" happens to collide. The test's invariant precondition (alice's and bob's keys are distinct) is asserted on the pre-bound inputs but consumed post-bound.

testFlowBasicValidateSignedContexts (same file, line ~53) shares the identical flaw — it just hasn't hit the colliding input at 100 runs / this seed yet. A fix should cover both.

Suggested fix

Constrain the bounded keys to differ, not the raw fuzz inputs, e.g. bound/boundPrivateKey first, then vm.assume(aliceKey != bobKey) (or assume vm.addr(aliceKey) != vm.addr(bobKey)). Do not weaken the InvalidSignature assertion itself — the bad-signature path must still be exercised across the input domain.

Note: rainix-sol-static is independently red on this repo (slither, tracked in #476); that is unrelated to this test.