Skip to content
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.

Sign up for GitHub

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

Wallet handle txs with conflicting account nonces #1864

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+1,098 −169
Open

Wallet handle txs with conflicting account nonces #1864

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7d743af
Remove descendants of conflicting txs
azarovh Dec 27, 2024
769da7f
Use ref for AccountType::From
azarovh Jan 10, 2025
7281875
Check nonces while updating conflicting txs
azarovh Dec 27, 2024
6c83ac1
Confirmed tx should not be marked as conflicted
azarovh Jan 21, 2025
8c5fdac
Fix comments
azarovh Jan 22, 2025
3327b22
Error on changing to same tx state
azarovh Jan 29, 2025
529d6bf
Fix abandoning conflicting txs
azarovh Jan 30, 2025
83b515a
Fix comments
azarovh Feb 3, 2025
caa2c45
Fix conflicting duplicates
azarovh Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions chainstate/test-framework/src/block_builder.rs
View file
Open in desktop
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);
}
};
});
Expand Down
4 changes: 2 additions & 2 deletions chainstate/test-framework/src/pos_block_builder.rs
View file
Open in desktop
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);
}
};
});
Expand Down
24 changes: 12 additions & 12 deletions chainstate/tx-verifier/src/transaction_verifier/mod.rs
View file
Open in desktop
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(_) => { /* do nothing */ }
TxInput::Account(outpoint) => {
self.unspend_input_from_account(outpoint.account().clone().into())?;
self.unspend_input_from_account(outpoint.account().into())?;
}
TxInput::AccountCommand(_, account_op) => {
self.unspend_input_from_account(account_op.clone().into())?;
self.unspend_input_from_account(account_op.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 @@ -831,7 +831,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 @@ -841,7 +841,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)
Expand Down
14 changes: 7 additions & 7 deletions common/src/chain/transaction/account_outpoint.rs
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl TxDependency {
fn from_account(acct: &AccountSpending, nonce: AccountNonce) -> Self {
match acct {
AccountSpending::DelegationBalance(_, _) => {
Self::DelegationAccount(TxAccountDependency::new(acct.clone().into(), nonce))
Self::DelegationAccount(TxAccountDependency::new(acct.into(), nonce))
}
}
}
Expand All @@ -74,10 +74,10 @@ impl TxDependency {
| AccountCommand::UnfreezeToken(_)
| AccountCommand::ChangeTokenMetadataUri(_, _)
| AccountCommand::ChangeTokenAuthority(_, _) => {
Self::TokenSupplyAccount(TxAccountDependency::new(acct.clone().into(), nonce))
Self::TokenSupplyAccount(TxAccountDependency::new(acct.into(), nonce))
}
AccountCommand::ConcludeOrder(_) | AccountCommand::FillOrder(_, _, _) => {
Self::OrderAccount(TxAccountDependency::new(acct.clone().into(), nonce))
Self::OrderAccount(TxAccountDependency::new(acct.into(), nonce))
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion mempool/src/pool/mod.rs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion node-gui/src/main_window/main_widget/tabs/wallet/delegation.rs
View file
Open in desktop
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
View file
Open in desktop
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
25 changes: 17 additions & 8 deletions wallet/src/account/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,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 @@ -85,7 +84,8 @@ use wallet_types::{
};

pub use self::output_cache::{
DelegationData, FungibleTokenInfo, PoolData, TxInfo, UnconfirmedTokenInfo, UtxoWithTxOutput,
DelegationData, FungibleTokenInfo, OrderData, PoolData, TxInfo, UnconfirmedTokenInfo,
UtxoWithTxOutput,
};
use self::output_cache::{OutputCache, TokenIssuanceData};
use self::transaction_list::{get_transaction_list, TransactionList};
Expand Down Expand Up @@ -900,6 +900,12 @@ impl Account {
.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 @@ -2056,7 +2062,8 @@ impl Account {
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 @@ -2084,15 +2091,17 @@ impl Account {
/// 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 in conflicting_txs {
let id = AccountWalletCreatedTxId::new(acc_id.clone(), tx_id);
db_tx.del_user_transaction(&id)?;
}

Ok(())
Expand Down
Loading
Loading