Skip to content

Commit 2729c09

Browse files
azarovhazarovh
authored
Merge pull request #1864 from mintlayer/fix/wallet_remove_conflicting_txs
Wallet handle txs with conflicting account nonces
2 parents 058c7f0 + be11da1 commit 2729c09

File tree

16 files changed

+2122
-293
lines changed

16 files changed

+2122
-293
lines changed

‎Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎chainstate/test-framework/src/block_builder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,10 @@ impl<'f> BlockBuilder<'f> {
210210
}
211211
TxInput::Account(outpoint) => {
212212
self.account_nonce_tracker
213-
.insert(outpoint.account().clone().into(), outpoint.nonce());
213+
.insert(outpoint.account().into(), outpoint.nonce());
214214
}
215215
TxInput::AccountCommand(nonce, op) => {
216-
self.account_nonce_tracker.insert(op.clone().into(), *nonce);
216+
self.account_nonce_tracker.insert(op.into(), *nonce);
217217
}
218218
TxInput::OrderAccountCommand(..) => {}
219219
};

‎chainstate/test-framework/src/pos_block_builder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,10 +450,10 @@ impl<'f> PoSBlockBuilder<'f> {
450450
}
451451
TxInput::Account(outpoint) => {
452452
self.account_nonce_tracker
453-
.insert(outpoint.account().clone().into(), outpoint.nonce());
453+
.insert(outpoint.account().into(), outpoint.nonce());
454454
}
455455
TxInput::AccountCommand(nonce, op) => {
456-
self.account_nonce_tracker.insert(op.clone().into(), *nonce);
456+
self.account_nonce_tracker.insert(op.into(), *nonce);
457457
}
458458
TxInput::OrderAccountCommand(..) => {}
459459
};

