Skip to content

Commit 74be598

Browse files
azarovhazarovh
committed
Error on changing to same tx state
1 parent 7ed77e0 commit 74be598

File tree

2 files changed

+140
-67
lines changed

2 files changed

+140
-67
lines changed

‎wallet/src/account/output_cache/mod.rs

+69-61
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,9 @@ impl OutputCache {
800800
confirmed_account_nonce,
801801
) {
802802
if let WalletTx::Tx(tx) = unconfirmed_tx {
803-
conflicting_txs.insert(tx.get_transaction().get_id());
803+
if confirmed_tx.get_id() != tx.get_transaction().get_id() {
804+
conflicting_txs.insert(tx.get_transaction().get_id());
805+
}
804806
}
805807
}
806808
}
@@ -811,14 +813,10 @@ impl OutputCache {
811813
let mut conflicting_txs_with_descendants = vec![];
812814

813815
for conflicting_tx in conflicting_txs {
814-
if conflicting_tx != confirmed_tx.get_id() {
815-
let descendants = self.remove_descendants_and_mark_as(
816-
conflicting_tx,
817-
TxState::Conflicted(block_id),
818-
)?;
816+
let descendants =
817+
self.remove_descendants_and_mark_as(conflicting_tx, TxState::Conflicted(block_id))?;
819818

820-
conflicting_txs_with_descendants.extend(descendants.into_iter());
821-
}
819+
conflicting_txs_with_descendants.extend(descendants.into_iter());
822820
}
823821

824822
Ok(conflicting_txs_with_descendants)
@@ -1468,65 +1466,75 @@ impl OutputCache {
14681466
match self.txs.entry(tx_id.into()) {
14691467
Entry::Occupied(mut entry) => match entry.get_mut() {
14701468
WalletTx::Block(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
1471-
WalletTx::Tx(tx) => match tx.state() {
1472-
TxState::Inactive(_) | TxState::Conflicted(_) | TxState::Abandoned => {
1473-
tx.set_state(new_state);
1474-
for input in tx.get_transaction().inputs() {
1475-
match input {
1476-
TxInput::Utxo(outpoint) => {
1477-
self.consumed.insert(outpoint.clone(), *tx.state());
1478-
}
1479-
TxInput::Account(outpoint) => match outpoint.account() {
1480-
AccountSpending::DelegationBalance(delegation_id, _) => {
1481-
if let Some(data) =
1482-
self.delegations.get_mut(delegation_id)
1483-
{
1484-
data.last_nonce = outpoint.nonce().decrement();
1485-
data.last_parent = find_parent(
1486-
&self.unconfirmed_descendants,
1487-
tx_id.into(),
1488-
);
1489-
}
1469+
WalletTx::Tx(tx) => {
1470+
ensure!(
1471+
tx.state() != &new_state,
1472+
WalletError::CannotChangeTransactionState(*tx.state(), new_state)
1473+
);
1474+
1475+
match tx.state() {
1476+
TxState::Inactive(_) | TxState::Conflicted(_) | TxState::Abandoned => {
1477+
tx.set_state(new_state);
1478+
for input in tx.get_transaction().inputs() {
1479+
match input {
1480+
TxInput::Utxo(outpoint) => {
1481+
self.consumed.insert(outpoint.clone(), *tx.state());
14901482
}
1491-
},
1492-
TxInput::AccountCommand(nonce, op) => match op {
1493-
AccountCommand::MintTokens(token_id, _)
1494-
| AccountCommand::UnmintTokens(token_id)
1495-
| AccountCommand::LockTokenSupply(token_id)
1496-
| AccountCommand::FreezeToken(token_id, _)
1497-
| AccountCommand::UnfreezeToken(token_id)
1498-
| AccountCommand::ChangeTokenMetadataUri(token_id, _)
1499-
| AccountCommand::ChangeTokenAuthority(token_id, _) => {
1500-
if let Some(data) =
1501-
self.token_issuance.get_mut(token_id)
1502-
{
1503-
data.last_nonce = nonce.decrement();
1504-
data.last_parent = find_parent(
1505-
&self.unconfirmed_descendants,
1506-
tx_id.into(),
1507-
);
1508-
data.unconfirmed_txs.remove(&tx_id.into());
1483+
TxInput::Account(outpoint) => match outpoint.account() {
1484+
AccountSpending::DelegationBalance(
1485+
delegation_id,
1486+
_,
1487+
) => {
1488+
if let Some(data) =
1489+
self.delegations.get_mut(delegation_id)
1490+
{
1491+
data.last_nonce = outpoint.nonce().decrement();
1492+
data.last_parent = find_parent(
1493+
&self.unconfirmed_descendants,
1494+
tx_id.into(),
1495+
);
1496+
}
15091497
}
1510-
}
1511-
AccountCommand::ConcludeOrder(order_id)
1512-
| AccountCommand::FillOrder(order_id, _, _) => {
1513-
if let Some(data) = self.orders.get_mut(order_id) {
1514-
data.last_nonce = nonce.decrement();
1515-
data.last_parent = find_parent(
1516-
&self.unconfirmed_descendants,
1517-
tx_id.into(),
1518-
);
1498+
},
1499+
TxInput::AccountCommand(nonce, op) => match op {
1500+
AccountCommand::MintTokens(token_id, _)
1501+
| AccountCommand::UnmintTokens(token_id)
1502+
| AccountCommand::LockTokenSupply(token_id)
1503+
| AccountCommand::FreezeToken(token_id, _)
1504+
| AccountCommand::UnfreezeToken(token_id)
1505+
| AccountCommand::ChangeTokenMetadataUri(token_id, _)
1506+
| AccountCommand::ChangeTokenAuthority(token_id, _) => {
1507+
if let Some(data) =
1508+
self.token_issuance.get_mut(token_id)
1509+
{
1510+
data.last_nonce = nonce.decrement();
1511+
data.last_parent = find_parent(
1512+
&self.unconfirmed_descendants,
1513+
tx_id.into(),
1514+
);
1515+
data.unconfirmed_txs.remove(&tx_id.into());
1516+
}
15191517
}
1520-
}
1521-
},
1518+
AccountCommand::ConcludeOrder(order_id)
1519+
| AccountCommand::FillOrder(order_id, _, _) => {
1520+
if let Some(data) = self.orders.get_mut(order_id) {
1521+
data.last_nonce = nonce.decrement();
1522+
data.last_parent = find_parent(
1523+
&self.unconfirmed_descendants,
1524+
tx_id.into(),
1525+
);
1526+
}
1527+
}
1528+
},
1529+
}
15221530
}
1531+
Ok(())
15231532
}
1524-
Ok(())
1533+
TxState::Confirmed(..) | TxState::InMempool(..) => Err(
1534+
WalletError::CannotChangeTransactionState(*tx.state(), new_state),
1535+
),
15251536
}
1526-
TxState::Confirmed(..) | TxState::InMempool(..) => Err(
1527-
WalletError::CannotChangeTransactionState(*tx.state(), new_state),
1528-
),
1529-
},
1537+
}
15301538
},
15311539
Entry::Vacant(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
15321540
}?;

‎wallet/src/wallet/tests.rs

+71-6
Original file line numberDiff line numberDiff line change
@@ -4344,6 +4344,13 @@ fn wallet_abandone_transactions(#[case] seed: Seed) {
43444344
wallet.scan_mempool(txs_to_keep.as_slice(), &WalletEventsNoOp).unwrap();
43454345
let coin_balance = get_coin_balance_with_inactive(&wallet);
43464346
assert_eq!(coin_balance, coins_after_abandon);
4347+
4348+
// Abandone the same tx again
4349+
let result = wallet.abandon_transaction(DEFAULT_ACCOUNT_INDEX, transaction_id);
4350+
assert_eq!(
4351+
result.unwrap_err(),
4352+
WalletError::CannotChangeTransactionState(TxState::Abandoned, TxState::Abandoned)
4353+
);
43474354
}
43484355

43494356
#[rstest]
@@ -6470,12 +6477,12 @@ fn create_order_fill_partially_conclude(#[case] seed: Seed) {
64706477
}
64716478
}
64726479

6473-
// Create 2 wallet from the same mnemonic.
6480+
// Create 2 wallets from the same mnemonic.
64746481
// Create a pool and a delegation with some stake for both wallets.
6475-
// Add 2 consequtive txs that spend share from delelgation to the first wallet as unconfirmed.
6482+
// Add 2 consecutive txs that spend share from delegation to the first wallet as unconfirmed.
64766483
// Add 1 txs that spends share from delegation with different amount (so that tx id is different)
64776484
// via the second wallet and submit it to the block.
6478-
// Check that 2 unconfirmed txs from the first wallet conflicts with confirmed tx and got removed.
6485+
// Check that 2 unconfirmed txs from the first wallet conflict with confirmed tx and got removed.
64796486
#[rstest]
64806487
#[trace]
64816488
#[case(Seed::from_entropy())]
@@ -6604,6 +6611,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
66046611
FeeRate::from_amount_per_kb(Amount::ZERO),
66056612
)
66066613
.unwrap();
6614+
let spend_from_delegation_tx_1_id = spend_from_delegation_tx_1.transaction().get_id();
66076615

66086616
wallet
66096617
.add_account_unconfirmed_tx(
@@ -6624,6 +6632,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
66246632
FeeRate::from_amount_per_kb(Amount::ZERO),
66256633
)
66266634
.unwrap();
6635+
let spend_from_delegation_tx_2_id = spend_from_delegation_tx_2.transaction().get_id();
66276636

66286637
wallet
66296638
.add_account_unconfirmed_tx(
@@ -6652,6 +6661,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
66526661
FeeRate::from_amount_per_kb(Amount::ZERO),
66536662
)
66546663
.unwrap();
6664+
let spend_from_delegation_tx_3_id = spend_from_delegation_tx_3.transaction().get_id();
66556665

66566666
let (_, block5) = create_block(
66576667
&chain_config,
@@ -6660,6 +6670,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
66606670
Amount::ZERO,
66616671
4,
66626672
);
6673+
let block5_id = block5.get_id();
66636674
scan_wallet(&mut wallet, BlockHeight::new(4), vec![block5]);
66646675

66656676
// if confirmed tx is added conflicting txs must be removed from the output cache
@@ -6690,13 +6701,62 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
66906701
coin_balance,
66916702
(coin_balance_after_delegating + withdraw_amount_3).unwrap()
66926703
);
6704+
6705+
assert_eq!(
6706+
*wallet
6707+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
6708+
.unwrap()
6709+
.state(),
6710+
TxState::Conflicted(block5_id.into())
6711+
);
6712+
assert_eq!(
6713+
*wallet
6714+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
6715+
.unwrap()
6716+
.state(),
6717+
TxState::Conflicted(block5_id.into())
6718+
);
6719+
assert_eq!(
6720+
*wallet
6721+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_3_id)
6722+
.unwrap()
6723+
.state(),
6724+
TxState::Confirmed(
6725+
BlockHeight::new(5),
6726+
chain_config.genesis_block().timestamp(),
6727+
0
6728+
)
6729+
);
6730+
6731+
// Abandone conflicting txs
6732+
wallet
6733+
.abandon_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
6734+
.unwrap();
6735+
assert_eq!(
6736+
*wallet
6737+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
6738+
.unwrap()
6739+
.state(),
6740+
TxState::Abandoned
6741+
);
6742+
6743+
wallet
6744+
.abandon_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
6745+
.unwrap();
6746+
assert_eq!(
6747+
*wallet
6748+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
6749+
.unwrap()
6750+
.state(),
6751+
TxState::Abandoned
6752+
);
66936753
}
66946754

66956755
// Create a pool and a delegation with some share.
6696-
// Create 2 consequtive txs that spend from delegation account and add them as unconfirmed.
6756+
// Create 2 consecutive txs that spend from delegation account and add them as unconfirmed.
66976757
// Check confirmed/unconfirmed balance and ensure that account nonce is incremented in OutputCache.
66986758
// Submit the first tx in a block.
6699-
// Check that Confirmed balance changed but
6759+
// Check that confirmed balance changed and unconfirmed stayed the same.
67006760
#[rstest]
67016761
#[trace]
67026762
#[case(Seed::from_entropy())]
@@ -6919,6 +6979,11 @@ fn conflicting_delegation_account_nonce_same_tx(#[case] seed: Seed) {
69196979
);
69206980
}
69216981

6982+
// Issue and mint some tokens
6983+
// Create an order selling tokens for coins
6984+
// Create 2 fill order txs and add them to a wallet as unconfirmed
6985+
// Confirm the first tx in a block and check that it is accounted in confirmed balance
6986+
// and also that unconfirmed balance has second tx.
69226987
#[rstest]
69236988
#[trace]
69246989
#[case(Seed::from_entropy())]
@@ -7033,7 +7098,7 @@ fn conflicting_order_account_nonce(#[case] seed: Seed) {
70337098
assert_eq!(actual_order_data.last_nonce, None);
70347099
assert_eq!(&actual_order_data.conclude_key, address2.as_object());
70357100

7036-
// Create 2 fill orders txs and put them in unconfirmed
7101+
// Create 2 fill order txs and put them in unconfirmed
70377102
let order_info = RpcOrderInfo {
70387103
conclude_key: address2.clone().into_object(),
70397104
initially_given: RpcOutputValue::Token {

0 commit comments

Comments
 (0)