diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a4b6f0b4..4defc16a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ The format is loosely based on [Keep a Changelog](https://keepachangelog.com/en/ - `wallet-create`/`wallet-recover`/`wallet-open` support the `ledger` subcommand, in addition to the existing `software` and `trezor`, which specifies the type of the wallet to operate on. +### Fixed + - Wallet: + - Fixed handling of confirmed and unconfirmed conflicting order transactions in the wallet. + ## [1.3.0] - 2026-04-09 ### Added diff --git a/wallet/src/account/output_cache/mod.rs b/wallet/src/account/output_cache/mod.rs index 651fcc32b..441c7ae85 100644 --- a/wallet/src/account/output_cache/mod.rs +++ b/wallet/src/account/output_cache/mod.rs @@ -783,9 +783,11 @@ impl OutputCache { confirmed_tx: &Transaction, block_id: Id, ) -> WalletResult, WalletTx)>> { + // TODO: maybe make it an enum struct ConflictCheck { frozen_token_id: Option, confirmed_account_nonce: Option<(AccountType, AccountNonce)>, + conflicting_order_command: Option<(OrderAccountCommandTag, OrderId)>, } let conflict_checks = confirmed_tx @@ -804,6 +806,7 @@ impl OutputCache { outpoint.account().into(), outpoint.nonce(), )), + conflicting_order_command: None, }), TxInput::AccountCommand(nonce, cmd) => match cmd { AccountCommand::MintTokens(_, _) @@ -816,13 +819,34 @@ impl OutputCache { | AccountCommand::FillOrder(_, _, _) => Some(ConflictCheck { frozen_token_id: None, confirmed_account_nonce: Some((cmd.into(), *nonce)), + conflicting_order_command: None, }), | AccountCommand::FreezeToken(token_id, _) => Some(ConflictCheck { frozen_token_id: Some(*token_id), confirmed_account_nonce: Some((cmd.into(), *nonce)), + conflicting_order_command: None, }), }, - TxInput::OrderAccountCommand(_) => None, + TxInput::OrderAccountCommand(cmd) => { + let order_id = match cmd { + OrderAccountCommand::FillOrder(order_id, _) + | OrderAccountCommand::FreezeOrder(order_id) + | OrderAccountCommand::ConcludeOrder(order_id) => *order_id, + }; + let cmd_tag: OrderAccountCommandTag = cmd.into(); + match cmd_tag { + // ConcludeOrder and FreezeOrder are exclusive: only one tx can win. + // Any unconfirmed tx with the same command for the same order conflicts. + OrderAccountCommandTag::ConcludeOrder + | OrderAccountCommandTag::FreezeOrder => Some(ConflictCheck { + frozen_token_id: None, + confirmed_account_nonce: None, + conflicting_order_command: Some((cmd_tag, order_id)), + }), + // Multiple fills for the same order can coexist. + OrderAccountCommandTag::FillOrder => None, + } + } } }) .collect::>(); @@ -857,6 +881,21 @@ impl OutputCache { continue; } } + + if let Some((confirmed_cmd_tag, confirmed_order_id)) = + conflict_check.conflicting_order_command + { + if confirmed_tx.get_id() != tx.get_transaction().get_id() + && uses_conflicting_order_command( + unconfirmed_tx, + confirmed_cmd_tag, + confirmed_order_id, + ) + { + conflicting_txs.insert(tx.get_transaction().get_id()); + continue; + } + } } WalletTx::Block(_) => { utils::debug_panic_or_log!("Cannot be block reward"); @@ -2062,6 +2101,41 @@ fn uses_conflicting_nonce( }) } +fn uses_conflicting_order_command( + unconfirmed_tx: &WalletTx, + confirmed_cmd_tag: OrderAccountCommandTag, + confirmed_order_id: OrderId, +) -> bool { + unconfirmed_tx.inputs().iter().any(|input| match input { + TxInput::OrderAccountCommand(cmd) => { + let unconfirmed_order_id = match cmd { + OrderAccountCommand::FillOrder(id, _) + | OrderAccountCommand::FreezeOrder(id) + | OrderAccountCommand::ConcludeOrder(id) => *id, + }; + // It is only a conflict if it is the same order id + if unconfirmed_order_id != confirmed_order_id { + return false; + } + + let unconfirmed_cmd_tag: OrderAccountCommandTag = cmd.into(); + match confirmed_cmd_tag { + // Confirmed fill orders do not conflict with anything + OrderAccountCommandTag::FillOrder => false, + // Confirmed conclude order conflict with any other unconfirmed operation on the + // order + OrderAccountCommandTag::ConcludeOrder => true, + // Confirmed Freeze order conflicts with any unconfirmed Fill or Freeze order + OrderAccountCommandTag::FreezeOrder => match unconfirmed_cmd_tag { + OrderAccountCommandTag::FillOrder | OrderAccountCommandTag::FreezeOrder => true, + OrderAccountCommandTag::ConcludeOrder => false, + }, + } + } + TxInput::Utxo(_) | TxInput::Account(_) | TxInput::AccountCommand(_, _) => false, + }) +} + #[derive(thiserror::Error, Debug, Eq, PartialEq)] pub enum OutputCacheInconsistencyError { #[error("Transaction from {0:?} is confirmed and among unconfirmed descendants")] diff --git a/wallet/src/account/output_cache/tests.rs b/wallet/src/account/output_cache/tests.rs index 5cfcbfef6..b174509e1 100644 --- a/wallet/src/account/output_cache/tests.rs +++ b/wallet/src/account/output_cache/tests.rs @@ -1267,6 +1267,338 @@ fn orders_state_update(#[case] seed: Seed) { ); } +// Regression test: when a confirmed V1 ConcludeOrder arrives and an unconfirmed ConcludeOrder, +// FreezeOrder or FillOrder for the same order is already in the cache, +// the unconfirmed one must be marked Conflicted and the confirmed one must be accepted without error. +// Previously, update_conflicting_txs returned None for OrderAccountCommand, so the unconfirmed +// conclude was never rolled back, causing add_tx to fail with OrderAlreadyConcluded. +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +fn update_conflicting_txs_order_v1_conclude(#[case] seed: Seed) { + let mut rng = make_seedable_rng(seed); + let chain_config = create_unit_test_config(); + let mut output_cache = OutputCache::empty(); + + // Create the order (confirmed). + let parent_tx_id = Id::::random_using(&mut rng); + let creation_tx = TransactionBuilder::new() + .add_input( + TxInput::from_utxo(parent_tx_id.into(), 0), + InputWitness::NoSignature(None), + ) + .add_output(TxOutput::CreateOrder(Box::new(OrderData::new( + Destination::AnyoneCanSpend, + OutputValue::Coin(Amount::from_atoms(1000)), + OutputValue::Coin(Amount::from_atoms(2000)), + )))) + .build(); + let order_id = make_order_id(creation_tx.inputs()).unwrap(); + let creation_tx_id = creation_tx.transaction().get_id(); + + output_cache + .add_tx( + &chain_config, + BlockHeight::new(1), + creation_tx_id.into(), + WalletTx::Tx(TxData::new( + creation_tx, + TxState::Confirmed(BlockHeight::new(1), BlockTimestamp::from_int_seconds(1), 0), + )), + ) + .unwrap(); + + // Add an unconfirmed fill order tx + let unconfirmed_fill_tx = TransactionBuilder::new() + .add_input( + TxInput::OrderAccountCommand(OrderAccountCommand::FillOrder( + order_id, + Amount::from_atoms(rng.gen_range(100..200)), + )), + InputWitness::NoSignature(None), + ) + .build(); + let unconfirmed_fill_tx_id = unconfirmed_fill_tx.transaction().get_id(); + + output_cache + .add_tx( + &chain_config, + BlockHeight::new(2), + unconfirmed_fill_tx_id.into(), + WalletTx::Tx(TxData::new( + unconfirmed_fill_tx.clone(), + TxState::InMempool(1), + )), + ) + .unwrap(); + + // Add an unconfirmed freeze tx — sets is_frozen = true. + let unconfirmed_freeze_tx = TransactionBuilder::new() + .add_input( + TxInput::OrderAccountCommand(OrderAccountCommand::FreezeOrder(order_id)), + InputWitness::NoSignature(None), + ) + .build(); + let unconfirmed_freeze_tx_id = unconfirmed_freeze_tx.transaction().get_id(); + + output_cache + .add_tx( + &chain_config, + BlockHeight::new(2), + unconfirmed_freeze_tx_id.into(), + WalletTx::Tx(TxData::new( + unconfirmed_freeze_tx.clone(), + TxState::InMempool(1), + )), + ) + .unwrap(); + + assert!(output_cache.orders[&order_id].is_frozen); + + // Add an unconfirmed conclude tx — sets is_concluded = true. + let unconfirmed_conclude_tx = TransactionBuilder::new() + .add_input( + TxInput::OrderAccountCommand(OrderAccountCommand::ConcludeOrder(order_id)), + InputWitness::NoSignature(None), + ) + .build(); + let unconfirmed_conclude_tx_id = unconfirmed_conclude_tx.transaction().get_id(); + + output_cache + .add_tx( + &chain_config, + BlockHeight::new(2), + unconfirmed_conclude_tx_id.into(), + WalletTx::Tx(TxData::new( + unconfirmed_conclude_tx.clone(), + TxState::InMempool(2), + )), + ) + .unwrap(); + + assert!(output_cache.orders[&order_id].is_concluded); + + // A *different* confirmed conclude tx for the same order arrives. + // update_conflicting_txs must mark the unconfirmed ones as Conflicted. + let confirmed_conclude_tx = TransactionBuilder::new() + .add_input( + TxInput::OrderAccountCommand(OrderAccountCommand::ConcludeOrder(order_id)), + InputWitness::NoSignature(None), + ) + .add_output(TxOutput::Transfer( + OutputValue::Coin(Amount::from_atoms(1)), + Destination::AnyoneCanSpend, + )) + .build(); + let confirmed_conclude_tx_id = confirmed_conclude_tx.transaction().get_id(); + assert_ne!(confirmed_conclude_tx_id, unconfirmed_conclude_tx_id); + + let block_id = Id::::random_using(&mut rng); + let mut conflicted = output_cache + .update_conflicting_txs(&chain_config, confirmed_conclude_tx.transaction(), block_id) + .unwrap(); + + // sort conflicted by tx id + conflicted.sort_by_key(|(tx_id, _)| *tx_id); + + let mut expected_conflicted = vec![ + ( + unconfirmed_fill_tx_id, + WalletTx::Tx(TxData::new( + unconfirmed_fill_tx, + TxState::Conflicted(block_id), + )), + ), + ( + unconfirmed_freeze_tx_id, + WalletTx::Tx(TxData::new( + unconfirmed_freeze_tx, + TxState::Conflicted(block_id), + )), + ), + ( + unconfirmed_conclude_tx_id, + WalletTx::Tx(TxData::new( + unconfirmed_conclude_tx, + TxState::Conflicted(block_id), + )), + ), + ]; + expected_conflicted.sort_by_key(|(tx_id, _)| *tx_id); + + assert_eq!(conflicted, expected_conflicted); + + // is_concluded and is_frozen must be rolled back after the conflict. + assert!(!output_cache.orders[&order_id].is_concluded); + assert!(!output_cache.orders[&order_id].is_frozen); + + // The confirmed conclude tx must now be addable without error. + output_cache + .add_tx( + &chain_config, + BlockHeight::new(2), + confirmed_conclude_tx_id.into(), + WalletTx::Tx(TxData::new( + confirmed_conclude_tx, + TxState::Confirmed(BlockHeight::new(2), BlockTimestamp::from_int_seconds(2), 0), + )), + ) + .unwrap(); + + assert!(output_cache.orders[&order_id].is_concluded); + assert!(!output_cache.orders[&order_id].is_frozen); +} + +// Regression test: same as above but for FreezeOrder. +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +fn update_conflicting_txs_order_v1_freeze(#[case] seed: Seed) { + let mut rng = make_seedable_rng(seed); + let chain_config = create_unit_test_config(); + let mut output_cache = OutputCache::empty(); + + // Create the order (confirmed). + let parent_tx_id = Id::::random_using(&mut rng); + let creation_tx = TransactionBuilder::new() + .add_input( + TxInput::from_utxo(parent_tx_id.into(), 0), + InputWitness::NoSignature(None), + ) + .add_output(TxOutput::CreateOrder(Box::new(OrderData::new( + Destination::AnyoneCanSpend, + OutputValue::Coin(Amount::from_atoms(1000)), + OutputValue::Coin(Amount::from_atoms(2000)), + )))) + .build(); + let order_id = make_order_id(creation_tx.inputs()).unwrap(); + let creation_tx_id = creation_tx.transaction().get_id(); + + output_cache + .add_tx( + &chain_config, + BlockHeight::new(1), + creation_tx_id.into(), + WalletTx::Tx(TxData::new( + creation_tx, + TxState::Confirmed(BlockHeight::new(1), BlockTimestamp::from_int_seconds(1), 0), + )), + ) + .unwrap(); + + // Add an unconfirmed fill order tx + let unconfirmed_fill_tx = TransactionBuilder::new() + .add_input( + TxInput::OrderAccountCommand(OrderAccountCommand::FillOrder( + order_id, + Amount::from_atoms(rng.gen_range(100..200)), + )), + InputWitness::NoSignature(None), + ) + .build(); + let unconfirmed_fill_tx_id = unconfirmed_fill_tx.transaction().get_id(); + + output_cache + .add_tx( + &chain_config, + BlockHeight::new(2), + unconfirmed_fill_tx_id.into(), + WalletTx::Tx(TxData::new( + unconfirmed_fill_tx.clone(), + TxState::InMempool(0), + )), + ) + .unwrap(); + + // Add an unconfirmed freeze tx — sets is_frozen = true. + let unconfirmed_freeze_tx = TransactionBuilder::new() + .add_input( + TxInput::OrderAccountCommand(OrderAccountCommand::FreezeOrder(order_id)), + InputWitness::NoSignature(None), + ) + .build(); + let unconfirmed_freeze_tx_id = unconfirmed_freeze_tx.transaction().get_id(); + + output_cache + .add_tx( + &chain_config, + BlockHeight::new(2), + unconfirmed_freeze_tx_id.into(), + WalletTx::Tx(TxData::new( + unconfirmed_freeze_tx.clone(), + TxState::InMempool(1), + )), + ) + .unwrap(); + + assert!(output_cache.orders[&order_id].is_frozen); + // concluded is still false + assert!(!output_cache.orders[&order_id].is_concluded); + + // A *different* confirmed freeze tx for the same order arrives. + // update_conflicting_txs must mark the unconfirmed one as Conflicted. + let confirmed_freeze_tx = TransactionBuilder::new() + .add_input( + TxInput::OrderAccountCommand(OrderAccountCommand::FreezeOrder(order_id)), + InputWitness::NoSignature(None), + ) + .add_output(TxOutput::Transfer( + OutputValue::Coin(Amount::from_atoms(1)), + Destination::AnyoneCanSpend, + )) + .build(); + let confirmed_freeze_tx_id = confirmed_freeze_tx.transaction().get_id(); + assert_ne!(confirmed_freeze_tx_id, unconfirmed_freeze_tx_id); + + let block_id = Id::::random_using(&mut rng); + let mut conflicted = output_cache + .update_conflicting_txs(&chain_config, confirmed_freeze_tx.transaction(), block_id) + .unwrap(); + + let mut expected_conflicted = vec![ + ( + unconfirmed_fill_tx_id, + WalletTx::Tx(TxData::new( + unconfirmed_fill_tx, + TxState::Conflicted(block_id), + )), + ), + ( + unconfirmed_freeze_tx_id, + WalletTx::Tx(TxData::new( + unconfirmed_freeze_tx, + TxState::Conflicted(block_id), + )), + ), + ]; + + // Sort by tx id + conflicted.sort_by_key(|(tx_id, _)| *tx_id); + expected_conflicted.sort_by_key(|(tx_id, _)| *tx_id); + + assert_eq!(conflicted, expected_conflicted); + + // is_frozen must be rolled back after the conflict. + assert!(!output_cache.orders[&order_id].is_frozen); + assert!(!output_cache.orders[&order_id].is_concluded); + + // The confirmed freeze tx must now be addable without error. + output_cache + .add_tx( + &chain_config, + BlockHeight::new(2), + confirmed_freeze_tx_id.into(), + WalletTx::Tx(TxData::new( + confirmed_freeze_tx, + TxState::Confirmed(BlockHeight::new(2), BlockTimestamp::from_int_seconds(2), 0), + )), + ) + .unwrap(); + + assert!(output_cache.orders[&order_id].is_frozen); + assert!(!output_cache.orders[&order_id].is_concluded); +} + fn add_random_transfer_tx( output_cache: &mut OutputCache, chain_config: &ChainConfig,