Skip to content

Commit 3327b22

Browse files
azarovhazarovh
committed
Error on changing to same tx state
1 parent 8c5fdac commit 3327b22

File tree

2 files changed

+140
-67
lines changed

2 files changed

+140
-67
lines changed

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

Lines changed: 69 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,9 @@ impl OutputCache {
748748
confirmed_account_nonce,
749749
) {
750750
if let WalletTx::Tx(tx) = unconfirmed_tx {
751-
conflicting_txs.insert(tx.get_transaction().get_id());
751+
if confirmed_tx.get_id() != tx.get_transaction().get_id() {
752+
conflicting_txs.insert(tx.get_transaction().get_id());
753+
}
752754
}
753755
}
754756
}
@@ -759,14 +761,10 @@ impl OutputCache {
759761
let mut conflicting_txs_with_descendants = vec![];
760762

761763
for conflicting_tx in conflicting_txs {
762-
if conflicting_tx != confirmed_tx.get_id() {
763-
let descendants = self.remove_descendants_and_mark_as(
764-
conflicting_tx,
765-
TxState::Conflicted(block_id),
766-
)?;
764+
let descendants =
765+
self.remove_descendants_and_mark_as(conflicting_tx, TxState::Conflicted(block_id))?;
767766

768-
conflicting_txs_with_descendants.extend(descendants.into_iter());
769-
}
767+
conflicting_txs_with_descendants.extend(descendants.into_iter());
770768
}
771769

772770
Ok(conflicting_txs_with_descendants)
@@ -1431,65 +1429,75 @@ impl OutputCache {
14311429
match self.txs.entry(tx_id.into()) {
14321430
Entry::Occupied(mut entry) => match entry.get_mut() {
14331431
WalletTx::Block(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
1434-
WalletTx::Tx(tx) => match tx.state() {
1435-
TxState::Inactive(_) | TxState::Conflicted(_) | TxState::Abandoned => {
1436-
tx.set_state(new_state);
1437-
for input in tx.get_transaction().inputs() {
1438-
match input {
1439-
TxInput::Utxo(outpoint) => {
1440-
self.consumed.insert(outpoint.clone(), *tx.state());
1441-
}
1442-
TxInput::Account(outpoint) => match outpoint.account() {
1443-
AccountSpending::DelegationBalance(delegation_id, _) => {
1444-
if let Some(data) =
1445-
self.delegations.get_mut(delegation_id)
1446-
{
1447-
data.last_nonce = outpoint.nonce().decrement();
1448-
data.last_parent = find_parent(
1449-
&self.unconfirmed_descendants,
1450-
tx_id.into(),
1451-
);
1452-
}
1432+
WalletTx::Tx(tx) => {
1433+
ensure!(
1434+
tx.state() != &new_state,
1435+
WalletError::CannotChangeTransactionState(*tx.state(), new_state)
1436+
);
1437+
1438+
match tx.state() {
1439+
TxState::Inactive(_) | TxState::Conflicted(_) | TxState::Abandoned => {
1440+
tx.set_state(new_state);
1441+
for input in tx.get_transaction().inputs() {
1442+
match input {
1443+
TxInput::Utxo(outpoint) => {
1444+
self.consumed.insert(outpoint.clone(), *tx.state());
14531445
}
1454-
},
1455-
TxInput::AccountCommand(nonce, op) => match op {
1456-
AccountCommand::MintTokens(token_id, _)
1457-
| AccountCommand::UnmintTokens(token_id)
1458-
| AccountCommand::LockTokenSupply(token_id)
1459-
| AccountCommand::FreezeToken(token_id, _)
1460-
| AccountCommand::UnfreezeToken(token_id)
1461-
| AccountCommand::ChangeTokenMetadataUri(token_id, _)
1462-
| AccountCommand::ChangeTokenAuthority(token_id, _) => {
1463-
if let Some(data) =
1464-
self.token_issuance.get_mut(token_id)
1465-
{
1466-
data.last_nonce = nonce.decrement();
1467-
data.last_parent = find_parent(
1468-
&self.unconfirmed_descendants,
1469-
tx_id.into(),
1470-
);
1471-
data.unconfirmed_txs.remove(&tx_id.into());
1446+
TxInput::Account(outpoint) => match outpoint.account() {
1447+
AccountSpending::DelegationBalance(
1448+
delegation_id,
1449+
_,
1450+
) => {
1451+
if let Some(data) =
1452+
self.delegations.get_mut(delegation_id)
1453+
{
1454+
data.last_nonce = outpoint.nonce().decrement();
1455+
data.last_parent = find_parent(
1456+
&self.unconfirmed_descendants,
1457+
tx_id.into(),
1458+
);
1459+
}
14721460
}
1473-
}
1474-
AccountCommand::ConcludeOrder(order_id)
1475-
| AccountCommand::FillOrder(order_id, _, _) => {
1476-
if let Some(data) = self.orders.get_mut(order_id) {
1477-
data.last_nonce = nonce.decrement();
1478-
data.last_parent = find_parent(
1479-
&self.unconfirmed_descendants,
1480-
tx_id.into(),
1481-
);
1461+
},
1462+
TxInput::AccountCommand(nonce, op) => match op {
1463+
AccountCommand::MintTokens(token_id, _)
1464+
| AccountCommand::UnmintTokens(token_id)
1465+
| AccountCommand::LockTokenSupply(token_id)
1466+
| AccountCommand::FreezeToken(token_id, _)
1467+
| AccountCommand::UnfreezeToken(token_id)
1468+
| AccountCommand::ChangeTokenMetadataUri(token_id, _)
1469+
| AccountCommand::ChangeTokenAuthority(token_id, _) => {
1470+
if let Some(data) =
1471+
self.token_issuance.get_mut(token_id)
1472+
{
1473+
data.last_nonce = nonce.decrement();
1474+
data.last_parent = find_parent(
1475+
&self.unconfirmed_descendants,
1476+
tx_id.into(),
1477+
);
1478+
data.unconfirmed_txs.remove(&tx_id.into());
1479+
}
14821480
}
1483-
}
1484-
},
1481+
AccountCommand::ConcludeOrder(order_id)
1482+
| AccountCommand::FillOrder(order_id, _, _) => {
1483+
if let Some(data) = self.orders.get_mut(order_id) {
1484+
data.last_nonce = nonce.decrement();
1485+
data.last_parent = find_parent(
1486+
&self.unconfirmed_descendants,
1487+
tx_id.into(),
1488+
);
1489+
}
1490+
}
1491+
},
1492+
}
14851493
}
1494+
Ok(())
14861495
}
1487-
Ok(())
1496+
TxState::Confirmed(..) | TxState::InMempool(..) => Err(
1497+
WalletError::CannotChangeTransactionState(*tx.state(), new_state),
1498+
),
14881499
}
1489-
TxState::Confirmed(..) | TxState::InMempool(..) => Err(
1490-
WalletError::CannotChangeTransactionState(*tx.state(), new_state),
1491-
),
1492-
},
1500+
}
14931501
},
14941502
Entry::Vacant(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
14951503
}?;

‎wallet/src/wallet/tests.rs

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3899,6 +3899,13 @@ fn wallet_abandone_transactions(#[case] seed: Seed) {
38993899
wallet.scan_mempool(txs_to_keep.as_slice(), &WalletEventsNoOp).unwrap();
39003900
let coin_balance = get_coin_balance_with_inactive(&wallet);
39013901
assert_eq!(coin_balance, coins_after_abandon);
3902+
3903+
// Abandone the same tx again
3904+
let result = wallet.abandon_transaction(DEFAULT_ACCOUNT_INDEX, transaction_id);
3905+
assert_eq!(
3906+
result.unwrap_err(),
3907+
WalletError::CannotChangeTransactionState(TxState::Abandoned, TxState::Abandoned)
3908+
);
39023909
}
39033910

39043911
#[rstest]
@@ -5940,12 +5947,12 @@ fn create_order_fill_partially_conclude(#[case] seed: Seed) {
59405947
}
59415948
}
59425949

5943-
// Create 2 wallet from the same mnemonic.
5950+
// Create 2 wallets from the same mnemonic.
59445951
// Create a pool and a delegation with some stake for both wallets.
5945-
// Add 2 consequtive txs that spend share from delelgation to the first wallet as unconfirmed.
5952+
// Add 2 consecutive txs that spend share from delegation to the first wallet as unconfirmed.
59465953
// Add 1 txs that spends share from delegation with different amount (so that tx id is different)
59475954
// via the second wallet and submit it to the block.
5948-
// Check that 2 unconfirmed txs from the first wallet conflicts with confirmed tx and got removed.
5955+
// Check that 2 unconfirmed txs from the first wallet conflict with confirmed tx and got removed.
59495956
#[rstest]
59505957
#[trace]
59515958
#[case(Seed::from_entropy())]
@@ -6071,6 +6078,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
60716078
FeeRate::from_amount_per_kb(Amount::ZERO),
60726079
)
60736080
.unwrap();
6081+
let spend_from_delegation_tx_1_id = spend_from_delegation_tx_1.transaction().get_id();
60746082

60756083
wallet
60766084
.add_account_unconfirmed_tx(
@@ -6091,6 +6099,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
60916099
FeeRate::from_amount_per_kb(Amount::ZERO),
60926100
)
60936101
.unwrap();
6102+
let spend_from_delegation_tx_2_id = spend_from_delegation_tx_2.transaction().get_id();
60946103

60956104
wallet
60966105
.add_account_unconfirmed_tx(
@@ -6119,6 +6128,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
61196128
FeeRate::from_amount_per_kb(Amount::ZERO),
61206129
)
61216130
.unwrap();
6131+
let spend_from_delegation_tx_3_id = spend_from_delegation_tx_3.transaction().get_id();
61226132

61236133
let (_, block5) = create_block(
61246134
&chain_config,
@@ -6127,6 +6137,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
61276137
Amount::ZERO,
61286138
4,
61296139
);
6140+
let block5_id = block5.get_id();
61306141
scan_wallet(&mut wallet, BlockHeight::new(4), vec![block5]);
61316142

61326143
// if confirmed tx is added conflicting txs must be removed from the output cache
@@ -6157,13 +6168,62 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
61576168
coin_balance,
61586169
(coin_balance_after_delegating + withdraw_amount_3).unwrap()
61596170
);
6171+
6172+
assert_eq!(
6173+
*wallet
6174+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
6175+
.unwrap()
6176+
.state(),
6177+
TxState::Conflicted(block5_id.into())
6178+
);
6179+
assert_eq!(
6180+
*wallet
6181+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
6182+
.unwrap()
6183+
.state(),
6184+
TxState::Conflicted(block5_id.into())
6185+
);
6186+
assert_eq!(
6187+
*wallet
6188+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_3_id)
6189+
.unwrap()
6190+
.state(),
6191+
TxState::Confirmed(
6192+
BlockHeight::new(5),
6193+
chain_config.genesis_block().timestamp(),
6194+
0
6195+
)
6196+
);
6197+
6198+
// Abandone conflicting txs
6199+
wallet
6200+
.abandon_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
6201+
.unwrap();
6202+
assert_eq!(
6203+
*wallet
6204+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
6205+
.unwrap()
6206+
.state(),
6207+
TxState::Abandoned
6208+
);
6209+
6210+
wallet
6211+
.abandon_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
6212+
.unwrap();
6213+
assert_eq!(
6214+
*wallet
6215+
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
6216+
.unwrap()
6217+
.state(),
6218+
TxState::Abandoned
6219+
);
61606220
}
61616221

61626222
// Create a pool and a delegation with some share.
6163-
// Create 2 consequtive txs that spend from delegation account and add them as unconfirmed.
6223+
// Create 2 consecutive txs that spend from delegation account and add them as unconfirmed.
61646224
// Check confirmed/unconfirmed balance and ensure that account nonce is incremented in OutputCache.
61656225
// Submit the first tx in a block.
6166-
// Check that Confirmed balance changed but
6226+
// Check that confirmed balance changed and unconfirmed stayed the same.
61676227
#[rstest]
61686228
#[trace]
61696229
#[case(Seed::from_entropy())]
@@ -6383,6 +6443,11 @@ fn conflicting_delegation_account_nonce_same_tx(#[case] seed: Seed) {
63836443
);
63846444
}
63856445

6446+
// Issue and mint some tokens
6447+
// Create an order selling tokens for coins
6448+
// Create 2 fill order txs and add them to a wallet as unconfirmed
6449+
// Confirm the first tx in a block and check that it is accounted in confirmed balance
6450+
// and also that unconfirmed balance has second tx.
63866451
#[rstest]
63876452
#[trace]
63886453
#[case(Seed::from_entropy())]
@@ -6496,7 +6561,7 @@ fn conflicting_order_account_nonce(#[case] seed: Seed) {
64966561
assert_eq!(actual_order_data.last_nonce, None);
64976562
assert_eq!(&actual_order_data.conclude_key, address2.as_object());
64986563

6499-
// Create 2 fill orders txs and put them in unconfirmed
6564+
// Create 2 fill order txs and put them in unconfirmed
65006565
let order_info = RpcOrderInfo {
65016566
conclude_key: address2.clone().into_object(),
65026567
initially_given: RpcOutputValue::Token {

0 commit comments

Comments
 (0)