fix: production bugs found by the unit-test coverage work

Summary

Production bugs found while raising unit-test coverage in !5179, split out so that MR stays test-only. Every fix here changes behaviour and comes with a test that fails on the previous code.

Stacked on !5179: this targets unit-test-expansion so the diff shows only the fixes (the tests that verify them live in the coverage test files that MR adds). Retarget to develop once !5179 merges.

Consensus-relevant: redeemSynthAssetToReserve atomicity (x/thorchain) and the x/denom mintTo ordering change state-machine code, so they need a coordinated release. This MR adds no proto changes.

Fixes

  • Startup liability reconstruction: both ethereum.go and evm/client.go now request the explicit pending block tag using rpc.PendingBlockNumber. Tests serve distinct pending/latest blocks and verify exact value-plus-maximum-gas liabilities, exclusion of stale/foreign transactions, readiness after startup reconstruction, no double-counting on retry, and not-ready when a pending transaction is missing.
  • Atomic synth redemption: NetworkMgr.redeemSynthAssetToReserve commits the Asgard→Reserve transfer, pool balance and ragnarok marker together using CacheContext. Injected send/pool-save failures must leave both module balances, the pool, marker and events unchanged; success, retry and balance-cap checks verify conservation.
  • Crash (P0): bifrost/oracle/providers/bitfinex.go — HandleWsMessage indexed tickerMsg[0]/[1] without a length check, so a websocket frame like [] or [42] panicked the unrecovered reader goroutine and killed bifrost. Now rejected like other malformed frames. GetAvailablePairs had the same unguarded index on REST ticker rows; also guarded.
  • Data race: bifrost/tss/tss_signer.go — KeySign.getVersion() read/wrote its version cache unsynchronised from the per-vault toLocalTSSSigner goroutines. Now mutex-guarded (concurrent misses collapse into one bridge query). New TestGetVersionConcurrent; the xrp/tron cov tests sign concurrently again as an end-to-end -race check.
  • Data race: bifrost/pkg/chainclients/evm/evm_block_scanner.go — getTxInOptimized's receipt goroutines all assigned the enclosing err, so one goroutine could see another's error and drop a valid inbound (BASE/BSC/POL, Concurrency > 1). The error is now goroutine-local; the cov test runs with Concurrency = 4 under -race.
  • Crash: evm/unstuck.go and ethereum/unstuck.go — unstuckTx dereferenced a nil TxOutItem (the selection loop already allows nil). Such an item can't be cancelled (the cancel is signed at the outbound's height). It is now kept for status polling (the record and its signer-cache entry stay until the tx is mined or leaves the mempool, which then releases the cache) instead of panicking the chain client; other records still proceed.
  • decimals() decoding: shared/evm/token_manager.go and ethereum/ethereum_block_scanner.go — the *big.Int branch used abi.ConvertType(x, new(*big.Int)), which can never type-assert, so it always fell back to 18. Only reachable with an ABI declaring uint256 (the bundled ERC20 ABIs declare uint8), so no live behaviour changes; values beyond uint64 are now rejected rather than truncated.
  • x/denom mintTo ordering: the recipient is now validated before MintCoins. Defensive only: MsgMintTokens.ValidateBasic already rejects bad recipients on both the tx path and wasmd's contract-dispatch path (handleSdkMessage calls ValidateBasic), and a failed message's writes are rolled back, so no reachable path changes consensus results.
  • Also: bifrost/pkg/chainclients/loadchains.go stops its retry ticker when the retry goroutine exits.

Removed after review: an inactive-vault MaxGas rebalance (it rewrote outbound amounts in TxOut EndBlock) was dropped. Review showed a requeued, already-broadcast refund could have its amount rewritten and then be signed again. The stall it targeted is tracked in confidential #3395; see also !5169 (merged) for the Bifrost-side UTXO approach.

Verification

  • make test-coverage-sum (mocknet unit tests plus the network-specific mainnet/stagenet/chainnet step): pass, 90.0% total.
  • test-race-bifrost equivalent (go test -race over ./bifrost/...): pass, including the concurrent xrp/tron keysign and evm Concurrency = 4 receipt tests that exercise the race fixes.
  • golangci-lint (repo config): clean.
  • Each fix's test was checked to fail with the production change reverted.

🤖 Generated with Claude Code

Edited by Son of Odin

Merge request reports

Loading
Loading