Skip to content

Commit

Permalink
feat: refactor configuration for chat so ffi can create and accept a …
Browse files Browse the repository at this point in the history 
…config file (#5426)

Description
---
This refactoring come on the heels of making testing easier via the chat
configuration. As a result we also solve the problem about peer seeds in
FFI. Previously the ffi required passing in a collection of peer seeds.
This wasn't in any way ideal. Now we take a standard tari configuration
setup, which will utilize the DNS seeds in the default configuration.

Motivation and Context
---
ChatFFI had a bad way to connect to peers. It needed fixing. Also
cleanup of the integration tests was desired and making configuration
simpler is helpful.

How Has This Been Tested?
---
Locally and CI

No new test was added for the configuration creation yet. As that config
defaults to real config, not local config.

What process can a PR reviewer use to test or verify this change?
---

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
  • Loading branch information
@brianp
brianp authored May 30, 2023
1 parent c29ab15 commit 9d0d8b5
Show file tree
Hide file tree
Showing 14 changed files with 632 additions and 344 deletions.
220 changes: 145 additions & 75 deletions Cargo.lock
View file

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ members = [
"base_layer/tari_mining_helper_ffi",
"clients/rust/base_node_grpc_client",
"clients/rust/wallet_grpc_client",
# "clients/rust/web_chat",
"comms/core",
"comms/dht",
"comms/rpc_macros",
Expand Down
3 changes: 1 addition & 2 deletions base_layer/chat_ffi/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,15 @@ version = "0.50.0-pre.2"
edition = "2018"

[dependencies]
tari_app_utilities = { path = "../../applications/tari_app_utilities" }
tari_chat_client = { path = "../contacts/examples/chat_client" }
tari_common = { path = "../../common" }
tari_common_types = { path = "../common_types" }
tari_comms = { path = "../../comms/core" }
tari_contacts = { path = "../contacts" }
tari_p2p = { path = "../p2p" }

libc = "0.2.65"
log = "0.4.6"
serde_json = "1.0.64"
thiserror = "1.0.26"
tokio = "1.23"

Expand Down
53 changes: 35 additions & 18 deletions base_layer/chat_ffi/chat.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,6 @@ struct ChatMessages;

struct ClientFFI;

struct ClientPeers;

/**
* Configuration for a comms node
*/
struct P2pConfig;

struct TariAddress;

#ifdef __cplusplus
Expand All @@ -27,16 +20,10 @@ extern "C" {

/**
* Creates a Chat Client
* TODO: This function takes a ptr to a collection of seed peers and this works fine in cucumber, or native rust but
* isn't at all ideal for a real FFI. We need to work with the mobile teams and come up with a better interface
* for supplying seed peers.
*
* ## Arguments
* `config` - The P2PConfig pointer
* `config` - The ApplicationConfig pointer
* `identity_file_path` - The path to the node identity file
* `db_path` - The path to the db file
* `seed_peers` - A ptr to a collection of seed peers
* `network_str` - The network to connect to
* `error_out` - Pointer to an int which will be modified
*
* ## Returns
Expand All @@ -46,11 +33,8 @@ extern "C" {
* # Safety
* The ```destroy_client``` method must be called when finished with a ClientFFI to prevent a memory leak
*/
struct ClientFFI *create_chat_client(struct P2pConfig *config,
struct ClientFFI *create_chat_client(ApplicationConfig *config,
const char *identity_file_path,
const char *db_path,
struct ClientPeers *seed_peers,
const char *network_str,
int *error_out);

/**
Expand All @@ -67,6 +51,39 @@ struct ClientFFI *create_chat_client(struct P2pConfig *config,
*/
void destroy_client_ffi(struct ClientFFI *client);

/**
* Creates a Chat Client config
*
* ## Arguments
* `network` - The network to run on
* `public_address` - The nodes public address
* `error_out` - Pointer to an int which will be modified
*
* ## Returns
* `*mut ApplicationConfig` - Returns a pointer to an ApplicationConfig
*
* # Safety
* The ```destroy_config``` method must be called when finished with a Config to prevent a memory leak
*/
ApplicationConfig *create_chat_config(const char *network_str,
const char *public_address,
const char *datastore_path,
int *error_out);

/**
* Frees memory for an ApplicationConfig
*
* ## Arguments
* `config` - The pointer of an ApplicationConfig
*
* ## Returns
* `()` - Does not return a value, equivalent to void in C
*
* # Safety
* None
*/
void destroy_config(ApplicationConfig *config);

/**
* Sends a message over a client
*
Expand Down
2 changes: 1 addition & 1 deletion base_layer/chat_ffi/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub enum InterfaceError {
NullError(String),
#[error("An error has occurred when trying to create the tokio runtime: `{0}`")]
TokioError(String),
#[error("Emoji ID is invalid")]
#[error("Something about the argument is invalid: `{0}`")]
InvalidArgument(String),
}

Expand Down
186 changes: 126 additions & 60 deletions base_layer/chat_ffi/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,19 @@

#![recursion_limit = "1024"]

use std::{ffi::CStr, fs::File, io::Read, path::PathBuf, ptr, str::FromStr};
use std::{ffi::CStr, path::PathBuf, ptr, str::FromStr, sync::Arc};

use libc::{c_char, c_int};
use tari_chat_client::{ChatClient, Client};
use tari_common::configuration::Network;
use tari_app_utilities::identity_management::load_from_json;
use tari_chat_client::{
config::{ApplicationConfig, ChatClientConfig},
ChatClient,
Client,
};
use tari_common::configuration::{MultiaddrList, Network};
use tari_common_types::tari_address::TariAddress;
use tari_comms::peer_manager::Peer;
use tari_comms::{multiaddr::Multiaddr, NodeIdentity};
use tari_contacts::contacts_service::types::Message;
use tari_p2p::P2pConfig;
use tokio::runtime::Runtime;

use crate::error::{InterfaceError, LibChatError};
Expand All @@ -40,25 +44,16 @@ mod error;
#[derive(Clone)]
pub struct ChatMessages(Vec<Message>);

#[derive(Clone)]
pub struct ClientPeers(Vec<Peer>);

pub struct ClientFFI {
client: Client,
runtime: Runtime,
}

/// Creates a Chat Client
/// TODO: This function takes a ptr to a collection of seed peers and this works fine in cucumber, or native rust but
/// isn't at all ideal for a real FFI. We need to work with the mobile teams and come up with a better interface
/// for supplying seed peers.
///
/// ## Arguments
/// `config` - The P2PConfig pointer
/// `config` - The ApplicationConfig pointer
/// `identity_file_path` - The path to the node identity file
/// `db_path` - The path to the db file
/// `seed_peers` - A ptr to a collection of seed peers
/// `network_str` - The network to connect to
/// `error_out` - Pointer to an int which will be modified
///
/// ## Returns
Expand All @@ -69,11 +64,8 @@ pub struct ClientFFI {
/// The ```destroy_client``` method must be called when finished with a ClientFFI to prevent a memory leak
#[no_mangle]
pub unsafe extern "C" fn create_chat_client(
config: *mut P2pConfig,
config: *mut ApplicationConfig,
identity_file_path: *const c_char,
db_path: *const c_char,
seed_peers: *mut ClientPeers,
network_str: *const c_char,
error_out: *mut c_int,
) -> *mut ClientFFI {
let mut error = 0;
Expand All @@ -89,28 +81,15 @@ pub unsafe extern "C" fn create_chat_client(
error = LibChatError::from(InterfaceError::InvalidArgument(e)).code;
ptr::swap(error_out, &mut error as *mut c_int);
};
let identity = match CStr::from_ptr(identity_file_path).to_str() {

let identity: Arc<NodeIdentity> = match CStr::from_ptr(identity_file_path).to_str() {
Ok(str) => {
let identity_path = PathBuf::from(str);
let mut buf = Vec::new();

match File::open(identity_path) {
Ok(mut f) => {
if let Err(e) = f.read_to_end(&mut buf) {
bad_identity(e.to_string());
return ptr::null_mut();
}
},
Err(e) => {
bad_identity(e.to_string());
return ptr::null_mut();
},
}

match serde_json::from_slice(&buf) {
Ok(identity) => identity,
Err(e) => {
bad_identity(e.to_string());
match load_from_json(identity_path) {
Ok(Some(identity)) => Arc::new(identity),
_ => {
bad_identity("No identity loaded".to_string());
return ptr::null_mut();
},
}
Expand All @@ -121,10 +100,67 @@ pub unsafe extern "C" fn create_chat_client(
},
};

let runtime = match Runtime::new() {
Ok(r) => r,
Err(e) => {
error = LibChatError::from(InterfaceError::TokioError(e.to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
},
};

let mut client = Client::new(identity, (*config).clone());
runtime.block_on(client.initialize());

let client_ffi = ClientFFI { client, runtime };

Box::into_raw(Box::new(client_ffi))
}

/// Frees memory for a ClientFFI
///
/// ## Arguments
/// `client` - The pointer of a ClientFFI
///
/// ## Returns
/// `()` - Does not return a value, equivalent to void in C
///
/// # Safety
/// None
#[no_mangle]
pub unsafe extern "C" fn destroy_client_ffi(client: *mut ClientFFI) {
if !client.is_null() {
drop(Box::from_raw(client))
}
}

/// Creates a Chat Client config
///
/// ## Arguments
/// `network` - The network to run on
/// `public_address` - The nodes public address
/// `error_out` - Pointer to an int which will be modified
///
/// ## Returns
/// `*mut ApplicationConfig` - Returns a pointer to an ApplicationConfig
///
/// # Safety
/// The ```destroy_config``` method must be called when finished with a Config to prevent a memory leak
#[no_mangle]
pub unsafe extern "C" fn create_chat_config(
network_str: *const c_char,
public_address: *const c_char,
datastore_path: *const c_char,
error_out: *mut c_int,
) -> *mut ApplicationConfig {
let mut error = 0;
ptr::swap(error_out, &mut error as *mut c_int);

let mut bad_network = |e| {
error = LibChatError::from(InterfaceError::InvalidArgument(e)).code;
ptr::swap(error_out, &mut error as *mut c_int);
};

let network = if network_str.is_null() {
bad_network("network is missing".to_string());
return ptr::null_mut();
Expand All @@ -144,49 +180,79 @@ pub unsafe extern "C" fn create_chat_client(
}
};

let db_path = match CStr::from_ptr(db_path).to_str() {
Ok(str) => PathBuf::from(str),
let datastore_path_string;
if datastore_path.is_null() {
error = LibChatError::from(InterfaceError::NullError("datastore_path".to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
} else {
match CStr::from_ptr(datastore_path).to_str() {
Ok(v) => {
datastore_path_string = v.to_owned();
},
_ => {
error = LibChatError::from(InterfaceError::InvalidArgument("datastore_path".to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
},
}
}
let datastore_path = PathBuf::from(datastore_path_string);

let public_address_string;
if public_address.is_null() {
error = LibChatError::from(InterfaceError::NullError("public_address".to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
} else {
match CStr::from_ptr(public_address).to_str() {
Ok(v) => {
public_address_string = v.to_owned();
},
_ => {
error = LibChatError::from(InterfaceError::InvalidArgument("public_address".to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
},
}
}
let address = match Multiaddr::from_str(&public_address_string) {
Ok(a) => a,
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();
},
};

let seed_peers = (*seed_peers).clone().0;
let mut chat_client_config = ChatClientConfig::default();
chat_client_config.network = network;
chat_client_config.p2p.transport.tcp.listener_address = address.clone();
chat_client_config.p2p.public_addresses = MultiaddrList::from(vec![address]);
chat_client_config.set_base_path(datastore_path);

let runtime = match Runtime::new() {
Ok(r) => r,
Err(e) => {
error = LibChatError::from(InterfaceError::TokioError(e.to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
},
let config = ApplicationConfig {
chat_client: chat_client_config,
..ApplicationConfig::default()
};

let mut client = Client::new(identity, (*config).clone(), seed_peers, db_path, network);

runtime.block_on(client.initialize());

let client_ffi = ClientFFI { client, runtime };

Box::into_raw(Box::new(client_ffi))
Box::into_raw(Box::new(config))
}

/// Frees memory for a ClientFFI
/// Frees memory for an ApplicationConfig
///
/// ## Arguments
/// `client` - The pointer of a ClientFFI
/// `config` - The pointer of an ApplicationConfig
///
/// ## Returns
/// `()` - Does not return a value, equivalent to void in C
///
/// # Safety
/// None
#[no_mangle]
pub unsafe extern "C" fn destroy_client_ffi(client: *mut ClientFFI) {
if !client.is_null() {
drop(Box::from_raw(client))
pub unsafe extern "C" fn destroy_config(config: *mut ApplicationConfig) {
if !config.is_null() {
drop(Box::from_raw(config))
}
}

Expand Down
Loading

0 comments on commit 9d0d8b5

Please sign in to comment.