Mid-loop finalization commits a stale root

Batched Merkle accumulators run two clocks over one storage slot. The insert loop folds new leaves into a local accumulator and writes the result to s.root once, at the end of the call. Everything else reads s.root directly. As long as those two clocks stay in step the field is correct. A subtree that seals in the middle of the loop reads the field while the loop still owes it a write, and the two disagree by exactly one call's worth of leaves.

contract Ledger {
    uint256 constant CAPACITY = 4;

    struct State {
        bytes32 root;
        uint256 leafCount;
        mapping(uint256 => bytes32) finalized;
        uint256 subtrees;
    }

    State internal s;
    error EmptyBatch();

    function insert(bytes32[] calldata leaves) external {
        if (leaves.length == 0) revert EmptyBatch();

        bytes32 running = s.root;
        for (uint256 i = 0; i < leaves.length; i++) {
            running = keccak256(abi.encodePacked(running, leaves[i]));
            s.leafCount++;

            uint256 leafCount_ = s.leafCount;
            if (leafCount_ != 0 && leafCount_ % CAPACITY == 0) {
                _finalize();
            }
        }
        s.root = running;
    }

    function _finalize() internal {
        bytes32 finalRoot = s.root;
        s.finalized[s.subtrees] = finalRoot;
        s.subtrees++;
    }
}

The loop keeps its progress in running. The seal reads s.root. Those are the same value only at the boundaries of a call, never inside it.

A worked instance

Capacity is 4, so a subtree seals every fourth leaf. Send the first four leaves as two calls.

Call one inserts leaves 1, 2, 3. leafCount reaches 3, no boundary trips, and the loop writes s.root = R3, the fold over three leaves.

Call two inserts leaves 4, 5, 6. On the first iteration running becomes R4, leafCount reaches 4, and 4 % 4 == 0 fires _finalize(). But _finalize reads s.root, which is still R3 from the end of call one. The write s.root = running has not happened yet; it runs after the loop. So finalized[0] is set to R3. The subtree that is supposed to commit to four leaves commits to three. Leaf 4 is now inside a subtree whose sealed root never included it, so no inclusion proof for it can verify.

The class

A cached aggregate is read by a side effect inside the loop that maintains it, before the loop writes the aggregate back. The side effect sees the value from the end of the previous call, off by one full batch. The trigger is any per-iteration condition that can fire on iteration i >= 0 of a fresh call: a modular boundary, a threshold, a capacity check. The damage is proportional to how much the loop advanced the local copy before the boundary hit.

Why it survives the test suite

The reassuring invariant holds: after any call, s.root equals the honest fold over every leaf inserted so far. Tests that insert a batch and then read the contract's root getter always see the right number, because that getter returns the field the loop wrote at the end. The corruption lives in finalized[], and the value there is only wrong when a boundary lands mid-call. Suites that check the sealed subtrees at all tend to check their count or that an entry is non-zero, not that each entry equals an independently recomputed subtree root. Compared against the contract's own getter, the stale value agrees with itself. It takes a test that folds the leaves by hand and compares, across a call boundary that splits a subtree, to catch it.

The fix

Seal the value the loop is holding, not the slot it has not written yet.

function insert(bytes32[] calldata leaves) external {
    if (leaves.length == 0) revert EmptyBatch();

    bytes32 running = s.root;
    for (uint256 i = 0; i < leaves.length; i++) {
        running = keccak256(abi.encodePacked(running, leaves[i]));
        s.leafCount++;
        if (s.leafCount % CAPACITY == 0) {
            s.finalized[s.subtrees++] = running; // the value this call produced
        }
    }
    s.root = running;
}

If a loop keeps a local copy of state, every effect inside the loop must read that copy, not the storage slot the loop writes once on exit.