[Version-unspecific] fix(bifrost): UTXO and observer hardening

Summary

Part 2 of the V136 chain-integration stack. Tracking issue: #325. This MR targets v136/00-chain-activation; review only the commits below. The stack table is in the 00 MR.

Bifrost-only fixes for the UTXO clients and the observer, ported from THORNode where THOR has them. No x/ code changes (one new x/mayachain test file). The one common/ change (MakeEVMGas) has only Bifrost callers.

Commits

Commit What
d7a0d6ff4 Mempool observations are never posted as final (from network version 1.136.0)
947d99e26 Reject bad prevout indexes in BTC, DASH, ZEC; tests for the bare-multisig sender check
72c2a3221 NULLFAIL is an error, not a signed tx (BTC, DASH)
979e50405 Zero EVM gas rounds up to one unit
a8f42c6b4 Drop invalid observations one at a time instead of losing the chunk
fd4ad559f Honour HALTSIGNING in BTC and DASH consolidation

Changes

Mempool observations are never final (observe.go)

Port of THOR 3886d30b3 and 42b7146a8, adapted.

A MAYA mempool deck has ConfirmationRequired == 0, so BlockHeight == FinaliseHeight and the observation is final on chain. A vault spend seen in the BTC mempool completed the outbound before it was mined. BTC OnObservedTxIn then marked the txid as reported, so the block scan skipped it.

  • sendDeck posts the vault-spend side of a MemPool deck once as not final (FinaliseHeight = BlockHeight + 1), without OnObservedTxIn. MAYAChain stores the votes and ObservedLink without completing the outbound, so LackSigning keeps it on the vault that paid. The block scan reports the tx as final once mined.
  • ObserveSigned no longer caches the txid in signedTxOutCache, so nodes with auto_observe=true observe their own outbounds from the block.
  • Gated in Bifrost at network version 1.136.0 (mempoolNotFinalVersion). Old binaries post the mempool observation and skip the block one; switching per binary would split the vote. On a version read error the mempool deck is held until the version is known, and ObserveSigned does not cache the txid. No state effect, so the MR stays [Version-unspecific].
  • Effect: BTC outbounds and internal txs complete about one block later. A tx dropped from the mempool no longer completes an outbound, but its pending payment still keeps LackSigning from moving the item to another vault.

Sender hardening (BTC, DASH, ZEC client.go)

  • Develop already rejects bare-multisig senders (THOR 928fbdfa9). This adds tests for it.
  • New prevOutput(vinTx, idx) in getSender and getGas returns an error for a nil tx, an index past the last output, or an output whose n differs from its index. Before, the first two panicked and the third was attributed silently. THOR has the same unchecked index.
  • ZEC getPrevoutOutput already bounded the index; it now rejects an n mismatch too.
  • BTC/DASH getGas return an error when outputs exceed inputs. Before, the fee wrapped to a huge uint64.
  • Not gated: honest txs give the same result; only malformed txs stop being observed, so no vote conflicts with old nodes.

NULLFAIL is an error (BTC, DASH signer.go)

verifySignedInput runs the script engine on each signed input and returns every failure. Before, NULLFAIL returned nil and the tx could be broadcast. NULLFAIL is logged CRITICAL and counted in signer_errors_total{error_name="script_verify_nullfail"}. ZEC signs through librustzcash and has no engine step.

Zero EVM gas (common/gas.go)

MakeEVMGas rounds 0 up to 1 unit (THOR e0b9b8901, gas half). Observations this changes were all rejected with code 6 before, so there is no vote to split.

Per-item observation validation (observe.go)

getMayachainTxIns drops and counts (observer_errors_total{error_name="invalid_observation"}) any item that fails ObservedTx.Valid or does not have the vault as sender or recipient. Before, one item without gas failed the whole MsgObservedTxIn with code 6 and up to 99 other items were lost, and a sender/recipient mismatch re-queued the chunk forever. broadcast.go is unchanged: with every item validated first, code 6 can only come from the message itself.

HALTSIGNING for consolidation (BTC, DASH)

Consolidation builds its own TxOutItem and never passed the sign.go halt check. utxo.IsSigningHalted checks HALTSIGNING and HALTSIGNING<CHAIN>; on a halt or read error consolidation waits for the next block. ZEC already had this. During a mixed-version window old nodes may still try to consolidate under a halt and blame new ones (slash points only); ZEC's gate has the same property.

Versioning / consensus impact

None on chain. The 1.136.0 gate for mempool decks is in Bifrost and reads the network version.

Tests

Each fix was tested against the pre-fix code first (fails), then with the fix (passes).

Fix Tests
Mempool observe_hardening_test.go: TestMempoolDeckNeverPostedAsFinal, TestObserveSignedAllowsBlockObservationAfterActivation, TestMempoolDeckLegacyBeforeActivation, TestBlockDeckStillPostedAfterActivation
Sender {bitcoin,dash,zcash}/sender_hardening_test.go: bare multisig, bad prevout index, getGas bad prevout, single-sig P2PKH/P2SH still accepted
NULLFAIL {bitcoin,dash}/nullfail_test.go: a real signature invalidated by changing an output
Gas common/gas_test.go TestETHGasFee
Per-item TestInvalidObservationDroppedIndividually
Halt shared/utxo/halt_test.go, {bitcoin,dash}/consolidate_halt_test.go

make test, make test-network-specific, gofumpt, vet and lint-versions pass. Regression: no export changes; the full suite on the stacked chain branches includes this code.

Deferred

  • DASH SignTx signs inputs in parallel on one shared wire.MsgTx (pre-existing data race under -race). Fix in a later MR.
  • Once the 1.136.0 gate is old, remove the legacy (final) mempool path and the then-dead signedTxOutCache / allowFutureObservation.
  • A bare-multisig-funded inbound strands funds without a refund, the same as THOR. To document in the inbound notes.

Review follow-up (round 4)

  • b0f8d3f27: mempool vault spends (BTC mempool scan, signer auto-observation) are posted once as not final from 1.136.0, instead of being dropped, so LackSigning does not move a pending payment to another vault. The block scan still posts the final observation. Tests: TestMempoolDeckNeverPostedAsFinal, TestAutoObservationPostedNotFinalThenBlockFinal, TestMempoolInternalTransferPostsVaultSpendOnly, TestMempoolDeckRequeuedWhenSendFails, and ObservedTxOutNotFinalSuite in x/mayachain/handler_observed_txout_not_final_test.go.
  • 67dcb9f5e: a failed network-version query holds the mempool deck and skips signedTxOutCache, instead of falling back to posting it as final. Tests: TestMempoolDeckHeldWhileVersionUnknown, TestObserveSignedNotCachedWhileVersionUnknown.
  • The observe slash point of a not-final outbound vote is given back once those votes reach consensus, in the V136 outbound preflight on !889 (839de26f5).
Edited by Bitol Maya

Merge request reports

Loading
Loading