fix: update cucumber tests for walletffi.feature

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 in wrapper to ptr Review comments Added waitForIterate to util. Refer to CString instead of string in interface types. Replace uint64 type with ulonglong type Downgrade code to be compatible with NodeJS v12 Update config.yml
tari-project · Sep 2, 2021 · 1e23c00 · 1e23c00
1 parent cedb1d4
commit 1e23c00
Show file tree

Hide file tree

Showing 24 changed files with 982 additions and 838 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -128,8 +128,14 @@ commands:
           command: cd integration_tests && node ./generate_report.js
           when: always
       - run:
-          name: Run ffi cucumber scenarios
+          name: Set the correct Node version for wallet FFI tests
+          shell: bash -l {0}
+          command: nvm install v12.22.6 && nvm use v12.22.6 && cd integration_tests && npm install
+          when: always
+      - run:
+          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"

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/integration_tests/features/WalletFFI.feature b/integration_tests/features/WalletFFI.feature
@@ -1,7 +1,10 @@
 @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*, ...)`
+    # Appears to run in NodeJS v12 consistently on mac (5 crash-less runs in a row).
+    # Crashes in v14+ intermittently on mac (more frequently than not) and completely broken on Linux.
+    # See issues:
+    # https://github.com/nodejs/node/issues/32463
+    # https://github.com/node-ffi-napi/node-ffi-napi/issues/97
 
     # 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
@@ -35,11 +38,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
@@ -101,12 +105,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

diff --git a/integration_tests/features/support/steps.js b/integration_tests/features/support/steps.js
@@ -13,6 +13,7 @@ const {
   consoleLogBalance,
   consoleLogTransactionDetails,
   withTimeout,
+  waitForIterate,
 } = require("../../helpers/util");
 const { ConnectivityStatus, PaymentType } = require("../../helpers/types");
 const TransactionBuilder = require("../../helpers/transactionBuilder");
@@ -3538,19 +3539,6 @@ When(
   }
 );
 
-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}`
-    );
-  }
-);
-
 Then(
   "I want to get emoji id of ffi wallet {word}",
   { timeout: 20 * 1000 },
@@ -3826,14 +3814,13 @@ Then(
       "Waiting for " + wallet_name + " to receive " + amount + " transaction(s)"
     );
 
-    await waitFor(
-      async () => {
+    await waitForIterate(
+      () => {
         return wallet.getCounters().received >= amount;
       },
       true,
-      700 * 1000,
-      5 * 1000,
-      5
+      1000,
+      700
     );
 
     if (!(wallet.getCounters().received >= amount)) {
@@ -3860,14 +3847,13 @@ Then(
         " transaction finalization(s)"
     );
 
-    await waitFor(
-      async () => {
+    await waitForIterate(
+      () => {
         return wallet.getCounters().finalized >= amount;
       },
       true,
-      700 * 1000,
-      5 * 1000,
-      5
+      1000,
+      700
     );
 
     if (!(wallet.getCounters().finalized >= amount)) {
@@ -3880,7 +3866,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);
@@ -3891,17 +3877,82 @@ Then(
         wallet_name +
         " to receive " +
         amount +
+        " transaction broadcast(s)"
+    );
+
+    await waitForIterate(
+      () => {
+        return wallet.getCounters().broadcast >= amount;
+      },
+      true,
+      1000,
+      700
+    );
+
+    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 waitForIterate(
+      () => {
+        return wallet.getCounters().mined >= amount;
+      },
+      true,
+      1000,
+      700
+    );
+
+    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 () => {
+    await waitForIterate(
+      () => {
         return wallet.getCounters().saf >= amount;
       },
       true,
-      700 * 1000,
-      5 * 1000,
-      5
+      1000,
+      700
     );
 
     if (!(wallet.getCounters().saf >= amount)) {
@@ -3924,23 +3975,21 @@ 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 waitForIterate(
+      () => {
+        return wallet.getBalance().available >= amount;
+      },
+      true,
+      1000,
+      700
+    );
 
     let balance = wallet.getBalance().available;
 
     if (!(balance >= amount)) {
       console.log("Balance not adequate!");
-    } else {
-      console.log(wallet.getBalance());
     }
+
     expect(balance >= amount).to.equal(true);
   }
 );
@@ -3961,10 +4010,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);

diff --git a/integration_tests/features/support/world.js b/integration_tests/features/support/world.js
@@ -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;
@@ -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.");
 });
 

diff --git a/integration_tests/helpers/ffi/byteVector.js b/integration_tests/helpers/ffi/byteVector.js
@@ -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;
     }
   }
 
@@ -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
     }
   }
 }