Skip to content

docs(L1): correct ProtocolVersions owner references and document uncapped iteration - #388

Merged
PelleKrab merged 2 commits into
mainfrom
docs/protocol-versions-natspec
Jul 29, 2026
Merged

docs(L1): correct ProtocolVersions owner references and document uncapped iteration#388
PelleKrab merged 2 commits into
mainfrom
docs/protocol-versions-natspec

Conversation

@PelleKrab

Copy link
Copy Markdown
Contributor

Summary

Comment-only changes to ProtocolVersions. No behavior change — initCodeHash is
unchanged; only sourceCodeHash moves.

Owner references. The NatSpec described the contract as Security Council-controlled,
but access control is enforced via _assertOnlyProxyAdminOwner, so the controlling party
is the ProxyAdmin owner — in production the 2-of-2 multisig. The contract-level @notice
now names that role, and the three owner-gated functions (registerUpgrade,
setMinimumProtocolVersion, setIncidentResponder) read ProxyAdmin owner only instead
of a bare Owner only.

Uncapped iteration. All five loops in this contract iterate _timestamps, so each one
reads as unbounded. Rather than repeat the rationale per loop, it is documented once at the
array declaration: the array only grows via owner-gated registerUpgrade, one entry per
hardfork, so the unbounded iteration poses no gas exhaustion risk.

Test plan

  • forge test — full suite passes
  • forge fmt --check clean on the changed file
  • just semver-lock — refreshed ProtocolVersions sourceCodeHash; initCodeHash
    identical, confirming the change is comment-only

The NatSpec described the contract as Security Council-controlled, but
access control is enforced via _assertOnlyProxyAdminOwner, so the
controlling party is the ProxyAdmin owner. Name that role consistently
on the contract and on each owner-gated function.
Every loop in ProtocolVersions iterates _timestamps, so each one reads
as unbounded iteration. Document the bound once at the array
declaration rather than per loop: the array only grows via owner-gated
registerUpgrade, one entry per hardfork.
@cb-heimdall

cb-heimdall commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

@PelleKrab
PelleKrab requested a review from jackchuma July 28, 2026 21:41
@PelleKrab
PelleKrab merged commit 99df7e6 into main Jul 29, 2026
6 checks passed
@PelleKrab
PelleKrab deleted the docs/protocol-versions-natspec branch July 29, 2026 13:41
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.

3 participants