test: use TCP node for daily sync test (#3464)

Description --- - uses TCP node (london) for sync tests - `--force-sync-peers` option now takes the pubkey/address of the peer rather than a seed peer index - Bypass tor for TCP in sync test - Display sync peer in status line for header sync - cleanup peer seeds and force sync peer JS array usage - make peer seeds optional (default to empty Vec) without having to specify a default for every network - fix `When a new node joins the network, it should receive all peers` cucumber Motivation and Context --- The primary goal of the daily sync test is to provide visibility into the performance of the sync protocol. Directly connecting to the same TCP node, bypassing the tor network, the network performance under which the sync protocol runs is more consistent. How Has This Been Tested? --- Run locally
tari-project · Oct 21, 2021 · a72f42f · a72f42f
1 parent 5d7fb20
commit a72f42f
Show file tree

Hide file tree

Showing 10 changed files with 73 additions and 80 deletions.
diff --git a/applications/daily_tests/automatic_sync_test.js b/applications/daily_tests/automatic_sync_test.js
@@ -4,6 +4,8 @@ const path = require("path");
 const helpers = require("./helpers");
 const BaseNodeProcess = require("integration_tests/helpers/baseNodeProcess");
 
+const NETWORK = "WEATHERWAX";
+
 const SyncType = {
   Archival: "Archival",
   Pruned: "Pruned",
@@ -18,8 +20,9 @@ async function main() {
       choices: [SyncType.Archival, SyncType.Pruned],
     })
     .option("force-sync-peer", {
-      type: "number",
-      description: "Enable force sync to peer_seeds i-th node.",
+      type: "string",
+      description:
+        "Pubkey and address string (formatted: `{pubkey}::{address}) of a the force sync peer.",
     })
     .option("log", {
       alias: "l",
@@ -30,11 +33,17 @@ async function main() {
     .help()
     .alias("help", "h").argv;
   try {
-    const { blockHeight, timeDiffMinutes, blockRate, forcedSyncPeer } =
-      await run(argv);
+    const options = {
+      ...argv,
+      forceSyncPeers: (argv.forceSyncPeers || "")
+        .split(",")
+        .map((s) => s.trim()),
+    };
+    const { blockHeight, timeDiffMinutes, blockRate } = await run(argv);
 
+    const { forcedSyncPeer, syncType } = argv;
     console.log(
-      `${argv.syncType} sync ${
+      `${syncType} sync ${
         forcedSyncPeer ? "forced to " + forcedSyncPeer : ""
       } to block height ${blockHeight} took ${timeDiffMinutes} minutes for a rate of ${blockRate} blocks/min`
     );
@@ -44,30 +53,25 @@ async function main() {
 }
 
 async function run(options) {
-  let forcedSyncPeer;
   const baseNode = new BaseNodeProcess("compile", true);
   await baseNode.init();
 
+  // Bypass tor for outbound TCP (faster but less private)
+  process.env[`TARI_BASE_NODE__${NETWORK}__TOR_PROXY_BYPASS_FOR_OUTBOUND_TCP`] =
+    "true";
   // Set pruning horizon in config file if `pruned` command line arg is present
   if (options.syncType === SyncType.Pruned) {
-    process.env.TARI_BASE_NODE__WEATHERWAX__PRUNING_HORIZON = 1000;
+    process.env[`TARI_BASE_NODE__${NETWORK}__PRUNING_HORIZON`] = 1000;
   }
 
-  // Check if we have a forced peer index.
-  if (options.forceSyncPeer !== undefined) {
-    const config = (
-      await fs.readFile(baseNode.baseDir + "/config/config.toml")
-    ).toString();
-    const peer = Array.from(
-      config.match(/\npeer_seeds = \[(.*?)\]/s)[1].matchAll(/\n[^#]*"(.*?)"/g),
-      (m) => m[1]
-    )[options.forceSyncPeer];
-    if (peer === undefined) {
-      // Check if index is within bounds of peer_seeds from config.
-      throw "Forced index out of bounds";
+  if (options.forceSyncPeer) {
+    let forcedSyncPeersStr = options.forceSyncPeer;
+    if (Array.isArray(options.forceSyncPeer)) {
+      forcedSyncPeersStr = options.forceSyncPeer.join(",");
     }
-    process.env.TARI_BASE_NODE__WEATHERWAX__FORCE_SYNC_PEERS = [peer];
-    forcedSyncPeer = peer;
+    console.log(`Force sync peer set to ${forcedSyncPeersStr}`);
+    process.env[`TARI_BASE_NODE__${NETWORK}__FORCE_SYNC_PEERS`] =
+      forcedSyncPeersStr;
   }
 
   await baseNode.start();
@@ -105,7 +109,6 @@ async function run(options) {
       blockRate: blockRate.toFixed(2),
       timeDiffMinutes: timeDiffMinutes.toFixed(2),
       blockHeight: syncResult.height,
-      forcedSyncPeer,
     };
   } catch (err) {
     await baseNode.stop();

diff --git a/applications/daily_tests/cron_jobs.js b/applications/daily_tests/cron_jobs.js
@@ -109,6 +109,9 @@ async function runBaseNodeSyncTest(syncType) {
       log: LOG_FILE,
       syncType,
       baseDir,
+      forceSyncPeers: [
+        "b0c1f788f137ba0cdc0b61e89ee43b80ebf5cca4136d3229561bf11eba347849::/ip4/3.8.193.254/tcp/18189",
+      ],
     });
 
     notify(

diff --git a/base_layer/core/src/base_node/state_machine_service/states/events_and_states.rs b/base_layer/core/src/base_node/state_machine_service/states/events_and_states.rs
@@ -191,14 +191,7 @@ impl StateInfo {
                 ),
                 HorizonSyncStatus::Finalizing => "Finalizing horizon sync".to_string(),
             },
-            BlockSync(info) => format!(
-                "Syncing blocks: ({}) {}",
-                info.sync_peers
-                    .first()
-                    .map(|n| n.short_str())
-                    .unwrap_or_else(|| "".to_string()),
-                info.sync_progress_string()
-            ),
+            BlockSync(info) => format!("Syncing blocks: {}", info.sync_progress_string()),
             Listening(_) => "Listening".to_string(),
             BlockSyncStarting => "Starting block sync".to_string(),
         }
@@ -287,7 +280,11 @@ impl BlockSyncInfo {
 
     pub fn sync_progress_string(&self) -> String {
         format!(
-            "{}/{} ({:.0}%)",
+            "({}) {}/{} ({:.0}%)",
+            self.sync_peers
+                .first()
+                .map(|n| n.short_str())
+                .unwrap_or_else(|| "--".to_string()),
             self.local_height,
             self.tip_height,
             (self.local_height as f64 / self.tip_height as f64 * 100.0)

diff --git a/common/src/configuration/global.rs b/common/src/configuration/global.rs
@@ -361,10 +361,9 @@ fn convert_node_config(
     // Peer seeds can be an array or a comma separated list (e.g. in an ENVVAR)
     let peer_seeds = match cfg.get_array(&key) {
         Ok(seeds) => seeds.into_iter().map(|v| v.into_str().unwrap()).collect(),
-        Err(..) => match cfg.get_str(&key) {
-            Ok(s) => s.split(',').map(|v| v.to_string()).collect(),
-            Err(err) => return Err(ConfigurationError::new(&key, &err.to_string())),
-        },
+        Err(..) => optional(cfg.get_str(&key))?
+            .map(|s| s.split(',').map(|v| v.trim().to_string()).collect())
+            .unwrap_or_default(),
     };
 
     let key = config_string("base_node", net_str, "dns_seeds_name_server");
@@ -405,7 +404,7 @@ fn convert_node_config(
     let force_sync_peers = match cfg.get_array(&key) {
         Ok(peers) => peers.into_iter().map(|v| v.into_str().unwrap()).collect(),
         Err(..) => match cfg.get_str(&key) {
-            Ok(s) => s.split(',').map(|v| v.to_string()).collect(),
+            Ok(s) => s.split(',').map(|v| v.trim().to_string()).collect(),
             Err(..) => vec![],
         },
     };

diff --git a/integration_tests/features/WalletCli.feature b/integration_tests/features/WalletCli.feature
@@ -15,6 +15,7 @@ Feature: Wallet CLI
         And I clear custom base node of wallet WALLET via command line
 
     Scenario: As a user I want to change password via command line
+        Given I have a seed node SEED
         Given I have wallet WALLET connected to all seed nodes
         When I stop wallet WALLET
         And I change the password of wallet WALLET to changedpwd via command line

diff --git a/integration_tests/features/support/steps.js b/integration_tests/features/support/steps.js
@@ -1566,8 +1566,7 @@ Then(/(.*) should have (\d+) peers/, async function (nodeName, peerCount) {
   await sleep(500);
   const client = this.getClient(nodeName);
   const peers = await client.getPeers();
-  // we add a non existing node when the node starts before adding any actual peers. So the count should always be 1 higher
-  expect(peers.length).to.equal(peerCount + 1);
+  expect(peers.length).to.equal(peerCount);
 });
 
 When("I print the world", function () {

diff --git a/integration_tests/features/support/world.js b/integration_tests/features/support/world.js
@@ -71,10 +71,13 @@ class CustomWorld {
 
   async createAndAddNode(name, addresses) {
     const node = this.createNode(name);
-    if (Array.isArray(addresses)) {
-      node.setPeerSeeds(addresses);
-    } else {
-      node.setPeerSeeds([addresses]);
+    console.log(`Creating node ${name} with ${addresses}`);
+    if (addresses) {
+      if (Array.isArray(addresses)) {
+        node.setPeerSeeds(addresses);
+      } else {
+        node.setPeerSeeds([addresses]);
+      }
     }
     await node.startNew();
     await this.addNode(name, node);

diff --git a/integration_tests/helpers/baseNodeProcess.js b/integration_tests/helpers/baseNodeProcess.js
@@ -91,11 +91,11 @@ class BaseNodeProcess {
   }
 
   setPeerSeeds(addresses) {
-    this.peerSeeds = addresses.join(",");
+    this.peerSeeds = addresses;
   }
 
   setForceSyncPeers(addresses) {
-    this.forceSyncPeers = addresses.join(",");
+    this.forceSyncPeers = addresses;
   }
 
   getGrpcAddress() {

diff --git a/integration_tests/helpers/config.js b/integration_tests/helpers/config.js
@@ -104,16 +104,11 @@ function baseEnvs(peerSeeds = [], forceSyncPeers = []) {
     TARI_MINING_NODE__VALIDATE_TIP_TIMEOUT_SEC: 2,
     TARI_WALLET__SCAN_FOR_UTXO_INTERVAL: 5,
   };
-  if (forceSyncPeers.length != 0) {
-    envs.TARI_BASE_NODE__LOCALNET__FORCE_SYNC_PEERS = forceSyncPeers;
+  if (forceSyncPeers.length > 0) {
+    envs.TARI_BASE_NODE__LOCALNET__FORCE_SYNC_PEERS = forceSyncPeers.join(",");
   }
-  if (peerSeeds.length != 0) {
-    envs.TARI_BASE_NODE__LOCALNET__PEER_SEEDS = peerSeeds;
-  } else {
-    //  Nowheresville
-    envs.TARI_BASE_NODE__LOCALNET__PEER_SEEDS = [
-      "5cfcf17f41b01980eb4fa03cec5ea12edbd3783496a2b5dabf99e4bf6410f460::/ip4/10.0.0.50/tcp/1",
-    ];
+  if (peerSeeds.length > 0) {
+    envs.TARI_BASE_NODE__LOCALNET__PEER_SEEDS = peerSeeds.join(",");
   }
 
   return envs;
@@ -140,32 +135,25 @@ function createEnv(
   const network =
     options && options.network ? options.network.toUpperCase() : "LOCALNET";
 
-  const configEnvs = {};
-  configEnvs[
-    `TARI_BASE_NODE__${network}__GRPC_BASE_NODE_ADDRESS`
-  ] = `${baseNodeGrpcAddress}:${baseNodeGrpcPort}`;
-  configEnvs[
-    `TARI_BASE_NODE__${network}__GRPC_CONSOLE_WALLET_ADDRESS`
-  ] = `${walletGrpcAddress}:${walletGrpcPort}`;
-  configEnvs[
-    `TARI_BASE_NODE__${network}__BASE_NODE_IDENTITY_FILE`
-  ] = `${nodeFile}`;
-  configEnvs[`TARI_BASE_NODE__${network}__TCP_LISTENER_ADDRESS`] =
-    "/ip4/127.0.0.1/tcp/" + (isWallet ? `${walletPort}` : `${baseNodePort}`);
-  configEnvs[`TARI_BASE_NODE__${network}__PUBLIC_ADDRESS`] =
-    "/ip4/127.0.0.1/tcp/" + (isWallet ? `${walletPort}` : `${baseNodePort}`);
-  configEnvs[
-    `TARI_MERGE_MINING_PROXY__${network}__PROXY_HOST_ADDRESS`
-  ] = `${proxyFullAddress}`;
-  configEnvs[
-    `TARI_STRATUM_TRANSCODER__${network}__TRANSCODER_HOST_ADDRESS`
-  ] = `${transcoderFullAddress}`;
-  configEnvs[`TARI_BASE_NODE__${network}__TRANSPORT`] = "tcp";
-  configEnvs[`TARI_WALLET__${network}__TRANSPORT`] = "tcp";
-  configEnvs[`TARI_WALLET__${network}__TCP_LISTENER_ADDRESS`] =
-    "/ip4/127.0.0.1/tcp/" + `${walletPort}`;
-  configEnvs[`TARI_WALLET__${network}__PUBLIC_ADDRESS`] =
-    "/ip4/127.0.0.1/tcp/" + `${walletPort}`;
+  const configEnvs = {
+    [`TARI_BASE_NODE__${network}__GRPC_BASE_NODE_ADDRESS`]: `${baseNodeGrpcAddress}:${baseNodeGrpcPort}`,
+    [`TARI_BASE_NODE__${network}__GRPC_CONSOLE_WALLET_ADDRESS`]: `${walletGrpcAddress}:${walletGrpcPort}`,
+
+    [`TARI_BASE_NODE__${network}__BASE_NODE_IDENTITY_FILE`]: `${nodeFile}`,
+
+    [`TARI_BASE_NODE__${network}__TRANSPORT`]: "tcp",
+    [`TARI_BASE_NODE__${network}__TCP_LISTENER_ADDRESS`]:
+      "/ip4/127.0.0.1/tcp/" + (isWallet ? `${walletPort}` : `${baseNodePort}`),
+    [`TARI_BASE_NODE__${network}__PUBLIC_ADDRESS`]:
+      "/ip4/127.0.0.1/tcp/" + (isWallet ? `${walletPort}` : `${baseNodePort}`),
+
+    [`TARI_WALLET__${network}__TRANSPORT`]: "tcp",
+    [`TARI_WALLET__${network}__TCP_LISTENER_ADDRESS`]: `/ip4/127.0.0.1/tcp/${walletPort}`,
+    [`TARI_WALLET__${network}__PUBLIC_ADDRESS`]: `/ip4/127.0.0.1/tcp/${walletPort}`,
+
+    [`TARI_MERGE_MINING_PROXY__${network}__PROXY_HOST_ADDRESS`]: `${proxyFullAddress}`,
+    [`TARI_STRATUM_TRANSCODER__${network}__TRANSCODER_HOST_ADDRESS`]: `${transcoderFullAddress}`,
+  };
 
   return { ...envs, ...configEnvs, ...mapEnvs(options || {}) };
 }

diff --git a/integration_tests/helpers/walletProcess.js b/integration_tests/helpers/walletProcess.js
@@ -56,7 +56,7 @@ class WalletProcess {
   }
 
   setPeerSeeds(addresses) {
-    this.peerSeeds = addresses.join(",");
+    this.peerSeeds = addresses;
   }
 
   run(cmd, args, saveFile, input_buffer, output) {