feat: Adds a reinitializer(2) entrypoint to TopUpFactory#136
Open
luiscfaria wants to merge 1 commit into
Open
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
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
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
Adds a
reinitializer(2)entrypoint toTopUpFactoryso a reservedEtherFiPlaceholderproxy at the canonical CREATE3 factory address (0xF4e147…D8CF) can be upgraded into a workingTopUpFactory. The placeholder already consumed OZ initializer v1, soTopUpFactory.initialize(alsoinitializer) reverts withInvalidInitialization()on the upgrade.reinitialize(address)re-wires the beacon while readingroleRegistry()from existing storage.This unblocks Asset Recovery on placeholder-reserved chains (Polygon, Plasma, and any future reserved chain).
Audit precedent
Byte-for-byte the same shape as the already-audited
EtherFiSafeFactory.reinitialize:TopUpFactory.solitself audited as "TopUp Contract Updates (Scroll Migration)" — sc-cash-internal#3.This change is the 11-line combination of those two audited components.
roleRegistry()is read from existingUpgradeableProxystorage (not re-supplied), so the upgrade cannot reassign it. Must be invoked atomically viaupgradeToAndCall.Changes
src/top-up/TopUpFactory.sol—+reinitialize(address)test/integration/fork/PolygonPlaceholderUpgradeFork.t.sol— fork test against real Polygon mainnet state: provesinitializereverts on theplaceholder,
reinitializesucceeds + preservesroleRegistry, andreinitializeis one-shot.scripts/recovery/DeployPolygonTopUpFactory.s.sol— deploys the impls and emits theupgradeToAndCall(reinitialize)3CP calldata.Note
Medium Risk
Touches upgradeable factory initialization on a canonical production proxy address; mitigated by audited EtherFiSafeFactory pattern, preserved roleRegistry, and fork tests, but still a privileged on-chain upgrade path.
Overview
Adds
TopUpFactory.reinitialize(address)(reinitializer(2)) so the reserved placeholder UUPS proxy at0xF4e147…D8CFcan be upgraded to a real factory without callinginitializeagain (placeholder already used OZ initializer v1). Reinit wires the beacon via__BeaconFactory_initializeusingroleRegistry()from existing proxy storage, not a new argument—intended for a single atomicupgradeToAndCall(factoryImpl, reinitialize(topUpImpl))from the RoleRegistry upgrader.DeployPolygonTopUpFactory.s.soldeploysTopUpFactory+TopUpimpls on Polygon and logs the 3CP upgrade calldata.PolygonPlaceholderUpgradeFork.t.solforks Polygon mainnet to proveinitializereverts on the live placeholder,reinitializesucceeds (beacon,getDeterministicAddress, preserved RR), and reinit is one-shot.Reviewed by Cursor Bugbot for commit 17c5111. Bugbot is set up for automated code reviews on this repo. Configure here.