fix: namespace TryMulticall reentrancy storage (L-02) - #85
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
chrismaree
left a comment
There was a problem hiding this comment.
Reviewed against L-02 / FRO-148 at 8fda1fd. All batching-lock reads/writes now use the ERC-7201 accessor; the base no longer contributes the sequential bool. Independently recomputed the namespace with cast keccak: 0x0dd1bd2d495db0d62878fd399d4ab6dc237ff1584371e8862b3e045d79dcd000 matches. The regression derives the namespace and checks actual guard behavior, addressing the previous review feedback. Caller/selector checks and enter/reset behavior remain intact.
Approval is for this change against #77. As documented, integrate before #78's first proxy deployment: removing the packed bool moves its permit2 offset from 1 to 0 and is not a layout-compatible upgrade for an already deployed proxy using that layout. Static review plus hash calculation; no tests or builds run.
Move the
TryMulticallreentrancy flag into the ERC-7201 namespaceuma.storage.TryMulticall, removing sequential storage from the reusable batching base.Base: #77. Tracks FRO-148 (L-02).
Integration: #77 uses immutable
permit2. Applying this change to the upgradeable version in #78 moves its sequentialpermit2from slot 0, offset 1 to offset 0. Incorporate it before that version's first deployment; an existing proxy requires compatible storage handling.Validation: 52 SignedProposer unit tests pass, including nested batching and a regression that derives the ERC-7201 slot and verifies the guard actually reads it. Independently verified the namespace and compiler storage layouts; formatting and diff checks pass.