Skip to content

test: pin every salt-derivation boundary as a fixed case - #88

Merged
thedavidmeister merged 5 commits into
mainfrom
2026-09-16-issue-70-salt-boundaries
Sep 19, 2026
Merged

thedavidmeister merged 5 commits into
mainfrom
2026-09-16-issue-70-salt-boundaries

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Asserts the boundary values of the salt derivations and clone paths as fixed cases, so covering them no longer depends on what the fuzzer draws. Test-only.

unit boundary test
effectiveSalt salt 0 / max testEffectiveSaltBoundarySalts
effectiveSalt deployer 0 / max testEffectiveSaltBoundaryDeployers
effectiveOpenSalt salt 0 / max testEffectiveOpenSaltBoundarySalts
effectiveOpenSalt data of 1, 32, 33, 2048 bytes testEffectiveOpenSaltBoundaryDataLengths
predictCloneAddress factory 0 / max testPredictCloneAddressBoundaryFactories
predictCloneAddress implementation 0 / max testPredictCloneAddressBoundaryImplementations
predictCloneAddress derived salt 0 / max testPredictCloneAddressBoundarySalts
predictDeterministicAddress deployer 0 testCloneDeterministicPredictZeroDeployer
cloneDeterministic salt 0 / max testCloneDeterministicExtremeSalts
cloneDeterministicOpenSalt salt 0 / max testCloneDeterministicOpenSaltExtremeSalts
cloneDeterministicOpenSalt empty data testCloneDeterministicOpenSaltEmptyData
cloneDeterministicOpenSalt 10,000-byte data testCloneDeterministicOpenSaltLargeData

[fuzz] seed stays unpinned: the fixed cases cover the boundaries on every run.

Supersedes #75 and #77. Closes #70

QA

  • forge test on a8098e2: 62 passed (main: 50). CI green.

  • Mutation probe (rainlanguage/adversarial-mutation-test@4e59ec0cc05248c14bcb1f27bc5c11b4d1e5589c), 32 boundary mutants of src/lib/LibICloneableFactoryV4.sol: whole suite 32/32 killed; the suite without this PR's 12 tests: 25/32, with 7 surviving (O06–O08, D01, D05, E06, E07). The other 25 die only when a fuzz run happens to draw the boundary value.

  • Each test was run alone against all 32 mutants. Listed below are the mutants it kills where it is the only test that fixes that operand at the boundary:

    test mutants
    testEffectiveSaltBoundarySalts S01
    testEffectiveSaltBoundaryDeployers S04
    testEffectiveOpenSaltBoundarySalts O01
    testEffectiveOpenSaltBoundaryDataLengths O03–O06
    testPredictCloneAddressBoundaryFactories P03, P04
    testPredictCloneAddressBoundaryImplementations P05, P06, C01, C02
    testPredictCloneAddressBoundarySalts P01, P02
    testCloneDeterministicPredictZeroDeployer D03
    testCloneDeterministicExtremeSalts D01, D02, D04, D05
    testCloneDeterministicOpenSaltExtremeSalts E01, E02
    testCloneDeterministicOpenSaltEmptyData E03–E05
    testCloneDeterministicOpenSaltLargeData O08, E06, E07
  • Mutant definitions, logs, JSON and the runner: /home/gildlab/artifacts/factory/88/trim/.

🤖 Generated with Claude Code

The derivations are specified over the whole input domain with no boundary
carved out, but the suite asserted the boundary values only when the fuzzer
happened to draw them, so which boundaries a green run had actually covered
varied with the seed. Each end of every operand domain that was still left to
the fuzzer is now a fixed case with the other operands fuzzed: salt and
deployer at zero and max in `effectiveSalt`, data at the word-relative length
boundaries in `effectiveOpenSalt`, implementation at zero and max in
`cloneCreationCode`, factory and implementation at zero and max in
`predictCloneAddress`. Expected values are recomputed from the interface
formula, from OZ `Clones`, or written out as literal bytes, never read back
from the library. The remaining two boundaries, the salt ends in
`effectiveOpenSalt` and `predictCloneAddress`, are pinned by PR #75.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Sep 16, 2026
@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 41ef7cad-ab7d-4425-8c50-133f38174ce2

📥 Commits

Reviewing files that changed from the base of the PR and between 79130af and a8098e2.

📒 Files selected for processing (4)
  • test/src/lib/LibICloneableFactoryV4.cloneDeterministic.t.sol
  • test/src/lib/LibICloneableFactoryV4.cloneDeterministicOpenSalt.t.sol
  • test/src/lib/LibICloneableFactoryV4.predictCloneAddress.t.sol
  • test/src/lib/LibICloneableFactoryV4.t.sol

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

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

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

thedavidmeister and others added 2 commits September 19, 2026 13:54
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings in, from #75, testEffectiveOpenSaltBoundarySalts and
testPredictCloneAddressBoundarySalts (salt at zero and max in the open-salt
derivation and in the CREATE2 prediction), and from #77 the end-to-end
boundary cases: testCloneDeterministicExtremeSalts,
testCloneDeterministicOpenSaltExtremeSalts, testCloneDeterministicPredictZeroDeployer,
testCloneDeterministicOpenSaltEmptyData and testCloneDeterministicOpenSaltLargeData.
Test bodies are unchanged; comments state the behaviour pinned.

testCloneCreationCodeBoundaryImplementations now builds its expectation from
the library's EIP1167_CREATION_CODE_PREFIX/_SUFFIX around the boundary
address bytes instead of copying the constants as hex literals.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thedavidmeister and others added 2 commits September 19, 2026 16:14
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drop testCloneCreationCodeBoundaryImplementations: its mutants are killed
by testPredictCloneAddressBoundaryImplementations. Drop assertions implied
by others in the same test or covered by main's tests, and cut NatSpec to
one line of behaviour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister
thedavidmeister merged commit 9262a20 into main Sep 19, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Boundary-value coverage of the salt derivations is fuzz-seed luck: max-salt mutants survive the whole suite

1 participant