fix(RollupCreator): refund only the caller's own ETH, not the full contract balance - #445
Open
JFKongphop wants to merge 1 commit into
Open
fix(RollupCreator): refund only the caller's own ETH, not the full contract balance#445JFKongphop wants to merge 1 commit into
JFKongphop wants to merge 1 commit into
Conversation
…ntract balance _deployFactories refunded the caller using the contract's entire balance (address(this).balance) rather than the ETH the caller sent for their own deployment. Snapshot the pre-existing balance (everything above msg.value) and exclude it from the refund so only the caller's own contribution is returned. Non-breaking: in the normal path preExistingBalance == 0. Adds test_createRollup_refundExcludesPreExistingBalance regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M5SZVUnwW9rAm6KzVvAM7T
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
RollupCreator._deployFactoriesrefunds the caller using the contract's entirebalance (
address(this).balance) rather than the ETH that caller actually sent fortheir own deployment. This changes it to refund only the caller's own contribution,
leaving any pre-existing balance untouched.
Current behavior
RollupCreatorintentionally accepts ETH (receive() external payable {}) so the L2factory deployer can return excess fees for the creator to refund. That part is by
design. The issue is only the refund amount: it sweeps the whole contract balance.
If any ETH is ever left in the contract — a stray transfer, dust from a prior
interaction, or a partial/failed deployment — the next caller of
createRollupreceives it along with their own refund. The ETH goes to an unrelated address rather
than staying put or being recoverable by its sender.
Impact
Low / hardening. In normal operation
RollupCreatorholds no ETH between transactions(it's a stateless deployment factory), so there is no standing balance to misattribute
and no live exploit. This is a defense-in-depth fix: the refund should return what the
caller paid, not whatever happens to be sitting in the contract. It also removes a
foot-gun for anyone integrating with or forking the factory.
Fix
Snapshot the balance that predates this call (everything above
msg.value) and excludeit from the refund.
msg.valueis still available inside the internal_deployFactories,and no ETH leaves the contract before the refund line, so
preExistingBalancecorrectlycaptures only the stray balance.
Compatibility
Non-breaking. In the normal path
preExistingBalance == 0, so the caller receives theexact same refund as before (their over-payment plus any excess returned by the factory
deployer). Behavior differs only when stray ETH was present — which is precisely the case
being fixed. No interface or storage-layout changes.
Testing
Adds one regression test (
test_createRollup_refundExcludesPreExistingBalance) thatforce-sends ETH to the creator before
createRollupand asserts the caller's refund doesnot include it. All existing
RollupCreatortests pass unchanged.