BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: high

Redundant Storage Reads After Write in `flagOutcome()` — Snapshot Values

Author Revealed upon completion

Root + Impact

Description

The flagOutcome() function writes snapshotTotalStaked and snapshotTotalBonus to storage at lines 357-358, then immediately reads them back from storage at lines 361-362 to compute corruptedReserve and bountyEntitlement. Solidity's optimizer does not eliminate these redundant SLOADs because the writes and reads are separated by other storage writes (snapshotSumStakeTime, snapshotSumStakeTimeSq at lines 359-360). The values are already available in the local variables totalEligibleStake and totalBonus (read from storage at lines 357-358). Caching them in memory locals avoids 4 warm SLOADs (~400 gas) on every flagOutcome call.

The same pattern occurs in claimExpired() at lines 524-527 and 545, where snapshot values are written then immediately read back for the corruptedReserve computation.

// ConfidencePool.sol — flagOutcome() lines 357-376
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_)
external onlyModerator
{
// ...
@> snapshotTotalStaked = totalEligibleStake; // SSTORE — value now in storage
@> snapshotTotalBonus = totalBonus; // SSTORE — value now in storage
snapshotSumStakeTime = sumStakeTime;
snapshotSumStakeTimeSq = sumStakeTimeSq;
@> corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED
@> ? snapshotTotalStaked + snapshotTotalBonus : 0; // 2 SLOADs — just wrote these!
@> bountyEntitlement = willBeGoodFaithCorrupted
@> ? snapshotTotalStaked + snapshotTotalBonus : 0; // 2 more SLOADs — same values
// ...
}

Risk

Likelihood: High

  • flagOutcome() is called at least once per pool resolution and potentially multiple times during the moderator's re-flag window

  • Every invocation of flagOutcome() pays the redundant SLOAD cost unconditionally

  • claimExpired() auto-resolution path has the same pattern — every first post-expiry call pays it

Impact: Low

  • Approximately 400 gas wasted per flagOutcome() call, and ~200 gas per claimExpired() auto-resolution

  • No correctness impact — the stored values are read back correctly

  • Purely a gas efficiency concern; does not affect security or functionality

Proof of Concept

File: G2-SloadAfterSstore-FlagOutcome.poc.t.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
// @notice PoC for G-2: Redundant SLOADs after SSTORE of snapshot values in flagOutcome()
contract G2_SloadAfterSstore_FlagOutcome_POC is BaseConfidencePoolTest {
function testPOC_G2_snapshotValuesReadAfterWriteInFlagOutcome() external {
_stake(alice, 100 * ONE);
_contributeBonus(bob, 50 * ONE);
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
// flagOutcome writes snapshotTotalStaked and snapshotTotalBonus to storage,
// then immediately reads them back at lines 361-362 to compute
// corruptedReserve and bountyEntitlement. Those 4 warm SLOADs (~400 gas)
// are avoidable by caching the values in memory.
​
uint256 gasBefore = gasleft();
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
uint256 gasUsed = gasBefore - gasleft();
​
// Verify correctness despite the redundant SLOADs
assertEq(pool.corruptedReserve(), pool.snapshotTotalStaked() + pool.snapshotTotalBonus());
assertEq(pool.bountyEntitlement(), pool.snapshotTotalStaked() + pool.snapshotTotalBonus());
​
assertGt(gasUsed, 0, "gas measurement sanity");
}
}

Run with:

forge test --match-path 'G2-SloadAfterSstore-FlagOutcome.poc.t.sol' -vv

Recommended Mitigation

Cache totalEligibleStake and totalBonus in memory locals (snapStaked, snapBonus) before writing them to the snapshot storage slots. Use the memory copies — not storage reads — when computing corruptedReserve and bountyEntitlement on the following lines. This eliminates 4 warm SLOADs (~400 gas) without changing any storage layout, event emission, or control flow. The memory values are guaranteed identical to the stored values because no other code executes between the SSTORE and the computation. Apply the same pattern to claimExpired() which has an identical write-then-read sequence.

// ConfidencePool.sol — flagOutcome() lines 357-376
+ uint256 snapStaked = totalEligibleStake;
+ uint256 snapBonus = totalBonus;
+ snapshotTotalStaked = snapStaked;
+ snapshotTotalBonus = snapBonus;
- snapshotTotalStaked = totalEligibleStake;
- snapshotTotalBonus = totalBonus;
snapshotSumStakeTime = sumStakeTime;
snapshotSumStakeTimeSq = sumStakeTimeSq;
- corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED
- ? snapshotTotalStaked + snapshotTotalBonus : 0;
- bountyEntitlement = willBeGoodFaithCorrupted
- ? snapshotTotalStaked + snapshotTotalBonus : 0;
+ corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED
+ ? snapStaked + snapBonus : 0;
+ bountyEntitlement = willBeGoodFaithCorrupted
+ ? snapStaked + snapBonus : 0;

Same pattern applied to claimExpired() lines 524-527, 545:

+ uint256 snapStaked = totalEligibleStake;
+ uint256 snapBonus = totalBonus;
+ snapshotTotalStaked = snapStaked;
+ snapshotTotalBonus = snapBonus;
- snapshotTotalStaked = totalEligibleStake;
- snapshotTotalBonus = totalBonus;
snapshotSumStakeTime = sumStakeTime;
snapshotSumStakeTimeSq = sumStakeTimeSq;
// ...
- corruptedReserve = snapshotTotalStaked + snapshotTotalBonus;
+ corruptedReserve = snapStaked + snapBonus;

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!