Skip to content

Wallet handle txs with conflicting account nonces #1864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Apr 15, 2025
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions chainstate/test-framework/src/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,10 @@ impl<'f> BlockBuilder<'f> {
}
TxInput::Account(outpoint) => {
self.account_nonce_tracker
.insert(outpoint.account().clone().into(), outpoint.nonce());
.insert(outpoint.account().into(), outpoint.nonce());
}
TxInput::AccountCommand(nonce, op) => {
self.account_nonce_tracker.insert(op.clone().into(), *nonce);
self.account_nonce_tracker.insert(op.into(), *nonce);
}
TxInput::OrderAccountCommand(..) => {}
};
Expand Down
4 changes: 2 additions & 2 deletions chainstate/test-framework/src/pos_block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,10 +450,10 @@ impl<'f> PoSBlockBuilder<'f> {
}
TxInput::Account(outpoint) => {
self.account_nonce_tracker
.insert(outpoint.account().clone().into(), outpoint.nonce());
.insert(outpoint.account().into(), outpoint.nonce());
}
TxInput::AccountCommand(nonce, op) => {
self.account_nonce_tracker.insert(op.clone().into(), *nonce);
self.account_nonce_tracker.insert(op.into(), *nonce);
}
TxInput::OrderAccountCommand(..) => {}
};
Expand Down
24 changes: 12 additions & 12 deletions chainstate/tx-verifier/src/transaction_verifier/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ where
let res = self
.spend_input_from_account(
outpoint.nonce(),
outpoint.account().clone().into(),
outpoint.account().into(),
)
.and_then(|_| {
// If the input spends from delegation account, this means the user is
Expand Down Expand Up @@ -496,10 +496,10 @@ where
match input {
TxInput::Utxo(_) | TxInput::OrderAccountCommand(_) => { /* do nothing */ }
TxInput::Account(outpoint) => {
self.unspend_input_from_account(outpoint.account().clone().into())?;
self.unspend_input_from_account(outpoint.account().into())?;
}
TxInput::AccountCommand(_, cmd) => {
self.unspend_input_from_account(cmd.clone().into())?;
self.unspend_input_from_account(cmd.into())?;
}
};
}
Expand Down Expand Up @@ -553,7 +553,7 @@ where
TxInput::AccountCommand(nonce, account_op) => match account_op {
AccountCommand::MintTokens(token_id, amount) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
self.tokens_accounting_cache
.mint_tokens(*token_id, *amount)
Expand All @@ -563,7 +563,7 @@ where
}
AccountCommand::UnmintTokens(ref token_id) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
// actual amount to unmint is determined by the number of burned tokens in the outputs
let total_burned =
Expand All @@ -582,7 +582,7 @@ where
}
AccountCommand::LockTokenSupply(token_id) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
self.tokens_accounting_cache
.lock_circulating_supply(*token_id)
Expand All @@ -592,7 +592,7 @@ where
}
AccountCommand::FreezeToken(token_id, is_unfreezable) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
self.tokens_accounting_cache
.freeze_token(*token_id, *is_unfreezable)
Expand All @@ -602,7 +602,7 @@ where
}
AccountCommand::UnfreezeToken(token_id) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
self.tokens_accounting_cache
.unfreeze_token(*token_id)
Expand All @@ -612,7 +612,7 @@ where
}
AccountCommand::ChangeTokenAuthority(token_id, new_authority) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
self.tokens_accounting_cache
.change_authority(*token_id, new_authority.clone())
Expand All @@ -622,7 +622,7 @@ where
}
AccountCommand::ChangeTokenMetadataUri(token_id, new_metadata_uri) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
self.tokens_accounting_cache
.change_metadata_uri(*token_id, new_metadata_uri.clone())
Expand Down Expand Up @@ -842,7 +842,7 @@ where
| AccountCommand::ChangeTokenMetadataUri(..) => None,
AccountCommand::ConcludeOrder(order_id) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
self.orders_accounting_cache
.conclude_order(*order_id)
Expand All @@ -852,7 +852,7 @@ where
}
AccountCommand::FillOrder(order_id, fill, _) => {
let res = self
.spend_input_from_account(*nonce, account_op.clone().into())
.spend_input_from_account(*nonce, account_op.into())
.and_then(|_| {
self.orders_accounting_cache
.fill_order(*order_id, *fill, OrdersVersion::V0)
Expand Down
14 changes: 7 additions & 7 deletions common/src/chain/transaction/account_outpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,26 @@ pub enum AccountType {
Order(OrderId),
}

impl From<AccountSpending> for AccountType {
fn from(spending: AccountSpending) -> Self {
impl From<&AccountSpending> for AccountType {
fn from(spending: &AccountSpending) -> Self {
match spending {
AccountSpending::DelegationBalance(id, _) => AccountType::Delegation(id),
AccountSpending::DelegationBalance(id, _) => AccountType::Delegation(*id),
}
}
}

impl From<AccountCommand> for AccountType {
fn from(op: AccountCommand) -> Self {
impl From<&AccountCommand> for AccountType {
fn from(op: &AccountCommand) -> Self {
match op {
AccountCommand::MintTokens(id, _)
| AccountCommand::UnmintTokens(id)
| AccountCommand::LockTokenSupply(id)
| AccountCommand::FreezeToken(id, _)
| AccountCommand::UnfreezeToken(id)
| AccountCommand::ChangeTokenAuthority(id, _)
| AccountCommand::ChangeTokenMetadataUri(id, _) => AccountType::Token(id),
| AccountCommand::ChangeTokenMetadataUri(id, _) => AccountType::Token(*id),
AccountCommand::ConcludeOrder(id) | AccountCommand::FillOrder(id, _, _) => {
AccountType::Order(id)
AccountType::Order(*id)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions mempool/src/pool/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl TxDependency {
fn from_account(account: &AccountSpending, nonce: AccountNonce) -> Self {
match account {
AccountSpending::DelegationBalance(_, _) => {
Self::DelegationAccount(TxAccountDependency::new(account.clone().into(), nonce))
Self::DelegationAccount(TxAccountDependency::new(account.into(), nonce))
}
}
}
Expand All @@ -77,10 +77,10 @@ impl TxDependency {
| AccountCommand::UnfreezeToken(_)
| AccountCommand::ChangeTokenMetadataUri(_, _)
| AccountCommand::ChangeTokenAuthority(_, _) => {
Self::TokenSupplyAccount(TxAccountDependency::new(cmd.clone().into(), nonce))
Self::TokenSupplyAccount(TxAccountDependency::new(cmd.into(), nonce))
}
AccountCommand::ConcludeOrder(_) | AccountCommand::FillOrder(_, _, _) => {
Self::OrderAccount(TxAccountDependency::new(cmd.clone().into(), nonce))
Self::OrderAccount(TxAccountDependency::new(cmd.into(), nonce))
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion mempool/src/pool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,9 @@ impl<'a> TxFinalizer<'a> {
TxAdditionOutcome::Rejected { transaction, error } => {
let tx_id = *transaction.tx_id();
let origin = transaction.origin();
log::trace!("Rejected transaction {tx_id}, checking orphan status");
log::trace!(
"Rejected transaction {tx_id} with error {error}. Checking orphan status"
);

self.try_add_orphan(tx_pool, transaction, error)
.inspect_err(|err| match &mut self.events_mode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub fn view_delegation(
.map(|(del_id, (pool_id, b))| (*del_id, *pool_id, *b))
{
let delegation_address = Address::new(chain_config, delegation_id)
.expect("Encoding pool id to address can't fail (GUI)");
.expect("Encoding delegation id to address can't fail (GUI)");
let pool_address = Address::new(chain_config, pool_id)
.expect("Encoding pool id to address can't fail (GUI)");
let delegate_staking_amount =
Expand Down
2 changes: 1 addition & 1 deletion test/functional/wallet_conflict.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ async def async_test(self):

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



Expand Down
2 changes: 2 additions & 0 deletions wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ thiserror.workspace = true
zeroize.workspace = true

[dev-dependencies]
chainstate-test-framework = { path = "../chainstate/test-framework" }
test-utils = { path = "../test-utils" }

serial_test = "3.2"

rstest.workspace = true
Expand Down
47 changes: 25 additions & 22 deletions wallet/src/account/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use common::size_estimation::{
use common::Uint256;
use crypto::key::hdkd::child_number::ChildNumber;
use mempool::FeeRate;
use output_cache::OrderData;
use serialization::hex_encoded::HexEncoded;
use utils::ensure;
pub use utxo_selector::UtxoSelectorError;
Expand Down Expand Up @@ -89,7 +88,8 @@ use wallet_types::{
};

pub use self::output_cache::{
DelegationData, OwnFungibleTokenInfo, PoolData, TxInfo, UnconfirmedTokenInfo, UtxoWithTxOutput,
DelegationData, OrderData, OwnFungibleTokenInfo, PoolData, TxInfo, UnconfirmedTokenInfo,
UtxoWithTxOutput,
};
use self::output_cache::{OutputCache, TokenIssuanceData};
use self::transaction_list::{get_transaction_list, TransactionList};
Expand Down Expand Up @@ -887,6 +887,12 @@ impl<K: AccountKeyChains> Account<K> {
.ok_or(WalletError::UnknownTokenId(*token_id))
}

pub fn get_orders(&self) -> impl Iterator<Item = (&OrderId, &OrderData)> {
self.output_cache
.orders()
.filter(|(_, data)| self.is_destination_mine(&data.conclude_key))
}

pub fn find_order(&self, order_id: &OrderId) -> WalletResult<&OrderData> {
self.output_cache
.order_data(order_id)
Expand Down Expand Up @@ -1947,16 +1953,10 @@ impl<K: AccountKeyChains> Account<K> {
.txs_with_unconfirmed()
.iter()
.filter_map(|(id, tx)| match tx.state() {
TxState::Confirmed(height, _, idx) => {
if height > common_block_height {
Some((
AccountWalletTxId::new(self.get_account_id(), id.clone()),
(height, idx),
))
} else {
None
}
}
TxState::Confirmed(height, _, idx) => (height > common_block_height).then_some((
AccountWalletTxId::new(self.get_account_id(), id.clone()),
(height, idx),
)),
TxState::Inactive(_)
| TxState::Conflicted(_)
| TxState::InMempool(_)
Expand All @@ -1970,7 +1970,7 @@ impl<K: AccountKeyChains> Account<K> {
for (tx_id, _) in revoked_txs {
db_tx.del_transaction(&tx_id)?;
let source = tx_id.into_item_id();
self.output_cache.remove_tx(&source)?;
self.output_cache.remove_confirmed_tx(&source)?;
wallet_events.del_transaction(self.account_index(), source);
}

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

self.update_conflicting_txs(signed_tx.transaction(), block, db_tx)?;

new_tx_was_added |= self
.add_wallet_tx_if_relevant_and_remove_from_user_txs(
Expand Down Expand Up @@ -2111,15 +2112,17 @@ impl<K: AccountKeyChains> Account<K> {
/// Check for any conflicting txs and update the new state in the DB
fn update_conflicting_txs<B: storage::Backend>(
&mut self,
wallet_tx: &WalletTx,
confirmed_tx: &Transaction,
block: &Block,
db_tx: &mut StoreTxRw<B>,
) -> WalletResult<()> {
let acc_id = self.get_account_id();
let conflicting_tx = self.output_cache.check_conflicting(wallet_tx, block.get_id().into());
for tx in conflicting_tx {
let id = AccountWalletTxId::new(acc_id.clone(), tx.id());
db_tx.set_transaction(&id, tx)?;
let conflicting_txs =
self.output_cache.update_conflicting_txs(confirmed_tx, block.get_id().into())?;

for (tx_id, tx) in conflicting_txs {
db_tx.set_transaction(&AccountWalletTxId::new(acc_id.clone(), tx.id()), &tx)?;
db_tx.del_user_transaction(&AccountWalletCreatedTxId::new(acc_id.clone(), tx_id))?;
}

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

for tx_id in abandoned_txs {
let id = AccountWalletCreatedTxId::new(acc_id.clone(), tx_id);
db_tx.del_user_transaction(&id)?;
for (tx_id, tx) in abandoned_txs {
db_tx.set_transaction(&AccountWalletTxId::new(acc_id.clone(), tx.id()), &tx)?;
db_tx.del_user_transaction(&AccountWalletCreatedTxId::new(acc_id.clone(), tx_id))?;
}

Ok(())
Expand Down
Loading