test: add Heap invariant harness#6555
Conversation
🦋 Changeset detectedLatest commit: 436418d The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR extends the Heap test suite with invariant-based testing infrastructure. A new 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/utils/structs/Heap.t.sol`:
- Around line 103-106: The handler currently signals model/handler desync by
calling revert("unexpected pop") / revert("unexpected replace") in the Heap test
handlers (see uses of _isMaxHeap, _heap.pop(Comparators.gt), root(), and
_removeFirst), which can be ignored by Foundry when fail_on_revert is false; fix
this by either setting fail_on_revert = true under the active [invariant]
profile in foundry.toml so these reverts cause invariant runs to fail, or
replace the revert paths in the handlers with explicit invariant assertions
(e.g., require/assert style checks that fail the test harness) to ensure
mismatches are always reported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 58816cf1-16d5-4a6d-a42e-962fe0a8e671
📒 Files selected for processing (1)
test/utils/structs/Heap.t.sol
| uint256 expected = root(); | ||
| uint256 removed = _isMaxHeap ? _heap.pop(Comparators.gt) : _heap.pop(); | ||
| if (removed != expected) revert("unexpected pop"); | ||
| _removeFirst(expected); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate Foundry config =="
fd -a '^foundry\.toml$'
echo "== Invariant-related settings =="
if [ -f foundry.toml ]; then
rg -n -C2 '^\s*\[invariant\]|^\s*\[profile\..*\.invariant\]|\bfail_on_revert\s*=' foundry.toml
else
echo "foundry.toml not found at repo root"
fi
echo "== Any other fail_on_revert declarations in repo =="
rg -n '\bfail_on_revert\s*='Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== All foundry.toml files =="
fd -a '^foundry\.toml$'
echo
echo "== Global search for fail_on_revert =="
rg -n '\bfail_on_revert\s*=' -S .
echo
echo "== Show any invariant-related config blocks in top-level foundry.toml =="
if [ -f foundry.toml ]; then
rg -n -C3 '\[(invariant|profile\.[^.]+\.invariant)[^\]]*\]' foundry.toml || true
rg -n -C3 'fail_on_revert' foundry.toml || true
else
echo "top-level foundry.toml not present"
fi
echo
echo "== List any foundry config extensions (e.g., *.toml) =="
fd -a '\.toml$' -t fRepository: OpenZeppelin/openzeppelin-contracts
Length of output: 180
🌐 Web query:
Foundry invariant fail_on_revert default value when not set
💡 Result:
The default value for fail_on_revert in Foundry's invariant testing configuration is false [1][2][3]. When this setting is false (the default), the fuzzer will ignore reverts that occur during the execution of function calls in an invariant test sequence [4][2][5]. This allows the fuzzer to continue testing other sequences even if specific calls revert [2][5]. If you wish to ensure that any revert within your handler functions triggers a test failure, you must explicitly set fail_on_revert = true in your foundry.toml file under the [invariant] section [1][6][2].
Citations:
- 1: https://rareskills.io/post/invariant-testing-solidity
- 2: https://foundry-book.zksync.io/forge/invariant-testing
- 3: https://www.getfoundry.sh/guides/invariant-testing
- 4: feat(invariant): add fail_on_assert for assertion failures in invariant campaigns foundry-rs/foundry#14275
- 5: https://paritytech.github.io/foundry-book-polkadot/forge/invariant-testing.html
- 6: Invariant test does not revert though fail_on_revert is set to true foundry-rs/foundry#6694
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== invariant-related sections in repo root foundry.toml =="
rg -n -C3 '^\s*\[invariant\]' foundry.toml || true
rg -n -C3 '^\s*\[profile\.[^.]+\.invariant\]' foundry.toml || true
echo
echo "== any mention of fail_on_revert (context) =="
rg -n -C2 'fail_on_revert' foundry.toml || trueRepository: OpenZeppelin/openzeppelin-contracts
Length of output: 185
Make handler mismatch revert(...) paths fail invariant runs (fail_on_revert)
test/utils/structs/Heap.t.sol(e.g., lines 103-106, and similarly 112-115) usesrevert("unexpected pop")/revert("unexpected replace")to detect handler/model desync.- The repo’s root
foundry.tomlhas nofail_on_revertsetting, so Foundry’s default (fail_on_revert = false) can cause these reverts during invariant target calls to be ignored—masking real heap regressions. - Add
fail_on_revert = trueunder the active[invariant](or invariant profile) config, or assert the mismatch from invariant targets instead of relying on handler reverts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/utils/structs/Heap.t.sol` around lines 103 - 106, The handler currently
signals model/handler desync by calling revert("unexpected pop") /
revert("unexpected replace") in the Heap test handlers (see uses of _isMaxHeap,
_heap.pop(Comparators.gt), root(), and _removeFirst), which can be ignored by
Foundry when fail_on_revert is false; fix this by either setting fail_on_revert
= true under the active [invariant] profile in foundry.toml so these reverts
cause invariant runs to fail, or replace the revert paths in the handlers with
explicit invariant assertions (e.g., require/assert style checks that fail the
test harness) to ensure mismatches are always reported.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/utils/structs/Heap.t.sol`:
- Around line 124-129: The root() function reads _model[0] and will revert on
empty arrays; add an explicit empty-array guard at the start of root() (e.g.,
require(_model.length > 0, "Heap: empty") or revert with a clear message) to
prevent out-of-bounds panics and make the precondition explicit when using
_model and _isMaxHeap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f15bbcc9-48cd-435e-9bc0-376bbed7d3af
📒 Files selected for processing (2)
.changeset/twelve-pants-listen.mdtest/utils/structs/Heap.t.sol
1 participant
Summary
Heap.Uint256Heapinsert,pop,replace, andclearpeek()consistency against a shadow modelTesting
forge fmt --check test/utils/structs/Heap.t.solPATH=/home/alex/.foundry/bin:$PATH forge test --match-path test/utils/structs/Heap.t.sol -vvCloses #5120