[A23-2] [MEDIUM] LibFlow.flow: kvs.length==0 short-circuit and store.set revert paths uncovered

[thedavidmeister]

Gap

LibFlow.flow has a short-circuit on kvs.length:

if (kvs.length > 0) {
    interpreterStore.set(DEFAULT_STATE_NAMESPACE, kvs);
}

(src/lib/LibFlow.sol:144-146)

The kvs.length > 0 branch is covered by Flow.time.t.sol:12 (testFlowBasicFlowTime) which uses vm.assume(writeToStore.length != 0) and asserts expectCall(STORE, set.selector, ...).

The kvs.length == 0 branch has no explicit negative assertion. Existing tests pass new uint256[](0) as writes; if set were erroneously called, the STORE is etched with REVERTING_MOCK_BYTECODE (test/abstract/InterpreterMockTest.sol:27) so the call would revert — that gives implicit detection but it is fragile (any future change that adds a fallback mock for set would silently break the short-circuit detection). There is also no test for the case where interpreterStore.set itself reverts.

Source

src/lib/LibFlow.sol:144-146

Existing tests

• test/src/concrete/Flow.time.t.sol:12 — covers kvs.length > 0 (positive assertion via vm.expectCall).
• All other Flow.*.t.sol tests pass new uint256[](0) for kvs — implicit but no explicit assertion.

Proposed tests

Add to test/src/concrete/Flow.time.t.sol:

import {VmSafe} from "forge-std/Vm.sol";

/// Negative test: empty kvs MUST NOT call interpreterStore.set.
function testFlowBasicFlowTimeNoStoreSetWhenKvsEmpty() public {
    (IFlowV5 flow, EvaluableV2 memory evaluable) = deployFlow();

    uint256[] memory stack = generateFlowStack(transferEmpty());
    interpreterEval2MockCall(stack, new uint256[](0));

    // Mock set so it does not revert; we use vm.expectCall with `count = 0`
    // (foundry: third arg is data, the count overload is .expectCall(addr,
    // data, 0)). Without this, accidental .set() would revert via the
    // store's REVERTING_MOCK_BYTECODE — but this assertion is explicit.
    vm.mockCall(address(STORE), abi.encodeWithSelector(IInterpreterStoreV2.set.selector), abi.encode());
    vm.expectCall(address(STORE), abi.encodeWithSelector(IInterpreterStoreV2.set.selector), 0);

    flow.flow(evaluable, new uint256[](0), new SignedContextV1[](0));
}

/// Negative test: store.set reverting bubbles up.
function testFlowBasicFlowTimeStoreSetRevertBubbles(uint256[] memory writeToStore) public {
    vm.assume(writeToStore.length != 0);
    (IFlowV5 flow, EvaluableV2 memory evaluable) = deployFlow();

    uint256[] memory stack = generateFlowStack(transferEmpty());
    interpreterEval2MockCall(stack, writeToStore);

    vm.mockCallRevert(
        address(STORE),
        abi.encodeWithSelector(IInterpreterStoreV2.set.selector, DEFAULT_STATE_NAMESPACE, writeToStore),
        "STORE_SET_REVERT"
    );

    vm.expectRevert("STORE_SET_REVERT");
    flow.flow(evaluable, writeToStore, new SignedContextV1[](0));
}

Mutation-test: invert the condition in LibFlow.flow to if (kvs.length >= 0) (always call set) and confirm testFlowBasicFlowTimeNoStoreSetWhenKvsEmpty fails. Then revert.