Skip to content

Commit

Permalink
fix: check for previously cancelled completed txn before accepting a …
Browse files Browse the repository at this point in the history
…repeat message (#3542)

Description
---
When handling a potentially repeated inbound transaction message there was a check for previously cancelled PENDING transactions but this did not check for a previously cancelled completed transaction.
This can occur if a transaction was completed and got rejected by the mempool. If this transaction exists then we should not add the pending transaction to our list.

This PR updates the check for previously cancelled transactions to include completed transactions.


How Has This Been Tested?
---
cargo test
  • Loading branch information
philipr-za committed Nov 8, 2021
1 parent 532e509 commit 911b83b
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 4 deletions.
10 changes: 6 additions & 4 deletions base_layer/wallet/src/transaction_service/service.rs
Expand Up @@ -1245,14 +1245,16 @@ where
);

// Check if this transaction has already been received and cancelled.
if let Ok(inbound_tx) = self.db.get_cancelled_pending_inbound_transaction(data.tx_id).await {
if inbound_tx.source_public_key != source_pubkey {
if let Ok(Some(any_tx)) = self.db.get_any_cancelled_transaction(data.tx_id).await {
let tx = CompletedTransaction::from(any_tx);

if tx.source_public_key != source_pubkey {
return Err(TransactionServiceError::InvalidSourcePublicKey);
}
trace!(
target: LOG_TARGET,
"A repeated Transaction (TxId: {}) has been received but has been previously cancelled",
inbound_tx.tx_id
"A repeated Transaction (TxId: {}) has been received but has been previously cancelled or rejected",
tx.tx_id
);
return Ok(());
}
Expand Down
18 changes: 18 additions & 0 deletions base_layer/wallet/src/transaction_service/storage/database.rs
Expand Up @@ -58,6 +58,12 @@ pub trait TransactionBackend: Send + Sync + Clone {

fn get_transactions_to_be_broadcast(&self) -> Result<Vec<CompletedTransaction>, TransactionStorageError>;

/// Check for presence of any form of cancelled transaction with this TxId
fn fetch_any_cancelled_transaction(
&self,
tx_id: TxId,
) -> Result<Option<WalletTransaction>, TransactionStorageError>;

/// Check if a record with the provided key exists in the backend.
fn contains(&self, key: &DbKey) -> Result<bool, TransactionStorageError>;
/// Modify the state the of the backend with a write operation
Expand Down Expand Up @@ -570,6 +576,18 @@ where T: TransactionBackend + 'static
Ok(t)
}

pub async fn get_any_cancelled_transaction(
&self,
tx_id: TxId,
) -> Result<Option<WalletTransaction>, TransactionStorageError> {
let db_clone = self.db.clone();

let tx = tokio::task::spawn_blocking(move || db_clone.fetch_any_cancelled_transaction(tx_id))
.await
.map_err(|err| TransactionStorageError::BlockingTaskSpawnError(err.to_string()))??;
Ok(tx)
}

async fn get_completed_transactions_by_cancelled(
&self,
cancelled: bool,
Expand Down
38 changes: 38 additions & 0 deletions base_layer/wallet/src/transaction_service/storage/sqlite_db.rs
Expand Up @@ -512,6 +512,44 @@ impl TransactionBackend for TransactionServiceSqliteDatabase {
Err(TransactionStorageError::ValuesNotFound)
}

fn fetch_any_cancelled_transaction(
&self,
tx_id: TxId,
) -> Result<Option<WalletTransaction>, TransactionStorageError> {
let conn = self.database_connection.acquire_lock();

match OutboundTransactionSql::find_by_cancelled(tx_id, true, &(*conn)) {
Ok(mut o) => {
self.decrypt_if_necessary(&mut o)?;

return Ok(Some(WalletTransaction::PendingOutbound(OutboundTransaction::try_from(
o,
)?)));
},
Err(TransactionStorageError::DieselError(DieselError::NotFound)) => (),
Err(e) => return Err(e),
};
match InboundTransactionSql::find_by_cancelled(tx_id, true, &(*conn)) {
Ok(mut i) => {
self.decrypt_if_necessary(&mut i)?;
return Ok(Some(WalletTransaction::PendingInbound(InboundTransaction::try_from(
i,
)?)));
},
Err(TransactionStorageError::DieselError(DieselError::NotFound)) => (),
Err(e) => return Err(e),
};
match CompletedTransactionSql::find_by_cancelled(tx_id, true, &(*conn)) {
Ok(mut c) => {
self.decrypt_if_necessary(&mut c)?;
return Ok(Some(WalletTransaction::Completed(CompletedTransaction::try_from(c)?)));
},
Err(TransactionStorageError::DieselError(DieselError::NotFound)) => (),
Err(e) => return Err(e),
};
Ok(None)
}

fn complete_outbound_transaction(
&self,
tx_id: u64,
Expand Down

0 comments on commit 911b83b

Please sign in to comment.