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.goandevm/client.gonow request the explicitpendingblock tag usingrpc.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.redeemSynthAssetToReservecommits the Asgard→Reserve transfer, pool balance and ragnarok marker together usingCacheContext. 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—HandleWsMessageindexedtickerMsg[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.GetAvailablePairshad 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-vaulttoLocalTSSSignergoroutines. Now mutex-guarded (concurrent misses collapse into one bridge query). NewTestGetVersionConcurrent; the xrp/tron cov tests sign concurrently again as an end-to-end-racecheck. - Data race:
bifrost/pkg/chainclients/evm/evm_block_scanner.go—getTxInOptimized's receipt goroutines all assigned the enclosingerr, 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 withConcurrency = 4under-race. - Crash:
evm/unstuck.goandethereum/unstuck.go—unstuckTxdereferenced a nilTxOutItem(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.goandethereum/ethereum_block_scanner.go— the*big.Intbranch usedabi.ConvertType(x, new(*big.Int)), which can never type-assert, so it always fell back to 18. Only reachable with an ABI declaringuint256(the bundled ERC20 ABIs declareuint8), so no live behaviour changes; values beyonduint64are now rejected rather than truncated.x/denommintToordering: the recipient is now validated beforeMintCoins. Defensive only:MsgMintTokens.ValidateBasicalready rejects bad recipients on both the tx path and wasmd's contract-dispatch path (handleSdkMessagecallsValidateBasic), and a failed message's writes are rolled back, so no reachable path changes consensus results.- Also:
bifrost/pkg/chainclients/loadchains.gostops 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-specificmainnet/stagenet/chainnetstep): pass, 90.0% total.test-race-bifrostequivalent (go test -raceover./bifrost/...): pass, including the concurrent xrp/tron keysign and evmConcurrency = 4receipt tests that exercise the race fixes.- golangci-lint (repo config): clean.
- Each fix's test was checked to fail with the production change reverted.