Submission Details
Impact:
Likelihood:
Inefficient Storage Variable Packing Increases Deployment and Read Gas Costs
Author Revealed upon completion
Root + Impact
Description
Savings: ~4 slots × 20,000 gas (cold SSTORE from zero) ≈ 80,000 gas saved once at initialize(), plus one fewer cold SLOAD (2,100 gas) whenever the combined fields are read together instead of separately.
// Root cause in the codebase with @> marks to highlight the relevant section
Risk
Likelihood:
Impact:
Optimized layout: 25 storage slots (4 slots saved — this is real, since none of these are mappings/arrays that get their own slot regardless).
Proof of Concept
Before you apply this: if this contract is already deployed (or is an implementation behind a proxy that's already live, or any pool clones already exist), reordering storage variables changes the storage layout and will silently corrupt state on upgrade. Only reorder pre-deployment, or add this as a fresh implementation with a migration path. I'd suggest a Foundry check (forge inspect ConfidencePool storage-layout) before/after to confirm no clash if there's any doubt.
/// @title StorageLayoutGasTest
/// @notice PoC proving the storage-packing gas finding with real numbers instead of a
/// theoretical slot count. Two things are measured:
/// 1. `initializeAll` — the cold-SSTORE cost, standing in for `IConfidencePool.initialize()`
/// being called once per pool via the clone in `createPool`.
/// 2. `readRiskWindowBundle` — a warm-SLOAD read of three fields that, in the optimized
/// layout, live in a single slot instead of two.
/// Run with: forge test --match-contract StorageLayoutGasTest -vvv
contract StorageLayoutGasTest is Test {
UnoptimizedLayout internal unopt;
OptimizedLayout internal opt;
address internal constant AGREEMENT = address(0x1111);
address internal constant MODERATOR = address(0x2222);
address internal constant RECOVERY = address(0x3333);
address internal constant STAKE_TOKEN = address(0x4444);
address internal constant REGISTRY = address(0x5555);
address internal constant ATTACKER = address(0x6666);
uint32 internal constant EXPIRY = uint32(block.timestamp + 30 days);
function setUp() public {
unopt = new UnoptimizedLayout();
opt = new OptimizedLayout();
}
/// @notice Core PoC: deploy-then-initialize gas cost, unoptimized vs optimized.
/// Expect the optimized version to spend meaningfully fewer cold SSTOREs because
/// several fields now share slots instead of each claiming a fresh one.
function test_InitializeGas_OptimizedIsCheaper() public {
uint256 gasBeforeUnopt = gasleft();
unopt.initializeAll(AGREEMENT, MODERATOR, RECOVERY, STAKE_TOKEN, REGISTRY, EXPIRY, ATTACKER);
uint256 gasUsedUnopt = gasBeforeUnopt - gasleft();
uint256 gasBeforeOpt = gasleft();
opt.initializeAll(AGREEMENT, MODERATOR, RECOVERY, STAKE_TOKEN, REGISTRY, EXPIRY, ATTACKER);
uint256 gasUsedOpt = gasBeforeOpt - gasleft();
emit log_named_uint("Unoptimized initializeAll gas", gasUsedUnopt);
emit log_named_uint("Optimized initializeAll gas", gasUsedOpt);
emit log_named_uint("Gas saved per pool creation", gasUsedUnopt - gasUsedOpt);
assertLt(
gasUsedOpt,
gasUsedUnopt,
"optimized layout should cost less gas to initialize than unoptimized layout"
);
}
/// @notice Confirms the read-side saving: bundling registry + both risk-window
/// timestamps into one slot means one SLOAD instead of two on every gated entrypoint
/// that checks risk-window state (e.g. `pokeRiskWindow`, stake eligibility checks).
function test_ReadBundleGas_OptimizedIsCheaper() public {
unopt.initializeAll(AGREEMENT, MODERATOR, RECOVERY, STAKE_TOKEN, REGISTRY, EXPIRY, ATTACKER);
opt.initializeAll(AGREEMENT, MODERATOR, RECOVERY, STAKE_TOKEN, REGISTRY, EXPIRY, ATTACKER);
uint256 gasBeforeUnopt = gasleft();
unopt.readRiskWindowBundle();
uint256 gasUsedUnopt = gasBeforeUnopt - gasleft();
uint256 gasBeforeOpt = gasleft();
opt.readRiskWindowBundle();
uint256 gasUsedOpt = gasBeforeOpt - gasleft();
emit log_named_uint("Unoptimized readRiskWindowBundle gas", gasUsedUnopt);
emit log_named_uint("Optimized readRiskWindowBundle gas", gasUsedOpt);
assertLt(
gasUsedOpt,
gasUsedUnopt,
"optimized layout should cost less gas to read the risk-window bundle"
);
}
/// @notice Proves the packing claim directly at the storage-slot level rather than
/// inferring it from gas alone: reads the raw slot for `riskWindowStart`/`riskWindowEnd`
/// in each contract and checks whether `safeHarborRegistry` (a 20-byte address) and the
/// two uint32 timestamps actually share one physical slot in the optimized layout.
function test_OptimizedFieldsShareASlot() public {
opt.initializeAll(AGREEMENT, MODERATOR, RECOVERY, STAKE_TOKEN, REGISTRY, EXPIRY, ATTACKER);
// safeHarborRegistry is declared at the start of the packed group in OptimizedLayout;
// slot index depends on declaration order above it (agreement+expiry+scopeLocked = slot 0,
// stakeToken+outcomeFlaggedAt = slot 1, so this group starts at slot 2).
bytes32 packedSlot = vm.load(address(opt), bytes32(uint256(2)));
// Lower 160 bits should decode back to the registry address.
address decodedRegistry = address(uint160(uint256(packedSlot)));
assertEq(decodedRegistry, REGISTRY, "registry address should occupy the low bytes of the packed slot");
// Bits 160-191 and 192-223 should hold the two uint32 timestamps (both zero here,
// but this proves they're addressable within the same slot rather than needing
// separate SLOADs, which is the actual gas-saving mechanism).
uint32 decodedStart = uint32(uint256(packedSlot) >> 160);
uint32 decodedEnd = uint32(uint256(packedSlot) >> 192);
assertEq(decodedStart, 0, "riskWindowStart should be packed into the same slot");
assertEq(decodedEnd, 0, "riskWindowEnd should be packed into the same slot");
}
}
Recommended Mitigation
Reorder declarations to cluster fields by type size so the compiler packs them into shared slots, without changing any field's visibility, type, or semantics. Confirm no clash pre/post via forge inspect ConfidencePool storage-layout, and apply only before first deployment (or via a proper upgrade-safe migration if already live).
- remove this code
uint256 private constant _MIN_EXPIRY_LEAD = 30 days;
uint256 public constant override CORRUPTED_CLAIM_WINDOW = 180 days;
/// @notice Grace period after `expiry` during which only the moderator can resolve a CORRUPTED
/// registry. After it elapses, any caller can finalize as bad-faith CORRUPTED via `claimExpired`
/// (the sweep to `recoveryAddress` then goes through `claimCorrupted`). Backstop against a
/// permanently-unavailable moderator; under the DAO-moderator trust model it should never fire.
/// @dev This backstop is scope-blind and staker-pessimistic, and its `riskWindowStart != 0`
/// gate is permissionlessly sealable (via `pokeRiskWindow`) by design — both intentional, with
/// accepted consequences documented in docs/DESIGN.md (auto-CORRUPTED backstop).
uint256 public constant override MODERATOR_CORRUPTED_GRACE = 180 days;
// Config (set at init; `expiry`/`recoveryAddress`/scope mutable per their own gates).
//
// Timestamps are `uint32` seconds, packed with addresses. uint32 saturates on 2106-02-07;
// expiry-derived timestamps are bounded by `expiry` (guarded at its setters), CORRUPTED-path
// ones by `block.timestamp + 180 days`. Every multiply into the k=2 score widens to uint256
// first (see `_bonusShare`, `_markRiskWindow*`, `_clampUserSums`).
address public agreement;
IERC20 public stakeToken;
IBattleChainSafeHarborRegistry public safeHarborRegistry;
address public outcomeModerator;
address public override recoveryAddress;
uint256 public minStake;
/// @notice Pool deadline. Past it, staking closes and `claimExpired` can mechanically resolve.
uint32 public expiry;
/// @notice Enumerable list of BattleChain accounts in pool scope.
address[] internal _scopeAccounts;
/// @notice O(1) membership lookup. True if `account` is in the pool's BattleChain scope.
mapping(address account => bool inScope) public override isAccountInScope;
/// @notice One-way flag flipped on the first interaction that observes the registry in any
/// state other than NOT_DEPLOYED or NEW_DEPLOYMENT (i.e. the agreement has moved past
/// pre-attack staging). Once true, the scope can no longer be changed.
bool public override scopeLocked;
// Live stake accounting (maintained incrementally on stake/withdraw).
uint256 public totalEligibleStake;
uint256 public totalBonus;
mapping(address staker => uint256 amount) public eligibleStake;
/// @notice Per-user Σ deposit_i.amount × deposit_i.entryTime. Pair with `userSumStakeTimeSq`
/// to compute the user's k=2 score in O(1) at claim time.
mapping(address staker => uint256 sum) public userSumStakeTime;
/// @notice Per-user Σ deposit_i.amount × deposit_i.entryTime². See `userSumStakeTime`.
mapping(address staker => uint256 sum) public userSumStakeTimeSq;
/// @notice Σ eligibleStake_u × entryTime_u across all users, maintained incrementally on
/// stake/withdraw. Together with `sumStakeTimeSq` and `totalEligibleStake`, computes the
/// k=2 bonus-distribution denominator in O(1) at claim time:
/// globalScore = T² × totalEligibleStake − 2T × sumStakeTime + sumStakeTimeSq
/// where T is `outcomeFlaggedAt`. Eagerly reset to `totalEligibleStake × riskWindowStart`
/// at the moment the risk window opens, so pre-risk parked capital earns no bonus credit.
uint256 public sumStakeTime;
/// @notice Σ eligibleStake_u × entryTime_u² across all users. See `sumStakeTime`.
uint256 public sumStakeTimeSq;
// Risk-window markers (one-way registry observations; see `_observePoolState`).
/// @notice Timestamp at which the registry was first observed in an active-risk state
/// (UNDER_ATTACK or PROMOTION_REQUESTED). Zero before the window opens. Bonus accrual treats
/// every staker as if they entered at `riskWindowStart` at the earliest; pre-risk deposits
/// earn no bonus credit. Observed lazily by gated entrypoints or eagerly via
/// `pokeRiskWindow`. Capped at `expiry` so a late observation doesn't push the accrual lower
/// bound above the bonus formula's upper bound `T`. See the contract-level natspec for the
/// "no observable risk" rules that apply when this stays zero.
uint32 public override riskWindowStart;
/// @notice Timestamp at which the registry was first observed in a terminal state (PRODUCTION
/// or CORRUPTED). Zero before observation. Acts as the upper bound `T` in the k=2 bonus
/// score so callers can't shift bonus distribution by varying when they trigger the
/// resolving call. Observed lazily or eagerly via `pokeRiskWindow`. Capped at `expiry`
/// for the same reason as `riskWindowStart`: accrual is bounded by the pool's lifecycle.
uint32 public override riskWindowEnd;
// Resolution outcome (frozen at `flagOutcome` / `claimExpired` resolution).
address public attacker;
PoolStates.Outcome public outcome;
bool public goodFaith;
bool public expiryLocked;
/// @notice Flipped true on the first successful post-resolution claim/sweep. Locks the
/// outcome against moderator re-flagging once any participant has acted on it.
bool public claimsStarted;
/// @notice The upper bound `T` used in the k=2 bonus formula. Set at outcome resolution to
/// `riskWindowEnd` for SURVIVED / CORRUPTED resolutions or `expiry` for EXPIRED. Pinned to
/// the first-observed terminal moment so the resolving call's `block.timestamp` can't shift
/// bonus shares.
uint32 public outcomeFlaggedAt;
// Resolution snapshot (live accounting captured at resolution; bonus-distribution inputs).
uint256 public snapshotTotalStaked;
uint256 public snapshotTotalBonus;
/// @notice `sumStakeTime` captured at `flagOutcome` time. Used in the bonus denominator.
uint256 public snapshotSumStakeTime;
/// @notice `sumStakeTimeSq` captured at `flagOutcome` time. Used in the bonus denominator.
uint256 public snapshotSumStakeTimeSq;
/// @notice Σ bonus shares paid to claimants so far. Subtracted from `snapshotTotalBonus`
/// to compute the bonus still owed to non-claimers, used by `sweepUnclaimedBonus` to
/// reserve their entitlement when sweeping excess (donations, dust).
uint256 public claimedBonus;
mapping(address staker => bool claimed) public hasClaimed;
// CORRUPTED-path accounting (bounty + sweep bookkeeping).
/// @notice CORRUPTED-path accounting marker tracking the snapshot amount destined for
/// `recoveryAddress` (less anything the attacker has claimed in good-faith). Decremented
/// opportunistically as `claimAttackerBounty` / `claimCorrupted` / `sweepUnclaimedCorrupted`
/// move funds. Not load-bearing for sweep gating — those use `stakeToken.balanceOf` directly
/// so post-resolution donations can be recovered.
uint256 public corruptedReserve;
uint256 public bountyClaimed;
uint256 public bountyEntitlement;
/// @notice `block.timestamp` of the first-ever good-faith CORRUPTED flag. Anchors
/// `corruptedClaimDeadline` so the moderator cannot mint a fresh attacker-claim window by
/// toggling the outcome out of and back into good-faith CORRUPTED. Set once, never reset.
uint32 internal _firstGoodFaithCorruptedAt;
uint32 public override corruptedClaimDeadline;
+ add this code
uint256 private constant _MIN_EXPIRY_LEAD = 30 days;
uint256 public constant override CORRUPTED_CLAIM_WINDOW = 180 days;
uint256 public constant override MODERATOR_CORRUPTED_GRACE = 180 days;
// --- Slot: pool identity + scope gate ---
// agreement (20) + expiry (4) + scopeLocked (1) = 25/32 bytes
address public agreement;
/// @notice Pool deadline. Past it, staking closes and `claimExpired` can mechanically resolve.
uint32 public expiry;
/// @notice One-way flag flipped on the first interaction that observes the registry in any
/// state other than NOT_DEPLOYED or NEW_DEPLOYMENT (i.e. the agreement has moved past
/// pre-attack staging). Once true, the scope can no longer be changed.
bool public override scopeLocked;
// --- Slot: stake token + resolution timestamp bound ---
// stakeToken (20) + outcomeFlaggedAt (4) = 24/32 bytes
IERC20 public stakeToken;
/// @notice The upper bound `T` used in the k=2 bonus formula. Set at outcome resolution to
/// `riskWindowEnd` for SURVIVED / CORRUPTED resolutions or `expiry` for EXPIRED. Pinned to
/// the first-observed terminal moment so the resolving call's `block.timestamp` can't shift
/// bonus shares.
uint32 public outcomeFlaggedAt;
// --- Slot: registry + risk-window markers (read together on every gated entrypoint) ---
// safeHarborRegistry (20) + riskWindowStart (4) + riskWindowEnd (4) = 28/32 bytes
IBattleChainSafeHarborRegistry public safeHarborRegistry;
/// @notice Timestamp at which the registry was first observed in an active-risk state
/// (UNDER_ATTACK or PROMOTION_REQUESTED). Zero before the window opens. Bonus accrual treats
/// every staker as if they entered at `riskWindowStart` at the earliest; pre-risk deposits
/// earn no bonus credit. Observed lazily by gated entrypoints or eagerly via
/// `pokeRiskWindow`. Capped at `expiry` so a late observation doesn't push the accrual lower
/// bound above the bonus formula's upper bound `T`. See the contract-level natspec for the
/// "no observable risk" rules that apply when this stays zero.
uint32 public override riskWindowStart;
/// @notice Timestamp at which the registry was first observed in a terminal state (PRODUCTION
/// or CORRUPTED). Zero before observation. Acts as the upper bound `T` in the k=2 bonus
/// score so callers can't shift bonus distribution by varying when they trigger the
/// resolving call. Observed lazily or eagerly via `pokeRiskWindow`. Capped at `expiry`
/// for the same reason as `riskWindowStart`: accrual is bounded by the pool's lifecycle.
uint32 public override riskWindowEnd;
// --- Slot: moderator + resolution outcome flags (read together in claim paths) ---
// outcomeModerator (20) + outcome (1) + goodFaith (1) + expiryLocked (1) + claimsStarted (1) = 24/32 bytes
address public outcomeModerator;
PoolStates.Outcome public outcome;
bool public goodFaith;
bool public expiryLocked;
/// @notice Flipped true on the first successful post-resolution claim/sweep. Locks the
/// outcome against moderator re-flagging once any participant has acted on it.
bool public claimsStarted;
// --- Slot: recovery address + CORRUPTED-path deadlines ---
// recoveryAddress (20) + _firstGoodFaithCorruptedAt (4) + corruptedClaimDeadline (4) = 28/32 bytes
address public override recoveryAddress;
/// @notice `block.timestamp` of the first-ever good-faith CORRUPTED flag. Anchors
/// `corruptedClaimDeadline` so the moderator cannot mint a fresh attacker-claim window by
/// toggling the outcome out of and back into good-faith CORRUPTED. Set once, never reset.
uint32 internal _firstGoodFaithCorruptedAt;
uint32 public override corruptedClaimDeadline;
// --- Slot: attacker (no remaining small fields to co-pack) ---
address public attacker;
// --- uint256 slots: one each, cannot be packed regardless of order ---
uint256 public minStake;
uint256 public totalEligibleStake;
uint256 public totalBonus;
/// @notice Σ eligibleStake_u × entryTime_u across all users, maintained incrementally on
/// stake/withdraw. Together with `sumStakeTimeSq` and `totalEligibleStake`, computes the
/// k=2 bonus-distribution denominator in O(1) at claim time:
/// globalScore = T² × totalEligibleStake − 2T × sumStakeTime + sumStakeTimeSq
/// where T is `outcomeFlaggedAt`. Eagerly reset to `totalEligibleStake × riskWindowStart`
/// at the moment the risk window opens, so pre-risk parked capital earns no bonus credit.
uint256 public sumStakeTime;
/// @notice Σ eligibleStake_u × entryTime_u². See `sumStakeTime`.
uint256 public sumStakeTimeSq;
uint256 public snapshotTotalStaked;
uint256 public snapshotTotalBonus;
/// @notice `sumStakeTime` captured at `flagOutcome` time. Used in the bonus denominator.
uint256 public snapshotSumStakeTime;
/// @notice `sumStakeTimeSq` captured at `flagOutcome` time. Used in the bonus denominator.
uint256 public snapshotSumStakeTimeSq;
/// @notice Σ bonus shares paid to claimants so far. Subtracted from `snapshotTotalBonus`
/// to compute the bonus still owed to non-claimers, used by `sweepUnclaimedBonus` to
/// reserve their entitlement when sweeping excess (donations, dust).
uint256 public claimedBonus;
/// @notice CORRUPTED-path accounting marker tracking the snapshot amount destined for
/// `recoveryAddress` (less anything the attacker has claimed in good-faith). Decremented
/// opportunistically as `claimAttackerBounty` / `claimCorrupted` / `sweepUnclaimedCorrupted`
/// move funds. Not load-bearing for sweep gating — those use `stakeToken.balanceOf` directly
/// so post-resolution donations can be recovered.
uint256 public corruptedReserve;
uint256 public bountyClaimed;
uint256 public bountyEntitlement;
// --- Mappings / dynamic array: always own slot, order doesn't affect gas ---
/// @notice Enumerable list of BattleChain accounts in pool scope.
address[] internal _scopeAccounts;
/// @notice O(1) membership lookup. True if `account` is in the pool's BattleChain scope.
mapping(address account => bool inScope) public override isAccountInScope;
mapping(address staker => uint256 amount) public eligibleStake;
/// @notice Per-user Σ deposit_i.amount × deposit_i.entryTime. Pair with `userSumStakeTimeSq`
/// to compute the user's k=2 score in O(1) at claim time.
mapping(address staker => uint256 sum) public userSumStakeTime;
/// @notice Per-user Σ deposit_i.amount × deposit_i.entryTime². See `userSumStakeTime`.
mapping(address staker => uint256 sum) public userSumStakeTimeSq;
mapping(address staker => bool claimed) public hasClaimed;
Prize pool breakdown
Total prize
7.25 ETH
nSLOC
5890
ETH / LOC
6
1.25
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.
Rewards Distribution
The contest is complete and the rewards are being distributed.