Skip to content

Commit

Permalink
feat: add a Rejected status to TransactionStatus (#3512)
Browse files Browse the repository at this point in the history
Description
---
This status will indicate the transaction was rejected by the mempool for being invalid. In the past it was just cancelled which is also the case when a Coinbase Transaction is manually cancelled during mining because the blockchain has moved on.

This PR adds the new status and applies is when a transaction is rejected during the `transaction_broadcast_protocol`. The Console Wallet is updated to display the rejected status.

How Has This Been Tested?
---
cargo test and manual testing
  • Loading branch information
philipr-za authored Nov 3, 2021
1 parent 299eaae commit c65a01c
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 11 deletions.
2 changes: 2 additions & 0 deletions applications/tari_app_grpc/proto/wallet.proto
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ enum TransactionStatus {
TRANSACTION_STATUS_MINED_CONFIRMED = 6;
// The transaction was not found by the wallet its in transaction database
TRANSACTION_STATUS_NOT_FOUND = 7;
// The transaction was rejected by the mempool
TRANSACTION_STATUS_REJECTED = 8;
}

message GetCompletedTransactionsRequest { }
Expand Down
1 change: 1 addition & 0 deletions applications/tari_app_grpc/src/conversions/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ impl From<tx::TransactionStatus> for grpc::TransactionStatus {
Imported => grpc::TransactionStatus::Imported,
Pending => grpc::TransactionStatus::Pending,
Coinbase => grpc::TransactionStatus::Coinbase,
Rejected => grpc::TransactionStatus::Rejected,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ impl TransactionsTab {
)));
let status = if (t.cancelled || !t.valid) && t.status == TransactionStatus::Coinbase {
"Abandoned".to_string()
} else if t.cancelled && t.status == TransactionStatus::Rejected {
"Rejected".to_string()
} else if t.cancelled {
"Cancelled".to_string()
} else if !t.valid {
Expand Down Expand Up @@ -331,7 +333,9 @@ impl TransactionsTab {
Span::styled(format!("{}", tx.fee), Style::default().fg(Color::White)),
fee_details,
]);
let status_msg = if tx.cancelled {
let status_msg = if tx.cancelled && tx.status == TransactionStatus::Rejected {
"Rejected".to_string()
} else if tx.cancelled {
"Cancelled".to_string()
} else if !tx.valid {
"Invalid".to_string()
Expand Down
4 changes: 4 additions & 0 deletions base_layer/common_types/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ pub enum TransactionStatus {
Coinbase,
/// This transaction is mined and confirmed at the current base node's height
MinedConfirmed,
/// This transaction was Rejected by the mempool
Rejected,
}

#[derive(Debug, Error)]
Expand All @@ -44,6 +46,7 @@ impl TryFrom<i32> for TransactionStatus {
4 => Ok(TransactionStatus::Pending),
5 => Ok(TransactionStatus::Coinbase),
6 => Ok(TransactionStatus::MinedConfirmed),
7 => Ok(TransactionStatus::Rejected),
code => Err(TransactionConversionError { code }),
}
}
Expand All @@ -66,6 +69,7 @@ impl Display for TransactionStatus {
TransactionStatus::Imported => write!(f, "Imported"),
TransactionStatus::Pending => write!(f, "Pending"),
TransactionStatus::Coinbase => write!(f, "Coinbase"),
TransactionStatus::Rejected => write!(f, "Rejected"),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ where
"Failed to Cancel outputs for TxId: {} after failed sending attempt with error {:?}", self.tx_id, e
);
}
if let Err(e) = self.resources.db.cancel_completed_transaction(self.tx_id).await {
if let Err(e) = self.resources.db.reject_completed_transaction(self.tx_id).await {
warn!(
target: LOG_TARGET,
"Failed to Cancel TxId: {} after failed sending attempt with error {:?}", self.tx_id, e
Expand Down
6 changes: 3 additions & 3 deletions base_layer/wallet/src/transaction_service/storage/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub trait TransactionBackend: Send + Sync + Clone {
/// Indicated that a completed transaction has been broadcast to the mempools
fn broadcast_completed_transaction(&self, tx_id: TxId) -> Result<(), TransactionStorageError>;
/// Cancel Completed transaction, this will update the transaction status
fn cancel_completed_transaction(&self, tx_id: TxId) -> Result<(), TransactionStorageError>;
fn reject_completed_transaction(&self, tx_id: TxId) -> Result<(), TransactionStorageError>;
/// Set cancellation on Pending transaction, this will update the transaction status
fn set_pending_transaction_cancellation_status(
&self,
Expand Down Expand Up @@ -624,9 +624,9 @@ where T: TransactionBackend + 'static
.and_then(|inner_result| inner_result)
}

pub async fn cancel_completed_transaction(&self, tx_id: TxId) -> Result<(), TransactionStorageError> {
pub async fn reject_completed_transaction(&self, tx_id: TxId) -> Result<(), TransactionStorageError> {
let db_clone = self.db.clone();
tokio::task::spawn_blocking(move || db_clone.cancel_completed_transaction(tx_id))
tokio::task::spawn_blocking(move || db_clone.reject_completed_transaction(tx_id))
.await
.map_err(|err| TransactionStorageError::BlockingTaskSpawnError(err.to_string()))??;
Ok(())
Expand Down
21 changes: 17 additions & 4 deletions base_layer/wallet/src/transaction_service/storage/sqlite_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,13 +620,13 @@ impl TransactionBackend for TransactionServiceSqliteDatabase {
Ok(())
}

fn cancel_completed_transaction(&self, tx_id: u64) -> Result<(), TransactionStorageError> {
fn reject_completed_transaction(&self, tx_id: u64) -> Result<(), TransactionStorageError> {
let start = Instant::now();
let conn = self.database_connection.acquire_lock();
let acquire_lock = start.elapsed();
match CompletedTransactionSql::find_by_cancelled(tx_id, false, &(*conn)) {
Ok(v) => {
v.cancel(&(*conn))?;
v.reject(&(*conn))?;
},
Err(TransactionStorageError::DieselError(DieselError::NotFound)) => {
return Err(TransactionStorageError::ValueNotFound(DbKey::CompletedTransaction(
Expand All @@ -637,7 +637,7 @@ impl TransactionBackend for TransactionServiceSqliteDatabase {
};
trace!(
target: LOG_TARGET,
"sqlite profile - cancel_completed_transaction: lock {} + db_op {} = {} ms",
"sqlite profile - reject_completed_transaction: lock {} + db_op {} = {} ms",
acquire_lock.as_millis(),
(start.elapsed() - acquire_lock).as_millis(),
start.elapsed().as_millis()
Expand Down Expand Up @@ -1627,6 +1627,19 @@ impl CompletedTransactionSql {
Ok(())
}

pub fn reject(&self, conn: &SqliteConnection) -> Result<(), TransactionStorageError> {
self.update(
UpdateCompletedTransactionSql {
cancelled: Some(1i32),
status: Some(TransactionStatus::Rejected as i32),
..Default::default()
},
conn,
)?;

Ok(())
}

pub fn cancel(&self, conn: &SqliteConnection) -> Result<(), TransactionStorageError> {
self.update(
UpdateCompletedTransactionSql {
Expand Down Expand Up @@ -2171,7 +2184,7 @@ mod test {
assert!(CompletedTransactionSql::find_by_cancelled(completed_tx1.tx_id, true, &conn).is_err());
CompletedTransactionSql::try_from(completed_tx1.clone())
.unwrap()
.cancel(&conn)
.reject(&conn)
.unwrap();
assert!(CompletedTransactionSql::find_by_cancelled(completed_tx1.tx_id, false, &conn).is_err());
assert!(CompletedTransactionSql::find_by_cancelled(completed_tx1.tx_id, true, &conn).is_ok());
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/tests/transaction_service/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ pub fn test_db_backend<T: TransactionBackend + 'static>(backend: T) {
.block_on(db.get_cancelled_completed_transaction(cancelled_tx_id))
.is_err());
runtime
.block_on(db.cancel_completed_transaction(cancelled_tx_id))
.block_on(db.reject_completed_transaction(cancelled_tx_id))
.unwrap();
let completed_txs_map = runtime.block_on(db.get_completed_transactions()).unwrap();
assert_eq!(completed_txs_map.len(), num_completed_txs - 1);
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet_ffi/src/callback_handler_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ mod test {
runtime
.block_on(db.insert_completed_transaction(5u64, completed_tx_cancelled.clone()))
.unwrap();
runtime.block_on(db.cancel_completed_transaction(5u64)).unwrap();
runtime.block_on(db.reject_completed_transaction(5u64)).unwrap();
runtime
.block_on(db.add_pending_outbound_transaction(3u64, outbound_tx.clone()))
.unwrap();
Expand Down

0 comments on commit c65a01c

Please sign in to comment.