[rasca]

Summary

ConsensusWorker.claim_next_appeal hand-builds the appeal transaction dict from a raw UPDATE ... RETURNING whose column list omitted transactions.contract_snapshot and transactions.consensus_history. Every claimed appeal was therefore rebuilt with contract_snapshot=None and an empty consensus_history. On a successful validator appeal of a non-deploy write transaction, process_validator_appeal then "restored" the contract's accepted state to {} — deleting the contract's storage including the code slot — after which the appeal re-run fails with invalid_contract absent_runner_comment and subsequent appeals self-confirm the error.

This PR adds the two columns to the claim query (root-cause fix) and hardens the rollback in process_validator_appeal so a missing snapshot can never again translate into a state wipe (defense in depth). Unit tests cover both layers.

Root cause

backend/consensus/worker.py:396-406 — the RETURNING list of claim_next_appeal selected 25 columns but not transactions.contract_snapshot or transactions.consensus_history, and the dict built from the row (worker.py:424-455) consequently had neither key.

That dict is passed to Transaction.from_dict in process_appeal (backend/consensus/worker.py:1794). from_dict treats both fields as optional (backend/domain/types.py:284 defaults consensus_history to {}; backend/domain/types.py:287-289 maps a missing contract_snapshot to None via ContractSnapshot.from_dict), so the omission produced no error — just a silently degraded Transaction on every appeal.

Note the columns exist in the DB and are populated at acceptance time; only this one claim query dropped them on the floor. The equivalent hand-built dict for fresh transactions (claim_next_transaction) is unaffected in practice because a pending transaction has no stored snapshot yet.

Impact

1. Contract state wipe (data loss). In process_validator_appeal, when a validator appeal succeeds the code restores the pre-execution contract state from transaction.contract_snapshot (backend/consensus/base.py:1196 region). With the snapshot always None, the pre-fix else branch set previous_contact_state = {} and wrote it via contract_processor.update_contract_state. The {} branch is only correct for deploy transactions (rolling back a deploy legitimately clears the account). For a RUN_CONTRACT write transaction, the contract's code lives inside states["accepted"] (see ContractSnapshot.extract_deployed_code_b64, backend/database_handler/contract_snapshot.py:83), so writing {} erased both storage and code. The subsequent appeal re-run then executed against a codeless account and the leader returned invalid_contract absent_runner_comment; the merged committee accepted that error result, and each later appeal drew fresh validators who re-executed against the already-wiped state, reproduced the same error, and voted agree — the corruption self-confirms and never heals.

Observed on a live localnet:

• Deploy tx 0xc68a8b2f7552deb2fb1cdd4afc337d3da254a67a09d427c096f109e0069c1531 — ACCEPTED, contract registered and working.
• Execute tx 0x23cb9ee07a6e4de6eef3e2da2f0f0eb3257f0414ae1c491693af4fc5e1967cde — ACCEPTED in round 0, then "Validator Appeal Successful", then the re-run leader errored with invalid_contract absent_runner_comment, the error outcome was accepted, and two further appeals failed while confirming the error.
• Post-mortem on the contract account: data.state.accepted = {} while data.state.finalized still contained the code slot — exactly the signature of update_contract_state(to_address, accepted_state={}) on the accepted branch only.

2. Biased appeal verification. The appeal committee is supposed to verify the original execution against the transaction's stored pre-execution snapshot. With contract_snapshot=None, TransactionContext.__init__ falls back to the snapshot factory, which loads the contract's current post-execution state from the DB. Appeal validators therefore re-executed the transaction on top of its own effects, systematically biasing votes toward disagree and making successful appeals more likely than the protocol intends — which is also what manufactured the wipe in consequence 1.

3. Validator re-draw. ConsensusAlgorithm.get_extra_validators (backend/consensus/base.py:1282) uses consensus_history to exclude validators and leaders already used in prior rounds. With the history always empty on the appeal path, appeal committees could re-draw already-used validators/leaders, weakening the appeal's independence guarantee.

The fix

Part 1 — root cause (backend/consensus/worker.py). Add transactions.contract_snapshot, transactions.consensus_history to the RETURNING list of claim_next_appeal and the two corresponding keys to the returned dict. The values are the raw JSONB payloads, the same shape TransactionsProcessor._parse_transaction_data produces and Transaction.from_dict already consumes on every other code path.

Part 2 — defense in depth (backend/consensus/base.py). The old rollback conflated "no snapshot" with "deploy rollback". The new logic in process_validator_appeal:

• snapshot present → restore states["accepted"] from it (unchanged);
• no snapshot and type == DEPLOY_CONTRACT → restore {} (unchanged deploy semantics, now explicit);
• no snapshot and non-deploy → re-fetch the transaction via transactions_processor.get_transaction_by_hash (which also hydrates snapshots archived to object storage, see _hydrate_archived_contract_snapshot) and restore from the re-fetched snapshot; if it is still unavailable, log an error and skip the restore entirely rather than write {}.

