From 777869d9c12588034cdbf2f03537d28b37a6505a Mon Sep 17 00:00:00 2001 From: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com> Date: Thu, 15 Aug 2024 19:49:52 +0200 Subject: [PATCH] feat: limit open ended vectors in chat (#6467) Description --- - Limited open-ended vectors in chat to guard against malicious network messages. - Fixed instability issues with chat FFI cucumber tests. Motivation and Context --- This is a _defense-in-depth_ exercise. How Has This Been Tested? --- - Existing unit tests pass. - Added new unit tests to test all limits What process can a PR reviewer use to test or verify this change? --- Code review Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify --- .../src/conversions/sidechain_feature.rs | 24 +-- .../src/ui/state/tasks.rs | 4 +- base_layer/chat_ffi/src/application_config.rs | 1 + base_layer/chat_ffi/src/callback_handler.rs | 46 ++++-- base_layer/chat_ffi/src/confirmation.rs | 11 +- base_layer/chat_ffi/src/contacts.rs | 4 + base_layer/chat_ffi/src/conversationalists.rs | 1 + base_layer/chat_ffi/src/lib.rs | 1 + base_layer/chat_ffi/src/message.rs | 33 +++- base_layer/chat_ffi/src/message_metadata.rs | 24 ++- base_layer/chat_ffi/src/messages.rs | 14 +- base_layer/common_types/src/lib.rs | 2 + .../src/max_size}/bytes.rs | 46 +++--- base_layer/common_types/src/max_size/mod.rs | 26 +++ .../src/max_size}/string.rs | 8 +- .../contacts/src/chat_client/src/client.rs | 30 ++-- .../contacts/src/chat_client/src/error.rs | 3 + .../contacts/src/contacts_service/error.rs | 7 + .../contacts/src/contacts_service/handle.rs | 10 +- .../contacts/src/contacts_service/service.rs | 2 +- .../src/contacts_service/storage/database.rs | 13 +- .../storage/types/messages.rs | 11 +- .../contacts_service/types/confirmation.rs | 33 +++- .../src/contacts_service/types/message.rs | 92 ++++++++--- .../contacts_service/types/message_builder.rs | 130 +++++++++++++-- .../types/message_dispatch.rs | 4 +- .../src/contacts_service/types/mod.rs | 2 +- base_layer/contacts/tests/contacts_service.rs | 2 + .../core/src/consensus/consensus_encoding.rs | 5 - base_layer/core/src/consensus/mod.rs | 2 +- base_layer/core/src/covenants/arguments.rs | 6 +- .../core/src/proto/sidechain_feature.rs | 6 +- .../transaction_components/encrypted_data.rs | 3 +- .../transaction_components/output_features.rs | 29 ++-- .../side_chain/template_registration.rs | 8 +- .../wallet/src/transaction_service/handle.rs | 3 +- base_layer/wallet_ffi/src/lib.rs | 10 +- common/src/lib.rs | 1 + integration_tests/src/chat_ffi.rs | 150 ++++++++++-------- .../tests/features/ChatFFI.feature | 4 - integration_tests/tests/steps/chat_steps.rs | 46 +++--- 41 files changed, 588 insertions(+), 269 deletions(-) rename base_layer/{core/src/consensus/consensus_encoding => common_types/src/max_size}/bytes.rs (63%) create mode 100644 base_layer/common_types/src/max_size/mod.rs rename base_layer/{core/src/consensus/consensus_encoding => common_types/src/max_size}/string.rs (98%) diff --git a/applications/minotari_app_grpc/src/conversions/sidechain_feature.rs b/applications/minotari_app_grpc/src/conversions/sidechain_feature.rs index 385b74faa3..a542b73477 100644 --- a/applications/minotari_app_grpc/src/conversions/sidechain_feature.rs +++ b/applications/minotari_app_grpc/src/conversions/sidechain_feature.rs @@ -22,18 +22,18 @@ use std::convert::{TryFrom, TryInto}; -use tari_common_types::types::{PublicKey, Signature}; -use tari_core::{ - consensus::MaxSizeString, - transactions::transaction_components::{ - BuildInfo, - CodeTemplateRegistration, - ConfidentialOutputData, - SideChainFeature, - TemplateType, - ValidatorNodeRegistration, - ValidatorNodeSignature, - }, +use tari_common_types::{ + types::{PublicKey, Signature}, + MaxSizeString, +}; +use tari_core::transactions::transaction_components::{ + BuildInfo, + CodeTemplateRegistration, + ConfidentialOutputData, + SideChainFeature, + TemplateType, + ValidatorNodeRegistration, + ValidatorNodeSignature, }; use tari_utilities::ByteArray; diff --git a/applications/minotari_console_wallet/src/ui/state/tasks.rs b/applications/minotari_console_wallet/src/ui/state/tasks.rs index d9f543a818..57d056ddcc 100644 --- a/applications/minotari_console_wallet/src/ui/state/tasks.rs +++ b/applications/minotari_console_wallet/src/ui/state/tasks.rs @@ -34,9 +34,11 @@ use rand::{random, rngs::OsRng}; use tari_common_types::{ tari_address::TariAddress, types::{PublicKey, Signature}, + MaxSizeBytes, + MaxSizeString, }; use tari_core::{ - consensus::{DomainSeparatedConsensusHasher, MaxSizeBytes, MaxSizeString}, + consensus::DomainSeparatedConsensusHasher, transactions::{ tari_amount::MicroMinotari, transaction_components::{encrypted_data::PaymentId, BuildInfo, OutputFeatures, TemplateType}, diff --git a/base_layer/chat_ffi/src/application_config.rs b/base_layer/chat_ffi/src/application_config.rs index df2d0db6d7..de4a6c6349 100644 --- a/base_layer/chat_ffi/src/application_config.rs +++ b/base_layer/chat_ffi/src/application_config.rs @@ -73,6 +73,7 @@ pub unsafe extern "C" fn create_chat_config( let mut bad_network = |e| { error = LibChatError::from(InterfaceError::InvalidArgument(e)).code; ptr::swap(error_out, &mut error as *mut c_int); + ptr::null_mut::() }; let network = if network_str.is_null() { diff --git a/base_layer/chat_ffi/src/callback_handler.rs b/base_layer/chat_ffi/src/callback_handler.rs index 9b36f5bd5a..aa00e20c9c 100644 --- a/base_layer/chat_ffi/src/callback_handler.rs +++ b/base_layer/chat_ffi/src/callback_handler.rs @@ -74,23 +74,40 @@ impl CallbackHandler { rec_message = chat_messages.recv() => { match rec_message { Ok(message_dispatch) => { - trace!(target: LOG_TARGET, "FFI Callback monitor received a new MessageDispatch"); match message_dispatch.deref() { MessageDispatch::Message(m) => { - trace!(target: LOG_TARGET, "FFI Callback monitor received a new Message"); + trace!( + target: LOG_TARGET, + "FFI Callback monitor received a new Message ({})", + m + ); self.trigger_message_received(m.clone()); } MessageDispatch::DeliveryConfirmation(c) => { - trace!(target: LOG_TARGET, "FFI Callback monitor received a new Delivery Confirmation"); + trace!( + target: LOG_TARGET, + "FFI Callback monitor received a new Delivery Confirmation ({})", + c + ); self.trigger_delivery_confirmation_received(c.clone()); }, MessageDispatch::ReadConfirmation(c) => { - trace!(target: LOG_TARGET, "FFI Callback monitor received a new Read Confirmation"); + trace!( + target: LOG_TARGET, + "FFI Callback monitor received a new Read Confirmation ({})", + c + ); self.trigger_read_confirmation_received(c.clone()); } }; }, - Err(_) => { debug!(target: LOG_TARGET, "FFI Callback monitor had an error receiving new messages")} + Err(e) => { + debug!( + target: LOG_TARGET, + "FFI Callback monitor had an error receiving new messages ({})", + e + ) + } } }, @@ -100,18 +117,27 @@ impl CallbackHandler { match liveness_event.deref() { ContactsLivenessEvent::StatusUpdated(data) => { trace!(target: LOG_TARGET, - "FFI Callback monitor received Contact Status Updated event" + "FFI Callback monitor received Contact Status Updated event ({})", data ); self.trigger_contact_status_change(data.deref().clone()); } ContactsLivenessEvent::NetworkSilence => {}, } }, - Err(_) => { debug!(target: LOG_TARGET, "FFI Callback monitor had an error with contacts liveness")} + Err(e) => { + debug!( + target: LOG_TARGET, + "FFI Callback monitor had an error with contacts liveness ({})", + e + ) + } } }, _ = self.shutdown.wait() => { - info!(target: LOG_TARGET, "ChatFFI Callback Handler shutting down because the shutdown signal was received"); + info!( + target: LOG_TARGET, + "ChatFFI Callback Handler shutting down because the shutdown signal was received" + ); break; }, } @@ -145,7 +171,7 @@ impl CallbackHandler { fn trigger_delivery_confirmation_received(&mut self, confirmation: Confirmation) { debug!( target: LOG_TARGET, - "Calling DeliveryConfirmationReceived callback function for message {:?}", + "Calling DeliveryConfirmationReceived callback function for message {}", confirmation.message_id, ); @@ -157,7 +183,7 @@ impl CallbackHandler { fn trigger_read_confirmation_received(&mut self, confirmation: Confirmation) { debug!( target: LOG_TARGET, - "Calling ReadConfirmationReceived callback function for message {:?}", + "Calling ReadConfirmationReceived callback function for message {}", confirmation.message_id, ); diff --git a/base_layer/chat_ffi/src/confirmation.rs b/base_layer/chat_ffi/src/confirmation.rs index 7f89fd5a29..4fca976615 100644 --- a/base_layer/chat_ffi/src/confirmation.rs +++ b/base_layer/chat_ffi/src/confirmation.rs @@ -57,11 +57,13 @@ pub unsafe extern "C" fn send_read_confirmation_for_message( if client.is_null() { error = LibChatError::from(InterfaceError::NullError("client".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return; } if message.is_null() { error = LibChatError::from(InterfaceError::NullError("message".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return; } let result = (*client) @@ -97,6 +99,7 @@ pub unsafe extern "C" fn read_confirmation_message_id( if confirmation.is_null() { error = LibChatError::from(InterfaceError::NullError("client".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); } let c = &(*confirmation); @@ -107,7 +110,7 @@ pub unsafe extern "C" fn read_confirmation_message_id( Err(e) => { error = LibChatError::from(InterfaceError::ConversionError(e.to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); - 0 + return ptr::null_mut(); }, }; @@ -162,7 +165,7 @@ pub unsafe extern "C" fn destroy_confirmation(ptr: *mut Confirmation) { #[cfg(test)] mod test { use tari_contacts::contacts_service::types::{Confirmation, MessageBuilder}; - use tari_utilities::epoch_time::EpochTime; + use tari_utilities::{epoch_time::EpochTime, ByteArray}; use crate::{ byte_vector::{chat_byte_vector_get_at, chat_byte_vector_get_length}, @@ -170,7 +173,7 @@ mod test { }; #[test] - fn test_reading_from_confrimation() { + fn test_reading_from_confirmation() { let message_id = MessageBuilder::new().build().message_id; let timestamp = EpochTime::now().as_u64(); let confirmation = Confirmation { @@ -190,7 +193,7 @@ mod test { read_id.push(chat_byte_vector_get_at(id_byte_vec, i, error_out)); } - assert_eq!(message_id, read_id) + assert_eq!(message_id.to_vec(), read_id) } unsafe { diff --git a/base_layer/chat_ffi/src/contacts.rs b/base_layer/chat_ffi/src/contacts.rs index 90b7972f8f..07ad3bc447 100644 --- a/base_layer/chat_ffi/src/contacts.rs +++ b/base_layer/chat_ffi/src/contacts.rs @@ -51,11 +51,13 @@ pub unsafe extern "C" fn add_chat_contact(client: *mut ChatClient, address: *mut if client.is_null() { error = LibChatError::from(InterfaceError::NullError("client".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return; } if address.is_null() { error = LibChatError::from(InterfaceError::NullError("receiver".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return; } let result = (*client).runtime.block_on((*client).client.add_contact(&(*address))); @@ -94,11 +96,13 @@ pub unsafe extern "C" fn check_online_status( if client.is_null() { error = LibChatError::from(InterfaceError::NullError("client".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return 0; } if receiver.is_null() { error = LibChatError::from(InterfaceError::NullError("receiver".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return 0; } let rec = (*receiver).clone(); diff --git a/base_layer/chat_ffi/src/conversationalists.rs b/base_layer/chat_ffi/src/conversationalists.rs index 30d02b18e4..70fc4a8886 100644 --- a/base_layer/chat_ffi/src/conversationalists.rs +++ b/base_layer/chat_ffi/src/conversationalists.rs @@ -56,6 +56,7 @@ pub unsafe extern "C" fn get_conversationalists( if client.is_null() { error = LibChatError::from(InterfaceError::NullError("client".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); } let result = (*client).runtime.block_on((*client).client.get_conversationalists()); diff --git a/base_layer/chat_ffi/src/lib.rs b/base_layer/chat_ffi/src/lib.rs index 346d6e125a..c360fce909 100644 --- a/base_layer/chat_ffi/src/lib.rs +++ b/base_layer/chat_ffi/src/lib.rs @@ -126,6 +126,7 @@ pub unsafe extern "C" fn create_chat_client( let mut bad_identity = |e| { error = LibChatError::from(InterfaceError::InvalidArgument(e)).code; ptr::swap(error_out, &mut error as *mut c_int); + ptr::null_mut::() }; let identity = match setup_node_identity( diff --git a/base_layer/chat_ffi/src/message.rs b/base_layer/chat_ffi/src/message.rs index ea1ee64289..e168f8248a 100644 --- a/base_layer/chat_ffi/src/message.rs +++ b/base_layer/chat_ffi/src/message.rs @@ -25,7 +25,7 @@ use std::{convert::TryFrom, ffi::CStr, ptr}; use libc::{c_char, c_int, c_uchar, c_uint, c_ulonglong}; use tari_chat_client::ChatClient as ChatClientTrait; use tari_common_types::tari_address::TariAddress; -use tari_contacts::contacts_service::types::{Message, MessageBuilder, MessageMetadata}; +use tari_contacts::contacts_service::types::{Message, MessageBuilder, MessageId, MessageMetadata}; use tari_utilities::ByteArray; use crate::{ @@ -60,10 +60,12 @@ pub unsafe extern "C" fn create_chat_message( if receiver.is_null() { error = LibChatError::from(InterfaceError::NullError("receiver".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); } if sender.is_null() { error = LibChatError::from(InterfaceError::NullError("sender".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); } let message_str = match CStr::from_ptr(message).to_str() { @@ -75,11 +77,18 @@ pub unsafe extern "C" fn create_chat_message( }, }; - let message_out = MessageBuilder::new() + let message_out = match MessageBuilder::new() .receiver_address((*receiver).clone()) .sender_address((*sender).clone()) .message(message_str) - .build(); + { + Ok(val) => val.build(), + Err(e) => { + error = LibChatError::from(InterfaceError::InvalidArgument(e.to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); + }, + }; Box::into_raw(Box::new(message_out)) } @@ -127,16 +136,26 @@ pub unsafe extern "C" fn get_chat_message( if client.is_null() { error = LibChatError::from(InterfaceError::NullError("client".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); } if message_id.is_null() { error = LibChatError::from(InterfaceError::NullError("message_id".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); } let id = process_vector(message_id, error_out); + let message_id = match MessageId::try_from(id) { + Ok(val) => val, + Err(e) => { + error = LibChatError::from(InterfaceError::ConversionError(format!("message_id ({})", e))).code; + ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); + }, + }; - let result = (*client).runtime.block_on((*client).client.get_message(&id)); + let result = (*client).runtime.block_on((*client).client.get_message(&message_id)); match result { Ok(message) => Box::into_raw(Box::new(message)), @@ -168,11 +187,13 @@ pub unsafe extern "C" fn send_chat_message(client: *mut ChatClient, message: *mu if client.is_null() { error = LibChatError::from(InterfaceError::NullError("client".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return; } if message.is_null() { error = LibChatError::from(InterfaceError::NullError("message".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return; } let result = (*client) @@ -563,7 +584,7 @@ mod test { message_id.push(chat_byte_vector_get_at(message_byte_vector, i, error_out)); } - assert_eq!(message.message_id, message_id); + assert_eq!(message.message_id.to_vec(), message_id); destroy_chat_message(message_ptr); chat_byte_vector_destroy(message_byte_vector); @@ -575,7 +596,7 @@ mod test { fn test_reading_message_body() { let body = "Hey there!"; let body_bytes = body.as_bytes(); - let message = MessageBuilder::new().message(body.into()).build(); + let message = MessageBuilder::new().message(body.into()).unwrap().build(); let message_ptr = Box::into_raw(Box::new(message)); let error_out = Box::into_raw(Box::new(0)); diff --git a/base_layer/chat_ffi/src/message_metadata.rs b/base_layer/chat_ffi/src/message_metadata.rs index 3d4a3c07a5..268809e5e0 100644 --- a/base_layer/chat_ffi/src/message_metadata.rs +++ b/base_layer/chat_ffi/src/message_metadata.rs @@ -23,7 +23,7 @@ use std::{convert::TryFrom, ptr}; use libc::{c_int, c_uint}; -use tari_contacts::contacts_service::types::{Message, MessageMetadata}; +use tari_contacts::contacts_service::types::{Message, MessageMetadata, MetadataData, MetadataKey}; use tari_utilities::ByteArray; use crate::{ @@ -62,10 +62,23 @@ pub unsafe extern "C" fn add_chat_message_metadata( } } - let metadata = MessageMetadata { - key: process_vector(key, error_out), - data: process_vector(data, error_out), + let key = match MetadataKey::try_from(process_vector(key, error_out)) { + Ok(val) => val, + Err(e) => { + error = LibChatError::from(InterfaceError::InvalidArgument(e.to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return; + }, + }; + let data = match MetadataData::try_from(process_vector(data, error_out)) { + Ok(val) => val, + Err(e) => { + error = LibChatError::from(InterfaceError::InvalidArgument(e.to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return; + }, }; + let metadata = MessageMetadata { key, data }; (*message).push(metadata); } @@ -197,7 +210,7 @@ mod test { let message = unsafe { Box::from_raw(message_ptr) }; assert_eq!(message.metadata.len(), 1); - assert_eq!(message.metadata[0].data, data_bytes); + assert_eq!(message.metadata[0].data.as_bytes(), data_bytes); unsafe { chat_byte_vector_destroy(data); @@ -211,6 +224,7 @@ mod test { let message_ptr = Box::into_raw(Box::new( MessageBuilder::new() .message("hello".to_string()) + .unwrap() .receiver_address(address.clone()) .sender_address(address) .build(), diff --git a/base_layer/chat_ffi/src/messages.rs b/base_layer/chat_ffi/src/messages.rs index 1e8a87b6d4..8e9c1331a0 100644 --- a/base_layer/chat_ffi/src/messages.rs +++ b/base_layer/chat_ffi/src/messages.rs @@ -65,11 +65,13 @@ pub unsafe extern "C" fn get_chat_messages( if client.is_null() { error = LibChatError::from(InterfaceError::NullError("client".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); } if address.is_null() { error = LibChatError::from(InterfaceError::NullError("address".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); } let mlimit = u64::from(limit); @@ -192,13 +194,13 @@ mod test { #[test] fn test_retrieving_messages_from_vector() { - let m = MessageBuilder::new().message("hello 2".to_string()).build(); + let m = MessageBuilder::new().message("hello 2".to_string()).unwrap().build(); let messages = MessageVector(vec![ - MessageBuilder::new().message("hello 0".to_string()).build(), - MessageBuilder::new().message("hello 1".to_string()).build(), + MessageBuilder::new().message("hello 0".to_string()).unwrap().build(), + MessageBuilder::new().message("hello 1".to_string()).unwrap().build(), m.clone(), - MessageBuilder::new().message("hello 3".to_string()).build(), - MessageBuilder::new().message("hello 4".to_string()).build(), + MessageBuilder::new().message("hello 3".to_string()).unwrap().build(), + MessageBuilder::new().message("hello 4".to_string()).unwrap().build(), ]); let messages_len = messages.0.len(); @@ -218,7 +220,7 @@ mod test { message_id.push(chat_byte_vector_get_at(message_byte_vector, i, error_out)); } - assert_eq!(m.message_id, message_id); + assert_eq!(m.message_id.to_vec(), message_id); destroy_message_vector(message_vector_ptr); destroy_chat_message(message_ptr); diff --git a/base_layer/common_types/src/lib.rs b/base_layer/common_types/src/lib.rs index b0d6c50ca1..848dd649d7 100644 --- a/base_layer/common_types/src/lib.rs +++ b/base_layer/common_types/src/lib.rs @@ -31,9 +31,11 @@ pub mod encryption; pub mod epoch; pub mod grpc_authentication; pub mod key_branches; +mod max_size; pub mod serializers; pub mod tari_address; pub mod transaction; mod tx_id; pub mod types; pub mod wallet_types; +pub use max_size::{MaxSizeBytes, MaxSizeBytesError, MaxSizeString, MaxSizeStringLengthError}; diff --git a/base_layer/core/src/consensus/consensus_encoding/bytes.rs b/base_layer/common_types/src/max_size/bytes.rs similarity index 63% rename from base_layer/core/src/consensus/consensus_encoding/bytes.rs rename to base_layer/common_types/src/max_size/bytes.rs index 3bd0eff08a..a2513a7695 100644 --- a/base_layer/core/src/consensus/consensus_encoding/bytes.rs +++ b/base_layer/common_types/src/max_size/bytes.rs @@ -1,35 +1,39 @@ -// Copyright 2022, The Tari Project +// Copyright 2022 The Tari Project // -// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the -// following conditions are met: +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the +// following conditions are met: // -// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following -// disclaimer. +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following +// disclaimer. // -// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the -// following disclaimer in the documentation and/or other materials provided with the distribution. +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the +// following disclaimer in the documentation and/or other materials provided with the distribution. // -// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote -// products derived from this software without specific prior written permission. +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote +// products derived from this software without specific prior written permission. // -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, -// INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE -// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR -// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, -// 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. +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, +// INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +// 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 +// +// Portions of this file were originally copyrighted (c) 2018 The Grin Developers, issued under the Apache License, +// Version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0. use std::{ cmp, convert::TryFrom, + fmt::Display, ops::{Deref, DerefMut}, }; use borsh::{BorshDeserialize, BorshSerialize}; use serde::{Deserialize, Serialize}; use tari_utilities::{ - hex::{from_hex, HexError}, + hex::{from_hex, to_hex, HexError}, ByteArray, ByteArrayError, }; @@ -141,12 +145,18 @@ impl ByteArray for MaxSizeBytes { }) } - /// Return the NodeId as a byte array + /// Return the data as a byte array fn as_bytes(&self) -> &[u8] { self.inner.as_ref() } } +impl Display for MaxSizeBytes { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", to_hex(&self.inner)) + } +} + #[derive(Debug, thiserror::Error)] pub enum MaxSizeBytesError { #[error("Invalid Bytes length: expected {expected}, got {actual}")] diff --git a/base_layer/common_types/src/max_size/mod.rs b/base_layer/common_types/src/max_size/mod.rs new file mode 100644 index 0000000000..e6926116bc --- /dev/null +++ b/base_layer/common_types/src/max_size/mod.rs @@ -0,0 +1,26 @@ +// Copyright 2022. The Tari Project +// +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the +// following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following +// disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the +// following disclaimer in the documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote +// products derived from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, +// INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +// 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. + +mod string; +pub use string::{MaxSizeString, MaxSizeStringLengthError}; +mod bytes; +pub use bytes::{MaxSizeBytes, MaxSizeBytesError}; diff --git a/base_layer/core/src/consensus/consensus_encoding/string.rs b/base_layer/common_types/src/max_size/string.rs similarity index 98% rename from base_layer/core/src/consensus/consensus_encoding/string.rs rename to base_layer/common_types/src/max_size/string.rs index 2df0a718b3..0a90b3caec 100644 --- a/base_layer/core/src/consensus/consensus_encoding/string.rs +++ b/base_layer/common_types/src/max_size/string.rs @@ -119,11 +119,8 @@ pub struct MaxSizeStringLengthError { #[cfg(test)] mod tests { - use super::*; - mod from_str_checked { - use super::*; - + use crate::MaxSizeString; #[test] fn it_returns_none_if_size_exceeded() { let s = MaxSizeString::<10>::from_str_checked("12345678901234567890"); @@ -152,8 +149,7 @@ mod tests { } mod from_utf8_bytes_checked { - use super::*; - + use crate::MaxSizeString; #[test] fn it_returns_none_if_size_exceeded() { let s = MaxSizeString::<10>::from_utf8_bytes_checked([0u8; 11]); diff --git a/base_layer/contacts/src/chat_client/src/client.rs b/base_layer/contacts/src/chat_client/src/client.rs index e04ecaf078..78af39d2cd 100644 --- a/base_layer/contacts/src/chat_client/src/client.rs +++ b/base_layer/contacts/src/chat_client/src/client.rs @@ -21,6 +21,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use std::{ + convert::TryFrom, fmt::{Debug, Formatter}, sync::Arc, time::Duration, @@ -34,7 +35,7 @@ use tari_comms::{peer_manager::PeerFeatures, CommsNode, NodeIdentity}; use tari_contacts::contacts_service::{ handle::ContactsServiceHandle, service::ContactOnlineStatus, - types::{Message, MessageBuilder, MessageMetadata}, + types::{Message, MessageBuilder, MessageId, MessageMetadata, MetadataData, MetadataKey}, }; use tari_shutdown::Shutdown; @@ -45,11 +46,11 @@ const LOG_TARGET: &str = "contacts::chat_client"; #[async_trait] pub trait ChatClient { async fn add_contact(&self, address: &TariAddress) -> Result<(), Error>; - fn add_metadata(&self, message: Message, metadata_type: String, data: String) -> Message; + fn add_metadata(&self, message: Message, metadata_type: String, data: String) -> Result; async fn check_online_status(&self, address: &TariAddress) -> Result; - fn create_message(&self, receiver: &TariAddress, message: String) -> Message; + fn create_message(&self, receiver: &TariAddress, message: String) -> Result; async fn get_messages(&self, sender: &TariAddress, limit: u64, page: u64) -> Result, Error>; - async fn get_message(&self, id: &[u8]) -> Result; + async fn get_message(&self, id: &MessageId) -> Result; async fn send_message(&self, message: Message) -> Result<(), Error>; async fn send_read_receipt(&self, message: Message) -> Result<(), Error>; async fn get_conversationalists(&self) -> Result, Error>; @@ -171,29 +172,30 @@ impl ChatClient for Client { Ok(()) } - fn add_metadata(&self, mut message: Message, key: String, data: String) -> Message { + fn add_metadata(&self, mut message: Message, key: String, data: String) -> Result { let metadata = MessageMetadata { - key: key.into_bytes(), - data: data.into_bytes(), + key: MetadataKey::try_from(key)?, + data: MetadataData::try_from(data)?, }; message.push(metadata); - message + Ok(message) } - fn create_message(&self, receiver: &TariAddress, message: String) -> Message { - MessageBuilder::new() + fn create_message(&self, receiver: &TariAddress, message: String) -> Result { + Ok(MessageBuilder::new() .receiver_address(receiver.clone()) .sender_address(self.address().clone()) - .message(message) - .build() + .message(message)? + .build()) } async fn check_online_status(&self, address: &TariAddress) -> Result { if let Some(mut contacts_service) = self.contacts.clone() { let contact = contacts_service.get_contact(address.clone()).await?; - return Ok(contacts_service.get_contact_online_status(contact).await?); + let status = contacts_service.get_contact_online_status(contact).await?; + return Ok(status); } Ok(ContactOnlineStatus::Offline) @@ -208,7 +210,7 @@ impl ChatClient for Client { Ok(messages) } - async fn get_message(&self, message_id: &[u8]) -> Result { + async fn get_message(&self, message_id: &MessageId) -> Result { match self.contacts.clone() { Some(mut contacts_service) => contacts_service.get_message(message_id).await.map_err(|e| e.into()), None => Err(Error::InitializationError( diff --git a/base_layer/contacts/src/chat_client/src/error.rs b/base_layer/contacts/src/chat_client/src/error.rs index 7c0f7a4f27..c5eda9635a 100644 --- a/base_layer/contacts/src/chat_client/src/error.rs +++ b/base_layer/contacts/src/chat_client/src/error.rs @@ -25,6 +25,7 @@ use std::io; use diesel::ConnectionError; use minotari_app_utilities::identity_management::IdentityError; use tari_common_sqlite::error::StorageError as SqliteStorageError; +use tari_common_types::MaxSizeBytesError; use tari_comms::peer_manager::PeerManagerError; use tari_contacts::contacts_service::error::ContactsServiceError; use tari_p2p::initialization::CommsInitializationError; @@ -38,6 +39,8 @@ pub enum Error { NetworkingError(#[from] NetworkingError), #[error("The client had a problem communication with the contacts service: {0}")] ContactsServiceError(#[from] ContactsServiceError), + #[error("Byte size conversion error: `{0}`")] + MaxSizeBytesError(#[from] MaxSizeBytesError), } #[derive(Debug, thiserror::Error)] diff --git a/base_layer/contacts/src/contacts_service/error.rs b/base_layer/contacts/src/contacts_service/error.rs index 1112f675da..74c5f22622 100644 --- a/base_layer/contacts/src/contacts_service/error.rs +++ b/base_layer/contacts/src/contacts_service/error.rs @@ -22,6 +22,7 @@ use diesel::result::Error as DieselError; use tari_common_sqlite::error::SqliteStorageError; +use tari_common_types::MaxSizeBytesError; use tari_comms::connectivity::ConnectivityError; use tari_comms_dht::outbound::DhtOutboundError; use tari_p2p::services::liveness::error::LivenessError; @@ -53,6 +54,10 @@ pub enum ContactsServiceError { MalformedMessageError(#[from] prost::DecodeError), #[error("Message source does not match authenticated origin")] MessageSourceDoesNotMatchOrigin, + #[error("Byte size conversion error: `{0}`")] + MaxSizeBytesError(#[from] MaxSizeBytesError), + #[error("Message is too large: `{0}`")] + MessageSizeExceeded(String), } #[derive(Debug, Error)] @@ -79,4 +84,6 @@ pub enum ContactsServiceStorageError { BlockingTaskSpawnError(String), #[error("We got an error")] UnknownError, + #[error("Byte size conversion error: `{0}`")] + MaxSizeBytesError(#[from] MaxSizeBytesError), } diff --git a/base_layer/contacts/src/contacts_service/handle.rs b/base_layer/contacts/src/contacts_service/handle.rs index 8e39fa647f..5327657d21 100644 --- a/base_layer/contacts/src/contacts_service/handle.rs +++ b/base_layer/contacts/src/contacts_service/handle.rs @@ -37,7 +37,7 @@ use tower::Service; use crate::contacts_service::{ error::ContactsServiceError, service::{ContactMessageType, ContactOnlineStatus}, - types::{Confirmation, Contact, Message, MessageDispatch}, + types::{Confirmation, Contact, Message, MessageDispatch, MessageId}, }; pub static DEFAULT_MESSAGE_LIMIT: u64 = 35; @@ -138,7 +138,7 @@ pub enum ContactsServiceRequest { GetContactOnlineStatus(Contact), SendMessage(TariAddress, Message), GetMessages(TariAddress, i64, i64), - GetMessage(Vec), + GetMessage(MessageId), SendReadConfirmation(TariAddress, Confirmation), GetConversationalists, } @@ -279,10 +279,10 @@ impl ContactsServiceHandle { } } - pub async fn get_message(&mut self, message_id: &[u8]) -> Result { + pub async fn get_message(&mut self, message_id: &MessageId) -> Result { match self .request_response_service - .call(ContactsServiceRequest::GetMessage(message_id.to_vec())) + .call(ContactsServiceRequest::GetMessage(message_id.clone())) .await?? { ContactsServiceResponse::Message(message) => Ok(message), @@ -307,7 +307,7 @@ impl ContactsServiceHandle { pub async fn send_read_confirmation( &mut self, address: TariAddress, - message_id: Vec, + message_id: MessageId, ) -> Result<(), ContactsServiceError> { match self .request_response_service diff --git a/base_layer/contacts/src/contacts_service/service.rs b/base_layer/contacts/src/contacts_service/service.rs index 006976e877..fc79aabd63 100644 --- a/base_layer/contacts/src/contacts_service/service.rs +++ b/base_layer/contacts/src/contacts_service/service.rs @@ -650,7 +650,7 @@ where T: ContactsBackend + 'static }, }; - trace!(target: LOG_TARGET, "Handling confirmation with details: message_id: {:?}, delivery: {:?}, read: {:?}", message_id, delivery, read); + trace!(target: LOG_TARGET, "Handling confirmation with details: message_id: {}, delivery: {:?}, read: {:?}", message_id, delivery, read); self.db.confirm_message(message_id, delivery, read)?; let _msg = self.message_publisher.send(Arc::new(dispatch)); diff --git a/base_layer/contacts/src/contacts_service/storage/database.rs b/base_layer/contacts/src/contacts_service/storage/database.rs index 593403c678..7f5c9e12da 100644 --- a/base_layer/contacts/src/contacts_service/storage/database.rs +++ b/base_layer/contacts/src/contacts_service/storage/database.rs @@ -30,10 +30,11 @@ use chrono::NaiveDateTime; use log::*; use tari_common_types::tari_address::TariAddress; use tari_comms::peer_manager::NodeId; +use tari_utilities::ByteArray; use crate::contacts_service::{ error::ContactsServiceStorageError, - types::{Contact, Message}, + types::{Contact, Message, MessageId}, }; const LOG_TARGET: &str = "contacts::contacts_service::database"; @@ -187,9 +188,9 @@ where T: ContactsBackend + 'static } } - pub fn get_message(&self, message_id: Vec) -> Result { + pub fn get_message(&self, message_id: MessageId) -> Result { let db_clone = self.db.clone(); - fetch!(db_clone, message_id, Message) + fetch!(db_clone, message_id.to_vec(), Message) } pub fn save_message(&self, message: Message) -> Result<(), ContactsServiceStorageError> { @@ -201,7 +202,7 @@ where T: ContactsBackend + 'static pub fn confirm_message( &self, - message_id: Vec, + message_id: MessageId, delivery_confirmation: Option, read_confirmation: Option, ) -> Result<(), ContactsServiceStorageError> { @@ -225,7 +226,9 @@ where T: ContactsBackend + 'static self.db .write(WriteOperation::Upsert(Box::new(DbKeyValuePair::MessageConfirmations( - message_id, delivery, read, + message_id.to_vec(), + delivery, + read, ))))?; Ok(()) diff --git a/base_layer/contacts/src/contacts_service/storage/types/messages.rs b/base_layer/contacts/src/contacts_service/storage/types/messages.rs index 1f7669de3a..8601a41bf8 100644 --- a/base_layer/contacts/src/contacts_service/storage/types/messages.rs +++ b/base_layer/contacts/src/contacts_service/storage/types/messages.rs @@ -26,11 +26,12 @@ use chrono::NaiveDateTime; use diesel::prelude::*; use tari_common_sqlite::util::diesel_ext::ExpectedRowsExtension; use tari_common_types::tari_address::TariAddress; +use tari_utilities::ByteArray; use crate::{ contacts_service::{ error::ContactsServiceStorageError, - types::{Direction, Message, MessageMetadata}, + types::{ChatBody, Direction, Message, MessageId, MessageMetadata}, }, schema::messages, }; @@ -183,7 +184,7 @@ impl TryFrom for Message { Ok(Self { metadata, - body: o.body, + body: ChatBody::try_from(o.body)?, receiver_address, sender_address, direction: Direction::from_byte( @@ -194,7 +195,7 @@ impl TryFrom for Message { stored_at: o.stored_at.timestamp() as u64, delivery_confirmation_at: Some(o.stored_at.timestamp() as u64), read_confirmation_at: Some(o.stored_at.timestamp() as u64), - message_id: o.message_id, + message_id: MessageId::try_from(o.message_id)?, }) } } @@ -209,8 +210,8 @@ impl TryFrom for MessagesSqlInsert { Ok(Self { receiver_address: o.receiver_address.to_vec(), sender_address: o.sender_address.to_vec(), - message_id: o.message_id, - body: o.body, + message_id: o.message_id.to_vec(), + body: o.body.to_vec(), metadata: metadata.into_bytes().to_vec(), stored_at: NaiveDateTime::from_timestamp_opt(o.stored_at as i64, 0) .ok_or(ContactsServiceStorageError::ConversionError)?, diff --git a/base_layer/contacts/src/contacts_service/types/confirmation.rs b/base_layer/contacts/src/contacts_service/types/confirmation.rs index f41b06a555..27af40c236 100644 --- a/base_layer/contacts/src/contacts_service/types/confirmation.rs +++ b/base_layer/contacts/src/contacts_service/types/confirmation.rs @@ -20,28 +20,45 @@ // 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. -use crate::contacts_service::proto; +use std::{convert::TryFrom, fmt::Display}; + +use tari_common_types::MaxSizeBytes; +use tari_utilities::ByteArray; + +use crate::contacts_service::{error::ContactsServiceError, proto, types::MessageId}; #[derive(Clone, Debug, Default)] pub struct Confirmation { - pub message_id: Vec, + pub message_id: MessageId, pub timestamp: u64, } -impl From for Confirmation { - fn from(confirmation: proto::Confirmation) -> Self { - Self { - message_id: confirmation.message_id, +impl TryFrom for Confirmation { + type Error = ContactsServiceError; + + fn try_from(confirmation: proto::Confirmation) -> Result { + Ok(Self { + message_id: MaxSizeBytes::try_from(confirmation.message_id)?, timestamp: confirmation.timestamp, - } + }) } } impl From for proto::Confirmation { fn from(confirmation: Confirmation) -> Self { Self { - message_id: confirmation.message_id, + message_id: confirmation.message_id.to_vec(), timestamp: confirmation.timestamp, } } } + +impl Display for Confirmation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Confirmation: message_id: {}, timestamp: {}", + self.message_id, self.timestamp + ) + } +} diff --git a/base_layer/contacts/src/contacts_service/types/message.rs b/base_layer/contacts/src/contacts_service/types/message.rs index 1238e714ad..d971e72bc3 100644 --- a/base_layer/contacts/src/contacts_service/types/message.rs +++ b/base_layer/contacts/src/contacts_service/types/message.rs @@ -20,20 +20,27 @@ // 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. -use std::convert::TryFrom; +use std::{convert::TryFrom, fmt::Display}; use num_derive::FromPrimitive; use num_traits::FromPrimitive; use serde::{Deserialize, Serialize}; -use tari_common_types::tari_address::TariAddress; +use tari_common_types::{tari_address::TariAddress, MaxSizeBytes}; use tari_comms_dht::domain_message::OutboundDomainMessage; use tari_p2p::tari_message::TariMessageType; +use tari_utilities::ByteArray; use crate::contacts_service::proto; +pub(crate) const MAX_MESSAGE_ID_SIZE: usize = 36; +pub type MessageId = MaxSizeBytes; +pub(crate) const MAX_BODY_SIZE: usize = 2 * 1024 * 1024; +pub type ChatBody = MaxSizeBytes; +pub(crate) const MAX_MESSAGE_SIZE: usize = MAX_BODY_SIZE + 512 * 1024; + #[derive(Clone, Debug, Default)] pub struct Message { - pub body: Vec, + pub body: ChatBody, pub metadata: Vec, pub receiver_address: TariAddress, pub sender_address: TariAddress, @@ -42,7 +49,7 @@ pub struct Message { pub stored_at: u64, pub delivery_confirmation_at: Option, pub read_confirmation_at: Option, - pub message_id: Vec, + pub message_id: MessageId, } impl Message { @@ -69,10 +76,23 @@ impl Direction { } } +pub(crate) const MAX_KEY_SIZE: usize = 256; +pub type MetadataKey = MaxSizeBytes; +pub(crate) const MAX_DATA_SIZE: usize = 2 * 1024 * 1024; +pub type MetadataData = MaxSizeBytes; + #[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct MessageMetadata { - pub key: Vec, - pub data: Vec, + pub key: MetadataKey, + pub data: MetadataData, +} + +impl Message { + pub fn data_byte_size(&self) -> usize { + self.body.len() + + self.metadata.iter().map(|m| m.data.len()).sum::() + + self.metadata.iter().map(|m| m.key.len()).sum::() + } } impl TryFrom for Message { @@ -81,17 +101,19 @@ impl TryFrom for Message { fn try_from(message: proto::Message) -> Result { let mut metadata = vec![]; for m in message.metadata { - metadata.push(m.into()); + metadata.push(MessageMetadata::try_from(m)?); } Ok(Self { - body: message.body, + body: ChatBody::try_from(message.body).map_err(|e| format!("body: ({})", e))?, metadata, - receiver_address: TariAddress::from_bytes(&message.receiver_address).map_err(|e| e.to_string())?, - sender_address: TariAddress::from_bytes(&message.sender_address).map_err(|e| e.to_string())?, + receiver_address: TariAddress::from_bytes(&message.receiver_address) + .map_err(|e| format!("receiver_address: ({})", e))?, + sender_address: TariAddress::from_bytes(&message.sender_address) + .map_err(|e| format!("sender_address: ({})", e))?, // A Message from a proto::Message will always be an inbound message direction: Direction::Inbound, - message_id: message.message_id, + message_id: MessageId::try_from(message.message_id).map_err(|e| format!("message_id: ({})", e))?, ..Message::default() }) } @@ -100,7 +122,7 @@ impl TryFrom for Message { impl From for proto::Message { fn from(message: Message) -> Self { Self { - body: message.body, + body: message.body.to_vec(), metadata: message .metadata .iter() @@ -109,7 +131,7 @@ impl From for proto::Message { receiver_address: message.receiver_address.to_vec(), sender_address: message.sender_address.to_vec(), direction: i32::from(message.direction.as_byte()), - message_id: message.message_id, + message_id: message.message_id.to_vec(), } } } @@ -120,20 +142,48 @@ impl From for OutboundDomainMessage { } } -impl From for MessageMetadata { - fn from(md: proto::MessageMetadata) -> Self { - Self { - data: md.data, - key: md.key, - } +impl TryFrom for MessageMetadata { + type Error = String; + + fn try_from(md: proto::MessageMetadata) -> Result { + Ok(Self { + data: MetadataData::try_from(md.data).map_err(|e| format!("metadata data: ({})", e))?, + key: MetadataKey::try_from(md.key).map_err(|e| format!("metadata key: ({})", e))?, + }) } } impl From for proto::MessageMetadata { fn from(md: MessageMetadata) -> Self { Self { - data: md.data, - key: md.key, + data: md.data.to_vec(), + key: md.key.to_vec(), } } } + +impl Display for Message { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Message {{ message_id: {}, receiver_address: {}, sender_address: {}, direction: {:?}, body: {}, \ + metadata: {:?}, sent_at: {}, stored_at: {}, delivery_confirmation_at: {:?}, read_confirmation_at: {:?} }}", + self.message_id, + self.receiver_address, + self.sender_address, + self.direction, + self.body, + format!( + "{:?}", + self.metadata + .iter() + .map(|m| format!("({}, {})", m.key, m.data)) + .collect::>() + ), + self.sent_at, + self.stored_at, + self.delivery_confirmation_at, + self.read_confirmation_at, + ) + } +} diff --git a/base_layer/contacts/src/contacts_service/types/message_builder.rs b/base_layer/contacts/src/contacts_service/types/message_builder.rs index 68511e34cd..af1c008971 100644 --- a/base_layer/contacts/src/contacts_service/types/message_builder.rs +++ b/base_layer/contacts/src/contacts_service/types/message_builder.rs @@ -20,10 +20,20 @@ // 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. +use std::convert::TryFrom; + use tari_common_types::tari_address::TariAddress; use uuid::Uuid; -use crate::contacts_service::types::{message::MessageMetadata, Message}; +use crate::contacts_service::{ + error::ContactsServiceError, + types::{ + message::{MessageMetadata, MAX_MESSAGE_SIZE}, + ChatBody, + Message, + MessageId, + }, +}; #[derive(Clone, Debug, Default)] pub struct MessageBuilder { @@ -33,12 +43,12 @@ pub struct MessageBuilder { impl MessageBuilder { pub fn new() -> Self { // We're forcing it to a String before bytes so we can have the same representation used in - // all places. Otherwise the UUID byte format will differ if displayed somewhere. + // all places, otherwise the UUID byte format will differ if displayed somewhere. let message_id = Uuid::new_v4().to_string().into_bytes(); Self { inner: Message { - message_id, + message_id: MessageId::from_bytes_truncate(message_id), ..Message::default() }, } @@ -62,26 +72,33 @@ impl MessageBuilder { } } - pub fn message(&self, body: String) -> Self { - let body = body.into_bytes(); - Self { - inner: Message { - body, - ..self.inner.clone() - }, - } + pub fn message(&self, body: String) -> Result { + let message = Message { + body: ChatBody::try_from(body.into_bytes())?, + ..self.inner.clone() + }; + self.finalize(message) } - pub fn metadata(&self, new_metadata: MessageMetadata) -> Self { + pub fn metadata(&self, new_metadata: MessageMetadata) -> Result { let mut metadata = self.inner.metadata.clone(); metadata.push(new_metadata); + let message = Message { + metadata, + ..self.inner.clone() + }; + self.finalize(message) + } - Self { - inner: Message { - metadata, - ..self.inner.clone() - }, + fn finalize(&self, message: Message) -> Result { + if message.data_byte_size() > MAX_MESSAGE_SIZE { + return Err(ContactsServiceError::MessageSizeExceeded(format!( + "current: {}, limit: {}", + message.data_byte_size(), + MAX_MESSAGE_SIZE + ))); } + Ok(Self { inner: message }) } pub fn build(&self) -> Message { @@ -96,3 +113,82 @@ impl From for MessageBuilder { } } } + +#[cfg(test)] +mod test { + use std::{convert::TryFrom, str::from_utf8_mut}; + + use uuid::Uuid; + + use crate::contacts_service::types::{ + message::{MAX_BODY_SIZE, MAX_DATA_SIZE, MAX_KEY_SIZE, MAX_MESSAGE_ID_SIZE}, + ChatBody, + MessageBuilder, + MessageId, + MessageMetadata, + MetadataData, + MetadataKey, + }; + + #[test] + fn test_message_id_size() { + for _ in 0..10 { + let message_id = Uuid::new_v4().to_string().into_bytes(); + assert!( + MessageId::try_from(message_id.clone()).is_ok(), + "Invalid size - MAX_MESSAGE_ID_SIZE length: {}, message_id length: {}", + MessageId::default().len(), + message_id.len() + ); + } + } + + #[test] + fn test_message_size() { + assert!(MetadataKey::try_from(vec![0u8; MAX_KEY_SIZE]).is_ok()); + assert!(MetadataKey::try_from(vec![0u8; MAX_KEY_SIZE + 1]).is_err()); + assert!(MetadataData::try_from(vec![0u8; MAX_DATA_SIZE]).is_ok()); + assert!(MetadataData::try_from(vec![0u8; MAX_DATA_SIZE + 1]).is_err()); + + assert!(ChatBody::try_from(vec![0u8; MAX_BODY_SIZE]).is_ok()); + assert!(ChatBody::try_from(vec![0u8; MAX_BODY_SIZE + 1]).is_err()); + + assert!(MessageId::try_from(vec![0u8; MAX_MESSAGE_ID_SIZE]).is_ok()); + assert!(MessageId::try_from(vec![0u8; MAX_MESSAGE_ID_SIZE + 1]).is_err()); + + let mut builder = MessageBuilder::new(); + builder = builder + .metadata(MessageMetadata { + key: Default::default(), + data: Default::default(), + }) + .unwrap(); + let message = builder.build(); + assert_eq!(message.metadata.len(), 1); + builder = builder + .metadata(MessageMetadata { + key: MetadataKey::try_from(vec![0u8; MAX_KEY_SIZE]).unwrap(), + data: MetadataData::try_from(vec![0u8; MAX_DATA_SIZE]).unwrap(), + }) + .unwrap(); + let message = builder.build(); + assert_eq!(message.metadata.len(), 2); + assert!(builder + .metadata(MessageMetadata { + key: MetadataKey::try_from(vec![0u8; MAX_KEY_SIZE]).unwrap(), + data: MetadataData::try_from(vec![0u8; MAX_DATA_SIZE]).unwrap() + }) + .is_err()); + let message = builder.build(); + assert_eq!(message.metadata.len(), 2); + + let builder = MessageBuilder::new(); + let mut body = vec![0u8; MAX_BODY_SIZE]; + let body_str = from_utf8_mut(&mut body).unwrap().to_string(); + builder.message(body_str.clone()).unwrap(); + assert!(builder.message(body_str).is_ok()); + let mut body = vec![0u8; MAX_BODY_SIZE + 1]; + let body_str = from_utf8_mut(&mut body).unwrap().to_string(); + assert!(builder.message(body_str).is_err()); + } +} diff --git a/base_layer/contacts/src/contacts_service/types/message_dispatch.rs b/base_layer/contacts/src/contacts_service/types/message_dispatch.rs index 0f6d81952b..b797afd3d1 100644 --- a/base_layer/contacts/src/contacts_service/types/message_dispatch.rs +++ b/base_layer/contacts/src/contacts_service/types/message_dispatch.rs @@ -44,10 +44,10 @@ impl TryFrom for MessageDispatch { Ok(match dispatch.contents { Some(proto::message_dispatch::Contents::Message(m)) => MessageDispatch::Message(Message::try_from(m)?), Some(proto::message_dispatch::Contents::DeliveryConfirmation(c)) => { - MessageDispatch::DeliveryConfirmation(Confirmation::from(c)) + MessageDispatch::DeliveryConfirmation(Confirmation::try_from(c).map_err(|e| e.to_string())?) }, Some(proto::message_dispatch::Contents::ReadConfirmation(c)) => { - MessageDispatch::ReadConfirmation(Confirmation::from(c)) + MessageDispatch::ReadConfirmation(Confirmation::try_from(c).map_err(|e| e.to_string())?) }, None => return Err("We didn't get any known type of chat message".to_string()), }) diff --git a/base_layer/contacts/src/contacts_service/types/mod.rs b/base_layer/contacts/src/contacts_service/types/mod.rs index 3faa818886..a46d83d404 100644 --- a/base_layer/contacts/src/contacts_service/types/mod.rs +++ b/base_layer/contacts/src/contacts_service/types/mod.rs @@ -24,7 +24,7 @@ mod contact; pub use contact::Contact; mod message; -pub use message::{Direction, Message, MessageMetadata}; +pub use message::{ChatBody, Direction, Message, MessageId, MessageMetadata, MetadataData, MetadataKey}; mod message_builder; pub use message_builder::MessageBuilder; diff --git a/base_layer/contacts/tests/contacts_service.rs b/base_layer/contacts/tests/contacts_service.rs index 98a8eb1a99..73acc051ae 100644 --- a/base_layer/contacts/tests/contacts_service.rs +++ b/base_layer/contacts/tests/contacts_service.rs @@ -243,6 +243,7 @@ pub fn test_message_pagination() { for num in 0..8 { let message = MessageBuilder::new() .message(format!("Test {:?}", num)) + .unwrap() .receiver_address(address.clone()) .sender_address(address.clone()) .build(); @@ -274,6 +275,7 @@ pub fn test_message_pagination() { for num in 0..3000 { let message = MessageBuilder::new() .message(format!("Test {:?}", num)) + .unwrap() .receiver_address(address.clone()) .sender_address(address.clone()) .build(); diff --git a/base_layer/core/src/consensus/consensus_encoding.rs b/base_layer/core/src/consensus/consensus_encoding.rs index ad361324b9..cc9abe8519 100644 --- a/base_layer/core/src/consensus/consensus_encoding.rs +++ b/base_layer/core/src/consensus/consensus_encoding.rs @@ -20,11 +20,6 @@ // 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. -mod bytes; mod hashing; -mod string; pub use hashing::DomainSeparatedConsensusHasher; -pub use string::MaxSizeString; - -pub use self::bytes::MaxSizeBytes; diff --git a/base_layer/core/src/consensus/mod.rs b/base_layer/core/src/consensus/mod.rs index 3d71b1a774..b8228fd1ef 100644 --- a/base_layer/core/src/consensus/mod.rs +++ b/base_layer/core/src/consensus/mod.rs @@ -30,7 +30,7 @@ mod consensus_manager; pub use consensus_manager::{ConsensusBuilderError, ConsensusManager, ConsensusManagerBuilder, ConsensusManagerError}; mod consensus_encoding; -pub use consensus_encoding::{DomainSeparatedConsensusHasher, MaxSizeBytes, MaxSizeString}; +pub use consensus_encoding::DomainSeparatedConsensusHasher; mod network; pub use network::NetworkConsensus; diff --git a/base_layer/core/src/covenants/arguments.rs b/base_layer/core/src/covenants/arguments.rs index 922b4cb98b..95e3dc2cfd 100644 --- a/base_layer/core/src/covenants/arguments.rs +++ b/base_layer/core/src/covenants/arguments.rs @@ -27,13 +27,15 @@ use std::{ use borsh::{BorshDeserialize, BorshSerialize}; use integer_encoding::VarIntWriter; -use tari_common_types::types::{Commitment, FixedHash, PublicKey}; +use tari_common_types::{ + types::{Commitment, FixedHash, PublicKey}, + MaxSizeBytes, +}; use tari_script::TariScript; use tari_utilities::{hex::Hex, ByteArray}; use super::decoder::CovenantDecodeError; use crate::{ - consensus::MaxSizeBytes, covenants::{ byte_codes, covenant::Covenant, diff --git a/base_layer/core/src/proto/sidechain_feature.rs b/base_layer/core/src/proto/sidechain_feature.rs index fbc29c98c1..997b851602 100644 --- a/base_layer/core/src/proto/sidechain_feature.rs +++ b/base_layer/core/src/proto/sidechain_feature.rs @@ -24,11 +24,13 @@ use std::convert::{TryFrom, TryInto}; -use tari_common_types::types::{PublicKey, Signature}; +use tari_common_types::{ + types::{PublicKey, Signature}, + MaxSizeString, +}; use tari_utilities::ByteArray; use crate::{ - consensus::MaxSizeString, proto, transactions::transaction_components::{ BuildInfo, diff --git a/base_layer/core/src/transactions/transaction_components/encrypted_data.rs b/base_layer/core/src/transactions/transaction_components/encrypted_data.rs index aa107aef68..6b8b3a762a 100644 --- a/base_layer/core/src/transactions/transaction_components/encrypted_data.rs +++ b/base_layer/core/src/transactions/transaction_components/encrypted_data.rs @@ -47,6 +47,7 @@ use serde::{Deserialize, Serialize}; use tari_common_types::{ tari_address::dual_address::DualAddress, types::{Commitment, PrivateKey}, + MaxSizeBytes, }; use tari_crypto::{hashing::DomainSeparatedHasher, keys::SecretKey}; use tari_hashing::TransactionSecureNonceKdfDomain; @@ -60,7 +61,7 @@ use thiserror::Error; use zeroize::{Zeroize, Zeroizing}; use super::EncryptedDataKey; -use crate::{consensus::MaxSizeBytes, transactions::tari_amount::MicroMinotari}; +use crate::transactions::tari_amount::MicroMinotari; // Useful size constants, each in bytes const SIZE_NONCE: usize = size_of::(); const SIZE_VALUE: usize = size_of::(); diff --git a/base_layer/core/src/transactions/transaction_components/output_features.rs b/base_layer/core/src/transactions/transaction_components/output_features.rs index 0db451450b..431ad9df19 100644 --- a/base_layer/core/src/transactions/transaction_components/output_features.rs +++ b/base_layer/core/src/transactions/transaction_components/output_features.rs @@ -28,22 +28,23 @@ use std::{ use borsh::{BorshDeserialize, BorshSerialize}; use serde::{Deserialize, Serialize}; -use tari_common_types::types::{PublicKey, Signature}; +use tari_common_types::{ + types::{PublicKey, Signature}, + MaxSizeBytes, + MaxSizeString, +}; use super::OutputFeaturesVersion; -use crate::{ - consensus::{MaxSizeBytes, MaxSizeString}, - transactions::transaction_components::{ - range_proof_type::RangeProofType, - side_chain::SideChainFeature, - BuildInfo, - CodeTemplateRegistration, - ConfidentialOutputData, - OutputType, - TemplateType, - ValidatorNodeRegistration, - ValidatorNodeSignature, - }, +use crate::transactions::transaction_components::{ + range_proof_type::RangeProofType, + side_chain::SideChainFeature, + BuildInfo, + CodeTemplateRegistration, + ConfidentialOutputData, + OutputType, + TemplateType, + ValidatorNodeRegistration, + ValidatorNodeSignature, }; /// Options for UTXO's diff --git a/base_layer/core/src/transactions/transaction_components/side_chain/template_registration.rs b/base_layer/core/src/transactions/transaction_components/side_chain/template_registration.rs index 1688deb3d3..732ef46014 100644 --- a/base_layer/core/src/transactions/transaction_components/side_chain/template_registration.rs +++ b/base_layer/core/src/transactions/transaction_components/side_chain/template_registration.rs @@ -22,9 +22,11 @@ use borsh::{BorshDeserialize, BorshSerialize}; use serde::{Deserialize, Serialize}; -use tari_common_types::types::{PublicKey, Signature}; - -use crate::consensus::{MaxSizeBytes, MaxSizeString}; +use tari_common_types::{ + types::{PublicKey, Signature}, + MaxSizeBytes, + MaxSizeString, +}; #[derive(Debug, Clone, Hash, PartialEq, Eq, Deserialize, Serialize, BorshSerialize, BorshDeserialize)] pub struct CodeTemplateRegistration { diff --git a/base_layer/wallet/src/transaction_service/handle.rs b/base_layer/wallet/src/transaction_service/handle.rs index 4fa9b0a5ca..439e48adaf 100644 --- a/base_layer/wallet/src/transaction_service/handle.rs +++ b/base_layer/wallet/src/transaction_service/handle.rs @@ -33,10 +33,11 @@ use tari_common_types::{ tari_address::TariAddress, transaction::{ImportStatus, TxId}, types::{FixedHash, HashOutput, PrivateKey, PublicKey, Signature}, + MaxSizeBytes, + MaxSizeString, }; use tari_comms::types::CommsPublicKey; use tari_core::{ - consensus::{MaxSizeBytes, MaxSizeString}, mempool::FeePerGramStat, proto, transactions::{ diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index e07cf9c0d4..d36af1724f 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -9225,7 +9225,7 @@ mod test { use once_cell::sync::Lazy; use tari_common_types::{emoji, tari_address::TariAddressFeatures, types::PrivateKey}; use tari_comms::peer_manager::PeerFeatures; - use tari_contacts::contacts_service::types::{Direction, Message, MessageMetadata}; + use tari_contacts::contacts_service::types::{ChatBody, Direction, Message, MessageId, MessageMetadata}; use tari_core::{ covenant, transactions::{ @@ -12276,7 +12276,7 @@ mod test { if alice_msg_count < 5 { let alice_message_result = alice_wallet_runtime.block_on(alice_wallet_contacts_service.send_message(Message { - body: vec![i], + body: ChatBody::try_from(vec![i]).unwrap(), metadata: vec![MessageMetadata::default()], receiver_address: alice_wallet_address.clone(), sender_address: bob_wallet_address.clone(), @@ -12285,7 +12285,7 @@ mod test { sent_at: u64::from(i), delivery_confirmation_at: None, read_confirmation_at: None, - message_id: vec![i], + message_id: MessageId::try_from(vec![i]).unwrap(), })); if alice_message_result.is_ok() { alice_msg_count += 1; @@ -12294,7 +12294,7 @@ mod test { if bob_msg_count < 5 { let bob_message_result = bob_wallet_runtime.block_on(bob_wallet_contacts_service.send_message(Message { - body: vec![i], + body: ChatBody::try_from(vec![i]).unwrap(), metadata: vec![MessageMetadata::default()], sender_address: alice_wallet_address.clone(), receiver_address: bob_wallet_address.clone(), @@ -12303,7 +12303,7 @@ mod test { sent_at: u64::from(i), delivery_confirmation_at: None, read_confirmation_at: None, - message_id: vec![i], + message_id: MessageId::try_from(vec![i]).unwrap(), })); if bob_message_result.is_ok() { bob_msg_count += 1; diff --git a/common/src/lib.rs b/common/src/lib.rs index 2056c59ae1..ba6a6ba96d 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -65,6 +65,7 @@ pub use configuration::{ utils::load_configuration, }; pub mod dir_utils; + pub use logging::initialize_logging; pub const DEFAULT_CONFIG: &str = "config/config.toml"; diff --git a/integration_tests/src/chat_ffi.rs b/integration_tests/src/chat_ffi.rs index da0ceb127a..5ca41dbe8a 100644 --- a/integration_tests/src/chat_ffi.rs +++ b/integration_tests/src/chat_ffi.rs @@ -29,19 +29,19 @@ use std::{ }; use async_trait::async_trait; - -type ClientFFI = c_void; - use libc::{c_char, c_int, c_uchar, c_uint}; use minotari_app_utilities::identity_management::setup_node_identity; -use tari_chat_client::{database, error::Error as ClientError, ChatClient}; +use tari_chat_client::{database, error::Error as ClientError, ChatClient as ChatClientTrait}; use tari_common::configuration::Network; use tari_common_types::tari_address::TariAddress; use tari_comms::{ multiaddr::Multiaddr, peer_manager::{Peer, PeerFeatures}, }; -use tari_contacts::contacts_service::{service::ContactOnlineStatus, types::Message}; +use tari_contacts::contacts_service::{ + service::ContactOnlineStatus, + types::{Message, MessageId}, +}; use crate::{chat_client::test_config, get_port}; @@ -65,63 +65,92 @@ extern "C" fn callback_read_confirmation_received(_state: *mut c_void) { *callback.read_confirmation_received.lock().unwrap() += 1; } +type FfiChatClient = c_void; +type FfiTariAddress = c_void; +type FFiApplicationConfig = c_void; +type FFiContactsServiceHandle = c_void; +type FfiMessage = c_void; +type FfiChatByteVector = c_void; +type FfiMessageVector = c_void; +type FfiConversationalistsVector = c_void; + +pub(crate) type CallbackContactStatusChange = unsafe extern "C" fn(*mut c_void); +pub(crate) type CallbackMessageReceived = unsafe extern "C" fn(*mut c_void); +pub(crate) type CallbackDeliveryConfirmationReceived = unsafe extern "C" fn(*mut c_void); +pub(crate) type CallbackReadConfirmationReceived = unsafe extern "C" fn(*mut c_void); + #[cfg_attr(windows, link(name = "minotari_chat_ffi.dll"))] #[cfg_attr(not(windows), link(name = "minotari_chat_ffi"))] extern "C" { pub fn create_chat_client( - config: *mut c_void, - callback_contact_status_change: unsafe extern "C" fn(*mut c_void), - callback_message_received: unsafe extern "C" fn(*mut c_void), - callback_delivery_confirmation_received: unsafe extern "C" fn(*mut c_void), - callback_read_confirmation_received: unsafe extern "C" fn(*mut c_void), - tari_address: *mut c_void, + config: *mut FFiApplicationConfig, + callback_contact_status_change: CallbackContactStatusChange, + callback_message_received: CallbackMessageReceived, + callback_delivery_confirmation_received: CallbackDeliveryConfirmationReceived, + callback_read_confirmation_received: CallbackReadConfirmationReceived, + tari_address: *mut FfiTariAddress, error_out: *const c_int, - ) -> *mut ClientFFI; + ) -> *mut FfiChatClient; pub fn sideload_chat_client( - config: *mut c_void, - contact_handle: *mut c_void, - callback_contact_status_change: unsafe extern "C" fn(*mut c_void), - callback_message_received: unsafe extern "C" fn(*mut c_void), - callback_delivery_confirmation_received: unsafe extern "C" fn(*mut c_void), - callback_read_confirmation_received: unsafe extern "C" fn(*mut c_void), - tari_address: *mut c_void, + config: *mut FFiApplicationConfig, + contact_handle: *mut FFiContactsServiceHandle, + callback_contact_status_change: CallbackContactStatusChange, + callback_message_received: CallbackMessageReceived, + callback_delivery_confirmation_received: CallbackDeliveryConfirmationReceived, + callback_read_confirmation_received: CallbackReadConfirmationReceived, + tari_address: *mut FfiTariAddress, error_out: *const c_int, - ) -> *mut ClientFFI; + ) -> *mut FfiChatClient; pub fn create_chat_message( - receiver: *mut c_void, - sender: *mut c_void, + receiver: *mut FfiTariAddress, + sender: *mut FfiTariAddress, message: *const c_char, error_out: *const c_int, - ) -> *mut c_void; - pub fn send_chat_message(client: *mut ClientFFI, message: *mut c_void, error_out: *const c_int); + ) -> *mut FfiMessage; + pub fn send_chat_message(client: *mut FfiChatClient, message: *mut FfiMessage, error_out: *const c_int); pub fn add_chat_message_metadata( - message: *mut c_void, - metadata_type: *const c_char, - data: *const c_char, + message: *mut FfiMessage, + key: *mut FfiChatByteVector, + data: *mut FfiChatByteVector, + error_out: *const c_int, + ); + pub fn add_chat_contact(client: *mut FfiChatClient, address: *mut FfiTariAddress, error_out: *const c_int); + pub fn check_online_status( + client: *mut FfiChatClient, + address: *mut FfiTariAddress, error_out: *const c_int, - ) -> *mut c_void; - pub fn add_chat_contact(client: *mut ClientFFI, address: *mut c_void, error_out: *const c_int); - pub fn check_online_status(client: *mut ClientFFI, address: *mut c_void, error_out: *const c_int) -> c_int; + ) -> c_uchar; pub fn get_chat_messages( - client: *mut ClientFFI, - sender: *mut c_void, - limit: c_int, - page: c_int, + client: *mut FfiChatClient, + sender: *mut FfiTariAddress, + limit: c_uint, + page: c_uint, + error_out: *const c_int, + ) -> *mut FfiMessageVector; + pub fn get_chat_message( + client: *mut FfiChatClient, + message_id: *mut FfiChatByteVector, error_out: *const c_int, - ) -> *mut c_void; - pub fn get_chat_message(client: *mut ClientFFI, message_id: *mut c_void, error_out: *const c_int) -> *mut c_void; - pub fn destroy_chat_client(client: *mut ClientFFI); + ) -> *mut FfiMessage; + pub fn destroy_chat_client(client: *mut FfiChatClient); pub fn chat_byte_vector_create( byte_array: *const c_uchar, element_count: c_uint, error_our: *const c_int, - ) -> *mut c_void; - pub fn send_read_confirmation_for_message(client: *mut ClientFFI, message: *mut c_void, error_out: *const c_int); - pub fn get_conversationalists(client: *mut ClientFFI, error_out: *const c_int) -> *mut c_void; + ) -> *mut FfiChatByteVector; + pub fn send_read_confirmation_for_message( + client: *mut FfiChatClient, + message: *mut FfiMessage, + error_out: *const c_int, + ); + pub fn get_conversationalists( + client: *mut FfiChatClient, + error_out: *const c_int, + ) -> *mut FfiConversationalistsVector; } #[derive(Debug)] -pub struct PtrWrapper(*mut ClientFFI); +pub struct PtrWrapper(*mut FfiChatClient); unsafe impl Send for PtrWrapper {} #[derive(Debug)] @@ -134,7 +163,7 @@ struct Conversationalists(Vec); struct MessagesVector(Vec); #[async_trait] -impl ChatClient for ChatFFI { +impl ChatClientTrait for ChatFFI { async fn add_contact(&self, address: &TariAddress) -> Result<(), ClientError> { let client = self.ptr.lock().unwrap(); @@ -148,7 +177,7 @@ impl ChatClient for ChatFFI { Ok(result) } - fn add_metadata(&self, message: Message, key: String, data: String) -> Message { + fn add_metadata(&self, message: Message, key: String, data: String) -> Result { let message_ptr = Box::into_raw(Box::new(message)) as *mut c_void; let error_out = Box::into_raw(Box::new(0)); @@ -161,31 +190,26 @@ impl ChatClient for ChatFFI { let byte_data = unsafe { chat_byte_vector_create(data_bytes.as_ptr(), len, error_out) }; unsafe { - add_chat_message_metadata( - message_ptr, - byte_key as *const c_char, - byte_data as *const c_char, - error_out, - ); - *Box::from_raw(message_ptr as *mut Message) + add_chat_message_metadata(message_ptr, byte_key, byte_data, error_out); + Ok(*Box::from_raw(message_ptr as *mut Message)) } } async fn check_online_status(&self, address: &TariAddress) -> Result { let client = self.ptr.lock().unwrap(); - let address_ptr = Box::into_raw(Box::new(address.clone())) as *mut c_void; - - let result; + let address_ptr = Box::into_raw(Box::new(address.clone())) as *mut FfiTariAddress; let error_out = Box::into_raw(Box::new(0)); - unsafe { result = check_online_status(client.0, address_ptr, error_out) } - Ok(ContactOnlineStatus::from_byte(u8::try_from(result).unwrap()).expect("A valid u8 from FFI status")) + unsafe { + let result = check_online_status(client.0, address_ptr, error_out); + Ok(ContactOnlineStatus::from_byte(result).expect("A valid u8 from FFI status")) + } } - fn create_message(&self, receiver: &TariAddress, message: String) -> Message { - let receiver_address_ptr = Box::into_raw(Box::new(receiver.to_owned())) as *mut c_void; - let sender_address_ptr = Box::into_raw(Box::new(self.address.clone())) as *mut c_void; + fn create_message(&self, receiver: &TariAddress, message: String) -> Result { + let receiver_address_ptr = Box::into_raw(Box::new(receiver.to_owned())) as *mut FfiTariAddress; + let sender_address_ptr = Box::into_raw(Box::new(self.address.clone())) as *mut FfiTariAddress; let message_c_str = CString::new(message).unwrap(); let message_c_char: *const c_char = CString::into_raw(message_c_str) as *const c_char; @@ -195,20 +219,20 @@ impl ChatClient for ChatFFI { unsafe { let message_ptr = create_chat_message(receiver_address_ptr, sender_address_ptr, message_c_char, error_out) as *mut Message; - *Box::from_raw(message_ptr) + Ok(*Box::from_raw(message_ptr)) } } async fn get_messages(&self, address: &TariAddress, limit: u64, page: u64) -> Result, ClientError> { let client = self.ptr.lock().unwrap(); - let address_ptr = Box::into_raw(Box::new(address.clone())) as *mut c_void; + let address_ptr = Box::into_raw(Box::new(address.clone())) as *mut FfiTariAddress; let messages; unsafe { let error_out = Box::into_raw(Box::new(0)); - let limit = i32::try_from(limit).expect("Truncation occurred") as c_int; - let page = i32::try_from(page).expect("Truncation occurred") as c_int; + let limit = i32::try_from(limit).expect("Truncation occurred") as c_uint; + let page = i32::try_from(page).expect("Truncation occurred") as c_uint; let all_messages = get_chat_messages(client.0, address_ptr, limit, page, error_out) as *mut MessagesVector; messages = (*all_messages).0.clone(); } @@ -216,7 +240,7 @@ impl ChatClient for ChatFFI { Ok(messages) } - async fn get_message(&self, message_id: &[u8]) -> Result { + async fn get_message(&self, message_id: &MessageId) -> Result { let client = self.ptr.lock().unwrap(); let error_out = Box::into_raw(Box::new(0)); diff --git a/integration_tests/tests/features/ChatFFI.feature b/integration_tests/tests/features/ChatFFI.feature index 466d116338..593ab55265 100644 --- a/integration_tests/tests/features/ChatFFI.feature +++ b/integration_tests/tests/features/ChatFFI.feature @@ -38,8 +38,6 @@ Feature: Chat FFI messaging Then there will be a ReadConfirmationReceived callback of at least 1 Then CHAT_A and CHAT_B will have a message 'Hey there' with matching read timestamps - # Also flaky on CI. Seems liveness has issues on CI - @broken Scenario: Callback for status change is received Given I have a seed node SEED_A When I have a chat FFI client CHAT_A connected to seed node SEED_A @@ -48,8 +46,6 @@ Feature: Chat FFI messaging When CHAT_A waits for contact CHAT_B to be online Then there will be a contact status update callback of at least 1 - #This is flaky, passes on local run time, but fails CI - @broken Scenario: A message is sent directly between two FFI clients Given I have a seed node SEED_A When I have a chat FFI client CHAT_A connected to seed node SEED_A diff --git a/integration_tests/tests/steps/chat_steps.rs b/integration_tests/tests/steps/chat_steps.rs index 705b10f532..fdfb7c9ca0 100644 --- a/integration_tests/tests/steps/chat_steps.rs +++ b/integration_tests/tests/steps/chat_steps.rs @@ -26,9 +26,10 @@ use cucumber::{then, when}; use tari_contacts::contacts_service::{ handle::{DEFAULT_MESSAGE_LIMIT, DEFAULT_MESSAGE_PAGE}, service::ContactOnlineStatus, - types::{Direction, Message, MessageMetadata}, + types::{ChatBody, Direction, Message, MessageId, MessageMetadata}, }; use tari_integration_tests::{chat_client::spawn_chat_client, TariWorld}; +use tari_utilities::ByteArray; use crate::steps::{HALF_SECOND, TWO_MINUTES_WITH_HALF_SECOND_SLEEP}; @@ -70,7 +71,7 @@ async fn send_message_to( let sender = world.chat_clients.get(&sender).unwrap(); let receiver = world.chat_clients.get(&receiver).unwrap(); - let message = sender.create_message(&receiver.address(), message); + let message = sender.create_message(&receiver.address(), message).unwrap(); sender.send_message(message).await?; Ok(()) @@ -100,17 +101,19 @@ async fn i_reply_to_message( let inbound_chat_message = messages .iter() - .find(|m| m.body == inbound_msg.clone().into_bytes()) + .find(|m| m.body == ChatBody::try_from(inbound_msg.clone().into_bytes()).unwrap()) .expect("no message with that content found") .clone(); - let message = sender.create_message(&address, outbound_msg); + let message = sender.create_message(&address, outbound_msg).unwrap(); - let message = sender.add_metadata( - message, - "reply".to_string(), - String::from_utf8(inbound_chat_message.message_id).expect("bytes to uuid"), - ); + let message = sender + .add_metadata( + message, + "reply".to_string(), + String::from_utf8(inbound_chat_message.message_id.to_vec()).expect("bytes to uuid"), + ) + .unwrap(); sender.send_message(message).await?; return Ok(()); @@ -174,7 +177,7 @@ async fn wait_for_contact_to_be_online(world: &mut TariWorld, client: String, co return Ok(()); } - tokio::time::sleep(Duration::from_millis(HALF_SECOND)).await; + tokio::time::sleep(Duration::from_millis(HALF_SECOND * 5)).await; } panic!( @@ -208,7 +211,7 @@ async fn have_replied_message( let inbound_chat_message = messages .iter() - .find(|m| m.body == inbound_reply.clone().into_bytes()) + .find(|m| m.body == ChatBody::try_from(inbound_reply.clone().into_bytes()).unwrap()) .expect("no message with that content found") .clone(); @@ -221,11 +224,12 @@ async fn have_replied_message( let metadata: &MessageMetadata = &inbound_chat_message.metadata[0]; // Metadata data is a reply type - assert_eq!(metadata.key, "reply".as_bytes(), "Metadata type is wrong"); + assert_eq!(metadata.key.as_bytes(), "reply".as_bytes(), "Metadata type is wrong"); // Metadata data contains id to original message assert_eq!( - metadata.data, outbound_chat_message.message_id, + metadata.data.as_bytes(), + outbound_chat_message.message_id.to_vec(), "Message id does not match" ); @@ -259,7 +263,7 @@ async fn matching_delivery_timestamps( let client_1_message = client_1_messages .iter() - .find(|m| m.body == msg.clone().into_bytes()) + .find(|m| m.body == ChatBody::try_from(msg.clone().into_bytes()).unwrap()) .expect("no message with that content found") .clone(); @@ -274,7 +278,7 @@ async fn matching_delivery_timestamps( let client_2_message = client_2_messages .iter() - .find(|m| m.body == msg.clone().into_bytes()) + .find(|m| m.body == ChatBody::try_from(msg.clone().into_bytes()).unwrap()) .expect("no message with that content found") .clone(); @@ -318,7 +322,7 @@ async fn matching_read_timestamps( let client_1_message = client_1_messages .iter() - .find(|m| m.body == msg.clone().into_bytes()) + .find(|m| m.body == ChatBody::try_from(msg.clone().into_bytes()).unwrap()) .expect("no message with that content found") .clone(); @@ -333,7 +337,7 @@ async fn matching_read_timestamps( let client_2_message = client_2_messages .iter() - .find(|m| m.body == msg.clone().into_bytes()) + .find(|m| m.body == ChatBody::try_from(msg.clone().into_bytes()).unwrap()) .expect("no message with that content found") .clone(); @@ -374,7 +378,7 @@ async fn send_read_receipt(world: &mut TariWorld, sender: String, receiver: Stri let message = messages .iter() - .find(|m| m.body == msg.clone().into_bytes()) + .find(|m| m.body == ChatBody::try_from(msg.clone().into_bytes()).unwrap()) .expect("no message with that content found") .clone(); @@ -419,8 +423,8 @@ async fn send_message_with_id_to( let sender = world.chat_clients.get(&sender).unwrap(); let receiver = world.chat_clients.get(&receiver).unwrap(); - let mut message = sender.create_message(&receiver.address(), message); - message.message_id = id.into_bytes(); + let mut message = sender.create_message(&receiver.address(), message).unwrap(); + message.message_id = MessageId::try_from(id).unwrap(); sender.send_message(message).await?; Ok(()) @@ -431,7 +435,7 @@ async fn find_message_with_id(world: &mut TariWorld, sender: String, message_id: let sender = world.chat_clients.get(&sender).unwrap(); sender - .get_message(message_id.as_bytes()) + .get_message(&MessageId::try_from(message_id.clone()).unwrap()) .await .unwrap_or_else(|_| panic!("Message not found with id {:?}", message_id));