Skip to content

Commit 529d6bf

Browse files
azarovhazarovh
committed
Fix abandoning conflicting txs
1 parent 3327b22 commit 529d6bf

File tree

2 files changed

+126
-82
lines changed

2 files changed

+126
-82
lines changed

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

+114-82
Original file line numberDiff line numberDiff line change
@@ -761,8 +761,40 @@ impl OutputCache {
761761
let mut conflicting_txs_with_descendants = vec![];
762762

763763
for conflicting_tx in conflicting_txs {
764-
let descendants =
765-
self.remove_descendants_and_mark_as(conflicting_tx, TxState::Conflicted(block_id))?;
764+
let descendants = self.remove_descendants(conflicting_tx);
765+
766+
// Mark conflicting tx and its descendants as Conflicting and update OutputCache data accordingly
767+
for tx_id in descendants.iter().rev().copied() {
768+
match self.txs.entry(tx_id.into()) {
769+
Entry::Occupied(mut entry) => match entry.get_mut() {
770+
WalletTx::Block(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
771+
WalletTx::Tx(tx) => match tx.state() {
772+
TxState::Inactive(_) => {
773+
tx.set_state(TxState::Conflicted(block_id));
774+
OutputCache::rollback_tx_data(
775+
tx,
776+
&self.unconfirmed_descendants,
777+
&mut self.consumed,
778+
&mut self.delegations,
779+
&mut self.token_issuance,
780+
&mut self.orders,
781+
);
782+
Ok(())
783+
}
784+
TxState::Abandoned
785+
| TxState::Confirmed(..)
786+
| TxState::InMempool(..)
787+
| TxState::Conflicted(..) => {
788+
Err(WalletError::CannotChangeTransactionState(
789+
*tx.state(),
790+
TxState::Conflicted(block_id),
791+
))
792+
}
793+
},
794+
},
795+
Entry::Vacant(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
796+
}?;
797+
}
766798

767799
conflicting_txs_with_descendants.extend(descendants.into_iter());
768800
}
@@ -1409,11 +1441,7 @@ impl OutputCache {
14091441
})
14101442
}
14111443

1412-
fn remove_descendants_and_mark_as(
1413-
&mut self,
1414-
tx_id: Id<Transaction>,
1415-
new_state: TxState,
1416-
) -> WalletResult<Vec<Id<Transaction>>> {
1444+
fn remove_descendants(&mut self, tx_id: Id<Transaction>) -> Vec<Id<Transaction>> {
14171445
let mut all_txs = Vec::new();
14181446
let mut to_update = BTreeSet::from_iter([OutPointSourceId::from(tx_id)]);
14191447

@@ -1425,85 +1453,56 @@ impl OutputCache {
14251453
}
14261454
}
14271455

1428-
for tx_id in all_txs.iter().rev().copied() {
1429-
match self.txs.entry(tx_id.into()) {
1430-
Entry::Occupied(mut entry) => match entry.get_mut() {
1431-
WalletTx::Block(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
1432-
WalletTx::Tx(tx) => {
1433-
ensure!(
1434-
tx.state() != &new_state,
1435-
WalletError::CannotChangeTransactionState(*tx.state(), new_state)
1436-
);
1456+
all_txs
1457+
}
14371458

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());
1445-
}
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-
}
1460-
}
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-
}
1480-
}
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-
}
1493-
}
1494-
Ok(())
1495-
}
1496-
TxState::Confirmed(..) | TxState::InMempool(..) => Err(
1497-
WalletError::CannotChangeTransactionState(*tx.state(), new_state),
1498-
),
1459+
// After tx is abandoned or marked as conflicted its effect on OutputCache should be rolled back
1460+
fn rollback_tx_data(
1461+
tx: &TxData,
1462+
unconfirmed_descendants: &BTreeMap<OutPointSourceId, BTreeSet<OutPointSourceId>>,
1463+
consumed: &mut BTreeMap<UtxoOutPoint, TxState>,
1464+
delegations: &mut BTreeMap<DelegationId, DelegationData>,
1465+
token_issuance: &mut BTreeMap<TokenId, TokenIssuanceData>,
1466+
orders: &mut BTreeMap<OrderId, OrderData>,
1467+
) {
1468+
let tx_id = tx.get_transaction().get_id();
1469+
for input in tx.get_transaction().inputs() {
1470+
match input {
1471+
TxInput::Utxo(outpoint) => {
1472+
consumed.insert(outpoint.clone(), *tx.state());
1473+
}
1474+
TxInput::Account(outpoint) => match outpoint.account() {
1475+
AccountSpending::DelegationBalance(delegation_id, _) => {
1476+
if let Some(data) = delegations.get_mut(delegation_id) {
1477+
data.last_nonce = outpoint.nonce().decrement();
1478+
data.last_parent = find_parent(unconfirmed_descendants, tx_id.into());
14991479
}
15001480
}
15011481
},
1502-
Entry::Vacant(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
1503-
}?;
1482+
TxInput::AccountCommand(nonce, op) => match op {
1483+
AccountCommand::MintTokens(token_id, _)
1484+
| AccountCommand::UnmintTokens(token_id)
1485+
| AccountCommand::LockTokenSupply(token_id)
1486+
| AccountCommand::FreezeToken(token_id, _)
1487+
| AccountCommand::UnfreezeToken(token_id)
1488+
| AccountCommand::ChangeTokenMetadataUri(token_id, _)
1489+
| AccountCommand::ChangeTokenAuthority(token_id, _) => {
1490+
if let Some(data) = token_issuance.get_mut(token_id) {
1491+
data.last_nonce = nonce.decrement();
1492+
data.last_parent = find_parent(unconfirmed_descendants, tx_id.into());
1493+
data.unconfirmed_txs.remove(&tx_id.into());
1494+
}
1495+
}
1496+
AccountCommand::ConcludeOrder(order_id)
1497+
| AccountCommand::FillOrder(order_id, _, _) => {
1498+
if let Some(data) = orders.get_mut(order_id) {
1499+
data.last_nonce = nonce.decrement();
1500+
data.last_parent = find_parent(unconfirmed_descendants, tx_id.into());
1501+
}
1502+
}
1503+
},
1504+
}
15041505
}
1505-
1506-
Ok(all_txs)
15071506
}
15081507

