Skip to content

Commit

Permalink
test: fix cucumber wallet fii status access violation (#6695)
Browse files Browse the repository at this point in the history
Description
---
Fix cucumber ffi import
- Added a unit test to compare the ffi lib with the ffi import function
signatures.
- Fixed all ffi import function signatures that did not match.

**Note:** Added better logging for cucumber merge mining process, but
`Scenario: Simple Merge Mining` is still broken,

Motivation and Context
---
There were memory access errors.

How Has This Been Tested?
---
`Scenario: As a client I want to be able to restore my ffi wallet from
seed words` now pass

What process can a PR reviewer use to test or verify this change?
---
Run cucumber
Code review

<!-- 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
hansieodendaal authored Nov 21, 2024
1 parent d62b40f commit b0d71de
Show file tree
Hide file tree
Showing 17 changed files with 350 additions and 77 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion applications/minotari_merge_mining_proxy/src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ mod test {
"id": "0",
"method": rpc_method,
"params": {
"wallet_address": "489r43gR8bDMJNBf4Q6sL9CNERvZQrTqjRCSESqgWQEWWq2UGAfj2voaw3zBtD7U8CQ391Nc1PDHUHiN85yhbZnCDasqzyX",
"wallet_address": "44AFFq5kSiGBoZ4NMDwYtN18obc8AemS33DBLWs3H7otXft3XjrpDtQGv7SqSsaBYBb98uNbr2VBBEt7f2wfn3RVGQBEP3A",
}
}),
Method::GetVersion => json!({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@ const LOG_TARGET: &str = "minotari_mm_proxy::proxy";

#[allow(clippy::too_many_lines)]
pub async fn start_merge_miner(cli: Cli) -> Result<(), anyhow::Error> {
trace!(target: LOG_TARGET, "{:?}", cli);
let config_path = cli.common.config_path();
let cfg = load_configuration(&config_path, true, cli.non_interactive_mode, &cli, cli.common.network)?;
trace!(target: LOG_TARGET, "{:?}", cfg);
let mut config = MergeMiningProxyConfig::load_from(&cfg)?;
config.set_base_path(cli.common.get_base_path());
trace!(target: LOG_TARGET, "{:?}", config);

// Get reputable monerod URLs
let mut assigned_dynamic_fail = false;
Expand Down
1 change: 1 addition & 0 deletions integration_tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ thiserror = "^1.0.20"
time = "0.3.36"
tokio = { version = "1.36", features = ["macros", "time", "sync", "rt-multi-thread"] }
tonic = "0.12.3"
regex = "1.11.0"

[package.metadata.cargo-machete]
ignored = ["minotari_wallet_ffi", "minotari_chat_ffi"]
Expand Down
7 changes: 4 additions & 3 deletions integration_tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ In its simplest form you can run the tests from the project route with `cargo te

## Concurrency

- The set of wallet FFI tests (`"@wallet-ffi and not @broken"`) should not be run with `--concurrency` greater than 1,
and because it defaults to 64, it should be limited to `--concurrency 1` on the command line. This is also true for
running it in CI.
- The set of wallet FFI tests (`"@wallet-ffi and not @broken"`) should not be run with `--concurrency` greater than 1.
We coded `.max_concurrent_scenarios(1)`, as concurrency defaults to 64, but it can also be limited to
`--concurrency 1` on the command line. This is also true for running it in CI.
**Note:** Specifying `--concurrency X` on the command line overrides `max_concurrent_scenarios(1)` set in the code.

```shell
cargo test --release --test cucumber -- --tags "@wallet-ffi and not @broken" --concurrency 1 --retry 2
Expand Down
22 changes: 21 additions & 1 deletion integration_tests/log4rs/cucumber.yml
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,22 @@ appenders:
pattern: "{{log_dir}}/log/libp2p.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} {f}.{L} {i} [{t}] {l:5} {m}{n}"
# An appender named "cucumber_detail" that writes to a file with a custom pattern encoder
cucumber_detail:
kind: rolling_file
path: "{{log_dir}}/log/cucumber_detail.log"
policy:
kind: compound
trigger:
kind: size
limit: 100mb
roller:
kind: fixed_window
base: 1
count: 5
pattern: "{{log_dir}}/log/cucumber_detail.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} {f}.{L} {i} [{t}] {l:5} {m}{n}"

# We don't want prints during cucumber test, everything useful will in logs.
# root:
Expand All @@ -167,6 +183,10 @@ loggers:
level: info # we have only single print, and it's info
appenders:
- stdout
cucumber_detail:
level: debug
appenders:
- cucumber_detail
c:
level: debug #trace #debug
appenders:
Expand Down Expand Up @@ -210,7 +230,7 @@ loggers:

# merge mining proxy
minotari_mm_proxy::proxy:
level: debug
level: debug #trace #debug
appenders:
- proxy
additive: false
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/src/base_node_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use tonic::transport::Channel;

use crate::{get_peer_seeds, get_port, wait_for_service, ServiceType, TariWorld};

const LOG_TARGET: &str = "cucumber::bas_node_process";
const LOG_TARGET: &str = "cucumber_detail::base_node_process";

#[derive(Clone)]
pub struct BaseNodeProcess {
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/src/ffi/contacts_liveness_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl ContactsLivenessData {
WalletAddress::from_ptr(ptr)
}

pub fn get_latency(&self) -> i32 {
pub fn get_latency(&self) -> u32 {
let latency;
let mut error = 0;
unsafe {
Expand Down
41 changes: 16 additions & 25 deletions integration_tests/src/ffi/ffi_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,47 +29,36 @@ pub type TariPendingInboundTransaction = c_void;
pub type TariCompletedTransaction = c_void;
pub type TariTransactionSendStatus = c_void;
pub type TariContactsLivenessData = c_void;
pub type TariRangeProof = c_void;
pub type TariBalance = c_void;
pub type TariWallet = c_void;
pub type TariWalletAddress = c_void;
pub type ByteVector = c_void;
#[allow(dead_code)]
pub type TariFeePerGramStat = c_void;
#[allow(dead_code)]
pub type TariTypeTag = c_void;
pub type TariVector = c_void;
pub type TariCoinPreview = c_void;
pub type TariTransactionKernel = c_void;
pub type TariPublicKey = c_void;
#[allow(dead_code)]
pub type TariPublicKeys = c_void;
#[allow(dead_code)]
pub type TariPrivateKey = c_void;
#[allow(dead_code)]
pub type TariComAndPubSignature = c_void;
#[allow(dead_code)]
pub type TariOutputFeatures = c_void;
#[allow(dead_code)]
pub type TariCovenant = c_void;
#[allow(dead_code)]
pub type TariEncryptedOpenings = c_void;
#[allow(dead_code)]
pub type TariUnblindedOutput = c_void;
#[allow(dead_code)]
pub type TariUnblindedOutputs = c_void;
pub type TariContact = c_void;
pub type TariContacts = c_void;
pub type TariCompletedTransactions = c_void;
pub type TariPendingOutboundTransactions = c_void;
pub type TariPendingOutboundTransaction = c_void;
pub type TariPendingInboundTransactions = c_void;
#[allow(dead_code)]
pub type TariUtxoSort = c_void;
#[allow(dead_code)]
pub type EmojiSet = c_void;
#[allow(dead_code)]
pub type TariFeePerGramStats = c_void;
pub type TariBaseNodeState = c_void;
pub type ContactsServiceHandle = c_void;

#[cfg_attr(windows, link(name = "minotari_wallet_ffi.dll"))]
#[cfg_attr(not(windows), link(name = "minotari_wallet_ffi"))]
Expand Down Expand Up @@ -107,11 +96,6 @@ extern "C" {
pub fn tari_address_create(bytes: *mut ByteVector, error_out: *mut c_int) -> *mut TariWalletAddress;
pub fn tari_address_destroy(address: *mut TariWalletAddress);
pub fn tari_address_get_bytes(address: *mut TariWalletAddress, error_out: *mut c_int) -> *mut ByteVector;
pub fn tari_address_from_private_key(
secret_key: *mut TariPrivateKey,
network: c_uint,
error_out: *mut c_int,
) -> *mut TariWalletAddress;
pub fn tari_address_from_base58(address: *const c_char, error_out: *mut c_int) -> *mut TariWalletAddress;
pub fn tari_address_to_emoji_id(address: *mut TariWalletAddress, error_out: *mut c_int) -> *mut c_char;
pub fn emoji_id_to_tari_address(emoji: *const c_char, error_out: *mut c_int) -> *mut TariWalletAddress;
Expand All @@ -137,6 +121,7 @@ extern "C" {
encrypted_data: *mut TariEncryptedOpenings,
minimum_value_promise: c_ulonglong,
script_lock_height: c_ulonglong,
range_proof: *mut TariRangeProof,
error_out: *mut c_int,
) -> *mut TariUnblindedOutput;
pub fn tari_unblinded_output_destroy(output: *mut TariUnblindedOutput);
Expand Down Expand Up @@ -176,6 +161,7 @@ extern "C" {
output_type: c_ushort,
maturity: c_ulonglong,
metadata: *const ByteVector,
range_proof_type: c_ushort,
error_out: *mut c_int,
) -> *mut TariOutputFeatures;
pub fn output_features_destroy(output_features: *mut TariOutputFeatures);
Expand All @@ -186,7 +172,12 @@ extern "C" {
) -> *mut TariSeedWords;
pub fn seed_words_get_length(seed_words: *const TariSeedWords, error_out: *mut c_int) -> c_uint;
pub fn seed_words_get_at(seed_words: *mut TariSeedWords, position: c_uint, error_out: *mut c_int) -> *mut c_char;
pub fn seed_words_push_word(seed_words: *mut TariSeedWords, word: *const c_char, error_out: *mut c_int) -> c_uchar;
pub fn seed_words_push_word(
seed_words: *mut TariSeedWords,
word: *const c_char,
passphrase: *const c_char,
error_out: *mut c_int,
) -> c_uchar;
pub fn seed_words_destroy(seed_words: *mut TariSeedWords);
pub fn contact_create(
alias: *const c_char,
Expand All @@ -204,7 +195,7 @@ extern "C" {
liveness_data: *mut TariContactsLivenessData,
error_out: *mut c_int,
) -> *mut TariWalletAddress;
pub fn liveness_data_get_latency(liveness_data: *mut TariContactsLivenessData, error_out: *mut c_int) -> c_int;
pub fn liveness_data_get_latency(liveness_data: *mut TariContactsLivenessData, error_out: *mut c_int) -> c_uint;
pub fn liveness_data_get_last_seen(
liveness_data: *mut TariContactsLivenessData,
error_out: *mut c_int,
Expand Down Expand Up @@ -381,7 +372,7 @@ extern "C" {
database_name: *const c_char,
datastore_path: *const c_char,
log_path: *const c_char,
log_level: c_int,
log_verbosity: c_int,
num_rolling_log_files: c_uint,
size_per_log_file_bytes: c_uint,
passphrase: *const c_char,
Expand Down Expand Up @@ -430,7 +421,7 @@ extern "C" {
callback_saf_messages_received: unsafe extern "C" fn(context: *mut c_void),
callback_connectivity_status: unsafe extern "C" fn(context: *mut c_void, u64),
callback_wallet_scanned_height: unsafe extern "C" fn(context: *mut c_void, u64),
callback_base_node_state_updated: unsafe extern "C" fn(context: *mut c_void, *mut TariBaseNodeState),
callback_base_node_state: unsafe extern "C" fn(context: *mut c_void, *mut TariBaseNodeState),
recovery_in_progress: *mut bool,
error_out: *mut c_int,
) -> *mut TariWallet;
Expand Down Expand Up @@ -508,8 +499,8 @@ extern "C" {
amount: c_ulonglong,
commitments: *mut TariVector,
fee_per_gram: c_ulonglong,
num_kernels: c_ulonglong,
num_outputs: c_ulonglong,
num_kernels: c_uint,
num_outputs: c_uint,
error_out: *mut c_int,
) -> c_ulonglong;
pub fn wallet_get_num_confirmations_required(wallet: *mut TariWallet, error_out: *mut c_int) -> c_ulonglong;
Expand Down Expand Up @@ -624,5 +615,5 @@ extern "C" {
error_out: *mut c_int,
) -> c_ulonglong;
pub fn fee_per_gram_stat_destroy(fee_per_gram_stat: *mut TariFeePerGramStat);
pub fn contacts_handle(wallet: *mut TariWallet, error_out: *mut c_int) -> *mut c_void;
pub fn contacts_handle(wallet: *mut TariWallet, error_out: *mut c_int) -> *mut ContactsServiceHandle;
}
Loading

0 comments on commit b0d71de

Please sign in to comment.