The second part exists because the state wipe is destructive and effectively unrecoverable once the error result is accepted, while the failure mode it guards against (a Transaction object built without its stored snapshot) has now happened once via this claim query and could be reintroduced by any future caller. Skipping the restore leaves the post-execution state in place — a recoverable inconsistency — instead of destroying the contract.

Test coverage

New file: tests/unit/consensus/test_appeal_state_restore.py (8 tests, mock-based, no DB or containers; follows the patterns of the existing tests/unit/consensus/test_appeal_snapshot.py).

• TestClaimNextAppealReturnsSnapshot — claim_next_appeal run against a mocked session: the returned dict contains contract_snapshot and consensus_history; the executed SQL's RETURNING clause selects both columns; and a claimed dict round-trips through Transaction.from_dict into a non-None contract_snapshot with the stored consensus_history (the exact properties the rollback and get_extra_validators depend on).
• TestValidatorAppealStateRestore — drives process_validator_appeal into the VALIDATOR_APPEAL_SUCCESSFUL branch with a mocked committing state and asserts all four restore branches: missing snapshot + non-deploy re-fetches from the DB and restores the stored state (never {}); missing snapshot + no DB row (or a row without a snapshot) skips update_contract_state entirely; missing snapshot + deploy still restores {}; in-memory snapshot restores directly without a DB round-trip.

Run (mirrors .github/workflows/unit-tests-pr.yml: Python 3.12, pip install -r backend/requirements.txt):

pytest tests/unit/consensus/test_appeal_state_restore.py -v

Local result: 8/8 pass; the full tests/unit/consensus/ directory passes (286 tests).

Blast radius

• Only the appeal claim path changes behavior. claim_next_transaction and the finalization claim query are untouched.
• The query change adds two JSONB columns to the RETURNING set of a query that claims at most one row per poll — negligible I/O. Snapshots archived out of the row come back as their stub and are handled by the existing consumers.
• Consumers of the dict are None-safe: Transaction.from_dict already handled missing/None values for both fields, so rows whose stored contract_snapshot is genuinely NULL behave exactly as before, except that the rollback now refuses to wipe state for them.
• Deploy-transaction rollback semantics are byte-for-byte identical.
• The new error log (missing snapshot on a non-deploy appeal rollback) should never fire after Part 1; if it does, it is surfacing a bug that previously destroyed contract state silently.

Reproduction notes for reviewers

On a localnet built without this patch:

1. Deploy a contract with a normal deploy transaction and wait for ACCEPTED (contract works, code slot present under data.state.accepted).
2. Send a separate write (execute) transaction to that contract and let it reach ACCEPTED — do not let it finalize. It must be a marginal/appealable acceptance so the appeal window is open.
3. File a validator appeal against the execute transaction and arrange for it to succeed (with the bug present, appeals are already biased toward success because the appeal committee verifies against post-execution state — consequence 2).
4. Observe: on "Validator Appeal Successful" the contract's data.state.accepted becomes {} (code slot gone; finalized still intact), the transaction re-runs and the leader errors with invalid_contract absent_runner_comment, the error result is accepted, and any further appeals fail while re-confirming the error.
5. With this patch, step 4 instead restores the pre-execution snapshot stored on the transaction row and the re-run executes normally.

🤖 Generated with Claude Code

[github-actions]

This PR targeted main, which is only the default/static branch.

I retargeted it to v0.123-dev, the active development branch. Pushes to v0.123-dev automatically fast-forward main.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 44d5d8ac-d5bc-43c4-9bf1-0623f1f0012c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[MuncleUscles]

Pushed 54903908 with the review follow-ups plus the architectural fix for this bug class:

Review findings applied

• Sibling omission (claim_next_transaction / claim_next_finalization had the same hand-maintained drift risk) — resolved structurally, see below.
• previous_contact_state → previous_contract_state.
• The skipped-restore path now also emits a consensus_event ERROR via msg_handler so it reaches monitoring, not just logs.

Architecture: shared claim-column manifest — all three claim queries now generate both their RETURNING clause and the row→dict conversion from one manifest (_TX_CLAIM_BASE_COLUMNS + opt-in groups). Query and dict physically cannot drift apart anymore. Heavy state columns stay an explicit opt-in (contract_snapshot has been observed at tens of MB — appeals need it, pending/finalization deliberately don't).

Same-class hunt found one more latent instance: PendingState.handle computed pre-effects from claim-built appeal flags (appeal_undetermined/appeal_leader_timeout) before its DB refresh — those silently default to False for re-processed appeals. Harmless today only because decide_pending_pre ignores its flag params; reordered refresh-first so nothing in the state can ever read pre-refresh claim data.

Class-killer test (test_claim_column_manifest.py): statically extracts every key Transaction.from_dict consumes and asserts each claim either returns the column or explicitly allowlists the omission with a rationale (allowlists are also checked for staleness both directions). Adding a from_dict field now breaks CI until someone makes a conscious per-claim decision.

1033 unit tests pass, black clean.