Skip to content

forge script --resume: positional receipt cursor can treat a reverted tx as complete and skip an already-mined later one #16566

Description

@gomesalexandre

Component

Forge

What command(s) is the bug in?

forge script --resume

Describe the bug

--resume tracks progress with a positional cursor — it assumes the completed transactions in a sequence are always a contiguous prefix — but a reverted transaction breaks that assumption, and the maintainers' own code comment already flags this as an open question.

The cursor:

// crates/script/src/broadcast.rs:454
let already_broadcasted = sequence.receipts.len();

and the pinned test makes the assumption explicit:

// crates/script/src/broadcast.rs:1535, remaining_transactions_skip_receipt_prefix
let mut sequence = ScriptSequence { transactions: [completed, second, third], receipts: vec![receipt()], .. };
assert_eq!(remaining_transactions(&sequence)..., vec![second, third]); // prefix semantics

The hole: crates/script/src/progress.rs:266-271 removes a reverted transaction from pending without adding a receipt:

Ok(TxStatus::Revert(receipt)) => {
    // consider:
    // if this is not removed from pending, then the script becomes
    // un-resumable. Is this desirable on reverts?
    warn!(tx_hash=?tx_hash, "Transaction Failure");
    deployment_sequence.remove_pending(receipt.transaction_hash());
    ...

That comment is the maintainers' own doubt about this exact behavior. If a later transaction in the same sequence still mines successfully (and does add a receipt), receipts.len() no longer corresponds to "everything before this index is done" — the reverted transaction leaves a hole in the middle of the sequence, not at the end.

Consequence on --resume: the reverted transaction is treated as already-completed (the cursor just counts receipts), so it's skipped, while a transaction that already mined can be re-sent — which fails nonce too low, retries a few times, then hard-fails. --resume is effectively unusable for that deployment. This matters because --resume is Foundry's own recommended recovery path (see the "Add information about using --resume if necessary" branch a few lines below the snippet above).

Two existing tests currently pin the prefix assumption as correct behavior — remaining_transactions_skip_receipt_prefix and remaining_unsigned_transactions_skip_completed_transactions (both in broadcast.rs) — so a fix would need to update both, not just the cursor logic.

A viable fix direction already exists in the codebase: transactions are hash-stamped when sent (add_pending records each transaction's hash), and format_receipt elsewhere already matches receipts to transactions by hash, not position. Matching completion by hash instead of receipts.len() would use data that's already being recorded.

Reachability — tested live, and it's narrower than it first looks

I built a small repro (4 broadcasts, #2 reverts, low-level .call() so the revert doesn't abort the script) and ran forge script --broadcast --skip-simulation against a local anvil.

It did not reproduce as "any reverting transaction." Foundry's own gas-estimation step catches most reverts before the transaction is ever broadcast — the script stops there entirely, so later transactions never get a chance to mine, and the completed set stays a valid prefix. The positional cursor handles that case correctly.

So the real trigger needs a transaction that passes gas estimation but still reverts when mined — an estimate-vs-mine state race (state changed between the two, or a block/timestamp-dependent condition). That's real, but narrower than the mechanism alone suggests, and I want to be upfront about that rather than imply I have a one-shot repro.

A second path looks like a more likely real-world trigger, though I haven't driven it live: PendingReceiptError at progress.rs:227 silently drops a pending transaction with no error surfaced at all, and execution can still reach "ONCHAIN EXECUTION COMPLETE & SUCCESSFUL." This could plausibly happen from a dropped/replaced transaction (e.g. a resubmission at higher gas replacing a stuck one) without needing the harder-to-hit estimate-vs-mine race — but I didn't reproduce it (it needs a dropped/replaced tx against anvil, which is fiddly to set up), so treat it as a lead, not a confirmed second repro.

Suggested direction

Match completed transactions to receipts by transaction hash (data already recorded via add_pending/format_receipt) rather than by position/count, so a mid-sequence hole doesn't misalign the cursor. Happy to attempt a PR once there's agreement on approach, given it touches two existing tests' pinned semantics.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions