[Version-unspecific] fix(bifrost): EVM client hardening

Summary

Part 3 of the V136 chain stack (00 to 06), tracked in #325. This MR targets v136/01-bifrost-utxo-hardening. Review only the commits on this branch; the parent branches have their own MRs.

This branch makes the generic EVM client (bifrost/pkg/chainclients/evm, used by ARB, AVAX and, from branch 03, BSC) and the ETH client (bifrost/pkg/chainclients/ethereum) safe to run a new chain on. Before it, any EVM chain other than ARB/AVAX hit logger.Fatal on its first observed deck. Most fixes are THORNode ports. The rest close gaps that THORNode also has.

Bifrost and config only. Nothing under x/, common/ or app/ changes, so there is no regression export churn.

Changes

Finality and confirmations

  • c0b5bc4a8 Replace logger.Fatal with a per-chain finality rule (evm/confirmations.go). ARB/AVAX keep built-in instant finality. Any other chain must configure finality (instant | confirmations | finalized) and confirmations, or the client refuses to start. Readiness failures hold the deck instead of exiting. The ETH client no longer fails open to 0 confirmations; it keeps the 2-block floor.
  • 47c795bae The finalized tag probe falls back to N confirmations only on a definite "unsupported" answer (-32601, -32602), logged at warn. Transient errors (-32005, -32000, transport) are retried 5 times and then fail startup. Before, one transient error at startup weakened finality until restart.
  • 1072b7be3 Off mocknet, a chain with a built-in rule cannot be configured to a different confirmations value. That value becomes ConfirmationRequired, which sets the voted FinaliseHeight, so a node with another value never matches the others. A mismatch refuses to start; it is never silently replaced.

Observation

  • 0e6021734 Recover EIP-7702 (type 4) senders with the Prague signer; ARB keeps its system-tx wrapper. The skip log is raised to error.
  • 510b71470 Observed gas uses receipt.EffectiveGasPrice from network version 1.136.0 (tx.GasPrice() before, so mixed-version validator sets vote the same). The gas oracle samples effective prices, excluding nil and zero, only after a block succeeds. The gas cache is copy-safe. A zero observed gas rounds up to 1 unit.
  • b475c4e56 Every credited Deposit event must come from the target vault's own router, else the tx is ignored. Uses the log emitter, not tx.To(), so aggregator deposits still pass. Migrations and vaults without a router are not checked.
  • 0a1e8776c Token decimals come from the token list; decimals() is only cross-checked. A failed lookup is never cached (it used to cache 18 forever). Cached entries that disagree with the list are healed.
  • 8bb0da786 Missing, extra or mismatched receipts, and a failed MAYAChain version lookup, fail the whole block so the scanner retries it. Before, the tx was dropped and the block marked scanned.
  • 1362fa5d1 A non-router tx is observed only when it is to or from a vault; a tx with an unrecoverable sender is ignored. Before, every native transfer went into block meta and was re-checked over RPC on each reorg. BSC is the first EVM-client chain with a reorg window (600 blocks), where this stalls the scanner for minutes.
  • 3b1ae989e The token-symbol sanitiser removes ., -, +, *, # instead of mapping them to characters NewSymbol rejects. Such whitelisted tokens (for example ARB CEX-AI, USD+) could never be credited. No live pool uses one, so no asset string in state changes.
  • 2f9f66516 A failed-outbound gas observation retries the block when the version lookup fails, instead of being dropped.

Signing

  • 019b662dd Chain-id pin. The expected id comes from expected_chain_id or a built-in table (mainnet/stagenet: ETH 1, ARB 42161, AVAX 43114; mocknet: 1337, 412346, 43112). A chain with neither, or an RPC reporting another id, refuses to start. The keysign wrapper uses the pinned id, refuses a typed tx for another chain, and checks EIP-155 protection and the recovered sender after signing.
  • e71d7f313 Unstuck honours HALTSIGNING / HALTSIGNING<CHAIN> (new shared/signing, MAYA semantics: any positive value halts). A mimir read error counts as halted for that pass.
  • a0b5b385a Unstuck clears the signer cache entry when the outbound is no longer known to the node, so a rescheduled outbound can be signed again.
  • 3f80205e4 SignTx refuses an item with empty Coins instead of panicking on Coins[0].

Solvency and network fee

  • 757f2875b Token balances are read from each vault's own router (vault.Routers, consensus data), not GetContracts(chain)[0]. A token balance without a router is an error.
  • 6314e89cf A stale network fee is rebroadcast hourly instead of daily.

Tests only

  • af9fdd818 On the 1.136.0 stack the ETH type-4 fixture is observed at the receipt effective gas price.

Versioning / consensus impact

All commits are [Version-unspecific]. No handler, keeper, manager or constant changes.

One behaviour is gated inside bifrost: observed gas switches to the receipt effective gas price at network version 1.136.0 (evm.EffectiveGasPriceVersion). Observed gas is voted on, so binaries must agree during a rolling upgrade. THORNode gated e0b9b8901 the same way.

THORNode references and parity

