fix(bob): Explicitly check for tx_punish#938
Conversation
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Can't vouch for the migration, rest LGTM |
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| json_extract(state, '$.Bob.ExecutionSetupDone.state2.tx_punish_fee') AS tx_punish_fee, | ||
| json_extract(state, '$.Bob.ExecutionSetupDone.state2.punish_address') AS punish_address | ||
| FROM swap_states | ||
| WHERE json_extract(state, '$.Bob.ExecutionSetupDone') IS NOT NULL; |
There was a problem hiding this comment.
Migration references nonexistent JSON path in database
High Severity
The migration extracts source data from $.Bob.ExecutionSetupDone.state2.tx_punish_fee and $.Bob.ExecutionSetupDone.state2.punish_address, but the Rust enum variant is SwapSetupCompleted(State2) with no #[serde(rename = "ExecutionSetupDone")] attribute. A codebase-wide search finds zero references to ExecutionSetupDone outside this migration file. Additionally, SwapSetupCompleted(State2) is a tuple variant, so serde serializes it without a state2 wrapper key. If DB records use the current variant name, the migration finds no source rows, causing either silent no-ops or assertion failures that block the upgrade.
|
|
||
| // Attempt to refund. Reachable from both Cancel and Punish (if tx_punish has not yet been published). | ||
| let (tx_refund, refund_type) = state.construct_best_bitcoin_refund_tx().context("Couldn't construct best Bitcoin refund transaction").map_err(backoff::Error::transient)?; | ||
| bitcoin_wallet.ensure_broadcasted(tx_refund, &refund_type.to_string()).await.map_err(|e| backoff::Error::transient(e.context("Couldn't publish best refund transaction")))?; |
There was a problem hiding this comment.
Refund failure no longer detects punishment via error codes
Medium Severity
The old code detected punishment by matching specific RPC error codes (RpcVerifyRejected/RpcVerifyError) when the refund broadcast failed, transitioning to BtcPunished. The new code removes this fallback entirely — all ensure_broadcasted errors are now transient retries. If construct_tx_punish().id() doesn't match the on-chain punish tx (e.g., Alice used a different fee or address, since these aren't enforced on-chain), the refund keeps failing and the code retries indefinitely, never reaching BtcPunished.
2 participants
Note
Medium Risk
Touches Bob swap state persistence and cancel/refund transition logic, plus adds a SQLite JSON migration; mistakes could misclassify punished vs refundable swaps or break resuming older swaps.
Overview
Fixes Bob getting stuck in
BtcCancelledby explicitly detecting punishment: whenExpiredTimelocks::Punishis reached, Bob now constructsTxPunishand queries the chain for its txid before attempting refunds, transitioning toBtcPunishedif found.To support this across resumes, Bob state structs
State3–State6now persisttx_punish_feeandpunish_address(andState6gainsconstruct_tx_punish()), with a new SQLite migration that backfills these fields into existing stored JSON states. Changelog notes the GUI-facing fix.Written by Cursor Bugbot for commit 3b0263d. This will update automatically on new commits. Configure here.