Skip to content

fix(giga): fall back to v2 on execution errors#3768

Open
philipsu522 wants to merge 3 commits into
mainfrom
cursor/giga-execution-error-fallback-56d1
Open

fix(giga): fall back to v2 on execution errors#3768
philipsu522 wants to merge 3 commits into
mainfrom
cursor/giga-execution-error-fallback-56d1

Conversation

@philipsu522

@philipsu522 philipsu522 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Describe your changes and provide context

This fixes a live RPC-level divergence, not only a defensive edge case. On main, the parity test reaches matching consensus fields but fails receipt parity: Giga stamps TxType=2 and gasUsed=gasLimit, while V2 writes an EndBlock synthetic receipt with zero values. Mixed Giga/V2 fleets can therefore return different receipts for the same transaction.

State-transition errors now route through the existing V2 fallback instead of duplicating V2's fee, nonce, receipt, deferred-info, and ABCI result handling. This adds an execution_failed abort sentinel and fallback metric reason. Because the Giga StateDB has not been finalized when ExecuteTransactionFeeCharged returns an error, its transaction-local cache is discarded before V2 re-runs the transaction. Both sequential and OCC Giga paths use the existing fallback plumbing.

The EIP-7623 floor-data-gas parity test covers deterministic ABCI fields, LastResultsHash, nonce, receipt, sender balance, fee-collector balance, and per-transaction coinbase balance in sequential and OCC modes.

Operational tradeoff: this creates the first deterministic single-transaction trigger that passes CheckTx at roughly 27.5k gas but reverts an entire OCC batch to V2. Operators should alert on app_giga_fallback_to_v2{reason="execution_failed"}; per-transaction OCC fallback should be tracked as follow-up work.

Testing performed to validate your change

  • go test ./giga/tests -run '^TestGiga_FailedExecutionFallsBackToV2$' -count=1
  • go test ./giga/tests -count=1
  • go test ./app -run 'TestGiga|TestOCC|TestProcess' -count=1
  • gofmt -s and goimports on all changed Go files

Slack Thread

Open in Web Open in Cursor 

Co-authored-by: Philip Su <philip.su.522@gmail.com>
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJul 17, 2026, 3:22 PM

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.02%. Comparing base (c2bae66) to head (99d8f3b).

Files with missing lines Patch % Lines
giga/executor/utils/errors.go 50.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3768      +/-   ##
==========================================
- Coverage   59.98%   59.02%   -0.97%     
==========================================
  Files        2291     2201      -90     
  Lines      190730   180267   -10463     
==========================================
- Hits       114411   106404    -8007     
+ Misses      66114    64506    -1608     
+ Partials    10205     9357     -848     
Flag Coverage Δ
sei-chain-pr 59.93% <71.42%> (?)
sei-db 70.41% <ø> (ø)
sei-db-state-db ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
app/app.go 71.52% <100.00%> (-0.05%) ⬇️
giga/executor/utils/errors.go 50.00% <50.00%> (ø)

... and 119 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bdchatham bdchatham left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a live bugfix, not just defense: the new test fails on main at receipt parity (giga stamps TxType=2, gasUsed=gasLimit; v2 writes the EndBlock synthetic receipt with zeros) while consensus fields match. State the red-proof in the description so the severity (RPC-level receipt divergence on mixed fleets) is on record.

Comment thread giga/tests/giga_test.go Outdated
Comment thread app/app.go Outdated
Comment thread app/app.go
Co-authored-by: FromTheRain <bdchatham@users.noreply.github.com>
@bdchatham

Copy link
Copy Markdown
Contributor

Live mixed-fleet validation on harbor

