fix(script): treat unknown tx as dropped to avoid receipt-poll hang #553
Merged
Conversation
feat: add Amsterdam hardfork mapping to SpecId Add EthereumHardfork::Amsterdam => SpecId::AMSTERDAM mapping in spec_id_from_ethereum_hardfork. Without this, Amsterdam would hit the unreachable!() catch-all arm. The default evm_version is already set to Osaka. Amp-Thread-ID: https://ampcode.com/threads/T-019e113f-5bda-711b-8ec5-4841042ba62c Co-authored-by: Centaur AI <ai@centaur.local>
Issue14212Test (test_rollForkToTxOnBase, test_transactDepositTxOnBase) depends on Base RPCs to fetch tx 0xe2f4bff... which intermittently fails with 'could not get transaction'. This is the same class of flakiness as the existing Polygon RPC skip. Move the contract into FLAKY_TESTDATA_CONTRACTS so it runs in the nightly flaky profile instead of blocking every CI run. Amp-Thread-ID: https://ampcode.com/threads/T-019e15de-2292-74cf-a885-b65acaa41d3c Co-authored-by: Centaur AI <ai@centaur.local>
* feat: improve forge build lint-failure UX * dedup lint-failure bypass message and revert parse emitter to stderr * address review: lead with bug-report CTA, add failure-notice tests - Rewrite LINT_FAILURE_NOTICE to lead with the report-the-bug CTA (deep-linked to BUG-FORM.yml) and demote --no-lint to a per-run workaround, with a docs link for temporarily disabling lint-on-build. - Drop the RUST_BACKTRACE suggestion: the failure is an eyre error built at a single call site, so a backtrace adds no signal. - Add integration tests for the failure notice (positive + negative --no-lint case) using COUNTER_A + deny=Notes to deterministically trigger the lint Err path without breaking compilation. - Add unit tests for the new convert_solar_errors non-buffer branch covering singular/plural messaging and the no-error path. Amp-Thread-ID: https://ampcode.com/threads/T-019e15e7-6907-723c-bd10-ac64558ed4ce Co-authored-by: Amp <amp@ampcode.com> * fix: fmt * use snapshot assertions for failure-notice tests; trim doc comment - Replace the fan-out of assert!(stderr.contains(...)) checks with a single snapbox stderr_eq snapshot, so the full expected output (lint diagnostic + notice + eyre cause chain) lives in one place and is trivially refreshed via SNAPSHOTS=overwrite. - Snapshot the negative --no-lint case as empty stderr too. - Drop the verbose doc comment on LINT_FAILURE_NOTICE; the constant's contents already speak for themselves. - Trim 'attach the full `forge build` output' to the more general 'attach the full output above'. Amp-Thread-ID: https://ampcode.com/threads/T-019e15e7-6907-723c-bd10-ac64558ed4ce Co-authored-by: Amp <amp@ampcode.com> * trim verbose test comments Amp-Thread-ID: https://ampcode.com/threads/T-019e15e7-6907-723c-bd10-ac64558ed4ce Co-authored-by: Amp <amp@ampcode.com> --------- Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Mablr <59505383+mablr@users.noreply.github.com>
) * feat(invariant): assert all invariants * Tests and Nits Amp-Thread-ID: https://ampcode.com/threads/T-019dbf48-3fb0-7762-a01f-b5e966339e73 Co-authored-by: Amp <amp@ampcode.com> * fix: check all invariants in afterInvariant gate and preflight Amp-Thread-ID: https://ampcode.com/threads/T-019dbf48-3fb0-7762-a01f-b5e966339e73 Co-authored-by: Amp <amp@ampcode.com> * fix: use per-invariant fail_on_revert when recording handler revert failures Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dbf48-3fb0-7762-a01f-b5e966339e73 * fix: commit state between txs in generate_counterexample Amp-Thread-ID: https://ampcode.com/threads/T-019dbf48-3fb0-7762-a01f-b5e966339e73 Co-authored-by: Amp <amp@ampcode.com> * fix: preflight check all invariants, not just primary Amp-Thread-ID: https://ampcode.com/threads/T-019dbf48-3fb0-7762-a01f-b5e966339e73 Co-authored-by: Amp <amp@ampcode.com> * fix: exclude secondary invariants from optimization mode runs Amp-Thread-ID: https://ampcode.com/threads/T-019dbf48-3fb0-7762-a01f-b5e966339e73 Co-authored-by: Amp <amp@ampcode.com> * refactor: rename invariant_fn to primary_invariant_fn, deterministic preflight error, debug_assert on empty invariants Amp-Thread-ID: https://ampcode.com/threads/T-019dbf48-3fb0-7762-a01f-b5e966339e73 Co-authored-by: Amp <amp@ampcode.com> * feat: show broken invariant count in progress bar during continuous runs Amp-Thread-ID: https://ampcode.com/threads/T-019dbf48-3fb0-7762-a01f-b5e966339e73 Co-authored-by: Amp <amp@ampcode.com> * feat(invariant): rename continuous_run to assert_all and default to true Renames the InvariantConfig field to better describe its semantics ("assert every invariant in the suite, don't stop on first failure") and flips the default to true so multi-invariant suites report all broken invariants by default, matching Echidna/Medusa behavior. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dcd68-66ac-76ed-ac5c-7ea722a9c9ae * feat(invariant): parameterize shrinker by target invariant + persisted failures footer - Generalize shrink_sequence, shrink_sequence_value, replay_run, replay_error to accept target_invariant: &Function (currently always primary; unblocks per-secondary shrinking). - Move reset_shrink_progress out of shrink fns; called once per invariant from replay_error. Progress label now 'Shrink: <invariant_name>'. - Add TestResult.invariant_failure_dir; Display appends 'N invariant failures persisted to <dir> — rerun to shrink' when secondary failures were written. Amp-Thread-ID: https://ampcode.com/threads/T-019dcd68-66ac-76ed-ac5c-7ea722a9c9ae Co-authored-by: Amp <amp@ampcode.com> * feat(invariant): structured InvariantOtherFailure for assert_all secondaries Promotes TestResult.other_failures from Vec<String> to Vec<InvariantOtherFailure> carrying name, reason, optional counterexample, and persisted path. Display renders each secondary symmetrically with [FAIL: reason] + [Sequence] block when a counterexample is available, falling back to the terse 'name: reason' one-liner otherwise. Amp-Thread-ID: https://ampcode.com/threads/T-019dcd68-66ac-76ed-ac5c-7ea722a9c9ae Co-authored-by: Amp <amp@ampcode.com> * feat(invariant): serial secondary shrinking + Ctrl-C persists un-shrunk secondaries PR-3 of the assert_all rollout. After the campaign finishes, every broken secondary invariant is shrunk in turn via replay_error so users get a ready-to-debug counterexample for each failure in a single run (matching how the primary is rendered: [FAIL: reason] <name> + [Sequence] block). On Ctrl-C, instead of dropping known secondaries (previous behavior was a 'break' before pushing them), the loop keeps recording every failure the campaign discovered. The shrink + replay step is skipped to honor the interrupt, but the un-shrunk sequence is persisted via BaseCounterExample::from_invariant_call (no execution required), so a re-run targeting that secondary picks up the saved counterexample and shrinks from there — same UX as re-running an interrupted primary. Output of an interrupted run now includes a terse '<invariant>: <reason>' line for each secondary the campaign saw, preserving visibility of all broken invariants while keeping the interrupt fast. Adds e2e coverage: - assert_all: extended to verify secondary failures render symmetrically with shrunk sequences and that re-running skips persisted secondaries. - assert_all_only_primary: new test confirming no secondary [FAIL] blocks or persisted-failures footer appear when only the primary breaks. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dcdd3-53f5-76b6-ac36-d59f06b58280 * feat(invariant): assert_all polish — [i/N] shrink counter, suite roll-up, opt-mode warning Three small UX wins for assert_all campaigns. No behavior change, no new dependencies. 1. Shrink progress bar gets an [i/N] queue counter when more than one invariant needs shrinking, so users see how many shrinkers are queued behind the current one (e.g. '[2/3] Shrink: invariant_X'). reset_shrink_progress and replay_error gain a position parameter; single-invariant call sites pass None. 2. Suite-level roll-up footer: when assert_all exercised >1 invariant and the test failed, render 'Suite assert_all: <broken>/<total> invariants broken' above the per-invariant blocks. Gives CI logs and Slack pastes a glanceable health line. New Option<usize> field on TestResult, populated only when meaningful. 3. Startup warning when assert_all + optimization-mode are combined. Optimization mode tracks one int256 return value, so any boolean secondary invariants in the same contract are filtered out before the campaign — previously silent. Now emits a once-per-suite warning naming the optimization invariant and every dropped boolean so users can move them to a separate contract. E2E tests: extend assert_all to assert the new 4/5 roll-up; assert_all_only_primary covers the 1/2 case; new assert_all_optimization_mode_warning verifies the warning fires with the dropped invariant names. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dcdd3-53f5-76b6-ac36-d59f06b58280 * feat(invariant): warn when assert_all skips invariants with persisted failures Symmetric with the primary's existing persisted-replay warning. Echidna and Medusa never silently drop properties between runs — properties are re-evaluated every campaign and a previous failure doesn't suppress them. Foundry's per-property failure file model meant secondaries with a stale persisted counterexample were filtered out of the campaign with no acknowledgment, so users coming from Echidna/Medusa would see fewer invariants in the report than their contract defines. Now emits one stderr line listing every skipped name and the cache dir to clean, e.g.: Warning: test/X.t.sol:Suite: 3 invariant(s) skipped due to persisted failures: invariant_a, invariant_b, invariant_c. Run `forge clean` or delete files in cache/invariant/failures/Suite to re-include. E2E: extends assert_all re-run case with stderr_eq snapshot asserting the warning fires with all 3 skipped names. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dcdd3-53f5-76b6-ac36-d59f06b58280 * fix(invariant): gate afterInvariant per-run under assert_all Previously afterInvariant was gated on failures.errors.is_empty() campaign-wide. Under assert_all that gate stayed closed for the rest of the campaign once any invariant broke, silently skipping the afterInvariant hook on every subsequent run. Any assertions or cleanup logic in afterInvariant therefore stopped running after the first unrelated invariant failure. Now snapshot failures.errors.len() at the start of each run and only skip afterInvariant when the current run produced a new failure. Preserves the legacy 'don't run afterInvariant on a run that already failed' semantics while letting it run on subsequent runs once an earlier invariant has broken. E2E: new assert_all_after_invariant_runs_after_earlier_failure case breaks invariant_first in run 1, keeps the campaign alive with a second never-breaking invariant, and asserts an always-reverting afterInvariant surfaces its marker in failure output. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dce2d-57c7-734a-bbc6-6fa5e34b25de * fix(invariant): re-evaluate secondary persisted failures on settings change The secondary persisted-failure skip used a bare `.exists()` check at two sites in runner.rs (the warning + InvariantContract::new filter, and the post-campaign shrink loop). Under the new `assert_all = true` default this meant any leftover failure file from a previous run was treated as still valid even after the user changed a tracked setting (target contracts/selectors, target/excluded senders, fail_on_revert), silently dropping the secondary from the campaign with a misleading 'skipped due to persisted failures' warning. Now both sites use the same settings-aware compatibility check the primary's replay path uses (persisted_call_sequence settings.diff). Stale caches fall back to a fresh evaluation; only secondaries whose persisted settings still match the current run are honored. Also hoists current_settings up so the new secondary_has_compatible_persisted closure can reuse it across all three call sites (warning, filter, shrink-loop skip). E2E: new assert_all_secondary_persisted_revalidates_on_settings_change runs once with fail_on_revert=false, flips it to true, re-runs and asserts the suite roll-up shows 2/2 invariants broken — proving the secondary was re-evaluated rather than silently filtered out. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dce2d-57c7-734a-bbc6-6fa5e34b25de * fix(invariant): drop hollow [FAIL] when only secondaries break under assert_all When the selected invariant test passes but a secondary breaks under assert_all, the report previously rendered a hollow '[FAIL]' header (no reason, no counterexample) for the primary and the suite roll-up overcounted broken invariants as '1 + other_failures.len()', attributing a non-existent primary failure. Now key the primary header on whether the primary actually broke (`reason.is_some() || counterexample.is_some()`) and skip the header when it didn't. Roll-up uses the same flag so the count reflects only invariants that actually broke (e.g., 1/2 instead of 2/2). JSON shape is unchanged: top-level reason/counterexample stay null when the selected primary didn't break, with full secondary detail in other_failures. E2E: new assert_all_secondary_only_failure_no_hollow_fail asserts a secondary-only break renders 'Suite assert_all: 1/2 invariants broken' followed by the secondary's '[FAIL: ...] <name>' block, with no hollow primary header. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dce2d-57c7-734a-bbc6-6fa5e34b25de * fix(invariant): scope assert_all hollow [FAIL] suppression to secondary-only case Previous commit suppressed the '[FAIL]' header whenever the primary had no top-level reason or counterexample, which also matches DS-style failures (they signal via the 'failed' flag and log events rather than through TestResult.reason). That regressed failure_assertions::ds_style_test_failing and test_cmd::core::legacy_assertions in CI. Now the suppression is scoped strictly to the assert_all secondary-only case: skip the primary header only when no primary failure AND assert_all is in play AND there is at least one secondary to render. DS-style, plain unit and single-invariant failures keep the original '[FAIL]'/'[FAIL: ...]' rendering. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dce2d-57c7-734a-bbc6-6fa5e34b25de * fix(invariant): attribute failure event to first broken invariant in declaration order The structured JSON 'failure' event emitted to stderr at campaign end (consumed by benchmark and CI tooling) used 'errors.values().next()' on a HashMap to pick its 'reason' field, while hardcoding the 'invariant' field to the primary's name. With HashMap RandomState, the same broken set of invariants produced a different reason string across runs, and the event was self-inconsistent (e.g., 'invariant: invariant_balance, reason: fee miscalculation'). Three sites used this pattern: in-run break path, afterInvariant break path, and the preflight check fallback. Now they walk 'invariant_contract.invariant_fns' in declaration order (a Vec, deterministic) and pick the first one with a recorded failure. Both 'invariant' and 'reason' fields refer to the same function, and the event is stable across runs. A new 'first_broken_event' helper centralizes the lookup. E2E: assert_all_failure_event_uses_declaration_order declares three invariants (a, b, c) that all break on the same call, runs with '--mt invariant_c' (primary is the last declared) and asserts the emitted event names invariant_a with reason 'a broken'. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dce2d-57c7-734a-bbc6-6fa5e34b25de * refactor(invariant): rename to InvariantSecondaryFailure / invariant_secondary_failures and serialize sparsely Renames TestResult.other_failures -> invariant_secondary_failures and the underlying InvariantOtherFailure struct -> InvariantSecondaryFailure. The previous names were generic ('other relative to what?'); the new names align with the existing 'primary/secondary' terminology used throughout the assert_all rollout and follow the Rust Vec<Foo>/foos plural-of-singular convention. Also marks the field with #[serde(default, skip_serializing_if = 'Vec::is_empty')] so it is omitted from JSON output for any test that has no secondary failure data — plain unit tests, fuzz tests, passing tests. Pre-PR JSON consumers continue to see the same shape on those results. invariant_failure_dir and assert_all_invariant_count already had Option::is_none guards. Updates the SimpleContractTest{NonVerbose,Verbose}.json fixtures to drop the now-skipped empty field. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019dce2d-57c7-734a-bbc6-6fa5e34b25de * test(invariant): assert_all + fail_on_revert=false attributes assert() to all live invariants Amp-Thread-ID: https://ampcode.com/threads/T-019dd262-ed81-723c-aaaa-8e1314bed45a Co-authored-by: Amp <amp@ampcode.com> * fix(clippy): use values() instead of iter() with unused key Amp-Thread-ID: https://ampcode.com/threads/T-019df149-03af-722b-abda-d8ab6332ee3b Co-authored-by: Amp <amp@ampcode.com> * fix(clippy): drop redundant references in trimmed_hex format args Amp-Thread-ID: https://ampcode.com/threads/T-019df149-03af-722b-abda-d8ab6332ee3b Co-authored-by: Amp <amp@ampcode.com> * Update crates/evm/evm/src/executors/invariant/error.rs Co-authored-by: stevencartavia <112043913+stevencartavia@users.noreply.github.com> * invariant: unify primary/secondary as one list and one failure type The "anchor" of an invariant campaign is the test entry point selected by `--mt` (or the per-`Test` invariant function the runner is currently exercising). Under `assert_all`, the anchor is the campaign's named test, and any other broken `invariant_*` functions are secondaries shrunk and reported alongside it. - InvariantContract: single invariant_fns list in declaration order with an anchor_idx pointing at the campaign's anchor invariant - TestResult: replace primary/secondary split with invariant_failures: Vec<InvariantFailure>; each entry carries is_anchor so the renderer can omit the function name suffix when a single failure is the anchor (its name is already on the trailing summary line) - assert_invariants/can_continue/assert_after_invariant return the broken invariant directly, removing the first_broken_event re-scan - collapse FailedInvariantCaseData::new + with_assertion_failure into InvariantRunCtx::failed_case - show invariant name on [FAIL: ...] line when not anchor or multiple failures (so secondaries in single-failure assert_all runs are still visible) * invariant: validate signatures upstream and reject parameterized invariants --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: George Niculae <george@gxn3ql7y5j.tail388b2e.ts.net> Co-authored-by: stevencartavia <112043913+stevencartavia@users.noreply.github.com>
…ne for backwards compatibility (#14689) * feat(forge): add noop --no-commit flag to forge install and forge clone for backwards compatibility `forge install` and `forge clone` no longer commit by default (commit is opt-in via `--commit`). Many AI tools and scripts still pass `--no-commit`, causing an unrecognized flag error. This adds a hidden, noop `--no-commit` flag so those invocations continue to work. Follow-up to #14649 which did the same for `forge init`. Amp-Thread-ID: https://ampcode.com/threads/T-019e15aa-9de3-77a6-9365-21ea300cd785 * test(forge): add test for noop --no-commit flag on forge clone Amp-Thread-ID: https://ampcode.com/threads/T-019e15aa-9de3-77a6-9365-21ea300cd785 --------- Co-authored-by: Centaur AI <ai@centaur.local> Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com>
Upstream reth (paradigmxyz/reth) now requires alloy-eip7928 ^0.3.6 (since commit d985041). The lockfile here still pinned 0.3.5, which causes resolution failures for any downstream that patches Foundry to use a recent reth revision. Bump the transitive dep to 0.3.7 (latest compatible) so the lockfile stays resolvable. Amp-Thread-ID: https://ampcode.com/threads/T-019e1699-eb69-75aa-afdc-c66246d2f217 Co-authored-by: Centaur AI <ai@centaur.local>
flake.lock: Update
Flake lock file updates:
• Updated input 'fenix':
'github:nix-community/fenix/74c1591' (2026-05-02)
→ 'github:nix-community/fenix/f54d645' (2026-05-09)
• Updated input 'fenix/rust-analyzer-src':
'github:rust-lang/rust-analyzer/64cdaeb' (2026-05-01)
→ 'github:rust-lang/rust-analyzer/73ca1d4' (2026-05-08)
• Updated input 'nixpkgs':
'github:NixOS/nixpkgs/c6d6588' (2026-05-01)
→ 'github:NixOS/nixpkgs/b3da656' (2026-05-08)
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* feat(lint): add missing-inheritance lint * fix(lint): consider dependency interfaces as candidates for missing-inheritance
…14626) * fix(script): treat unknown tx as dropped to avoid receipt-poll hang `provider.get_transaction_by_hash(hash)` returns `Result<Option<Tx>, _>`. `check_tx_status` matched `Ok(_)` for the 'node knows the tx' path, incorrectly bucketing `Ok(None)` (tx unknown) as known. In the `get_receipt`-error branch this fed `RetryError::Continue`, which is unbounded — producing an infinite `eth_blockNumber` + `eth_getTransactionReceipt` loop whenever a tx was rejected at submission or dropped from the mempool after a halt (e.g. anvil's new -32003 mapping for non-REVERT EVM halts). Distinguish `Ok(Some(_))` from `Ok(None)`; treat the latter the same as a transport error and terminate as `TxStatus::Dropped`. Amp-Thread-ID: https://ampcode.com/threads/T-019dfe94-2b0d-71bc-a863-633e05594581 Co-authored-by: Amp <amp@ampcode.com> * test(script): cover check_tx_status dropped/unknown tx paths Amp-Thread-ID: https://ampcode.com/threads/T-019e0814-2cda-7271-a8d6-c59e6739946b Co-authored-by: Amp <amp@ampcode.com> --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Co-authored-by: Mablr <59505383+mablr@users.noreply.github.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Dargon789, your pull request is larger than the review limit of 150000 diff characters
|
@stevencartavia is attempting to deploy a commit to the Foundry development Team on Vercel. A member of the Team first needs to authorize it. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the assert_all feature for invariant testing, allowing campaigns to continue after the first failure and report all broken invariants. It updates the invariant executor, result structures, and reporting logic to handle multiple failures per test suite. Additionally, a new lint rule missing-inheritance is added to detect contracts implementing interface APIs without explicit inheritance. The forge build command now includes a --no-lint flag and improved error notices. In crates/script, transaction status checking is refined to correctly identify dropped transactions. Review feedback suggests refining error handling in transaction status checks to distinguish between transport errors and dropped transactions, and clarifying the lint failure notice to distinguish between configuration-based denials and actual engine failures.
|
Deployment failed with the following error: Learn More: https://vercel.com/dargon789-forge?upgradeToPro=build-rate-limit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Solution
PR Checklist