‎chainstate/tx-verifier/src/transaction_verifier/mod.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ where
367367
let res = self
368368
.spend_input_from_account(
369369
outpoint.nonce(),
370-
outpoint.account().clone().into(),
370+
outpoint.account().into(),
371371
)
372372
.and_then(|_| {
373373
// If the input spends from delegation account, this means the user is
@@ -496,10 +496,10 @@ where
496496
match input {
497497
TxInput::Utxo(_) | TxInput::OrderAccountCommand(_) => { /* do nothing */ }
498498
TxInput::Account(outpoint) => {
499-
self.unspend_input_from_account(outpoint.account().clone().into())?;
499+
self.unspend_input_from_account(outpoint.account().into())?;
500500
}
501501
TxInput::AccountCommand(_, cmd) => {
502-
self.unspend_input_from_account(cmd.clone().into())?;
502+
self.unspend_input_from_account(cmd.into())?;
503503
}
504504
};
505505
}
@@ -553,7 +553,7 @@ where
553553
TxInput::AccountCommand(nonce, account_op) => match account_op {
554554
AccountCommand::MintTokens(token_id, amount) => {
555555
let res = self
556-
.spend_input_from_account(*nonce, account_op.clone().into())
556+
.spend_input_from_account(*nonce, account_op.into())
557557
.and_then(|_| {
558558
self.tokens_accounting_cache
559559
.mint_tokens(*token_id, *amount)
@@ -563,7 +563,7 @@ where
563563
}
564564
AccountCommand::UnmintTokens(ref token_id) => {
565565
let res = self
566-
.spend_input_from_account(*nonce, account_op.clone().into())
566+
.spend_input_from_account(*nonce, account_op.into())
567567
.and_then(|_| {
568568
// actual amount to unmint is determined by the number of burned tokens in the outputs
569569
let total_burned =
@@ -582,7 +582,7 @@ where
582582
}
583583
AccountCommand::LockTokenSupply(token_id) => {
584584
let res = self
585-
.spend_input_from_account(*nonce, account_op.clone().into())
585+
.spend_input_from_account(*nonce, account_op.into())
586586
.and_then(|_| {
587587
self.tokens_accounting_cache
588588
.lock_circulating_supply(*token_id)
@@ -592,7 +592,7 @@ where
592592
}
593593
AccountCommand::FreezeToken(token_id, is_unfreezable) => {
594594
let res = self
595-
.spend_input_from_account(*nonce, account_op.clone().into())
595+
.spend_input_from_account(*nonce, account_op.into())
596596
.and_then(|_| {
597597
self.tokens_accounting_cache
598598
.freeze_token(*token_id, *is_unfreezable)
@@ -602,7 +602,7 @@ where
602602
}
603603
AccountCommand::UnfreezeToken(token_id) => {
604604
let res = self
605-
.spend_input_from_account(*nonce, account_op.clone().into())
605+
.spend_input_from_account(*nonce, account_op.into())
606606
.and_then(|_| {
607607
self.tokens_accounting_cache
608608
.unfreeze_token(*token_id)
@@ -612,7 +612,7 @@ where
612612
}
613613
AccountCommand::ChangeTokenAuthority(token_id, new_authority) => {
614614
let res = self
615-
.spend_input_from_account(*nonce, account_op.clone().into())
615+
.spend_input_from_account(*nonce, account_op.into())
616616
.and_then(|_| {
617617
self.tokens_accounting_cache
618618
.change_authority(*token_id, new_authority.clone())
@@ -622,7 +622,7 @@ where
622622
}
623623
AccountCommand::ChangeTokenMetadataUri(token_id, new_metadata_uri) => {
624624
let res = self
625-
.spend_input_from_account(*nonce, account_op.clone().into())
625+
.spend_input_from_account(*nonce, account_op.into())
626626
.and_then(|_| {
627627
self.tokens_accounting_cache
628628
.change_metadata_uri(*token_id, new_metadata_uri.clone())
@@ -842,7 +842,7 @@ where
842842
| AccountCommand::ChangeTokenMetadataUri(..) => None,
843843
AccountCommand::ConcludeOrder(order_id) => {
844844
let res = self
845-
.spend_input_from_account(*nonce, account_op.clone().into())
845+
.spend_input_from_account(*nonce, account_op.into())
846846
.and_then(|_| {
847847
self.orders_accounting_cache
848848
.conclude_order(*order_id)
@@ -852,7 +852,7 @@ where
852852
}
853853
AccountCommand::FillOrder(order_id, fill, _) => {
854854
let res = self
855-
.spend_input_from_account(*nonce, account_op.clone().into())
855+
.spend_input_from_account(*nonce, account_op.into())
856856
.and_then(|_| {
857857
self.orders_accounting_cache
858858
.fill_order(*order_id, *fill, OrdersVersion::V0)

‎common/src/chain/transaction/account_outpoint.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,26 @@ pub enum AccountType {
3636
Order(OrderId),
3737
}
3838

39-
impl From<AccountSpending> for AccountType {
40-
fn from(spending: AccountSpending) -> Self {
39+
impl From<&AccountSpending> for AccountType {
40+
fn from(spending: &AccountSpending) -> Self {
4141
match spending {
42-
AccountSpending::DelegationBalance(id, _) => AccountType::Delegation(id),
42+
AccountSpending::DelegationBalance(id, _) => AccountType::Delegation(*id),
4343
}
4444
}
4545
}
4646

47-
impl From<AccountCommand> for AccountType {
48-
fn from(op: AccountCommand) -> Self {
47+
impl From<&AccountCommand> for AccountType {
48+
fn from(op: &AccountCommand) -> Self {
4949
match op {
5050
AccountCommand::MintTokens(id, _)
5151
| AccountCommand::UnmintTokens(id)
5252
| AccountCommand::LockTokenSupply(id)
5353
| AccountCommand::FreezeToken(id, _)
5454
| AccountCommand::UnfreezeToken(id)
5555
| AccountCommand::ChangeTokenAuthority(id, _)
56-
| AccountCommand::ChangeTokenMetadataUri(id, _) => AccountType::Token(id),
56+
| AccountCommand::ChangeTokenMetadataUri(id, _) => AccountType::Token(*id),
5757
AccountCommand::ConcludeOrder(id) | AccountCommand::FillOrder(id, _, _) => {
58-
AccountType::Order(id)
58+
AccountType::Order(*id)
5959
}
6060
}
6161
}

‎mempool/src/pool/entry.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ impl TxDependency {
6363
fn from_account(account: &AccountSpending, nonce: AccountNonce) -> Self {
6464
match account {
6565
AccountSpending::DelegationBalance(_, _) => {
66-
Self::DelegationAccount(TxAccountDependency::new(account.clone().into(), nonce))
66+
Self::DelegationAccount(TxAccountDependency::new(account.into(), nonce))
6767
}
6868
}
6969
}
@@ -77,10 +77,10 @@ impl TxDependency {
7777
| AccountCommand::UnfreezeToken(_)
7878
| AccountCommand::ChangeTokenMetadataUri(_, _)
7979
| AccountCommand::ChangeTokenAuthority(_, _) => {
80-
Self::TokenSupplyAccount(TxAccountDependency::new(cmd.clone().into(), nonce))
80+
Self::TokenSupplyAccount(TxAccountDependency::new(cmd.into(), nonce))
8181
}
8282
AccountCommand::ConcludeOrder(_) | AccountCommand::FillOrder(_, _, _) => {
83-
Self::OrderAccount(TxAccountDependency::new(cmd.clone().into(), nonce))
83+
Self::OrderAccount(TxAccountDependency::new(cmd.into(), nonce))
8484
}
8585
}
8686
}

‎mempool/src/pool/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,9 @@ impl<'a> TxFinalizer<'a> {
378378
TxAdditionOutcome::Rejected { transaction, error } => {
379379
let tx_id = *transaction.tx_id();
380380
let origin = transaction.origin();
381-
log::trace!("Rejected transaction {tx_id}, checking orphan status");
381+
log::trace!(
382+
"Rejected transaction {tx_id} with error {error}. Checking orphan status"
383+
);
382384

383385
self.try_add_orphan(tx_pool, transaction, error)
384386
.inspect_err(|err| match &mut self.events_mode {

‎node-gui/src/main_window/main_widget/tabs/wallet/delegation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ pub fn view_delegation(
8787
.map(|(del_id, (pool_id, b))| (*del_id, *pool_id, *b))
8888
{
8989
let delegation_address = Address::new(chain_config, delegation_id)
90-
.expect("Encoding pool id to address can't fail (GUI)");
90+
.expect("Encoding delegation id to address can't fail (GUI)");
9191
let pool_address = Address::new(chain_config, pool_id)
9292
.expect("Encoding pool id to address can't fail (GUI)");
9393
let delegate_staking_amount =

‎test/functional/wallet_conflict.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ async def async_test(self):
198198

199199
# check we cannot abandon an already confirmed transaction
200200
assert_in("Success", await wallet.select_account(1))
201-
assert_in("Cannot abandon a transaction in Confirmed at height 6", await wallet.abandon_transaction(new_transfer_tx_id))
201+
assert_in("Cannot change a transaction's state from Confirmed", await wallet.abandon_transaction(new_transfer_tx_id))
202202

203203

204204

‎wallet/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ thiserror.workspace = true
3737
zeroize.workspace = true
3838

3939
[dev-dependencies]
40+
chainstate-test-framework = { path = "../chainstate/test-framework" }
4041
test-utils = { path = "../test-utils" }
42+
4143
serial_test = "3.2"
4244

4345
rstest.workspace = true

‎wallet/src/account/mod.rs

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ use common::size_estimation::{
3535
use common::Uint256;
3636
use crypto::key::hdkd::child_number::ChildNumber;
3737
use mempool::FeeRate;
38-
use output_cache::OrderData;
3938
use serialization::hex_encoded::HexEncoded;
4039
use utils::ensure;
4140
pub use utxo_selector::UtxoSelectorError;
@@ -89,7 +88,8 @@ use wallet_types::{
8988
};
9089

9190
pub use self::output_cache::{
92-
DelegationData, OwnFungibleTokenInfo, PoolData, TxInfo, UnconfirmedTokenInfo, UtxoWithTxOutput,
91+
DelegationData, OrderData, OwnFungibleTokenInfo, PoolData, TxInfo, UnconfirmedTokenInfo,
92+
UtxoWithTxOutput,
9393
};
9494
use self::output_cache::{OutputCache, TokenIssuanceData};
9595
use self::transaction_list::{get_transaction_list, TransactionList};
@@ -887,6 +887,12 @@ impl<K: AccountKeyChains> Account<K> {
887887
.ok_or(WalletError::UnknownTokenId(*token_id))
888888
}
889889

890+
pub fn get_orders(&self) -> impl Iterator<Item = (&OrderId, &OrderData)> {
891+
self.output_cache
892+
.orders()
893+
.filter(|(_, data)| self.is_destination_mine(&data.conclude_key))
894+
}
895+
890896
pub fn find_order(&self, order_id: &OrderId) -> WalletResult<&OrderData> {
891897
self.output_cache
892898
.order_data(order_id)
@@ -1947,16 +1953,10 @@ impl<K: AccountKeyChains> Account<K> {
19471953
.txs_with_unconfirmed()
19481954
.iter()
19491955
.filter_map(|(id, tx)| match tx.state() {
1950-
TxState::Confirmed(height, _, idx) => {
1951-
if height > common_block_height {
1952-
Some((
1953-
AccountWalletTxId::new(self.get_account_id(), id.clone()),
1954-
(height, idx),
1955-
))
1956-
} else {
1957-
None
1958-
}
1959-
}
1956+
TxState::Confirmed(height, _, idx) => (height > common_block_height).then_some((
1957+
AccountWalletTxId::new(self.get_account_id(), id.clone()),
1958+
(height, idx),
1959+
)),
19601960
TxState::Inactive(_)
19611961
| TxState::Conflicted(_)
19621962
| TxState::InMempool(_)
@@ -1970,7 +1970,7 @@ impl<K: AccountKeyChains> Account<K> {
19701970
for (tx_id, _) in revoked_txs {
19711971
db_tx.del_transaction(&tx_id)?;
19721972
let source = tx_id.into_item_id();
1973-
self.output_cache.remove_tx(&source)?;
1973+
self.output_cache.remove_confirmed_tx(&source)?;
19741974
wallet_events.del_transaction(self.account_index(), source);
19751975
}
19761976

@@ -2083,7 +2083,8 @@ impl<K: AccountKeyChains> Account<K> {
20832083
let tx_state =
20842084
TxState::Confirmed(block_height, block.timestamp(), idx as u64);
20852085
let wallet_tx = WalletTx::Tx(TxData::new(signed_tx.clone(), tx_state));
2086-
self.update_conflicting_txs(&wallet_tx, block, db_tx)?;
2086+
2087+
self.update_conflicting_txs(signed_tx.transaction(), block, db_tx)?;
20872088

20882089
new_tx_was_added |= self
20892090
.add_wallet_tx_if_relevant_and_remove_from_user_txs(
@@ -2111,15 +2112,17 @@ impl<K: AccountKeyChains> Account<K> {
21112112
/// Check for any conflicting txs and update the new state in the DB
21122113
fn update_conflicting_txs<B: storage::Backend>(
21132114
&mut self,
2114-
wallet_tx: &WalletTx,
2115+
confirmed_tx: &Transaction,
21152116
block: &Block,
21162117
db_tx: &mut StoreTxRw<B>,
21172118
) -> WalletResult<()> {
21182119
let acc_id = self.get_account_id();
2119-
let conflicting_tx = self.output_cache.check_conflicting(wallet_tx, block.get_id().into());
2120-
for tx in conflicting_tx {
2121-
let id = AccountWalletTxId::new(acc_id.clone(), tx.id());
2122-
db_tx.set_transaction(&id, tx)?;
2120+
let conflicting_txs =
2121+
self.output_cache.update_conflicting_txs(confirmed_tx, block.get_id().into())?;
2122+
2123+
for (tx_id, tx) in conflicting_txs {
2124+
db_tx.set_transaction(&AccountWalletTxId::new(acc_id.clone(), tx.id()), &tx)?;
2125+
db_tx.del_user_transaction(&AccountWalletCreatedTxId::new(acc_id.clone(), tx_id))?;
21232126
}
21242127

21252128
Ok(())
@@ -2279,9 +2282,9 @@ impl<K: AccountKeyChains> Account<K> {
22792282
let abandoned_txs = self.output_cache.abandon_transaction(tx_id)?;
22802283
let acc_id = self.get_account_id();
22812284

2282-
for tx_id in abandoned_txs {
2283-
let id = AccountWalletCreatedTxId::new(acc_id.clone(), tx_id);
2284-
db_tx.del_user_transaction(&id)?;
2285+
for (tx_id, tx) in abandoned_txs {
2286+
db_tx.set_transaction(&AccountWalletTxId::new(acc_id.clone(), tx.id()), &tx)?;
2287+
db_tx.del_user_transaction(&AccountWalletCreatedTxId::new(acc_id.clone(), tx_id))?;
22852288
}
22862289

22872290
Ok(())

0 commit comments

Comments
 (0)