Ported or adapted:

  • 46e986bbb, 4a8c22818: confirmation rules. MAYA uses a config table with a finalized mode instead of hardcoded counts.
  • 6a0ac1a08: Prague signer.
  • c873ffe50, ETH half of e0b9b8901: effective gas price (version-gated).
  • c61ca97af, aa6b1cfcd: router-origin check and per-vault router balances. MAYA checks every credited Deposit (THOR checks the first) and keeps inactive vaults' routers, because MAYA lists inactive vaults only while they can still be refunded from.
  • 3e6436bc0: vault asset set, already in MAYA since V133.
  • b5b11633e: unstuck HALTSIGNING, with MAYA's non-height semantics.
  • 0fced2f7b4: vault-only observation.
  • cc8097283b: re-sign dropped outbounds.
  • 89c6fc0f64: symbol sanitiser.
  • 5294ac17fc: empty-coins guard.
  • 1df4d86490: hourly fee rebroadcast (cadence only; the "fee is 0" branch 4eba8849f2 is not needed, reportNetworkFee already posts the first fee).

MAYA fixes THORNode does not have: chain-id pin, receipt completeness (including the block hash), token-list decimals, pinned built-in confirmations, narrowed finalized-tag fallback, failed-outbound retry, recoverable reorg rescans.

Handled in separate MRs on develop: signer-cache expiry (THOR a759cb4f99, 9f17b66f8d) in !880 (merged), and EVM RPC bearer/basic auth (THOR 337d8f9317) in !881 (merged).

Tests

  • Unit tests for every fix: evm/confirmations_test.go, signer_test.go, gas_observation_test.go, vault_router_test.go, receipt_completeness_test.go, vault_relevance_test.go, unstuck_dropped_test.go, empty_coins_test.go, failed_outbound_test.go; the same set in ethereum/; shared/evm tests for gas price, deposit router, token decimals, chain id, receipts and symbols; shared/signing/halt_test.go; blockscanner TestNetworkFeeCheckBlocks.
  • make test, go vet, gofumpt and lint-versions, lint-handlers, lint-managers, tools/analyze pass on this branch.
  • Regression suite: not affected (no x/ or common/ change). The full suite and the BSC mocknet simulation run on branch 03 with these fixes underneath.

Deferred / open items

  • Scanner prefetch and concurrent receipt processing for 0.45 s blocks (THOR bd66c5254, f6abd5730, 45775159af). Measure BSC scan lag on a mainnet node first.
  • signer/sign.go is not moved onto shared/signing, to keep this MR small.
  • A transient symbol() RPC failure on a whitelisted token's first sighting still skips that coin on this node.
  • ConvertAmount still assumes 18 decimals when metadata cannot be loaded (non-whitelisted tokens only).

Operator notes

  • New per-chain keys: finality, confirmations, expected_chain_id (env BIFROST_CHAINS_<CHAIN>_...). ETH, ARB and AVAX need no change: their built-in values apply.
  • An RPC endpoint for the wrong chain now stops the client at startup instead of signing for another chain.

CI follow-up

  • 6a25d214f clears the trunk check findings on this branch: else if in the blockscanner fee check (gocritic), test renames for gosec G101 false positives, shadowed err in two EVM tests. No behaviour change.
  • a1e5e91ac raises the RPC timeout in the two new token-decimals tests from 1 s to 10 s. TestTokenManagerDecimals hit context deadline exceeded once on a loaded CI runner; the stub server answers immediately.

Review follow-up (round 4)

  • a447d0ca3: a receipt must also carry the block's hash; one from another fork's block at the same height fails the block. Skipped for AVAX, whose hash go-ethereum cannot recompute.
  • eaa1c74a2: unstuck cancels an outbound that is a full signing period old (already rescheduled) on the first pass, so a signing halt that outlasted a period no longer delays the cancel to the next buffer window.
  • 9f2b76235: the generic EVM scanner checks for a reorg on empty blocks too; an empty first block after a reorg used to hide the replaced parent.
  • bbf730100: reorg rescans are stored as pending before the new block hashes and rescanned by every block until one delivers them. Failures retry the block, and restarts keep them. EVM and ETH scanners; the reorg tests run against a mock node in shared/evm/evmtest.

Follow-up after review round 4

  • 06a5c8994: reprocessTxs declares err at function scope since the round-4 change, so the getHeader result gets its own name (govet shadow, flagged by trunk check --all in CI). No behaviour change.

Follow-up after review round 6

  • 0bc33a842: ETH and EVM SignTx no longer charge a re-signed outbound for its own old copy: the reservation at the signing nonce is left out, and a nonce keeps one reservation (the larger). Tests: liability_retry_test.go in both clients.
  • c495bdeae: ErrRetryBlock wraps ErrRetryableBlockScan, so receipt failures take the scanner's retryable branch. Tests: receipt-completeness tests assert IsRetryableBlockScanError.
  • 7cbfa99de: the deposit router check covers only the Deposit events the parser credits (GetTxInItemAndDepositRouters), so an ignored event from another router no longer rejects a valid deposit. Tests: deposit_router_test.go, deposit_router_scan_test.go in both clients.

Follow-up after review round 7

  • bb1f981fc: native ETH/ARB/AVAX deposits are credited whichever known router took them, because every MAYA router forwards msg.value to the vault before emitting Deposit. Only ERC-20 deposits, which stay in the router's allowance, are router-checked. This differs from THORChain on purpose. The router tests now cover ERC-20 in the parser and in both scanners.
  • b50863d2f: the ETH scanner samples gas only after the block was processed, and the EVM scanner only after MarkRescanDelivered, so a retried block is sampled once. Test: TestRetriedBlockSampledOnce in both packages.
Edited by Bitol Maya

Merge request reports

Loading
Loading