Validated this PR end-to-end on an ephemeral harbor chain: 4 validators + one v2 RPC node (giga_executor.enabled=false, occ_enabled=false) on nightly-20260715-65411b1 (= #3753 merge, pre-this-PR), plus one giga+OCC RPC node swapped between the pre-fix image and sei/sei-chain:pr3768-6ab6c68 (built from head 6ab6c68 via ecr.yml workflow_dispatch).

Probe: EIP-7623 floor-data-gas tx (1000 zero calldata bytes, gasLimit 27500 — clears the ante's intrinsic check, fails inside geth's Execute()), exercised via qa-testing's tests/chain_tests/floorDataGas.spec.ts (sei-protocol/qa-testing#143/#145).

Pre-fix giga node — divergence reproduced. Same tx hash, eth_getTransactionReceipt on both nodes:

field giga (pre-fix) v2
gasUsed 0x6b6c (gas limit) 0x0
cumulativeGasUsed 0x6b6c 0x0
effectiveGasPrice 0x165a0bc00 0x0
status / type 0x0 / 0x2 0x0 / 0x2

Consensus held throughout (nonce, sender balance, and fee charge matched v2 on both executors) — receipt-only divergence, invisible to the AppHash/LastResultsHash gate, per the RCA.

This PR's image — fixed. Giga node on pr3768-6ab6c68, peered against unfixed validators and the unfixed v2 node:

  • qa-testing floor-data-gas spec: 4/4 passing (was 3/4, failing on exactly gasUsed 27500 vs 0)
  • Fresh same-tx cross-node diff: receipts byte-identical{status: 0x0, gasUsed: 0x0, cumulativeGasUsed: 0x0, type: 0x2, effectiveGasPrice: 0x0}
  • Fallback observable on the node's metrics: sei_chain_app_giga_fallback_to_v2_total{reason="execution_failed", scope="batch"} 2 — one per probe tx, scope="batch" as expected under OCC
  • Node synced ~4,300 blocks produced under the old image and stayed in consensus with the unfixed fleet

The qa-testing spec now guards this class continuously: it locks the canonical v2 receipt shape at the RPC surface, so a regression on any executor fails it regardless of which node serves the query.

@bdchatham

Copy link
Copy Markdown
Contributor

Live A/B validation on harbor (ephemeral chains, 4 v2 validators + 1 RPC follower with giga_executor.enabled/occ_enabled=true from birth, qa-testing parity spec tests/chain_tests/floorDataGas.spec.ts @ b43a61ed):

  • Pre-fix (ca3bcfca, giga executor ENABLED — attested via seid log): parity spec red — failing EIP-7623 floor tx got a receipt with gasUsed = 27500 (= gas limit) where v2 canon writes 0. Receipt-exists / nonce-bumped-once / full-fee assertions all green.
  • This PR's head (6ab6c68c, same topology, executor ENABLED): 4/4 green — canonical v2 failure receipt served from the giga executor path.

Same spec, image, topology, and platform on both legs; only the seid commit differs. Confirms the divergence reproduces on a live giga-serving node and that this PR resolves it to v2 canon.

🤖 Generated with Claude Code

@philipsu522
philipsu522 marked this pull request as ready for review July 17, 2026 15:21

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR correctly fixes a real receipt-parity divergence by routing Giga state-transition errors (e.g. EIP-7623 floor-data-gas) through the existing v2 fallback instead of duplicating v2's fee/nonce/receipt/deferred-info handling. The change is safe, consistent with existing abort-fallback patterns, and adds a strong cross-mode (v2 / Giga-sequential / Giga-OCC) parity test; the only notable point is the operational tradeoff of whole-batch OCC fallback, which the author already documents.

Findings: 0 blocking | 6 non-blocking | 2 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • Both second-opinion review files were empty: codex-review.md (OpenAI Codex) and cursor-review.md (Cursor) produced no output, so this synthesis reflects only the direct review.
  • Correctness is sound: ExecutionFailedAbortError implements IsAbortError(), so ShouldExecutionAbort recognizes it via the vm.AbortError cast, and both the sequential (line ~1627) and OCC (line ~2330) callers already handle abort errors by re-running the tx through v2. Since stateDB.Finalize() is not called before the early return, defer stateDB.Cleanup() discards the pre-charged-fee cache — matching the existing self-destruct fallback precedent, so no stale giga state leaks into the v2 re-run.
  • Net simplification: removes ~60 lines of hand-rolled v2-mirroring logic (nonce bump, Finalize, WriteReceipt, AppendToEvmTxDeferredInfo) that had to be kept in lockstep with v2, replacing it with a single authoritative fallback. This is a maintainability win and eliminates a class of future drift bugs.
  • The rewritten TestGiga_FailedExecutionFallsBackToV2 is a meaningful upgrade — it asserts parity on deterministic ABCI fields, LastResultsHash, nonce, receipt, and sender/fee-collector/coinbase balances across v2, Giga-sequential, and Giga-OCC — but it only covers the EIP-7623 floor-data-gas trigger. Consider a follow-up covering at least one other Execute()-path failure to guard the comment's claim that 'other Execute() failure classes are pre-checked upstream.'
  • 2 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread app/app.go
// fee, nonce, receipt, and ABCI result behavior lives in v2. The
// stateDB has not been finalized, so Cleanup discards its
// transaction-local cache before the caller re-runs the tx through v2.
return nil, gigautils.ErrExecutionFailed

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Correctness here is fine — falling back to v2 for any execErr is safe since v2 is authoritative. Worth flagging the performance/DoS tradeoff the PR body already notes: in OCC mode a single execution_failed abort reverts the entire EVM batch to v2 (scope="batch" at app.go:1861). This is the first deterministic single-tx trigger (passes CheckTx at ~27.5k gas but fails floor-data-gas) that can force a full batch fallback, so an attacker could cheaply negate Giga's parallelism by seeding batches with such txs. The author acknowledges this and defers per-tx OCC fallback to follow-up; please ensure the app_giga_fallback_to_v2{reason="execution_failed"} alert is actually wired up before/at rollout.

Comment thread app/app.go
GasWanted: int64(ethTx.Gas()), //nolint:gosec
Log: fmt.Sprintf("giga executor apply message error: %v", execErr),
}, nil
// EIP-7623's floor-data-gas check is the known natural failure here:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] nit: the comment asserts "other Execute() failure classes are pre-checked upstream" so EIP-7623 floor-data-gas is "the known natural failure here." The fallback is safe regardless of which class triggers it, but if this assumption ever drifts (e.g. a new geth Execute() error path that upstream checks miss), the only signal will be an uptick in the fallback metric. Consider logging the underlying execErr at debug level before returning so unexpected triggers are diagnosable.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants