feat(symbolic): persist replayable counterexample artifacts#15217
Conversation
figtracer
left a comment
There was a problem hiding this comment.
reviewed the replay paths and reproduced one config-drift edge case locally.
grandizzy
left a comment
There was a problem hiding this comment.
Please check findings below
| let filter = self.filter(&config)?; | ||
| let replay_symbolic_artifact = self.load_symbolic_artifact_replay()?; | ||
| if replay_symbolic_artifact.is_some() { | ||
| config.symbolic.enabled = true; |
There was a problem hiding this comment.
🟡 non-blocking: sequence replay needs no solver, yet this enables symbolic mode globally for all replays. If enabling symbolic has side effects (solver validation, test discovery), prefer an explicit replay execution mode over flipping this flag.
| /// Executes all the tests in the project. | ||
| /// | ||
| /// See [`Self::compile_and_run`] for more details. | ||
| #[allow(clippy::too_many_arguments)] |
There was a problem hiding this comment.
🟡 non-blocking: run_tests now needs #[allow(clippy::too_many_arguments)] (also at L1296/L1335). Since replay_symbolic_artifact already lives in TestRunnerConfig, consider a RunTestsOptions struct instead of threading another positional arg.
| "raw_args" | ||
| ], | ||
| "properties": { | ||
| "warp": { |
There was a problem hiding this comment.
🟡 non-blocking: the schema requires compact 0x hex for warp/roll/value, but the loader doesn't validate against the schema and Alloy's U256 deserializer may accept other forms — a schema-contract mismatch on input. Add a producer test that Some(U256::ZERO) serializes to "0x0" (schema allows 0x0, forbids leading-zero forms). Also note the external HTTPS $refs mean the offline test only checks ref existence, not real validation.
Re-reviewed after latest commits (2d56330): blockers 1, 2, and 4 are fixed and blocker 3 is mitigated. Dismissing in favor of an approval.
grandizzy
left a comment
There was a problem hiding this comment.
Re-reviewed after the latest commits. All four earlier blockers are resolved or mitigated:
- Single-call replay semantics (
a3ffefe): now gates onreplay.status == Confirmed, rejectswarp/roll, usescall_raw+is_raw_call_successto mirror the original symbolic path, decodes the live revert reason (falling back to the artifact reason only when empty), and routesEvmError::Skiptosingle_skip. ✅ check_sequenceexecuted-call counts (96ea2e7): independentcalls_executedcounter replacesidx + 1/sequence.len(). ✅- Artifact filename identity (
415d8b5): 16-byte keccak over the fullcontract\0value\0kindidentity, with a collision test. ✅ - Double
replay_persisted_call_sequence: still executes twice, but the second pass now recomputes metrics for the (possibly changed) sequence returned byreplay_error, so it's a minor efficiency follow-up rather than a correctness issue. ✅ (mitigated)
Most non-blocking items were also addressed: #[serde(deny_unknown_fields)] on the artifact structs, loader rejecting non-confirmed artifacts, a U256::ZERO compact-hex test, doc-comments scoping execute_tx/execute_tx_and_register_created to invariant replay, and the legacy singular counterexample_artifact field documented in favor of the plural.
Remaining nits are optional follow-ups: config.symbolic.enabled = true for sequence replay, the run_tests argument count, and true offline schema validation for the external $refs.
LGTM, nice work.
Description
Closes OSS-349
Adds symbolic counterexample artifacts that can be replayed independently from the original symbolic run. When
forge test --symbolicconfirms a counterexample, Forge writes a schema-versioned JSON artifact under the project cache and surfaces its path in JSON output and non-JSON test output.Artifacts can be replayed with:
Replay supports both single-call symbolic counterexamples and invariant/stateful sequence counterexamples.
What changed
foundry:symbolic.counterexample@v1artifacts.--replay-symbolic-artifact <PATH>to reconstruct the target contract/test/path and replay the stored calls.counterexample_artifacts.PR Checklist