Skip to content

fix(RollupCreator): refund only the caller's own ETH, not the full contract balance - #445

Open
JFKongphop wants to merge 1 commit into
OffchainLabs:mainfrom
JFKongphop:fix/rollupcreator-exact-refund
Open

fix(RollupCreator): refund only the caller's own ETH, not the full contract balance#445
JFKongphop wants to merge 1 commit into
OffchainLabs:mainfrom
JFKongphop:fix/rollupcreator-exact-refund

Conversation

@JFKongphop

Copy link
Copy Markdown

Summary

RollupCreator._deployFactories refunds the caller using the contract's entire
balance (address(this).balance) rather than the ETH that caller actually sent for
their own deployment. This changes it to refund only the caller's own contribution,
leaving any pre-existing balance untouched.

Current behavior

// refund the caller
(bool sent,) = msg.sender.call{value: address(this).balance}("");
require(sent, "Refund failed");

RollupCreator intentionally accepts ETH (receive() external payable {}) so the L2
factory 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 createRollup
receives 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 RollupCreator holds 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 exclude
it from the refund. msg.value is still available inside the internal _deployFactories,
and no ETH leaves the contract before the refund line, so preExistingBalance correctly
captures only the stray balance.

Compatibility

Non-breaking. In the normal path preExistingBalance == 0, so the caller receives the
exact 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) that
force-sends ETH to the creator before createRollup and asserts the caller's refund does
not include it. All existing RollupCreator tests pass unchanged.

…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
@CLAassistant

CLAassistant commented Aug 4, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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