feat: add DepositCapStateGuard batch pre-check#2
Open
maxencerb wants to merge 4 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds a new ChangesDeposit Cap State Guard
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 `@README.md`:
- Around line 54-56: The fenced address snippet in the README is missing a
language tag and triggers markdownlint MD040. Update that fenced block to use a
deterministic text language tag (for example, a plain text fence) so the address
line is still rendered as code while satisfying the linter.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3cfe7a38-796f-44c0-b072-1780acf57556
📒 Files selected for processing (9)
README.mdbroadcast/DeployDepositCapStateGuard.s.sol/1/run-1783338513185.jsonbroadcast/DeployDepositCapStateGuard.s.sol/1/run-1783338539903.jsonbroadcast/DeployDepositCapStateGuard.s.sol/1/run-latest.jsonbroadcast/DeployDepositCapStateGuard.s.sol/11155111/run-1783338690064.jsonbroadcast/DeployDepositCapStateGuard.s.sol/11155111/run-1783338727468.jsonbroadcast/DeployDepositCapStateGuard.s.sol/11155111/run-1783338776508.jsonbroadcast/DeployDepositCapStateGuard.s.sol/11155111/run-latest.jsonscript/DeployDepositCapStateGuard.s.sol
✅ Files skipped from review due to trivial changes (8)
- broadcast/DeployDepositCapStateGuard.s.sol/1/run-latest.json
- script/DeployDepositCapStateGuard.s.sol
- broadcast/DeployDepositCapStateGuard.s.sol/11155111/run-1783338727468.json
- broadcast/DeployDepositCapStateGuard.s.sol/1/run-1783338513185.json
- broadcast/DeployDepositCapStateGuard.s.sol/11155111/run-1783338776508.json
- broadcast/DeployDepositCapStateGuard.s.sol/11155111/run-latest.json
- broadcast/DeployDepositCapStateGuard.s.sol/11155111/run-1783338690064.json
- broadcast/DeployDepositCapStateGuard.s.sol/1/run-1783338539903.json
Comment on lines
+54
to
+56
| ``` | ||
| DepositCapStateGuard: 0x3Fa77E57baC28940621d1aaE1a136058c4064623 | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language tag to the fenced address block.
This trips markdownlint (MD040) and is easy to fix. Use something like text for the deterministic address snippet.
Proposed fix
-```
+```text
DepositCapStateGuard: 0x3Fa77E57baC28940621d1aaE1a136058c4064623
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@README.md` around lines 54 - 56, The fenced address snippet in the README is
missing a language tag and triggers markdownlint MD040. Update that fenced block
to use a deterministic text language tag (for example, a plain text fence) so
the address line is still rendered as code while satisfying the linter.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
src/checks/DepositCapStateGuard.sol, a small stateless, read-only guard that asserts a Facility intent's deposit cap and total supply match caller expectations, and reverts otherwise.Intended usage
The guard must be used as the first call of a batch transaction (e.g. an EIP-7702 batch) executed in the mode where any failing call reverts the whole batch. The batch calls
requireStatefirst; if the on-chain state no longer matches what was expected when the batch was signed (a deposit cap was changed, or deposits/withdrawals moved the supply), the read-only call reverts and the rest of the batch reverts with it — so no state-changing call ever executes against stale state.Design notes
[minSupply, maxSupply]range rather than an exact value, so benign concurrent deposits/withdrawals within the tolerated band don't race the batch into a revert. PassingminSupply == maxSupplydegenerates to an exact-supply check.Facility.totalSupply(uint256)is a public getter on the implementation but is not declared onIFacility, so the guard declares the one view it needs locally.Testing
test/DepositCapStateGuard.t.solsuite (11 tests, incl. a fuzz test over the supply range logic) covering in-range/edge/exact passes, below/above range, stale cap, inverted range, multi-intent batches, and empty checks.MockFacilitygainedtotalSupply/setTotalSupplyknobs; full suite passes (45/45).Summary by CodeRabbit
New Features
DepositCapStateGuard, a read-only batch precheck that validates each intent’s total supply is within an inclusive range and that the deposit cap matches exactly.Tests
Documentation
Deployment
script/DeployDepositCapStateGuard.s.soldeploys the guard through the canonical CREATE2 factory (0x4e59b44847b379578588920cA78FbF26c0B4956C) with a salt ground for a0x3fa77e57("3F ATTEST") vanity prefix, so it lands on the same address on every chain:The script asserts the deterministic address post-deploy and reverts on mismatch; the address is also documented in the README. It is a pure function of the init code, so the salt must be re-ground if the guard source or compiler profile changes.