Skip to content

Commit

Permalink
fix: update cucumber tests for walletffi.feature
Browse files Browse the repository at this point in the history
Made types more explicit for ffi interface and easier to maintain going forwards.
Initialized InterfaceFFI in world rather than WalletFFIClient.
Updated steps in WalletFFI.feature
Updated some const variables in wrapper classes to be let.
Better handling of callback function pointer variables in wallet.
Added additional steps to tests.

Update WalletFFI.feature

Update wallet.js

Fixed leak in wallet

Fixed leak for transport in walletFFI client.

Renamed pointer variable for each struct wrapper to ptr.

Update walletFFIClient.js

Review comments
  • Loading branch information
StriderDM committed Sep 1, 2021
1 parent 95ac87d commit 7b549a9
Show file tree
Hide file tree
Showing 22 changed files with 709 additions and 588 deletions.
3 changes: 2 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,9 @@ commands:
command: cd integration_tests && node ./generate_report.js
when: always
- run:
name: Run ffi cucumber scenarios
name: Run FFI wallet library cucumber scenarios
command: cd integration_tests && mkdir -p cucumber_output && node_modules/.bin/cucumber-js --tags "not @long-running and not @broken and not @flaky and @wallet-ffi" --format json:cucumber_output/tests_ffi.cucumber --exit
when: always
- run:
name: Generate report (ffi)
command: cd integration_tests && node ./generate_report.js "cucumber_output/tests_ffi.cucumber" "temp/reports/cucumber_ffi_report.html"
Expand Down
34 changes: 23 additions & 11 deletions integration_tests/features/WalletFFI.feature
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
@wallet-ffi
Feature: Wallet FFI
# Increase heap memory available to nodejs if frequent crashing occurs with
# error being be similar to this: `0x1a32cd5 V8_Fatal(char const*, ...)`
# Increase heap memory available to nodejs if frequent crashing occurs.

# Note, there is a known bug in napi-ffi that intermittently causes crashes which has the following output:
#
# Fatal error in , line 0
# Check failed: result.second.
#

# It's just calling the encrypt function, we don't test if it's actually encrypted
Scenario: As a client I want to be able to protect my wallet with a passphrase
Expand Down Expand Up @@ -35,11 +40,12 @@ Feature: Wallet FFI
And I stop ffi wallet FFI_WALLET
And I stop node BASE1
And I wait 5 seconds
And I restart ffi wallet FFI_WALLET
# Possibly check SAF messages, no way to get current connected base node peer from the library itself afaik
# Good idea just to add a fn to do this to the library.
# Then I wait for ffi wallet FFI_WALLET to receive 1 SAF message
And I wait 5 seconds
# Broken step with reason base node is not persisted
# See details on:
# Scenario: As a client I want to receive Tari via my Public Key sent while I am offline when I come back online
# And I restart ffi wallet FFI_WALLET
And I restart ffi wallet FFI_WALLET connected to base node BASE2
Then I wait for ffi wallet FFI_WALLET to receive at least 1 SAF message
And I stop ffi wallet FFI_WALLET

Scenario: As a client I want to cancel a transaction
Expand Down Expand Up @@ -101,12 +107,18 @@ Feature: Wallet FFI
And I wait 10 seconds
And I send 2000000 uT from wallet SENDER to wallet FFI_WALLET at fee 100
And I wait 5 seconds
And I restart ffi wallet FFI_WALLET
# Broken step with reason base node is not persisted
# Log:
# [wallet::transaction_service::callback_handler] DEBUG Calling Received Finalized Transaction callback function for TxId: 7595706993517535281
# [wallet::transaction_service::service] WARN Error broadcasting completed transaction TxId: 7595706993517535281 to mempool: NoBaseNodeKeysProvided
# And I restart ffi wallet FFI_WALLET
And I restart ffi wallet FFI_WALLET connected to base node BASE
Then I wait for ffi wallet FFI_WALLET to receive 1 transaction
Then I wait for ffi wallet FFI_WALLET to receive 1 finalization
# Assume tx will be mined to reduce time taken for test, balance is tested in later scenarios.
# And mining node MINER mines 10 blocks
# Then I wait for ffi wallet FFI_WALLET to have at least 1000000 uT
Then I wait for ffi wallet FFI_WALLET to receive 1 broadcast
And mining node MINER mines 10 blocks
Then I wait for ffi wallet FFI_WALLET to receive 1 mined
Then I wait for ffi wallet FFI_WALLET to have at least 1000000 uT
And I stop ffi wallet FFI_WALLET