15091508
/// Mark a transaction and its descendants as abandoned
@@ -1512,7 +1511,40 @@ impl OutputCache {
15121511
&mut self,
15131512
tx_id: Id<Transaction>,
15141513
) -> WalletResult<Vec<Id<Transaction>>> {
1515-
self.remove_descendants_and_mark_as(tx_id, TxState::Abandoned)
1514+
let all_abandoned = self.remove_descendants(tx_id);
1515+
1516+
for tx_id in all_abandoned.iter().rev().copied() {
1517+
match self.txs.entry(tx_id.into()) {
1518+
Entry::Occupied(mut entry) => match entry.get_mut() {
1519+
WalletTx::Block(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
1520+
WalletTx::Tx(tx) => match tx.state() {
1521+
TxState::Inactive(_) => {
1522+
tx.set_state(TxState::Abandoned);
1523+
OutputCache::rollback_tx_data(
1524+
tx,
1525+
&self.unconfirmed_descendants,
1526+
&mut self.consumed,
1527+
&mut self.delegations,
1528+
&mut self.token_issuance,
1529+
&mut self.orders,
1530+
);
1531+
Ok(())
1532+
}
1533+
TxState::Conflicted(_) => {
1534+
tx.set_state(TxState::Abandoned);
1535+
Ok(())
1536+
}
1537+
state => Err(WalletError::CannotChangeTransactionState(
1538+
*state,
1539+
TxState::Abandoned,
1540+
)),
1541+
},
1542+
},
1543+
Entry::Vacant(_) => Err(WalletError::CannotFindTransactionWithId(tx_id)),
1544+
}?;
1545+
}
1546+
1547+
Ok(all_abandoned)
15161548
}
15171549

15181550
pub fn get_transaction(&self, transaction_id: Id<Transaction>) -> WalletResult<&TxData> {

‎wallet/src/wallet/tests.rs

+12
Original file line numberDiff line numberDiff line change
@@ -6207,6 +6207,12 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
62076207
TxState::Abandoned
62086208
);
62096209

6210+
let mut delegations = wallet.get_delegations(DEFAULT_ACCOUNT_INDEX).unwrap().collect_vec();
6211+
assert_eq!(delegations.len(), 1);
6212+
let (deleg_id, deleg_data) = delegations.pop().unwrap();
6213+
assert_eq!(*deleg_id, delegation_id);
6214+
assert_eq!(deleg_data.last_nonce, Some(AccountNonce::new(0)));
6215+
62106216
wallet
62116217
.abandon_transaction(DEFAULT_ACCOUNT_INDEX, spend_from_delegation_tx_2_id)
62126218
.unwrap();
@@ -6217,6 +6223,12 @@ fn conflicting_delegation_account_nonce(#[case] seed: Seed) {
62176223
.state(),
62186224
TxState::Abandoned
62196225
);
6226+
6227+
let mut delegations = wallet.get_delegations(DEFAULT_ACCOUNT_INDEX).unwrap().collect_vec();
6228+
assert_eq!(delegations.len(), 1);
6229+
let (deleg_id, deleg_data) = delegations.pop().unwrap();
6230+
assert_eq!(*deleg_id, delegation_id);
6231+
assert_eq!(deleg_data.last_nonce, Some(AccountNonce::new(0)));
62206232
}
62216233

62226234
// Create a pool and a delegation with some share.

0 commit comments

Comments
 (0)