[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.
sendDeckposts the vault-spend side of aMemPooldeck once as not final (FinaliseHeight = BlockHeight + 1), withoutOnObservedTxIn. MAYAChain stores the votes andObservedLinkwithout completing the outbound, so LackSigning keeps it on the vault that paid. The block scan reports the tx as final once mined.ObserveSignedno longer caches the txid insignedTxOutCache, so nodes withauto_observe=trueobserve 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, andObserveSigneddoes 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)ingetSenderandgetGasreturns an error for a nil tx, an index past the last output, or an output whosendiffers from its index. Before, the first two panicked and the third was attributed silently. THOR has the same unchecked index. - ZEC
getPrevoutOutputalready bounded the index; it now rejects annmismatch too. - BTC/DASH
getGasreturn an error when outputs exceed inputs. Before, the fee wrapped to a hugeuint64. - 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
SignTxsigns inputs in parallel on one sharedwire.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, andObservedTxOutNotFinalSuiteinx/mayachain/handler_observed_txout_not_final_test.go.67dcb9f5e: a failed network-version query holds the mempool deck and skipssignedTxOutCache, 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).