Multisig hub catch-up panics when the operation's change UTXO was spent while the wallet was behind - #102
Open
nklyy wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A multisig cosigner catching up on hub history panics in
get_change_utxo_idxwhen the replayed operation assigned its RGB change to a UTXO that was created and spent while that cosigner was not syncing:Through FFI bindings the panic aborts the host process, and since operations are processed sequentially the same operation is replayed on every restart: the wallet crash-loops with no recovery short of manual DB surgery. We hit this in production-like use (a wallet restored from an older copy catching up on hub history) on
v0.3.0-beta.28;masterpanics identically - the test in this PR is red on currentmaster.Mechanism
get_change_utxo_idxtreats its two arms asymmetrically:btc_changearm tolerates a missing TXO and inserts a placeholder row (exists: false);change_utxo_outpointarm does a baretxn.get_txo(outpoint)?.expect("should exist")(online.rs:2934).Nothing guarantees that row exists:
include_spent = false- unspent only - so an outpoint created and spent inside the wallet's offline window is never backfilled;save_transfershave a rescue path (FastSync+include_spent = true) for missing input TXOs, but the change outpoint is looked up without any rescue;A wallet that never synced is silently rescued - its missing inputs trigger the spent-assignments backfill, which happens to also insert the change TXO. A wallet that was merely behind (the realistic recovery case: it knows the inputs from before it went dark, so the rescue never fires) panics.
Reproducer
sync_with_hub_replay_survives_inflation_change(2-of-3 multisig, IFA):btc_change: None(1000-sat UTXOs make the BTC remainder sub-dust);At the moment of the panic the lagging wallet's
txotable holds the still-unspent siblings of the change tx but not the spent change target - the exact pattern we found in the live incident's wallet DB.Suggested direction
Mirroring the spent-assignments rescue in the
change_utxo_outpointarm (on a miss:sync_wallet(FastSync, include_spent = true), then retry the lookup) makes this test pass end-to-end on our fork, with the rest of the multisig suite unaffected. Not included here on purpose - this looks like another instance of the "every hub operation should have a deterministic outcome, identical for all cosigners" bucket discussed in #94: here the outcome depends on the cosigner's localtxotable state. Happy to adapt the test or the fix to whatever shape that wider rework takes.