From c8746fb3378df791c5a6cd548e050fda436d58c0 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Tue, 5 May 2026 15:26:30 +0400 Subject: [PATCH 1/3] test kvs short-circuit and store.set revert paths in LibFlow.flow Two new tests: - testFlowBasicFlowTimeNoStoreSetWhenKvsEmpty: explicit count=0 expectCall on STORE.set when kvs is empty. Pins the existing short-circuit so a future change that drops it is caught. Mutation verified: removing the `kvs.length > 0` guard makes the test fail; reverting passes. - testFlowBasicFlowTimeStoreSetRevertBubbles: when kvs is non-empty and store.set reverts, the revert MUST propagate out of flow(). Closes #329. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/src/concrete/Flow.time.t.sol | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/src/concrete/Flow.time.t.sol b/test/src/concrete/Flow.time.t.sol index 055cfa33..f733a4c0 100644 --- a/test/src/concrete/Flow.time.t.sol +++ b/test/src/concrete/Flow.time.t.sol @@ -27,4 +27,40 @@ contract FlowTimeTest is FlowTest { flow.flow(evaluable, writeToStore, new SignedContextV1[](0)); } + + /// `LibFlow.flow` short-circuits the `interpreterStore.set` call when + /// `kvs.length == 0`. Pin this with an explicit count=0 expectCall so + /// a future refactor that drops the short-circuit is caught. + function testFlowBasicFlowTimeNoStoreSetWhenKvsEmpty() public { + (IFlowV5 flow, EvaluableV2 memory evaluable) = deployFlow(); + + uint256[] memory stack = generateFlowStack(transferEmpty()); + interpreterEval2MockCall(stack, new uint256[](0)); + + // Mock set to a no-op so the existing REVERTING_MOCK_BYTECODE on + // STORE doesn't accidentally pass the test for the wrong reason. + 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)); + } + + /// A revert from `interpreterStore.set` MUST propagate out of `flow` + /// rather than being caught and swallowed. + /// forge-config: default.fuzz.runs = 100 + 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), abi.encode("STORE_SET_FAILED") + ); + + vm.expectRevert(abi.encode("STORE_SET_FAILED")); + flow.flow(evaluable, writeToStore, new SignedContextV1[](0)); + } } From 68268c7fa9bc525a5ef66befecf87d24560a4d93 Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sat, 9 May 2026 14:00:52 +0400 Subject: [PATCH 2/3] use raw string literals for mockCallRevert payload + expectRevert match Per Foundry convention `vm.mockCallRevert(retdata)` and `vm.expectRevert(revertData)` take raw revert bytes; passing `abi.encode("...")` produces ABI-encoded bytes (offset+length+padded data) which are self-consistent here but diverge from idiomatic usage in the Foundry book. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/src/concrete/Flow.time.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/src/concrete/Flow.time.t.sol b/test/src/concrete/Flow.time.t.sol index f733a4c0..2cf8006e 100644 --- a/test/src/concrete/Flow.time.t.sol +++ b/test/src/concrete/Flow.time.t.sol @@ -57,10 +57,10 @@ contract FlowTimeTest is FlowTest { interpreterEval2MockCall(stack, writeToStore); vm.mockCallRevert( - address(STORE), abi.encodeWithSelector(IInterpreterStoreV2.set.selector), abi.encode("STORE_SET_FAILED") + address(STORE), abi.encodeWithSelector(IInterpreterStoreV2.set.selector), "STORE_SET_FAILED" ); - vm.expectRevert(abi.encode("STORE_SET_FAILED")); + vm.expectRevert("STORE_SET_FAILED"); flow.flow(evaluable, writeToStore, new SignedContextV1[](0)); } } From c1d6de082d119f83004c531d30d025186d8e711e Mon Sep 17 00:00:00 2001 From: thedavidmeister Date: Sat, 9 May 2026 14:33:42 +0400 Subject: [PATCH 3/3] forge fmt sweep after string-literal change Co-Authored-By: Claude Opus 4.7 (1M context) --- test/src/concrete/Flow.time.t.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/src/concrete/Flow.time.t.sol b/test/src/concrete/Flow.time.t.sol index 2cf8006e..0c178c3e 100644 --- a/test/src/concrete/Flow.time.t.sol +++ b/test/src/concrete/Flow.time.t.sol @@ -56,9 +56,7 @@ contract FlowTimeTest is FlowTest { uint256[] memory stack = generateFlowStack(transferEmpty()); interpreterEval2MockCall(stack, writeToStore); - vm.mockCallRevert( - address(STORE), abi.encodeWithSelector(IInterpreterStoreV2.set.selector), "STORE_SET_FAILED" - ); + vm.mockCallRevert(address(STORE), abi.encodeWithSelector(IInterpreterStoreV2.set.selector), "STORE_SET_FAILED"); vm.expectRevert("STORE_SET_FAILED"); flow.flow(evaluable, writeToStore, new SignedContextV1[](0));