foundry-rs / foundry-rs/foundry
forge script --resume: positional receipt cursor can treat a reverted tx as complete and skip an already-mined later one
- Dominant language
- Rust
- Stars
- 10.6k
- Forks
- 2.6k
- Avg merge
- 18h 20m
- Merged PRs (30d)
- 510
Description
### 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:**
```rust
// crates/script/src/broadcast.rs:454
let already_broadcasted = sequence.receipts.len();
```
and the pinned test makes the assumption explicit:
```rust
// 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:
```rust
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.
Contributor guide
Research direction
Start in crates/script/src/broadcast.rs at the positional receipt cursor and remaining_transactions tests, then read crates/script/src/progress.rs around reverted and pending receipt handling. Run remaining_transactions_skip_receipt_prefix and remaining_unsigned_transactions_skip_completed_transactions first. Done means resume progress remains correct when a sequence has a mid-sequence missing receipt, with both pinned tests updated to the agreed semantics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- blockchain, cli, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100