Skip to content

fix: close the re-audit findings - #16

Merged
Zena-park merged 1 commit into
mainfrom
fix/reaudit-followups
Aug 21, 2026
Merged

fix: close the re-audit findings#16
Zena-park merged 1 commit into
mainfrom
fix/reaudit-followups

Conversation

@Zena-park

Copy link
Copy Markdown
Owner

Why

Re-audit of main after the audit fixes (a0bcad0) found no regression but raised R-1…R-8 (Low 4 · Info 4). This closes R-1 through R-7; R-8 (IEip2612) is left as optional.

What

  • R-1 MinterControl_requireAppointable also refuses a Minter that has its own appointment pending as a Controller (PendingAppointment), so schedule(A,B) + schedule(B,A) can no longer be announced and left for a third party to resolve via permissionless executeController. The same checks run at scheduling and execution; the one shape scheduling cannot see (an appointment pending over a Minter) fails at execution — ADR-003, CHANGELOG and the review doc now say exactly that. Tests: two-call cycle; chain through a pending Minter.
  • R-2 — CHANGELOG and docs/deploying.md: CREATE2 addresses are bound to a repository revision; v0.1.0 predictions no longer match the current tree.
  • R-3foundry.toml pins evm_version = "cancun" next to solc; verified bytecode unchanged (it was already the effective target). ci.yml comment corrected.
  • R-4package.json carries @openzeppelin/contracts and @openzeppelin/contracts-upgradeable at 5.1.0 as an advisory canary (alerts + Dependabot release bumps fire; a bump is the signal to move the submodules). Nothing compiles against them — explicit remappings, and node_modules is outside the Foundry root. Documented in CONTRIBUTING.
  • R-5 — test: receiveWithAuthorization (v,r,s) form refuses a non-payee.
  • R-6npm run lock:check (scripts/check-foundry-lock.mjs) compares foundry.lock with the submodule checkout; part of npm run check, so no CI-only step and the header's "package.json owns the gates" stays true.
  • R-7 — review doc no longer says the ERC-1822 check "prevents bricking"; it guards accidents, the delay and veto guard the rest.

Checks

  • npm run check — fmt · lint · lock:check · 139 tests (8 suites); Slither 0 locally; Halmos in CI

🤖 Generated with Claude Code

MinterControl: the appointment checks also refuse a Minter that has its
own appointment pending as a Controller (PendingAppointment), so a cycle
cannot be announced in two calls and left to whoever executes first.
The same checks run at scheduling and at execution; the one shape
scheduling cannot see -- an appointment pending over a Minter -- fails
at execution. Docs that said 'refused at scheduling' now say exactly
that.

foundry.toml pins evm_version alongside solc: forge otherwise passes
its own default, so creation code -- and every CREATE2 address --
depended on the forge release. Bytecode is unchanged by the pin (cancun
was already the effective target). CHANGELOG and docs/deploying.md now
say that addresses are bound to a repository revision and that v0.1.0
predictions no longer match.

npm run lock:check (scripts/check-foundry-lock.mjs) compares
foundry.lock with the submodule checkout and is part of npm run check,
so the lock is enforced locally and in CI without a CI-only step.

package.json carries the two OpenZeppelin packages at the submodules'
release as an advisory canary: GitHub alerts and Dependabot release
bumps fire for them, and a bump is the signal to move the submodules.
Nothing compiles against the npm copies.

Tests: the EIP-form receiveWithAuthorization payee refusal, the
two-call cycle, the chain through a pending Minter. security-review.md
no longer overstates the ERC-1822 guard.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 21, 2026 15:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Zena-park
Zena-park merged commit 867f9f4 into main Aug 21, 2026
3 checks passed
@Zena-park
Zena-park deleted the fix/reaudit-followups branch August 21, 2026 15:30
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.

2 participants