Feat/evm confirmation and reorg - #230
Open
ALLEN-AYODEJI wants to merge 2 commits into
Open
Conversation
Adds octo-evm-core (BIP-32/BIP-44 derivation, EIP-55 checksums) and a minimal octo-chain ChainAdapter, a migration adding chain_kind/derivation columns to wallets/addresses, Store::allocate_evm_address (same row-lock atomicity as the Stellar path), and chain-aware API responses. Docs updated for the EVM deposit model and the xpub/sibling-key risk. Refs Octo-Protocol-org#220
Stellar deposits credit on sight because Stellar finality is instant. EVM blocks reorg, so crediting on sight would let an attacker deposit, get credited, force/exploit a reorg, and withdraw against a balance that no longer exists. This closes that window. Store layer (migration 0022, already on this branch): - transactions gain confirmation_state (detected/confirming/confirmed/ orphaned), evm_tx_hash/log_index/block_number/block_hash/ confirmations/orphaned_at; wallets gain confirmation_depth/ reorg_rewind_bound (per-wallet, not hard-coded). - record_evm_deposit, confirming_evm_transactions, progress_evm_confirmation, orphan_evm_deposits_from_block, and evm_block_headers/ingest_cursor helpers for hash-chained reorg detection. - Fixed wallets_due_for_poll to filter chain_kind = 'stellar' (it had no chain_kind filter at all, so an EVM wallet sharing the same `network` value would already have been handed to the Horizon poller); added the EVM analogue, evm_wallets_due_for_poll. Ingest layer (new this commit): - crates/ingest/src/evm_rpc.rs: a minimal eth_getBlockByNumber-only JSON-RPC client (no alloy/ethers, per the workspace's narrow- primitives ADR), with the same retry/circuit-breaker treatment as the existing Horizon client. - crates/ingest/src/confirmation.rs: ConfirmationTracker (one per EVM wallet) and EvmSupervisor (fans it out, mirroring Supervisor). Each tick: (1) extends the wallet's verified block-hash chain from its cursor to the current tip, detecting a reorg via parent-hash continuity rather than a number-only comparison, with a bounded backward search for the last common ancestor (reorg_rewind_bound) — hitting the bound alerts instead of guessing or looping; (2) recomputes confirmations for every still-accumulating deposit and promotes it to confirmed (spendable) at confirmation_depth, firing deposit.confirmed; a reorg instead marks affected rows orphaned (never deleted) and fires deposit.orphaned. - Wired into bin/server behind an optional EVM_RPC_URL (no default — unlike Horizon there's no sane public default to fall back to; the tracker simply doesn't start if unset). Audited every existing balance/aggregate query: sum_deposits_for_address(es) already filter status = 'confirmed', so an EVM deposit is invisible to them for free until this tracker promotes it — no query changes needed. Tests: - crates/ingest/tests/confirmation_tests.rs runs against a real Anvil node (evm_snapshot/evm_revert to force genuine reorgs, not mocked ones): progressive confirmation counting + promotion at exactly the configured depth + not spendable below it; the acceptance scenario (deposit reaches confirmed, a reorg reverts its block, it's marked orphaned, balance drops, deposit.orphaned fires); a reorg deeper than reorg_rewind_bound alerts without touching state. - crates/store/tests/store_tests.rs: record_evm_deposit dedups on (evm_tx_hash, log_index), so a post-reorg rescan re-detecting a survived transaction can't double-credit it. Docs: docs/deposit-model.md gets a "Confirmation depth and reorg handling" section (per-chain depth guidance, the state machine, and why an already-withdrawn deposit can't be reorged away); docs/threat- model.md gets the corresponding threat rows and known-limitations entries (depth is an operator trade-off, the header window is finite). Refs Octo-Protocol-org#222
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
1 participant
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.
Description
This is the highest-risk issue in the epic. Octo currently credits a deposit the moment it sees
it, because on Stellar that is correct: a transaction_successful payment is final and the ledger
does not reorganise. That assumption is wired into the design — see the guarantees at the top of
crates/ingest/src/lib.rs and
crates/store/src/lib.rs.
On EVM, blocks reorg. A deposit seen at block N can vanish. If Octo credits on sight, an attacker
can deposit, get credited, force or exploit a reorg, and withdraw against a balance that no longer
exists. Even absent an attacker, ordinary 1–2 block reorgs happen routinely on L1 and L2s.
Introduce a confirmation state machine and a reorg detector that can reverse a credit.
Requirements and context
Deposits move through explicit states: detected → confirming → confirmed → creditable.
Only confirmed funds are spendable. A separate orphaned terminal state records reversals for
audit — never delete the row; the ledger is append-only and reversals must be visible.
Confirmation depth is per chain, configured in #216. Ethereum L1 and an L2 with a centralised
sequencer have very different risk profiles, and L2s can have deep reorgs on sequencer
failover. Do not hard-code a number.
Reorg detection: store the block hash alongside the block number for each processed block, and
on each poll verify the parent hash still matches. A number-only cursor cannot detect a reorg —
the same height with a different hash looks identical.
On reorg: rewind the cursor to the last common ancestor, mark affected deposits orphaned, emit a
reversal webhook, and re-scan. Rewinding must be bounded — an unbounded rewind on a malicious RPC
is a DoS.
Security: the window between crediting and finality is the exploitable window. State clearly
in the threat model what depth is used per chain and what that implies. Consider whether an
orphaned deposit that was already withdrawn against is possible, and what the system does about
it — an honest "this is prevented by requiring N confirmations before spendability" is the
expected answer.
Consider finalized / safe block tags (post-Merge) as a stronger signal than depth counting
where the provider supports them.
Suggested execution
Branch: feat/evm-confirmation-and-reorg
Implement changes
Migration 00NN_deposit_confirmations.sql: add confirmation_state, block_number,
block_hash, confirmations, and orphaned_at to transactions; add a partial index over
rows still confirming, since that is the hot query.
Add a confirmation tracker that re-checks confirming deposits each tick, promotes them at depth,
and emits deposit.confirmed.
Add reorg detection via parent-hash chaining, with a bounded rewind depth (configurable, default
≥ 2× confirmation depth) and a loud alert if the bound is hit.
Add reversal: mark orphaned, adjust balances, emit deposit.orphaned via
octo-webhooks.
Ensure balance queries only sum confirmed rows. Audit every existing balance/aggregate query
for this — one missed query makes unconfirmed funds spendable and defeats the whole issue.
Test and commit
Reorg integration test using Anvil snapshot/revert (#219): deposit at block N, confirm it, force
a reorg, assert the deposit is marked orphaned, the balance is reduced, and the webhook fires.
This is the acceptance test for the issue.
Test that a deposit below confirmation depth is not spendable — attempt a withdrawal against
it and assert rejection.
Test progressive confirmation counting and promotion at exactly the configured depth.
Test deep-reorg bounding: a reorg deeper than the rewind bound alerts rather than looping.
Test that re-scanning after a reorg re-detects a transaction that survived, without
double-crediting.
Test that Stellar deposits are unaffected and still credit immediately.
Update docs/threat-model.md and docs/deposit-model.md
with the per-chain depths and the reasoning behind each.
Example commit message
feat(ingest): confirmation depth and reorg handling for EVM
Stellar finality is instant, so Octo credits on sight. EVM blocks
reorg, so crediting on sight would let an attacker deposit, withdraw,
and reorg away the deposit.
Deposits now progress detected → confirming → confirmed, with only
confirmed rows spendable, and reorgs are detected by parent-hash
chaining (a number-only cursor cannot see a same-height different-hash
reorg). Affected deposits are marked orphaned and never deleted.
Closes #222