Skip to content

Commit

Permalink
feat!: expose reason for transaction cancellation for callback in wal…
Browse files Browse the repository at this point in the history
…let_ffi (#3601)

Description
---
Exposes reason for transaction cancellation to wallet_ffi via the transaction cancellation callback.

Additionally removes outdated docs and fixes Clippy errors.

Motivation and Context
---

How Has This Been Tested?
---
cargo test --all
  • Loading branch information
StriderDM authored Nov 23, 2021
1 parent cc846cd commit 3b3da21
Show file tree
Hide file tree
Showing 18 changed files with 130 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl WalletEventMonitor {
self.trigger_balance_refresh();
notifier.transaction_mined(tx_id);
},
TransactionEvent::TransactionCancelled(tx_id) => {
TransactionEvent::TransactionCancelled(tx_id, _) => {
self.trigger_tx_state_refresh(tx_id).await;
self.trigger_balance_refresh();
notifier.transaction_cancelled(tx_id);
Expand Down
4 changes: 2 additions & 2 deletions base_layer/p2p/src/services/liveness/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ impl LivenessState {
self.failed_pings
.entry(node_id)
.and_modify(|v| {
*v = *v + 1;
*v += 1;
})
.or_insert(1);
}
Expand All @@ -167,7 +167,7 @@ impl LivenessState {
/// a latency sample is added and calculated. The given `peer` must match the recorded peer
pub fn record_pong(&mut self, nonce: u64, sent_by: &NodeId) -> Option<u32> {
self.inc_pongs_received();
self.failed_pings.remove_entry(&sent_by);
self.failed_pings.remove_entry(sent_by);

let (node_id, _) = self.inflight_pings.get(&nonce)?;
if node_id == sent_by {
Expand Down
3 changes: 2 additions & 1 deletion base_layer/wallet/src/transaction_service/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use tari_service_framework::reply_channel::SenderService;

use crate::transaction_service::{
error::TransactionServiceError,
protocols::TxRejection,
storage::models::{CompletedTransaction, InboundTransaction, OutboundTransaction, WalletTransaction},
};

Expand Down Expand Up @@ -150,7 +151,7 @@ pub enum TransactionEvent {
TransactionDirectSendResult(TxId, bool),
TransactionCompletedImmediately(TxId),
TransactionStoreForwardSendResult(TxId, bool),
TransactionCancelled(TxId),
TransactionCancelled(TxId, TxRejection),
TransactionBroadcast(TxId),
TransactionImported(TxId),
TransactionMined {
Expand Down
11 changes: 11 additions & 0 deletions base_layer/wallet/src/transaction_service/protocols/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@
// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub enum TxRejection {
Unknown, // 0
UserCancelled, // 1
Timeout, // 2
DoubleSpend, // 3
Orphan, // 4
TimeLocked, // 5
InvalidTransaction, // 6
}

pub mod transaction_broadcast_protocol;
pub mod transaction_receive_protocol;
pub mod transaction_send_protocol;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use crate::{
transaction_service::{
error::{TransactionServiceError, TransactionServiceProtocolError},
handle::TransactionEvent,
protocols::TxRejection,
service::TransactionServiceResources,
storage::{database::TransactionBackend, models::CompletedTransaction},
},
Expand Down Expand Up @@ -215,10 +216,31 @@ where

self.cancel_transaction().await;

let reason = match response.rejection_reason {
TxSubmissionRejectionReason::None | TxSubmissionRejectionReason::ValidationFailed => {
TransactionServiceError::MempoolRejectionInvalidTransaction
},
TxSubmissionRejectionReason::DoubleSpend => TransactionServiceError::MempoolRejectionDoubleSpend,
TxSubmissionRejectionReason::Orphan => TransactionServiceError::MempoolRejectionOrphan,
TxSubmissionRejectionReason::TimeLocked => TransactionServiceError::MempoolRejectionTimeLocked,
_ => TransactionServiceError::UnexpectedBaseNodeResponse,
};

let cancellation_event_reason = match reason {
TransactionServiceError::MempoolRejectionInvalidTransaction => TxRejection::InvalidTransaction,
TransactionServiceError::MempoolRejectionDoubleSpend => TxRejection::DoubleSpend,
TransactionServiceError::MempoolRejectionOrphan => TxRejection::Orphan,
TransactionServiceError::MempoolRejectionTimeLocked => TxRejection::TimeLocked,
_ => TxRejection::Unknown,
};

let _ = self
.resources
.event_publisher
.send(Arc::new(TransactionEvent::TransactionCancelled(self.tx_id)))
.send(Arc::new(TransactionEvent::TransactionCancelled(
self.tx_id,
cancellation_event_reason,
)))
.map_err(|e| {
trace!(
target: LOG_TARGET,
Expand All @@ -228,15 +250,6 @@ where
e
});

let reason = match response.rejection_reason {
TxSubmissionRejectionReason::None | TxSubmissionRejectionReason::ValidationFailed => {
TransactionServiceError::MempoolRejectionInvalidTransaction
},
TxSubmissionRejectionReason::DoubleSpend => TransactionServiceError::MempoolRejectionDoubleSpend,
TxSubmissionRejectionReason::Orphan => TransactionServiceError::MempoolRejectionOrphan,
TxSubmissionRejectionReason::TimeLocked => TransactionServiceError::MempoolRejectionTimeLocked,
_ => TransactionServiceError::UnexpectedBaseNodeResponse,
};
return Err(TransactionServiceProtocolError::new(self.tx_id, reason));
} else if response.rejection_reason == TxSubmissionRejectionReason::AlreadyMined {
info!(
Expand Down Expand Up @@ -342,7 +355,10 @@ where
let _ = self
.resources
.event_publisher
.send(Arc::new(TransactionEvent::TransactionCancelled(self.tx_id)))
.send(Arc::new(TransactionEvent::TransactionCancelled(
self.tx_id,
TxRejection::InvalidTransaction,
)))
.map_err(|e| {
trace!(
target: LOG_TARGET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use tari_common_types::transaction::{TransactionDirection, TransactionStatus, Tx
use tari_comms::types::CommsPublicKey;
use tokio::sync::{mpsc, oneshot};

use crate::connectivity_service::WalletConnectivityInterface;
use crate::{connectivity_service::WalletConnectivityInterface, transaction_service::protocols::TxRejection};
use tari_common_types::types::HashOutput;
use tari_core::transactions::{
transaction_entities::Transaction,
Expand Down Expand Up @@ -504,7 +504,10 @@ where
let _ = self
.resources
.event_publisher
.send(Arc::new(TransactionEvent::TransactionCancelled(self.id)))
.send(Arc::new(TransactionEvent::TransactionCancelled(
self.id,
TxRejection::Timeout,
)))
.map_err(|e| {
trace!(
target: LOG_TARGET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use crate::{
config::TransactionRoutingMechanism,
error::{TransactionServiceError, TransactionServiceProtocolError},
handle::{TransactionEvent, TransactionServiceResponse},
protocols::TxRejection,
service::TransactionServiceResources,
storage::{
database::TransactionBackend,
Expand Down Expand Up @@ -826,7 +827,10 @@ where
let _ = self
.resources
.event_publisher
.send(Arc::new(TransactionEvent::TransactionCancelled(self.id)))
.send(Arc::new(TransactionEvent::TransactionCancelled(
self.id,
TxRejection::Timeout,
)))
.map_err(|e| {
trace!(
target: LOG_TARGET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub struct TransactionValidationProtocol<TTransactionBackend, TWalletConnectivit
event_publisher: TransactionEventSender,
output_manager_handle: OutputManagerHandle,
}
use crate::transaction_service::protocols::TxRejection;
use tari_common_types::types::Signature;

#[allow(unused_variables)]
Expand Down Expand Up @@ -416,7 +417,7 @@ where
);
};

self.publish_event(TransactionEvent::TransactionCancelled(tx_id));
self.publish_event(TransactionEvent::TransactionCancelled(tx_id, TxRejection::Orphan));

Ok(())
}
Expand Down
6 changes: 5 additions & 1 deletion base_layer/wallet/src/transaction_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::{
transaction_receive_protocol::{TransactionReceiveProtocol, TransactionReceiveProtocolStage},
transaction_send_protocol::{TransactionSendProtocol, TransactionSendProtocolStage},
transaction_validation_protocol::TransactionValidationProtocol,
TxRejection,
},
storage::{
database::{TransactionBackend, TransactionDatabase},
Expand Down Expand Up @@ -1301,7 +1302,10 @@ where

let _ = self
.event_publisher
.send(Arc::new(TransactionEvent::TransactionCancelled(tx_id)))
.send(Arc::new(TransactionEvent::TransactionCancelled(
tx_id,
TxRejection::UserCancelled,
)))
.map_err(|e| {
trace!(
target: LOG_TARGET,
Expand Down
14 changes: 7 additions & 7 deletions base_layer/wallet/tests/transaction_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2184,7 +2184,7 @@ fn test_transaction_cancellation() {
loop {
tokio::select! {
event = alice_event_stream.recv() => {
if let TransactionEvent::TransactionCancelled(_) = &*event.unwrap() {
if let TransactionEvent::TransactionCancelled(..) = &*event.unwrap() {
cancelled = true;
break;
}
Expand Down Expand Up @@ -2380,7 +2380,7 @@ fn test_transaction_cancellation() {
loop {
tokio::select! {
event = alice_event_stream.recv() => {
if let TransactionEvent::TransactionCancelled(_) = &*event.unwrap() {
if let TransactionEvent::TransactionCancelled(..) = &*event.unwrap() {
cancelled = true;
break;
}
Expand Down Expand Up @@ -3524,7 +3524,7 @@ fn test_coinbase_abandoned() {
loop {
tokio::select! {
event = alice_event_stream.recv() => {
if let TransactionEvent::TransactionCancelled(tx_id) = &*event.unwrap() {
if let TransactionEvent::TransactionCancelled(tx_id, _) = &*event.unwrap() {
if tx_id == &tx_id1 {
count += 1;
}
Expand Down Expand Up @@ -3684,7 +3684,7 @@ fn test_coinbase_abandoned() {
count += 1;
}
},
TransactionEvent::TransactionCancelled(tx_id) => {
TransactionEvent::TransactionCancelled(tx_id, _) => {
if tx_id == &tx_id2 {
count += 1;
}
Expand Down Expand Up @@ -3771,7 +3771,7 @@ fn test_coinbase_abandoned() {
count += 1;
}
},
TransactionEvent::TransactionCancelled(tx_id) => {
TransactionEvent::TransactionCancelled(tx_id, _) => {
if tx_id == &tx_id1 {
count += 1;
}
Expand Down Expand Up @@ -4755,7 +4755,7 @@ fn test_transaction_timeout_cancellation() {
loop {
tokio::select! {
event = carol_event_stream.recv() => {
if let TransactionEvent::TransactionCancelled(t) = &*event.unwrap() {
if let TransactionEvent::TransactionCancelled(t, _) = &*event.unwrap() {
if t == &tx_id {
transaction_cancelled = true;
break;
Expand Down Expand Up @@ -5074,7 +5074,7 @@ fn transaction_service_tx_broadcast() {
loop {
tokio::select! {
event = alice_event_stream.recv() => {
if let TransactionEvent::TransactionCancelled(tx_id) = &*event.unwrap(){
if let TransactionEvent::TransactionCancelled(tx_id, _) = &*event.unwrap(){
if tx_id == &tx_id2 {
tx2_cancelled = true;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ async fn tx_broadcast_protocol_submit_rejection() {
loop {
tokio::select! {
event = event_stream.recv() => {
if let TransactionEvent::TransactionCancelled(_) = &*event.unwrap() {
if let TransactionEvent::TransactionCancelled(..) = &*event.unwrap() {
cancelled = true;
}
},
Expand Down Expand Up @@ -547,7 +547,7 @@ async fn tx_broadcast_protocol_submit_success_followed_by_rejection() {
loop {
tokio::select! {
event = event_stream.recv() => {
if let TransactionEvent::TransactionCancelled(_) = &*event.unwrap() {
if let TransactionEvent::TransactionCancelled(..) = &*event.unwrap() {
cancelled = true;
}
},
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/tests/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ fn test_store_and_forward_send_tx() {
event = carol_event_stream.recv() => {
match &*event.unwrap() {
TransactionEvent::ReceivedTransaction(_) => tx_recv = true,
TransactionEvent::TransactionCancelled(_) => tx_cancelled = true,
TransactionEvent::TransactionCancelled(..) => tx_cancelled = true,
_ => (),
}
if tx_recv && tx_cancelled {
Expand Down
12 changes: 6 additions & 6 deletions base_layer/wallet_ffi/src/callback_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ where TBackend: TransactionBackend + 'static
callback_transaction_mined_unconfirmed: unsafe extern "C" fn(*mut CompletedTransaction, u64),
callback_direct_send_result: unsafe extern "C" fn(TxId, bool),
callback_store_and_forward_send_result: unsafe extern "C" fn(TxId, bool),
callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction),
callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction, u64),
callback_txo_validation_complete: unsafe extern "C" fn(u64, u8),
callback_balance_updated: unsafe extern "C" fn(*mut Balance),
callback_transaction_validation_complete: unsafe extern "C" fn(u64, u8),
Expand Down Expand Up @@ -123,7 +123,7 @@ where TBackend: TransactionBackend + 'static
callback_transaction_mined_unconfirmed: unsafe extern "C" fn(*mut CompletedTransaction, u64),
callback_direct_send_result: unsafe extern "C" fn(TxId, bool),
callback_store_and_forward_send_result: unsafe extern "C" fn(TxId, bool),
callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction),
callback_transaction_cancellation: unsafe extern "C" fn(*mut CompletedTransaction, u64),
callback_txo_validation_complete: unsafe extern "C" fn(TxId, u8),
callback_balance_updated: unsafe extern "C" fn(*mut Balance),
callback_transaction_validation_complete: unsafe extern "C" fn(TxId, u8),
Expand Down Expand Up @@ -242,8 +242,8 @@ where TBackend: TransactionBackend + 'static
self.receive_store_and_forward_send_result(tx_id, result);
self.trigger_balance_refresh().await;
},
TransactionEvent::TransactionCancelled(tx_id) => {
self.receive_transaction_cancellation(tx_id).await;
TransactionEvent::TransactionCancelled(tx_id, reason) => {
self.receive_transaction_cancellation(tx_id, reason as u64).await;
self.trigger_balance_refresh().await;
},
TransactionEvent::TransactionBroadcast(tx_id) => {
Expand Down Expand Up @@ -425,7 +425,7 @@ where TBackend: TransactionBackend + 'static
}
}

async fn receive_transaction_cancellation(&mut self, tx_id: TxId) {
async fn receive_transaction_cancellation(&mut self, tx_id: TxId, reason: u64) {
let mut transaction = None;
if let Ok(tx) = self.db.get_cancelled_completed_transaction(tx_id).await {
transaction = Some(tx);
Expand All @@ -451,7 +451,7 @@ where TBackend: TransactionBackend + 'static
);
let boxing = Box::into_raw(Box::new(tx));
unsafe {
(self.callback_transaction_cancellation)(boxing);
(self.callback_transaction_cancellation)(boxing, reason);
}
},
}
Expand Down
18 changes: 14 additions & 4 deletions base_layer/wallet_ffi/src/callback_handler_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ mod test {
};

use crate::{callback_handler::CallbackHandler, output_manager_service_mock::MockOutputManagerService};
use tari_wallet::transaction_service::protocols::TxRejection;

struct CallbackState {
pub received_tx_callback_called: bool,
Expand Down Expand Up @@ -168,7 +169,7 @@ mod test {
drop(lock);
}

unsafe extern "C" fn tx_cancellation_callback(tx: *mut CompletedTransaction) {
unsafe extern "C" fn tx_cancellation_callback(tx: *mut CompletedTransaction, _reason: u64) {
let mut lock = CALLBACK_STATE.lock().unwrap();
match (*tx).tx_id {
3 => lock.tx_cancellation_callback_called_inbound = true,
Expand Down Expand Up @@ -415,7 +416,10 @@ mod test {
mock_output_manager_service_state.set_balance(balance.clone());
// Balance updated should be detected with following event, total = 4 times
transaction_event_sender
.send(Arc::new(TransactionEvent::TransactionCancelled(3u64)))
.send(Arc::new(TransactionEvent::TransactionCancelled(
3u64,
TxRejection::UserCancelled,
)))
.unwrap();
let start = Instant::now();
while start.elapsed().as_secs() < 10 {
Expand All @@ -431,11 +435,17 @@ mod test {
assert_eq!(callback_balance_updated, 4);

transaction_event_sender
.send(Arc::new(TransactionEvent::TransactionCancelled(4u64)))
.send(Arc::new(TransactionEvent::TransactionCancelled(
4u64,
TxRejection::UserCancelled,
)))
.unwrap();

transaction_event_sender
.send(Arc::new(TransactionEvent::TransactionCancelled(5u64)))
.send(Arc::new(TransactionEvent::TransactionCancelled(
5u64,
TxRejection::UserCancelled,
)))
.unwrap();

oms_event_sender
Expand Down
Loading

0 comments on commit 3b3da21

Please sign in to comment.