[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
c0b5bc4a8Replacelogger.Fatalwith a per-chain finality rule (evm/confirmations.go). ARB/AVAX keep built-in instant finality. Any other chain must configurefinality(instant|confirmations|finalized) andconfirmations, 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.47c795baeThefinalizedtag 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.1072b7be3Off mocknet, a chain with a built-in rule cannot be configured to a different confirmations value. That value becomesConfirmationRequired, which sets the votedFinaliseHeight, so a node with another value never matches the others. A mismatch refuses to start; it is never silently replaced.
Observation
0e6021734Recover EIP-7702 (type 4) senders with the Prague signer; ARB keeps its system-tx wrapper. The skip log is raised to error.510b71470Observed gas usesreceipt.EffectiveGasPricefrom 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.b475c4e56Every creditedDepositevent must come from the target vault's own router, else the tx is ignored. Uses the log emitter, nottx.To(), so aggregator deposits still pass. Migrations and vaults without a router are not checked.0a1e8776cToken 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.8bb0da786Missing, 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.1362fa5d1A 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.3b1ae989eThe token-symbol sanitiser removes.,-,+,*,#instead of mapping them to charactersNewSymbolrejects. Such whitelisted tokens (for example ARBCEX-AI,USD+) could never be credited. No live pool uses one, so no asset string in state changes.2f9f66516A failed-outbound gas observation retries the block when the version lookup fails, instead of being dropped.
Signing
019b662ddChain-id pin. The expected id comes fromexpected_chain_idor 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.e71d7f313Unstuck honoursHALTSIGNING/HALTSIGNING<CHAIN>(newshared/signing, MAYA semantics: any positive value halts). A mimir read error counts as halted for that pass.a0b5b385aUnstuck clears the signer cache entry when the outbound is no longer known to the node, so a rescheduled outbound can be signed again.3f80205e4SignTxrefuses an item with emptyCoinsinstead of panicking onCoins[0].
Solvency and network fee
757f2875bToken balances are read from each vault's own router (vault.Routers, consensus data), notGetContracts(chain)[0]. A token balance without a router is an error.6314e89cfA stale network fee is rebroadcast hourly instead of daily.
Tests only
af9fdd818On 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 afinalizedmode instead of hardcoded counts.6a0ac1a08: Prague signer.c873ffe50, ETH half ofe0b9b8901: 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" branch4eba8849f2is not needed,reportNetworkFeealready 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 inethereum/;shared/evmtests for gas price, deposit router, token decimals, chain id, receipts and symbols;shared/signing/halt_test.go;blockscannerTestNetworkFeeCheckBlocks. make test,go vet, gofumpt andlint-versions,lint-handlers,lint-managers,tools/analyzepass on this branch.- Regression suite: not affected (no
x/orcommon/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.gois not moved ontoshared/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. ConvertAmountstill assumes 18 decimals when metadata cannot be loaded (non-whitelisted tokens only).
Operator notes
- New per-chain keys:
finality,confirmations,expected_chain_id(envBIFROST_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
6a25d214fclears the trunk check findings on this branch:else ifin the blockscanner fee check (gocritic), test renames for gosec G101 false positives, shadowederrin two EVM tests. No behaviour change.a1e5e91acraises the RPC timeout in the two new token-decimals tests from 1 s to 10 s.TestTokenManagerDecimalshitcontext deadline exceededonce 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 inshared/evm/evmtest.
Follow-up after review round 4
06a5c8994:reprocessTxsdeclareserrat function scope since the round-4 change, so thegetHeaderresult gets its own name (govet shadow, flagged bytrunk check --allin CI). No behaviour change.
Follow-up after review round 6
0bc33a842: ETH and EVMSignTxno 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.goin both clients.c495bdeae:ErrRetryBlockwrapsErrRetryableBlockScan, so receipt failures take the scanner's retryable branch. Tests: receipt-completeness tests assertIsRetryableBlockScanError.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.goin 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 forwardsmsg.valueto 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 afterMarkRescanDelivered, so a retried block is sampled once. Test:TestRetriedBlockSampledOncein both packages.