[ci] adopt forge 1.8.0 - #978
Open
claude[bot] wants to merge 2 commits into
Open
Conversation
…orce Foundry's stable channel moved 1.7.1 -> 1.8.0 on 2026-08-27. The toolchain action is pinned by sha but the forge binary it installs is not, so the expanded 1.8.0 linter reddened the lint job with no change on our side: 125 findings, 106 in src/, 18 in certora/ and 1 in a transitive lib/ file. Stop linting certora/: the rules that fire there (repeated literals, one contract per file, external-function) describe the normal shape of a spec harness rather than defects. That disposes of the 18 certora findings. The repo has no script/ directory, so the target is just src. Also adopt the rule exclusions settled on in midnight#1168, which covers 64 of the 106 src findings. --deny notes stays, so strictness is unchanged for the code that ships; only the scope narrows. Co-authored-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
The 42 src findings the exclusions do not cover are acknowledged at the site with the repo's existing forge-lint directive idiom, each naming the reason the rule does not apply: - reentrancy-events (14): the emit is last on purpose, so it reports the post-interaction state. In the three factories the external call is the CREATE2 deployment whose address the event carries. - missing-events-access-control (6): every flagged write is reported by the Allocate, Deallocate, Deposit or Transfer event alongside it. - unsafe-typecast (6): block.timestamp fits uint64, the msg.data casts want only the leading bytes4, and allocation is bounded by absoluteCap. - uninitialized-local (4): each flagged local is read at its zero default deliberately - two loop counters, and minted/burned shares that are zero exactly when no assets moved. - unused-import (4): src/imports/*.sol are compile-forcing shims, so the imported symbol is deliberately unreferenced. - ecrecover (3): s-malleability is handled by the nonce, not the signature. - costly-loop (2): swap-and-pop writes storage once, then breaks. - reentrancy-no-eth (1): adapters are set through a timelock. - unused-error (1), cache-array-length (1). Comments only: the src diff adds 38 directive lines and removes nothing, so the bytecode is unchanged. Co-authored-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
MathisGD
reviewed
Aug 28, 2026
Comment on lines
+166
to
+167
| // forge-lint: disable-next-item(reentrancy-events) the event is emitted after the vault calls on purpose, once | ||
| // the cap check passed. |
Collaborator
There was a problem hiding this comment.
you infer too much things ^^
Suggested change
| // forge-lint: disable-next-item(reentrancy-events) the event is emitted after the vault calls on purpose, once | |
| // the cap check passed. | |
| // forge-lint: disable-next-item(reentrancy-events) ack. |
Comment on lines
+140
to
+141
| // forge-lint: disable-next-item(reentrancy-events) the event is emitted after the vault calls on purpose, once | ||
| // the cap check passed. |
Comment on lines
+17
to
18
| // forge-lint: disable-next-item(unused-error) part of the error catalogue, kept even though no code path uses it. | ||
| error CapExceeded(); |
Comment on lines
+592
to
593
| // forge-lint: disable-next-item(uninitialized-local) i is meant to start at zero. | ||
| for (uint256 i; i < ids.length; i++) { |
Collaborator
There was a problem hiding this comment.
actually initialise i (and double check that it doesn't change the compiled bytecode)
Comment on lines
+606
to
+607
| // forge-lint: disable-next-item(reentrancy-events) the event is emitted last so it reports the allocation after | ||
| // the adapter call. |
Comment on lines
+595
to
+596
| // forge-lint: disable-next-item(missing-events-access-control,unsafe-typecast) the Allocate event below | ||
| // reports the change, and allocation is bounded by absoluteCap < 2**128. |
Collaborator
There was a problem hiding this comment.
why do we even have the missing event here?
| { | ||
| require(isAdapter[adapter], ErrorsLib.NotAdapter()); | ||
|
|
||
| // forge-lint: disable-next-item(reentrancy-no-eth) adapters are set through a timelock, so they are trusted. |
Collaborator
There was a problem hiding this comment.
Suggested change
| // forge-lint: disable-next-item(reentrancy-no-eth) adapters are set through a timelock, so they are trusted. | |
| // forge-lint: disable-next-item(reentrancy-no-eth) adapters are trusted to not reenter. |
| // forge-lint: disable-next-item(reentrancy-no-eth) adapters are set through a timelock, so they are trusted. | ||
| (bytes32[] memory ids, int256 change) = IAdapter(adapter).deallocate(data, assets, msg.sig, msg.sender); | ||
|
|
||
| // forge-lint: disable-next-item(uninitialized-local) i is meant to start at zero. |
| for (uint256 i; i < ids.length; i++) { | ||
| Caps storage _caps = caps[ids[i]]; | ||
| require(_caps.allocation > 0, ErrorsLib.ZeroAllocation()); | ||
| // forge-lint: disable-next-item(missing-events-access-control,unsafe-typecast) the Deallocate event below |
Comment on lines
+635
to
+636
| // forge-lint: disable-next-item(reentrancy-events) the event is emitted last so it reports the allocation after | ||
| // the adapter call. |
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
2 participants
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.
Foundry's stable channel went 1.7.1 -> 1.8.0 on 2026-08-27, and the toolchain action pins itself but not the forge binary, so the expanded linter reddened
linton main with no change on our side: 125 findings, 106 insrc/, 18 incertora/, 1 in a transitivelib/file.This stops linting
certora/(the target is now justsrc, there is noscript/) and adopts the rule exclusions settled in morpho-org/midnight#1168, which together cover 64 of the 106 src findings. The remaining 42 are acknowledged in place with the existingforge-lintdirective idiom. No version pin, and--deny notesstays.Comments only in
src/: the diff adds 38 directive lines and removes nothing, so the bytecode is unchanged.Generated by Claude Code