[Testing] Harden EVM UUID partition-0 test coverage

[janezpodhostnik]

Follow-up hardening of the forced UUID-partition-0 tests introduced in #8637,
addressing the actionable findings from the reviews on #8634 and #8637.

Changes

• Guard the forced-partition-zero tests with requireUUIDPartitionZero, so they
  fail loudly instead of silently testing the common branch if the block forcing
  ever stops selecting partition 0
• Bound the search loop in blockFixtureWithUUIDPartitionZero and fail via
  t.Fatalf instead of hanging if the block fixture stops being random
• Add TestCOADryCall_UUIDPartitionZero and
  TestCOADryCallWithSigAndArgs_UUIDPartitionZero, giving the partition-0 branch
  of assertUpdatedRegisterCount (one register fewer) deterministic coverage;
  the shared COA setup is extracted into createCOAAndRunEVMTx
• Deduplicate the partition check into usesLegacyUUIDPartition, stating the
  sha256(blockID)[0] + txnIndex 0 assumption in one place

Verified: full package passes with -race, 5x with -shuffle=on, and the
changed tests 20x repeated without flakes.

Related: #8629, #8634, #8637

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 37049933-3cc3-4b65-bd1b-f8c9f80a9666

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

FVM review

3 findings, 1 of them important. Test-only diff (fvm/evm/evm_test.go); no production FVM code is touched, so there are no determinism/HCU, metering, or error-taxonomy implications.

Important

requireUUIDPartitionZero is tautological and cannot deliver the protection it documents — fvm/evm/evm_test.go:7247.

The guard's stated purpose is to "fail loudly instead of silently passing via the non-zero-partition branch if the block forcing ever breaks." But it re-derives the identical predicate that was used to pick the block:

• blockFixtureWithUUIDPartitionZero (:7266) selects on sha256(block.ID())[0] == 0
• runWithNewEnvironmentWithBlock sets fvm.WithBlockHeader(block1.ToHeader())
• usesLegacyUUIDPartition (:7228) tests sha256(ctx.BlockHeader.ID())[0] == 0

GenericBlock.ID() is defined as b.ToHeader().ID() (model/flow/block.go:40), and WithBlockHeader stores the header verbatim (fvm/context.go:180). So the guard's input and formula are bit-for-bit the same as the fixture's, and require.True can never fire.

Concrete failure scenario: environment.uuidPartition is changed to use a different hash byte (or to stop deriving the partition from the block ID at all). The test-local copy of the formula in usesLegacyUUIDPartition still agrees with itself, so the fixture keeps "succeeding", the guard keeps passing, and both TestCOADryCall_UUIDPartitionZero and TestDryRunFollowedByRun_UUIDPartitionZero silently run against a non-zero partition while asserting the partition-0 register count / computation — exactly the silent-wrong-branch outcome the guard was added to prevent. The guard adds no coverage over the fixture itself.

To be non-tautological the check has to observe the production behaviour rather than restate the test's own arithmetic. environment.uuidPartition is unexported, but the observable consequence is available: the legacy register flow.UUIDRegisterID(0) ({Owner: "", Key: "uuid"}) vs. uuid_N. assertUpdatedRegisterCount already receives the *snapshot.ExecutionSnapshot, so it can branch on what actually happened instead of on ctx:

_, legacy := state.WriteSet[flow.UUIDRegisterID(0)]
if legacy {
    expected--
}

and requireUUIDPartitionZero can assert legacy after the run. That also removes the txnIndex == 0 assumption below entirely. (Worth confirming the legacy register isn't written for any unrelated reason in the non-zero-partition case before adopting it.)

Nits

1. usesLegacyUUIDPartition bakes in txnIndex == 0 with nothing enforcing it — fvm/evm/evm_test.go:7228. environment.uuidPartition computes (sha256(blockID)[0] + txnIndex) % 256 (fvm/environment/uuids.go:80); the helper drops the txnIndex term and only documents the assumption. If a future test calls assertUpdatedRegisterCount on a transaction built with fvm.Transaction(txBody, 1), the helper returns the wrong branch and the assertion is off by one, with no compile-time or runtime signal. Taking txnIndex uint32 and mirroring the production formula would cost one parameter. I verified this is latent, not live: assertUpdatedRegisterCount has exactly one caller (:3094), and both it and dryRunFollowedByRunCase (:4126) submit fvm.Transaction(txBody, 0).

2. blockFixtureWithUUIDPartitionZero should call t.Helper() — fvm/evm/evm_test.go:7266. Without it, the t.Fatalf at :7277 reports the helper's line rather than the test that invoked it. (The unreachable return nil after t.Fatalf is fine — Goexit never reaches it, and Go requires the statement.)

3. Minor redundancy in the extracted helpers — createCOAAndRunEVMTx (:3026), coaDryCallCase (:3116), and coaDryCallWithSigAndArgsCase (:3205) each recompute systemcontracts.SystemContractsForChain(chain.ChainID()), and createCOAAndRunEVMTx takes both chain and ctx when ctx.Chain already carries it. Harmless, just more parameters than needed.

Verified correct

• The extraction is behaviour-preserving. The two removed inline bodies had byte-identical first transactions; the extracted createCOAAndRunEVMTx keeps the same script, the same single PrepareSignAndEncodeTx call (so nonce consumption is unchanged), the same 3-event assertions and ordering, and the same assertUpdatedRegisterCount(..., 13). Snapshot threading that was snapshot = snapshot.Append(state) inline is preserved through the return value, and the dry-call halves keep the 0 events / 0 updated registers assertions.
• All non-zero partitions are equivalent for these assertions, so a single countNonZeroPartition value is sound: a fresh uuid_P register yields the same counter start for every P != 0.
• for range maxAttempts over an int is valid — go.mod declares go 1.25.1.
• require/t.Fatalf are all invoked on the test goroutine. runWithNewEnvironmentWithBlock and the nested RunWith* helpers call f synchronously, so FailNow/Goexit semantics are correct and the helpers' deferred cleanups still run.

I was not able to execute go test/go vet in this environment, so the register-count values themselves (13 / 12) are not independently confirmed here — CI covers them, and a wrong value now fails deterministically rather than at 1/256, which is the improvement this PR is after.

_{Automated FVM review (Claude, trial mode) — findings only, no approval.}

[janezpodhostnik]

Applied in 25fefe8:

• assertUpdatedRegisterCount now branches on the observed write set (state.WriteSet[flow.UUIDRegisterID(0)]), removing the ctx/txnIndex assumption.
• createCOAAndRunEVMTx cross-checks the test-local formula against the observed write on every run, so a change to environment.uuidPartition fails the forced COA tests deterministically.
• usesLegacyUUIDPartition takes an explicit txnIndex and mirrors the production formula.
• Redundant chain params dropped in favor of ctx.Chain.

Not applied:

• The write-set check can't replace the guard for TestDryRunFollowedByRun_UUIDPartitionZero: verified empirically that this transaction writes no uuid register at either partition (the uuid interaction stays in discarded speculative state). Its partition-dependent computation assertion already fails deterministically (79 vs 74) if production diverges, so the header-based guard remains for wiring/fixture drift only, with its comment corrected accordingly.
• t.Helper() in blockFixtureWithUUIDPartitionZero: intentionally omitted. If that Fatalf fires, the defect is in the fixture itself, so the helper's line is the useful one; the failing test's name is in the output regardless.

All divergence paths were negative-tested (sabotaged formula/fixture → guard, cross-check, and assertion each fire as expected).

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!