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 1 commit
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
130 changes: 69 additions & 61 deletions wallet/src/account/output_cache/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,9 @@ impl OutputCache {
confirmed_account_nonce,
) {
if let WalletTx::Tx(tx) = unconfirmed_tx {
conflicting_txs.insert(tx.get_transaction().get_id());
if confirmed_tx.get_id() != tx.get_transaction().get_id() {
conflicting_txs.insert(tx.get_transaction().get_id());
}
}
}
}
Expand All @@ -759,14 +761,10 @@ impl OutputCache {
let mut conflicting_txs_with_descendants = vec![];

for conflicting_tx in conflicting_txs {
if conflicting_tx != confirmed_tx.get_id() {
let descendants = self.remove_descendants_and_mark_as(
conflicting_tx,
TxState::Conflicted(block_id),
)?;
let descendants =
self.remove_descendants_and_mark_as(conflicting_tx, TxState::Conflicted(block_id))?;

conflicting_txs_with_descendants.extend(descendants.into_iter());
}
conflicting_txs_with_descendants.extend(descendants.into_iter());
}

Ok(conflicting_txs_with_descendants)
Expand Down Expand Up @@ -1431,65 +1429,75 @@ impl OutputCache {
match self.txs.entry(tx_id.into()) {
Entry::Occupied(mut entry) => match entry.get_mut() {
WalletTx::Block(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
WalletTx::Tx(tx) => match tx.state() {
TxState::Inactive(_) | TxState::Conflicted(_) | TxState::Abandoned => {
tx.set_state(new_state);
for input in tx.get_transaction().inputs() {
match input {
TxInput::Utxo(outpoint) => {
self.consumed.insert(outpoint.clone(), *tx.state());
}
TxInput::Account(outpoint) => match outpoint.account() {
AccountSpending::DelegationBalance(delegation_id, _) => {
if let Some(data) =
self.delegations.get_mut(delegation_id)
{
data.last_nonce = outpoint.nonce().decrement();
data.last_parent = find_parent(
&self.unconfirmed_descendants,
tx_id.into(),
);
}
WalletTx::Tx(tx) => {
ensure!(
tx.state() != &new_state,
WalletError::CannotChangeTransactionState(*tx.state(), new_state)
);

match tx.state() {
TxState::Inactive(_) | TxState::Conflicted(_) | TxState::Abandoned => {
tx.set_state(new_state);
for input in tx.get_transaction().inputs() {
match input {
TxInput::Utxo(outpoint) => {
self.consumed.insert(outpoint.clone(), *tx.state());
}
},
TxInput::AccountCommand(nonce, op) => match op {
AccountCommand::MintTokens(token_id, _)
| AccountCommand::UnmintTokens(token_id)
| AccountCommand::LockTokenSupply(token_id)
| AccountCommand::FreezeToken(token_id, _)
| AccountCommand::UnfreezeToken(token_id)
| AccountCommand::ChangeTokenMetadataUri(token_id, _)
| AccountCommand::ChangeTokenAuthority(token_id, _) => {
if let Some(data) =
self.token_issuance.get_mut(token_id)
{
data.last_nonce = nonce.decrement();
data.last_parent = find_parent(
&self.unconfirmed_descendants,
tx_id.into(),
);
data.unconfirmed_txs.remove(&tx_id.into());
TxInput::Account(outpoint) => match outpoint.account() {
AccountSpending::DelegationBalance(
delegation_id,
_,
) => {
if let Some(data) =
self.delegations.get_mut(delegation_id)
{
data.last_nonce = outpoint.nonce().decrement();
data.last_parent = find_parent(
&self.unconfirmed_descendants,
tx_id.into(),
);
}
}
}
AccountCommand::ConcludeOrder(order_id)
| AccountCommand::FillOrder(order_id, _, _) => {
if let Some(data) = self.orders.get_mut(order_id) {
data.last_nonce = nonce.decrement();
data.last_parent = find_parent(
&self.unconfirmed_descendants,
tx_id.into(),
);
},
TxInput::AccountCommand(nonce, op) => match op {
AccountCommand::MintTokens(token_id, _)
| AccountCommand::UnmintTokens(token_id)
| AccountCommand::LockTokenSupply(token_id)
| AccountCommand::FreezeToken(token_id, _)
| AccountCommand::UnfreezeToken(token_id)
| AccountCommand::ChangeTokenMetadataUri(token_id, _)
| AccountCommand::ChangeTokenAuthority(token_id, _) => {
if let Some(data) =
self.token_issuance.get_mut(token_id)
{
data.last_nonce = nonce.decrement();
data.last_parent = find_parent(
&self.unconfirmed_descendants,
tx_id.into(),
);
data.unconfirmed_txs.remove(&tx_id.into());
}
}
}
},
AccountCommand::ConcludeOrder(order_id)
| AccountCommand::FillOrder(order_id, _, _) => {
if let Some(data) = self.orders.get_mut(order_id) {
data.last_nonce = nonce.decrement();
data.last_parent = find_parent(
&self.unconfirmed_descendants,
tx_id.into(),
);
}
}
},
}
}
Ok(())
}
Ok(())
TxState::Confirmed(..) | TxState::InMempool(..) => Err(
WalletError::CannotChangeTransactionState(*tx.state(), new_state),
),
}
TxState::Confirmed(..) | TxState::InMempool(..) => Err(
WalletError::CannotChangeTransactionState(*tx.state(), new_state),
),
},
}
},
Entry::Vacant(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
}?;
Expand Down
77 changes: 71 additions & 6 deletions wallet/src/wallet/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3899,6 +3899,13 @@ fn wallet_abandone_transactions(#[case] seed: Seed) {
wallet.scan_mempool(txs_to_keep.as_slice(), &WalletEventsNoOp).unwrap();
let coin_balance = get_coin_balance_with_inactive(&wallet);
assert_eq!(coin_balance, coins_after_abandon);

// Abandone the same tx again
azarovh marked this conversation as resolved.
Show resolved Hide resolved
let result = wallet.abandon_transaction(DEFAULT_ACCOUNT_INDEX, transaction_id);
assert_eq!(
result.unwrap_err(),
WalletError::CannotChangeTransactionState(TxState::Abandoned, TxState::Abandoned)
);
}

#[rstest]
Expand Down Expand Up @@ -5940,12 +5947,12 @@ fn create_order_fill_partially_conclude(#[case] seed: Seed) {
}
}

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

wallet
.add_account_unconfirmed_tx(
Expand All @@ -6091,6 +6099,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
FeeRate::from_amount_per_kb(Amount::ZERO),
)
.unwrap();
let spend_from_delegation_tx_2_id = spend_from_delegation_tx_2.transaction().get_id();

wallet
.add_account_unconfirmed_tx(
Expand Down Expand Up @@ -6119,6 +6128,7 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
FeeRate::from_amount_per_kb(Amount::ZERO),
)
.unwrap();
let spend_from_delegation_tx_3_id = spend_from_delegation_tx_3.transaction().get_id();

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

// if confirmed tx is added conflicting txs must be removed from the output cache
Expand Down Expand Up @@ -6157,13 +6168,62 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
coin_balance,
(coin_balance_after_delegating + withdraw_amount_3).unwrap()
);

assert_eq!(
*wallet
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
.unwrap()
.state(),
TxState::Conflicted(block5_id.into())
);
assert_eq!(
*wallet
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
.unwrap()
.state(),
TxState::Conflicted(block5_id.into())
);
assert_eq!(
*wallet
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_3_id)
.unwrap()
.state(),
TxState::Confirmed(
BlockHeight::new(5),
chain_config.genesis_block().timestamp(),
0
)
);

// Abandone conflicting txs
azarovh marked this conversation as resolved.
Show resolved Hide resolved
wallet
.abandon_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
.unwrap();
assert_eq!(
*wallet
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_1_id)
.unwrap()
.state(),
TxState::Abandoned
);

wallet
.abandon_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
.unwrap();
assert_eq!(
*wallet
.get_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
.unwrap()
.state(),
TxState::Abandoned
);
}

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

// Issue and mint some tokens
// Create an order selling tokens for coins
// Create 2 fill order txs and add them to a wallet as unconfirmed
// Confirm the first tx in a block and check that it is accounted in confirmed balance
// and also that unconfirmed balance has second tx.
azarovh marked this conversation as resolved.
Show resolved Hide resolved
#[rstest]
#[trace]
#[case(Seed::from_entropy())]
Expand Down Expand Up @@ -6496,7 +6561,7 @@ fn conflicting_order_account_nonce(#[case] seed: Seed) {
assert_eq!(actual_order_data.last_nonce, None);
assert_eq!(&actual_order_data.conclude_key, address2.as_object());

// Create 2 fill orders txs and put them in unconfirmed
// Create 2 fill order txs and put them in unconfirmed
let order_info = RpcOrderInfo {
conclude_key: address2.clone().into_object(),
initially_given: RpcOutputValue::Token {
Expand Down