# Scenario: As a client I want to get my balance
Expand Down
123 changes: 98 additions & 25 deletions integration_tests/features/support/steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -3827,9 +3827,7 @@ Then(
);

await waitFor(
async () => {
return wallet.getCounters().received >= amount;
},
async () => wallet.getCounters().received >= amount,
true,
700 * 1000,
5 * 1000,
Expand Down Expand Up @@ -3861,9 +3859,7 @@ Then(
);

await waitFor(
async () => {
return wallet.getCounters().finalized >= amount;
},
async () => wallet.getCounters().finalized >= amount,
true,
700 * 1000,
5 * 1000,
Expand All @@ -3880,7 +3876,7 @@ Then(
);

Then(
/I wait for ffi wallet (.*) to receive (.*) SAF message/,
/I wait for ffi wallet (.*) to receive (.*) broadcast/,
{ timeout: 710 * 1000 },
async function (wallet_name, amount) {
let wallet = this.getWallet(wallet_name);
Expand All @@ -3891,13 +3887,75 @@ Then(
wallet_name +
" to receive " +
amount +
" transaction broadcast(s)"
);

await waitFor(
async () => wallet.getCounters().broadcast >= amount,
true,
700 * 1000,
5 * 1000,
5
);

if (!(wallet.getCounters().broadcast >= amount)) {
console.log("Counter not adequate!");
} else {
console.log(wallet.getCounters());
}
expect(wallet.getCounters().broadcast >= amount).to.equal(true);
}
);

Then(
/I wait for ffi wallet (.*) to receive (.*) mined/,
{ timeout: 710 * 1000 },
async function (wallet_name, amount) {
let wallet = this.getWallet(wallet_name);

console.log("\n");
console.log(
"Waiting for " +
wallet_name +
" to receive " +
amount +
" transaction mined"
);

await waitFor(
async () => wallet.getCounters().mined >= amount,
true,
700 * 1000,
5 * 1000,
5
);

if (!(wallet.getCounters().mined >= amount)) {
console.log("Counter not adequate!");
} else {
console.log(wallet.getCounters());
}
expect(wallet.getCounters().mined >= amount).to.equal(true);
}
);

Then(
/I wait for ffi wallet (.*) to receive at least (.*) SAF message/,
{ timeout: 710 * 1000 },
async function (wallet_name, amount) {
let wallet = this.getWallet(wallet_name);

console.log("\n");
console.log(
"Waiting for " +
wallet_name +
" to receive at least " +
amount +
" SAF messages(s)"
);

await waitFor(
async () => {
return wallet.getCounters().saf >= amount;
},
async () => wallet.getCounters().saf >= amount,
true,
700 * 1000,
5 * 1000,
Expand All @@ -3924,23 +3982,20 @@ Then(
"Waiting for " + wallet_name + " balance to be at least " + amount + " uT"
);

let count = 0;

while (!(wallet.getBalance().available >= amount)) {
await sleep(1000);
count++;
if (count > 700) {
break;
}
}
await waitFor(
async () => wallet.getBalance().available >= amount,
true,
700 * 1000,
5 * 1000,
5
);

let balance = wallet.getBalance().available;

if (!(balance >= amount)) {
console.log("Balance not adequate!");
} else {
console.log(wallet.getBalance());
}

expect(balance >= amount).to.equal(true);
}
);
Expand All @@ -3961,10 +4016,28 @@ When(/I start ffi wallet (.*)/, async function (walletName) {
await wallet.startNew(null, null);
});

When(/I restart ffi wallet (.*)/, async function (walletName) {
let wallet = this.getWallet(walletName);
await wallet.restart();
});
When(
/I restart ffi wallet (.*) connected to base node (.*)/,
async function (walletName, node) {
let wallet = this.getWallet(walletName);
await wallet.restart();
let peer = this.nodes[node].peerAddress().split("::");
wallet.addBaseNodePeer(peer[0], peer[1]);
}
);

Then(
"I want to get public key of ffi wallet {word}",
{ timeout: 20 * 1000 },
function (name) {
let wallet = this.getWallet(name);
let public_key = wallet.identify();
expect(public_key.length).to.be.equal(
64,
`Public key has wrong length : ${public_key}`
);
}
);

When(/I stop ffi wallet (.*)/, function (walletName) {
let wallet = this.getWallet(walletName);
Expand Down
5 changes: 4 additions & 1 deletion integration_tests/features/support/world.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const TransactionBuilder = require("../../helpers/transactionBuilder");
const glob = require("glob");
const fs = require("fs");
const archiver = require("archiver");
const InterfaceFFI = require("../../helpers/ffi/ffiInterface");

class CustomWorld {
constructor({ attach, parameters }) {
// this.variable = 0;
Expand Down Expand Up @@ -381,7 +383,8 @@ BeforeAll({ timeout: 1200000 }, async function () {
await miningNode.compile();

console.log("Compiling wallet FFI...");
await WalletFFIClient.Init();
await InterfaceFFI.compile();
await InterfaceFFI.init();
console.log("Finished compilation.");
});

Expand Down
20 changes: 10 additions & 10 deletions integration_tests/helpers/ffi/byteVector.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
const InterfaceFFI = require("./ffiInterface");

class ByteVector {
#byte_vector_ptr;
#ptr;

pointerAssign(ptr) {
// Prevent pointer from being leaked in case of re-assignment
if (this.#byte_vector_ptr) {
if (this.#ptr) {
this.destroy();
this.#byte_vector_ptr = ptr;
this.#ptr = ptr;
} else {
this.#byte_vector_ptr = ptr;
this.#ptr = ptr;
}
}

Expand All @@ -30,21 +30,21 @@ class ByteVector {
}

getLength() {
return InterfaceFFI.byteVectorGetLength(this.#byte_vector_ptr);
return InterfaceFFI.byteVectorGetLength(this.#ptr);
}

getAt(position) {
return InterfaceFFI.byteVectorGetAt(this.#byte_vector_ptr, position);
return InterfaceFFI.byteVectorGetAt(this.#ptr, position);
}

getPtr() {
return this.#byte_vector_ptr;
return this.#ptr;
}

destroy() {
if (this.#byte_vector_ptr) {
InterfaceFFI.byteVectorDestroy(this.#byte_vector_ptr);
this.#byte_vector_ptr = undefined; //prevent double free segfault
if (this.#ptr) {
InterfaceFFI.byteVectorDestroy(this.#ptr);
this.#ptr = undefined; //prevent double free segfault
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions integration_tests/helpers/ffi/commsConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const InterfaceFFI = require("./ffiInterface");
const utf8 = require("utf8");

class CommsConfig {
#comms_config_ptr;
#ptr;

constructor(
public_address,
Expand All @@ -17,7 +17,7 @@ class CommsConfig {
let sanitize_db_name = utf8.encode(database_name);
let sanitize_db_path = utf8.encode(datastore_path);
let sanitize_network = utf8.encode(network);
this.#comms_config_ptr = InterfaceFFI.commsConfigCreate(
this.#ptr = InterfaceFFI.commsConfigCreate(
sanitize_address,
transport_ptr,
sanitize_db_name,
Expand All @@ -29,13 +29,13 @@ class CommsConfig {
}

getPtr() {
return this.#comms_config_ptr;
return this.#ptr;
}

destroy() {
if (this.#comms_config_ptr) {
InterfaceFFI.commsConfigDestroy(this.#comms_config_ptr);
this.#comms_config_ptr = undefined; //prevent double free segfault
if (this.#ptr) {
InterfaceFFI.commsConfigDestroy(this.#ptr);
this.#ptr = undefined; //prevent double free segfault
}
}
}
Expand Down
Loading

0 comments on commit 7b549a9

Please sign in to comment.