feat(L1): bind aggregate proofs to ProtocolVersions schedule#359
Open
PelleKrab wants to merge 20 commits into
Open
feat(L1): bind aggregate proofs to ProtocolVersions schedule#359PelleKrab wants to merge 20 commits into
PelleKrab wants to merge 20 commits into
Conversation
Add the Security Council-controlled upgrade activation schedule contract, its interface, and tests. Maintains an ordered registry of upgrades and their L2 activation timestamps, committed via scheduleId. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
…o conventions Replace the history-dependent rolling hash with a per-upgrade cumulative hash chain reproducible from (l2ChainId, address, ordered keys, current timestamps). When a timestamp changes, only the affected suffix is recomputed (O(n-j) bubble-up) rather than hashing the full state on every mutation. - Add MIN_NOTICE (1 hour) guard on setTimestamp; expose it in the interface - Require non-zero protocolVersion; use _protocolVersions as existence sentinel, removing the separate _upgradeIndex mapping - Move all errors and events into IProtocolVersions; add MIN_NOTICE to interface - Replace assembly in _keyFromUpgradeId with bytes32(raw) Solidity cast - Rename constructor params to leading-underscore convention - Restructure test file into grouped contracts with snake_case naming, external visibility, and @notice on each test function per repo style Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Returns the full ordered upgrade schedule in a single eth_call — name, activation timestamp, protocol version, and cumulative scheduleId per entry. Name is recovered from the bytes32 key at read time via _nameFromKey, avoiding a separate storage mapping. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Replace the per-call keccak256(abi.encode(l2ChainId, address(this))) in _refreshScheduleId with a cached immutable _seed set once in the constructor, saving a hash + encode on every schedule mutation. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Reflects bytecode change from caching hash chain seed as immutable. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
…s control - Replace Ownable with ProxyAdminOwnedBase; drop explicit _owner constructor param and ProtocolVersions_ZeroOwner error - Replace per-upgrade protocolVersion mapping with a single latestProtocolVersion uint256 updated on each registerUpgrade call - Remove redundant storage vars: lastUpdatedAtBlock, l2ChainId immutable, _scheduleId (scheduleId() now reads the tip of _upgradeScheduleId directly) - Remove redundant view functions: upgradeCount, upgradeIdAt, upgradeIds, getProtocolVersion (getSchedule() covers all registry reads) - Inline _registerUpgrade into registerUpgrade and _applyTimestamp into setTimestamp; inline onlyChainTeam guard into delayTimestamp - Drop ScheduleIdUpdated old-scheduleId param; enforce MIN_NOTICE in delayTimestamp Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
…y pattern Replace the string-based upgrade registry (bytes32 key from packed upgradeId) with a purely numeric scheme: each upgrade gets an ascending uint256 id equal to its registration index. Human-readable names are kept off-chain; the id is permanent and stable because the registry is strictly append-only. Convert the constructor to an initializer so the contract can be deployed behind an OP proxy. _seed is now stored (not immutable) so it can be set inside initialize() against the proxy's address(this), binding the schedule commitment to the proxy address that callers interact with. Replace the mapping-based storage (_registered, _timestamps, _upgradeScheduleId keyed by bytes32) with parallel arrays indexed by id, simplifying the hash chain and eliminating the three-mapping structure. Drop getTimestamp(string) and the duplicate-registration guard; add setLatestProtocolVersion() so the informational field can be updated independently of registrations. Add LatestProtocolVersionUpdated and ScheduleIdUpdated events; update the interface to inherit IProxyAdminOwnedBase and IReinitializableBase. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Seven findings from a multi-angle review; fixes applied: * emit LatestProtocolVersionUpdated in registerUpgrade — previously only setLatestProtocolVersion fired the event, so indexers missed every version bump that came through registration. * Guard scheduleId() before initialization — returns bytes32(0) while _seed is unset, which could be picked up as a valid schedule commitment; now reverts with ProtocolVersions_NotInitialized. * Dedicate ProtocolVersions_InsufficientNotice for setTimestamp — the MIN_NOTICE check in setTimestamp reused DelayMustBeLater with current=0 as the floor, which was misleading; a new error carries only the offending timestamp. * Split delayTimestamp compound condition — the combined || condition reported current as the floor even when only the MIN_NOTICE leg fired; each constraint now reverts independently with the correct floor value. * Extract _setTimestamp internal — setTimestamp and delayTimestamp duplicated the activation-already-passed check and the write+emit+refresh triple; _setTimestamp holds those shared invariants and each public function keeps only its distinct preconditions. * Drop redundant zero-write in registerUpgrade — push(bytes32(0)) wrote zero to a slot that _refreshScheduleId immediately overwrites; replaced with push() (no argument) to save one SSTORE. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
…elper, init guard Rename latestProtocolVersion → minimumProtocolVersion throughout (state variable, event, setter, interface, tests). The field represents the minimum version nodes must run, not an informational latest; the old name implied recommendation rather than a floor. Remove the protocolVersion parameter from registerUpgrade. Upgrade registration is purely a scheduling concern (assign id, extend hash chain); the minimum version is set independently via setMinimumProtocolVersion. Drops the InvalidProtocolVersion check from registerUpgrade and removes the protocolVersion field from the UpgradeRegistered event. Extract _writeTimestamp(id, newTs) as the shared write helper for setTimestamp and delayTimestamp. Each public function owns its complete validation sequence; _writeTimestamp is a pure write (store + emit TimestampSet + refreshScheduleId). Eliminates the mixed-responsibility _setTimestamp that owned some validation but not all, which caused the ActivationAlreadyPassed check to appear in two places. Guard registerUpgrade against pre-initialization calls. _seed is zero between upgradeTo and initialize; any registerUpgrade call in that window would compute hash chain links from a zero seed, permanently desynchronizing scheduleId from its reproducible derivation (l2ChainId, address(this)). Revert with NotInitialized when _seed == 0, consistent with the existing guard in scheduleId(). Regenerate ABI, storage layout, and semver-lock snapshots. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Deploy the ProtocolVersions impl + per-chain proxy as part of SystemDeploy and initialize it via initialize(l2ChainId), rather than excluding it from initializer tracking. Adds it to Types.Implementations/DeployOutput, artifact saves, getImplementations, and _assertValidImplementations. Tags the contract @Custom:proxied true so the initializer test verifies both impl and proxy, and wires it into Setup (via getAddress for fork safety). Updates the ProtocolVersions sourceCodeHash in semver-lock accordingly. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
PelleKrab
force-pushed
the
proof-schedule-binding
branch
from
641b312 to
0964d31
Compare
PelleKrab
force-pushed
the
dynamic-hardfork-protocol-id
branch
from
0570814 to
7cb1c5d
Compare
PelleKrab
force-pushed
the
proof-schedule-binding
branch
from
0964d31 to
9c49406
Compare
…no-op setTimestamp Apply @jackchuma review nits on ProtocolVersions: - Rename the chainTeam role to incidentResponder; delaying a bad hardfork activation is an incident-response action. NatSpec scopes the power so the shared name doesn't imply SuperchainConfig's pause authority. - Drop the redundant chainTeam deploy config key and source the role from superchainConfigIncidentResponder (same entity). - setTimestamp: short-circuit no-op writes ahead of the change-validation guards so idempotent resubmissions succeed. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
PelleKrab
force-pushed
the
dynamic-hardfork-protocol-id
branch
from
7cb1c5d to
e1ebc49
Compare
PelleKrab
force-pushed
the
proof-schedule-binding
branch
from
9c49406 to
a09ecb3
Compare
Run ProtocolVersions tests through CommonTest so they use the SystemDeploy-created proxy and implementation. Reorder ProtocolVersions members to match the Solidity style guide and restrict implementation internals to private. Co-authored-by: Codex <codex-noreply@coinbase.com>
PelleKrab
force-pushed
the
proof-schedule-binding
branch
from
a09ecb3 to
a3fbbb6
Compare
Events before errors, and mutating externals before view getters, mirroring the ProtocolVersions contract body and the repo style-guide ordering. ABI is generator-sorted, so no snapshot or semver-lock impact. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
PelleKrab
force-pushed
the
proof-schedule-binding
branch
from
a3fbbb6 to
90bf7d3
Compare
Collaborator
🟡 Heimdall Review Status
|
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.
Summary
Builds on #353.
Pins each
AggregateVerifiergame to theProtocolVersions.scheduleId()that is active when the gameis initialized, and includes that pinned value in the TEE and ZK proof journals.
PROTOCOL_VERSIONS.scheduleId()once during initialization andstore it as
scheduleIdbound to the upgrade schedule that existed when the game was created
ProtocolVersionsproxy into
AggregateVerifierIAggregateVerifierexposesPROTOCOL_VERSIONSandscheduleId; tests coverthat later schedule changes do not alter an initialized game's pinned value
Test plan
forge test --match-path test/L1/proofs/AggregateVerifier.t.solforge test --match-path test/L1/proofs/DisputeGameFactory.t.solforge test --match-path test/L1/OptimismPortal2.t.solforge test --match-path 'test/L1/proofs/*.t.sol'forge fmt --check