From 50de54aef1e750a40f98ec5ada17e6cfe9d4047e Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:51 +0200
Subject: [PATCH 01/25] netns: remove empty connections defs

Like in the netns defintion for bitcoind.
---
 modules/netns-isolation.nix | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 7016f22..05fc0d2 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -106,12 +106,10 @@ in {
       spark-wallet = {
         id = 17;
         # communicates with clightning over lightning-rpc socket
-        connections = [];
       };
       lightning-charge = {
         id = 18;
         # communicates with clightning over lightning-rpc socket
-        connections = [];
       };
       nanopos = {
         id = 19;
@@ -120,11 +118,9 @@ in {
       recurring-donations = {
         id = 20;
         # communicates with clightning over lightning-rpc socket
-        connections = [];
       };
       nginx = {
         id = 21;
-        connections = [];
       };
       lightning-loop = {
         id = 22;

From 4eb92df08c7ae58d27d74140742826d5fa3a28c7 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:52 +0200
Subject: [PATCH 02/25] netns: remove redundant filter

The 'availableNetns' connection matrix only consists of enabled entries,
so no extra filtering is needed.
Reason: availableNetns starts with the filtered 'base' and is then symmetrised.
---
 modules/netns-isolation.nix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 05fc0d2..408676a 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -8,7 +8,7 @@ let
   netns = builtins.mapAttrs (n: v: {
     inherit (v) id;
     address = "169.254.${toString cfg.addressblock}.${toString v.id}";
-    availableNetns = builtins.filter isEnabled availableNetns.${n};
+    availableNetns = availableNetns.${n};
   }) enabledServices;
 
   # Symmetric netns connection matrix

From 74f1610668960dd1579f313707264d8c60b15e56 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:53 +0200
Subject: [PATCH 03/25] netns: clarify addressblock description

---
 modules/netns-isolation.nix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 408676a..2d66224 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -44,7 +44,7 @@ in {
       type = types.ints.u8;
       default = "1";
       description = ''
-        Specify the N address block in 169.254.N.0/24.
+        The address block N in 169.254.N.0/24, used as the prefix for netns addresses.
       '';
     };
 

From 5a81693ef3e709a630d510c9fbf23c1bca89522d Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:54 +0200
Subject: [PATCH 04/25] netns: add range check for netns ids

---
 modules/netns-isolation.nix | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 2d66224..9d5e164 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -53,12 +53,11 @@ in {
       type = types.attrsOf (types.submodule {
         options = {
           id = mkOption {
-            # TODO: Exclude 10
             # TODO: Assert uniqueness
-            type = types.int;
+            type = types.ints.between 11 255;
             description = ''
-              id for the netns, that is used for the IP address host part and
-              naming the interfaces. Must be unique. Must not be 10.
+              id for the netns, used for the IP address host part and
+              for naming the interfaces. Must be unique. Must be greater than 10.
             '';
           };
           connections = mkOption {

From b7fc819be5cae75a1417c5afcf8376df07f8f03a Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:55 +0200
Subject: [PATCH 05/25] netns: consistent var naming

n is used elsewhere in similar contexts.
---
 modules/netns-isolation.nix | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 9d5e164..fa36387 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -210,8 +210,8 @@ in {
       ];
       rpcallowip = [
         "127.0.0.1"
-      ] ++ lib.lists.concatMap (s: [
-        "${netns.${s}.address}"
+      ] ++ lib.lists.concatMap (n: [
+        "${netns.${n}.address}"
       ]) netns.bitcoind.availableNetns;
       cli = pkgs.writeScriptBin "bitcoin-cli" ''
         netns-exec nb-bitcoind ${config.services.bitcoind.package}/bin/bitcoin-cli -datadir='${config.services.bitcoind.dataDir}' "$@"
@@ -253,8 +253,8 @@ in {
       ];
       rpcallowip = [
         "127.0.0.1"
-      ] ++ lib.lists.concatMap (s: [
-        "${netns.${s}.address}"
+      ] ++ lib.lists.concatMap (n: [
+        "${netns.${n}.address}"
       ]) netns.liquidd.availableNetns;
       mainchainrpchost = netns.bitcoind.address;
       cli = pkgs.writeScriptBin "elements-cli" ''

From a3ae8668e639159d5b63dd3fdc00d166c9c12e11 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:56 +0200
Subject: [PATCH 06/25] netns: use map instead of concatMap

---
 modules/netns-isolation.nix | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index fa36387..e51d89e 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -210,9 +210,7 @@ in {
       ];
       rpcallowip = [
         "127.0.0.1"
-      ] ++ lib.lists.concatMap (n: [
-        "${netns.${n}.address}"
-      ]) netns.bitcoind.availableNetns;
+      ] ++ map (n: "${netns.${n}.address}") netns.bitcoind.availableNetns;
       cli = pkgs.writeScriptBin "bitcoin-cli" ''
         netns-exec nb-bitcoind ${config.services.bitcoind.package}/bin/bitcoin-cli -datadir='${config.services.bitcoind.dataDir}' "$@"
       '';
@@ -253,9 +251,7 @@ in {
       ];
       rpcallowip = [
         "127.0.0.1"
-      ] ++ lib.lists.concatMap (n: [
-        "${netns.${n}.address}"
-      ]) netns.liquidd.availableNetns;
+      ] ++ map (n: "${netns.${n}.address}") netns.liquidd.availableNetns;
       mainchainrpchost = netns.bitcoind.address;
       cli = pkgs.writeScriptBin "elements-cli" ''
         netns-exec nb-liquidd ${pkgs.nix-bitcoin.elementsd}/bin/elements-cli -datadir='${config.services.liquidd.dataDir}' "$@"

From 0f0f6ddbb97716de7af9a5bbd4519c0ec4e53b95 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:57 +0200
Subject: [PATCH 07/25] netns: add comment about undesirable algorithmic
 complexity
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We don't want to be Accidentally Quadratic™
---
 modules/netns-isolation.nix | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index e51d89e..050e183 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -16,6 +16,12 @@ let
   #   availableNetns.bitcoind = [ "clighting" ];
   #   and
   #   availableNetns.clighting = [ "bitcoind" ];
+  #
+  # FIXME: Although negligible for our purposes, this calculation's runtime
+  # is in the order of (number of connections * number of services),
+  # because attrsets and lists are fully copied on each update with '//' or '++'.
+  # This can only be improved with an update in the nix language.
+  #
   availableNetns = let
     # base = { clightning = [ "bitcoind" ]; ... }
     base = builtins.mapAttrs (n: v:

From d0b8d77de2018207d8b2e598990ba65db499c08d Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:58 +0200
Subject: [PATCH 08/25] netns: remove conditionals for service settings

Going without the conditionals (like in secure-node.nix) adds
readability and doesn't reduce evaluation performance (in fact, it
even slightly improves performance due to implementation details
of mkIf).

To avoid errors, remove use of disabled services in secure-node.nix and
nix-bitcoin-webindex.nix.
---
 modules/netns-isolation.nix      | 18 +++++++++---------
 modules/nix-bitcoin-webindex.nix |  7 +++----
 modules/presets/secure-node.nix  |  4 +++-
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 050e183..ddff07b 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -223,13 +223,13 @@ in {
     };
 
     # clightning: Custom netns configs
-    services.clightning = mkIf config.services.clightning.enable {
+    services.clightning = {
       bitcoin-rpcconnect = netns.bitcoind.address;
       bind-addr = netns.clightning.address;
     };
 
     # lnd: Custom netns configs
-    services.lnd = mkIf config.services.lnd.enable {
+    services.lnd = {
       listen = netns.lnd.address;
       rpclisten = [
         "${netns.lnd.address}"
@@ -249,7 +249,7 @@ in {
     };
 
     # liquidd: Custom netns configs
-    services.liquidd = mkIf config.services.liquidd.enable {
+    services.liquidd = {
       bind = netns.liquidd.address;
       rpcbind = [
         "${netns.liquidd.address}"
@@ -268,31 +268,31 @@ in {
     };
 
     # electrs: Custom netns configs
-    services.electrs = mkIf config.services.electrs.enable {
+    services.electrs = {
       address = netns.electrs.address;
       daemonrpc = "${netns.bitcoind.address}:${toString config.services.bitcoind.rpc.port}";
     };
 
     # spark-wallet: Custom netns configs
-    services.spark-wallet = mkIf config.services.spark-wallet.enable {
+    services.spark-wallet = {
       host = netns.spark-wallet.address;
       extraArgs = "--no-tls";
     };
 
     # lightning-charge: Custom netns configs
-    services.lightning-charge.host = mkIf config.services.lightning-charge.enable netns.lightning-charge.address;
+    services.lightning-charge.host = netns.lightning-charge.address;
 
     # nanopos: Custom netns configs
-    services.nanopos = mkIf config.services.nanopos.enable {
+    services.nanopos = {
       charged-url = "http://${netns.lightning-charge.address}:9112";
       host = netns.nanopos.address;
     };
 
     # nginx: Custom netns configs
-    services.nix-bitcoin-webindex.host = mkIf config.services.nix-bitcoin-webindex.enable netns.nginx.address;
+    services.nix-bitcoin-webindex.host = netns.nginx.address;
 
     # loop: Custom netns configs
-    services.lightning-loop = mkIf config.services.lightning-loop.enable {
+    services.lightning-loop = {
       cli = pkgs.writeScriptBin "loop"
       # Switch user because lnd makes datadir contents readable by user only
       ''
diff --git a/modules/nix-bitcoin-webindex.nix b/modules/nix-bitcoin-webindex.nix
index b75ab2e..7375f6e 100644
--- a/modules/nix-bitcoin-webindex.nix
+++ b/modules/nix-bitcoin-webindex.nix
@@ -77,13 +77,12 @@ in {
     systemd.services.create-web-index = {
       description = "Get node info";
       wantedBy = [ "multi-user.target" ];
-      path  = with pkgs; [
+      path = with pkgs; [
         config.programs.nodeinfo
-        config.services.clightning.cli
-        config.services.lnd.cli
         jq
         sudo
-      ];
+      ] ++ optional config.services.lnd.enable config.services.lnd.cli
+        ++ optional config.services.clightning.enable config.services.clightning.cli;
       serviceConfig = nix-bitcoin-services.defaultHardening // {
         ExecStart="${pkgs.bash}/bin/bash ${createWebIndex}";
         User = "root";
diff --git a/modules/presets/secure-node.nix b/modules/presets/secure-node.nix
index 133f649..e666ca1 100644
--- a/modules/presets/secure-node.nix
+++ b/modules/presets/secure-node.nix
@@ -194,7 +194,9 @@ in {
       port = 50001;
       enforceTor = true;
     };
-    services.tor.hiddenServices.electrs = mkHiddenService { port = cfg.electrs.port; toHost = cfg.electrs.address; };
+    services.tor.hiddenServices.electrs = mkIf cfg.electrs.enable (mkHiddenService {
+      port = cfg.electrs.port; toHost = cfg.electrs.address;
+    });
 
     services.spark-wallet = {
       onion-service = true;

From e385c732567a1cdf958f8b6690011e03765cd53b Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:35:59 +0200
Subject: [PATCH 09/25] netns: separate implementation and service configs

This greatly improves clarity.

Especially the bitcoind-import-banlist.serviceConfig definition was out
of place.
---
 modules/netns-isolation.nix | 115 +++++++++++++++++-------------------
 1 file changed, 55 insertions(+), 60 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index ddff07b..b18782d 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -75,8 +75,10 @@ in {
     };
   };
 
-  config = mkIf cfg.enable {
-    # Prerequisites
+  config = mkIf cfg.enable (mkMerge [
+
+  # Base infrastructure
+  {
     networking.dhcpcd.denyInterfaces = [ "br0" "br-nb*" "nb-veth*" ];
     services.tor.client.socksListenAddress = "${bridgeIp}:9050";
     networking.firewall.interfaces.br0.allowedTCPPorts = [ 9050 ];
@@ -88,51 +90,6 @@ in {
       permissions = "u+rx,g+rx,o-rwx";
     };
 
-    nix-bitcoin.netns-isolation.services = {
-      bitcoind = {
-        id = 12;
-      };
-      clightning = {
-        id = 13;
-        connections = [ "bitcoind" ];
-      };
-      lnd = {
-        id = 14;
-        connections = [ "bitcoind" ];
-      };
-      liquidd = {
-        id = 15;
-        connections = [ "bitcoind" ];
-      };
-      electrs = {
-        id = 16;
-        connections = [ "bitcoind" ];
-      };
-      spark-wallet = {
-        id = 17;
-        # communicates with clightning over lightning-rpc socket
-      };
-      lightning-charge = {
-        id = 18;
-        # communicates with clightning over lightning-rpc socket
-      };
-      nanopos = {
-        id = 19;
-        connections = [ "nginx" "lightning-charge" ];
-      };
-      recurring-donations = {
-        id = 20;
-        # communicates with clightning over lightning-rpc socket
-      };
-      nginx = {
-        id = 21;
-      };
-      lightning-loop = {
-        id = 22;
-        connections = [ "lnd" ];
-      };
-    };
-
     systemd.services = {
       netns-bridge = {
         description = "Create bridge";
@@ -153,8 +110,6 @@ in {
           RemainAfterExit = "yes";
         };
       };
-
-      bitcoind-import-banlist.serviceConfig.NetworkNamespacePath = "/var/run/netns/nb-bitcoind";
     } //
     (let
       makeNetnsServices = n: v: let
@@ -206,8 +161,55 @@ in {
     in foldl (services: n:
       services // (makeNetnsServices n netns.${n})
     ) {} (builtins.attrNames netns));
+  }
+
+  # Service-specific config
+  {
+    nix-bitcoin.netns-isolation.services = {
+      bitcoind = {
+        id = 12;
+      };
+      clightning = {
+        id = 13;
+        connections = [ "bitcoind" ];
+      };
+      lnd = {
+        id = 14;
+        connections = [ "bitcoind" ];
+      };
+      liquidd = {
+        id = 15;
+        connections = [ "bitcoind" ];
+      };
+      electrs = {
+        id = 16;
+        connections = [ "bitcoind" ];
+      };
+      spark-wallet = {
+        id = 17;
+        # communicates with clightning over lightning-rpc socket
+      };
+      lightning-charge = {
+        id = 18;
+        # communicates with clightning over lightning-rpc socket
+      };
+      nanopos = {
+        id = 19;
+        connections = [ "nginx" "lightning-charge" ];
+      };
+      recurring-donations = {
+        id = 20;
+        # communicates with clightning over lightning-rpc socket
+      };
+      nginx = {
+        id = 21;
+      };
+      lightning-loop = {
+        id = 22;
+        connections = [ "lnd" ];
+      };
+    };
 
-    # bitcoin: Custom netns configs
     services.bitcoind = {
       bind = netns.bitcoind.address;
       rpcbind = [
@@ -221,14 +223,13 @@ in {
         netns-exec nb-bitcoind ${config.services.bitcoind.package}/bin/bitcoin-cli -datadir='${config.services.bitcoind.dataDir}' "$@"
       '';
     };
+    systemd.services.bitcoind-import-banlist.serviceConfig.NetworkNamespacePath = "/var/run/netns/nb-bitcoind";
 
-    # clightning: Custom netns configs
     services.clightning = {
       bitcoin-rpcconnect = netns.bitcoind.address;
       bind-addr = netns.clightning.address;
     };
 
-    # lnd: Custom netns configs
     services.lnd = {
       listen = netns.lnd.address;
       rpclisten = [
@@ -248,7 +249,6 @@ in {
       '';
     };
 
-    # liquidd: Custom netns configs
     services.liquidd = {
       bind = netns.liquidd.address;
       rpcbind = [
@@ -267,31 +267,25 @@ in {
       '';
     };
 
-    # electrs: Custom netns configs
     services.electrs = {
       address = netns.electrs.address;
       daemonrpc = "${netns.bitcoind.address}:${toString config.services.bitcoind.rpc.port}";
     };
 
-    # spark-wallet: Custom netns configs
     services.spark-wallet = {
       host = netns.spark-wallet.address;
       extraArgs = "--no-tls";
     };
 
-    # lightning-charge: Custom netns configs
     services.lightning-charge.host = netns.lightning-charge.address;
 
-    # nanopos: Custom netns configs
     services.nanopos = {
       charged-url = "http://${netns.lightning-charge.address}:9112";
       host = netns.nanopos.address;
     };
 
-    # nginx: Custom netns configs
     services.nix-bitcoin-webindex.host = netns.nginx.address;
 
-    # loop: Custom netns configs
     services.lightning-loop = {
       cli = pkgs.writeScriptBin "loop"
       # Switch user because lnd makes datadir contents readable by user only
@@ -299,5 +293,6 @@ in {
         netns-exec nb-lightning-loop sudo -u lnd ${config.services.lightning-loop.package}/bin/loop "$@"
       '';
     };
-  };
+  }
+  ]);
 }

From 9715134f066fee42101f798ec347d9c3d72057ad Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:00 +0200
Subject: [PATCH 10/25] netns: don't repeat cli definitions

1. Saves some code.
2. Guarantees that the netns and no-netns cli defs are always in sync.
---
 modules/bitcoind.nix             | 14 +++++---------
 modules/lightning-loop.nix       |  3 ++-
 modules/liquid.nix               | 10 ++++++----
 modules/lnd.nix                  |  3 ++-
 modules/netns-isolation.nix      | 32 ++++++++++----------------------
 modules/nix-bitcoin-services.nix |  7 +++++++
 6 files changed, 32 insertions(+), 37 deletions(-)

diff --git a/modules/bitcoind.nix b/modules/bitcoind.nix
index 5362710..440b6ed 100644
--- a/modules/bitcoind.nix
+++ b/modules/bitcoind.nix
@@ -265,20 +265,16 @@ in {
       };
       cli = mkOption {
         type = types.package;
-        default = cfg.cli-nonetns-exec;
+        # Overriden on netns-isolation
+        default = cfg.cliBase;
         description = "Binary to connect with the bitcoind instance.";
       };
-      # Needed because bitcoin-cli commands executed through systemd already
-      # run inside nb-bitcoind, hence they don't need netns-exec prefixed.
-      cli-nonetns-exec = mkOption {
+      cliBase = mkOption {
         readOnly = true;
         type = types.package;
         default = pkgs.writeScriptBin "bitcoin-cli" ''
           exec ${cfg.package}/bin/bitcoin-cli -datadir='${cfg.dataDir}' "$@"
         '';
-        description = ''
-          Binary to connect with the bitcoind instance without netns-exec.
-        '';
       };
       enforceTor =  nix-bitcoin-services.enforceTor;
     };
@@ -315,7 +311,7 @@ in {
         fi
       '';
       postStart = ''
-        cd ${cfg.cli-nonetns-exec}/bin
+        cd ${cfg.cliBase}/bin
         # Poll until bitcoind accepts commands. This can take a long time.
         while ! ./bitcoin-cli getnetworkinfo &> /dev/null; do
           sleep 1
@@ -342,7 +338,7 @@ in {
       bindsTo = [ "bitcoind.service" ];
       after = [ "bitcoind.service" ];
       script = ''
-        cd ${cfg.cli-nonetns-exec}/bin
+        cd ${cfg.cliBase}/bin
         echo "Importing node banlist..."
         cat ${./banlist.cli.txt} | while read line; do
             if ! err=$(eval "$line" 2>&1) && [[ $err != *already\ banned* ]]; then
diff --git a/modules/lightning-loop.nix b/modules/lightning-loop.nix
index 0c84873..65a6981 100644
--- a/modules/lightning-loop.nix
+++ b/modules/lightning-loop.nix
@@ -30,10 +30,11 @@ in {
       default = pkgs.writeScriptBin "loop"
       # Switch user because lnd makes datadir contents readable by user only
       ''
-        exec sudo -u lnd ${cfg.package}/bin/loop "$@"
+        ${cfg.cliExec} sudo -u lnd ${cfg.package}/bin/loop "$@"
       '';
       description = "Binary to connect with the lnd instance.";
     };
+    inherit (nix-bitcoin-services) cliExec;
     enforceTor =  nix-bitcoin-services.enforceTor;
   };
 
diff --git a/modules/liquid.nix b/modules/liquid.nix
index bf93d30..ae2b07e 100644
--- a/modules/liquid.nix
+++ b/modules/liquid.nix
@@ -210,17 +210,19 @@ in {
         '';
       };
       cli = mkOption {
+        readOnly = true;
         default = pkgs.writeScriptBin "elements-cli" ''
-          exec ${pkgs.nix-bitcoin.elementsd}/bin/elements-cli -datadir='${cfg.dataDir}' "$@"
+          ${cfg.cliExec} ${pkgs.nix-bitcoin.elementsd}/bin/elements-cli -datadir='${cfg.dataDir}' "$@"
         '';
         description = "Binary to connect with the liquidd instance.";
       };
-      swap-cli = mkOption {
+      swapCli = mkOption {
         default = pkgs.writeScriptBin "liquidswap-cli" ''
-          exec ${pkgs.nix-bitcoin.liquid-swap}/bin/liquidswap-cli -c '${cfg.dataDir}/elements.conf' "$@"
+          ${cfg.cliExec} ${pkgs.nix-bitcoin.liquid-swap}/bin/liquidswap-cli -c '${cfg.dataDir}/elements.conf' "$@"
         '';
         description = "Binary for managing liquid swaps.";
       };
+      inherit (nix-bitcoin-services) cliExec;
       enforceTor =  nix-bitcoin-services.enforceTor;
     };
   };
@@ -229,7 +231,7 @@ in {
     environment.systemPackages = [
       pkgs.nix-bitcoin.elementsd
       (hiPrio cfg.cli)
-      (hiPrio cfg.swap-cli)
+      (hiPrio cfg.swapCli)
     ];
 
     systemd.tmpfiles.rules = [
diff --git a/modules/lnd.nix b/modules/lnd.nix
index 998440f..621dc03 100644
--- a/modules/lnd.nix
+++ b/modules/lnd.nix
@@ -115,11 +115,12 @@ in {
       default = pkgs.writeScriptBin "lncli"
       # Switch user because lnd makes datadir contents readable by user only
       ''
-        exec sudo -u lnd ${cfg.package}/bin/lncli --tlscertpath ${secretsDir}/lnd-cert \
+        ${cfg.cliExec} sudo -u lnd ${cfg.package}/bin/lncli --tlscertpath ${secretsDir}/lnd-cert \
           --macaroonpath '${cfg.dataDir}/chain/bitcoin/mainnet/admin.macaroon' "$@"
       '';
       description = "Binary to connect with the lnd instance.";
     };
+    inherit (nix-bitcoin-services) cliExec;
     enforceTor =  nix-bitcoin-services.enforceTor;
   };
 
diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index b18782d..cca9bbc 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -9,6 +9,7 @@ let
     inherit (v) id;
     address = "169.254.${toString cfg.addressblock}.${toString v.id}";
     availableNetns = availableNetns.${n};
+    netnsName = "nb-${n}";
   }) enabledServices;
 
   # Symmetric netns connection matrix
@@ -42,6 +43,7 @@ let
 
   bridgeIp = "169.254.${toString cfg.addressblock}.10";
 
+  mkCliExec = service: "exec netns-exec ${netns.${service}.netnsName}";
 in {
   options.nix-bitcoin.netns-isolation = {
     enable = mkEnableOption "netns isolation";
@@ -114,7 +116,7 @@ in {
     (let
       makeNetnsServices = n: v: let
         vethName = "nb-veth-${toString v.id}";
-        netnsName = "nb-${n}";
+        inherit (v) netnsName;
         ipNetns = "${ip} -n ${netnsName}";
         netnsIptables = "${ip} netns exec ${netnsName} ${config.networking.firewall.package}/bin/iptables";
       in {
@@ -219,8 +221,10 @@ in {
       rpcallowip = [
         "127.0.0.1"
       ] ++ map (n: "${netns.${n}.address}") netns.bitcoind.availableNetns;
-      cli = pkgs.writeScriptBin "bitcoin-cli" ''
-        netns-exec nb-bitcoind ${config.services.bitcoind.package}/bin/bitcoin-cli -datadir='${config.services.bitcoind.dataDir}' "$@"
+      cli = let
+        inherit (config.services.bitcoind) cliBase;
+      in pkgs.writeScriptBin cliBase.name ''
+        exec netns-exec ${netns.bitcoind.netnsName} ${cliBase}/bin/${cliBase.name} "$@"
       '';
     };
     systemd.services.bitcoind-import-banlist.serviceConfig.NetworkNamespacePath = "/var/run/netns/nb-bitcoind";
@@ -241,12 +245,7 @@ in {
         "127.0.0.1"
       ];
       bitcoind-host = netns.bitcoind.address;
-      cli = pkgs.writeScriptBin "lncli"
-      # Switch user because lnd makes datadir contents readable by user only
-      ''
-        netns-exec nb-lnd sudo -u lnd ${config.services.lnd.package}/bin/lncli --tlscertpath ${config.nix-bitcoin.secretsDir}/lnd-cert \
-          --macaroonpath '${config.services.lnd.dataDir}/chain/bitcoin/mainnet/admin.macaroon' "$@"
-      '';
+      cliExec = mkCliExec "lnd";
     };
 
     services.liquidd = {
@@ -259,12 +258,7 @@ in {
         "127.0.0.1"
       ] ++ map (n: "${netns.${n}.address}") netns.liquidd.availableNetns;
       mainchainrpchost = netns.bitcoind.address;
-      cli = pkgs.writeScriptBin "elements-cli" ''
-        netns-exec nb-liquidd ${pkgs.nix-bitcoin.elementsd}/bin/elements-cli -datadir='${config.services.liquidd.dataDir}' "$@"
-      '';
-      swap-cli = pkgs.writeScriptBin "liquidswap-cli" ''
-        netns-exec nb-liquidd ${pkgs.nix-bitcoin.liquid-swap}/bin/liquidswap-cli -c '${config.services.liquidd.dataDir}/elements.conf' "$@"
-      '';
+      cliExec = mkCliExec "liquidd";
     };
 
     services.electrs = {
@@ -286,13 +280,7 @@ in {
 
     services.nix-bitcoin-webindex.host = netns.nginx.address;
 
-    services.lightning-loop = {
-      cli = pkgs.writeScriptBin "loop"
-      # Switch user because lnd makes datadir contents readable by user only
-      ''
-        netns-exec nb-lightning-loop sudo -u lnd ${config.services.lightning-loop.package}/bin/loop "$@"
-      '';
-    };
+    services.lightning-loop.cliExec = mkCliExec "lightning-loop";
   }
   ]);
 }
diff --git a/modules/nix-bitcoin-services.nix b/modules/nix-bitcoin-services.nix
index 097567e..e8e2f9a 100644
--- a/modules/nix-bitcoin-services.nix
+++ b/modules/nix-bitcoin-services.nix
@@ -55,4 +55,11 @@ with lib;
     set -eo pipefail
     ${src}
   '';
+
+  cliExec = mkOption {
+    # Used by netns-isolation to execute the cli in the service's private netns
+    internal = true;
+    type = types.str;
+    default = "exec";
+  };
 }

From 121301337bd81d1d8a5d7b500b58366347abd467 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:01 +0200
Subject: [PATCH 11/25] netns: add option 'allowedUser' for modules-only usage

The dependency on secure-node.nix prevented using nix-bitcoin by just
importing modules.nix.
---
 modules/netns-isolation.nix     | 10 +++++++++-
 modules/presets/secure-node.nix |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index cca9bbc..83230ed 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -75,6 +75,14 @@ in {
         };
       });
     };
+
+    allowedUser = mkOption {
+      type = types.str;
+      description = ''
+        User that is allowed to execute commands in the service network namespaces.
+        The user's group is also authorized.
+      '';
+    };
   };
 
   config = mkIf cfg.enable (mkMerge [
@@ -88,7 +96,7 @@ in {
     security.wrappers.netns-exec = {
       source  = "${pkgs.nix-bitcoin.netns-exec}/netns-exec";
       capabilities = "cap_sys_admin=ep";
-      owner = "${config.nix-bitcoin.operatorName}";
+      owner = cfg.allowedUser;
       permissions = "u+rx,g+rx,o-rwx";
     };
 
diff --git a/modules/presets/secure-node.nix b/modules/presets/secure-node.nix
index e666ca1..d49307f 100644
--- a/modules/presets/secure-node.nix
+++ b/modules/presets/secure-node.nix
@@ -238,6 +238,7 @@ in {
             [ cfg.hardware-wallets.group ]);
       openssh.authorizedKeys.keys = config.users.users.root.openssh.authorizedKeys.keys;
     };
+    nix-bitcoin.netns-isolation.allowedUser = operatorName;
     # Give operator access to onion hostnames
     services.onion-chef.enable = true;
     services.onion-chef.access.${operatorName} = [ "bitcoind" "clightning" "nginx" "liquidd" "spark-wallet" "electrs" "sshd" ];

From 32e70a7516a0d9602b6884c0281abc59a85d3228 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:02 +0200
Subject: [PATCH 12/25] netns: move webindex config for modules-only usage

webindex is only available in secure-node.
---
 modules/netns-isolation.nix      | 8 ++++++--
 modules/nix-bitcoin-webindex.nix | 5 ++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 83230ed..e54550c 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -83,6 +83,12 @@ in {
         The user's group is also authorized.
       '';
     };
+
+    netns = mkOption {
+      default = netns;
+      readOnly = true;
+      description = "Exposes netns parameters.";
+    };
   };
 
   config = mkIf cfg.enable (mkMerge [
@@ -286,8 +292,6 @@ in {
       host = netns.nanopos.address;
     };
 
-    services.nix-bitcoin-webindex.host = netns.nginx.address;
-
     services.lightning-loop.cliExec = mkCliExec "lightning-loop";
   }
   ]);
diff --git a/modules/nix-bitcoin-webindex.nix b/modules/nix-bitcoin-webindex.nix
index 7375f6e..8982f6c 100644
--- a/modules/nix-bitcoin-webindex.nix
+++ b/modules/nix-bitcoin-webindex.nix
@@ -41,7 +41,10 @@ in {
     };
     host = mkOption {
       type = types.str;
-      default = "localhost";
+      default = if config.nix-bitcoin.netns-isolation.enable then
+        config.nix-bitcoin.netns-isolation.netns.nginx.address
+      else
+        "localhost";
       description = "HTTP server listen address.";
     };
     enforceTor =  nix-bitcoin-services.enforceTor;

From 8bfb7bb2f878aa6ad50dd3bae3361c68af82e47f Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:03 +0200
Subject: [PATCH 13/25] netns: rename bridge br0 -> nb-br

br0 has a high risk of name clashes when nix-bitcoin used as part of a
larger config.
Use a more specific name.
---
 modules/netns-isolation.nix | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index e54550c..1de1ebe 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -95,9 +95,9 @@ in {
 
   # Base infrastructure
   {
-    networking.dhcpcd.denyInterfaces = [ "br0" "br-nb*" "nb-veth*" ];
+    networking.dhcpcd.denyInterfaces = [ "nb-br" "br-nb*" "nb-veth*" ];
     services.tor.client.socksListenAddress = "${bridgeIp}:9050";
-    networking.firewall.interfaces.br0.allowedTCPPorts = [ 9050 ];
+    networking.firewall.interfaces.nb-br.allowedTCPPorts = [ 9050 ];
     boot.kernel.sysctl."net.ipv4.ip_forward" = true;
     security.wrappers.netns-exec = {
       source  = "${pkgs.nix-bitcoin.netns-exec}/netns-exec";
@@ -112,14 +112,14 @@ in {
         requiredBy = [ "tor.service" ];
         before = [ "tor.service" ];
         script = ''
-          ${ip} link add name br0 type bridge
-          ${ip} link set br0 up
-          ${ip} addr add ${bridgeIp}/24 brd + dev br0
+          ${ip} link add name nb-br type bridge
+          ${ip} link set nb-br up
+          ${ip} addr add ${bridgeIp}/24 brd + dev nb-br
           ${iptables} -w -t nat -A POSTROUTING -s 169.254.${toString cfg.addressblock}.0/24 -j MASQUERADE
         '';
         preStop = ''
           ${iptables} -w -t nat -D POSTROUTING -s 169.254.${toString cfg.addressblock}.0/24 -j MASQUERADE
-          ${ip} link del br0
+          ${ip} link del nb-br
         '';
         serviceConfig = {
           Type = "oneshot";
@@ -150,7 +150,7 @@ in {
             ${ipNetns} addr add ${v.address}/24 dev ${vethName}
             ${ip} link set br-${vethName} up
             ${ipNetns} link set ${vethName} up
-            ${ip} link set br-${vethName} master br0
+            ${ip} link set br-${vethName} master nb-br
             ${ipNetns} route add default via ${bridgeIp}
             ${netnsIptables} -w -P INPUT DROP
             ${netnsIptables} -w -A INPUT -s 127.0.0.1,${bridgeIp},${v.address} -j ACCEPT

From b7450877a0445151fd9e9e983d436b1ca514d820 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:04 +0200
Subject: [PATCH 14/25] netns: rename bridge peer devices br-nb-veth* ->
 nb-veth-br*

This ensures a consistent 'nb-' namespace and simplifies the
dhcpcd.denyInterfaces rules.

Also rename vethName -> veth.
---
 modules/netns-isolation.nix | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 1de1ebe..1d39d40 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -95,7 +95,7 @@ in {
 
   # Base infrastructure
   {
-    networking.dhcpcd.denyInterfaces = [ "nb-br" "br-nb*" "nb-veth*" ];
+    networking.dhcpcd.denyInterfaces = [ "nb-br" "nb-veth*" ];
     services.tor.client.socksListenAddress = "${bridgeIp}:9050";
     networking.firewall.interfaces.nb-br.allowedTCPPorts = [ 9050 ];
     boot.kernel.sysctl."net.ipv4.ip_forward" = true;
@@ -129,7 +129,8 @@ in {
     } //
     (let
       makeNetnsServices = n: v: let
-        vethName = "nb-veth-${toString v.id}";
+        veth = "nb-veth-${toString v.id}";
+        peer = "nb-veth-br-${toString v.id}";
         inherit (v) netnsName;
         ipNetns = "${ip} -n ${netnsName}";
         netnsIptables = "${ip} netns exec ${netnsName} ${config.networking.firewall.package}/bin/iptables";
@@ -145,12 +146,12 @@ in {
           script = ''
             ${ip} netns add ${netnsName}
             ${ipNetns} link set lo up
-            ${ip} link add ${vethName} type veth peer name br-${vethName}
-            ${ip} link set ${vethName} netns ${netnsName}
-            ${ipNetns} addr add ${v.address}/24 dev ${vethName}
-            ${ip} link set br-${vethName} up
-            ${ipNetns} link set ${vethName} up
-            ${ip} link set br-${vethName} master nb-br
+            ${ip} link add ${veth} type veth peer name ${peer}
+            ${ip} link set ${veth} netns ${netnsName}
+            ${ipNetns} addr add ${v.address}/24 dev ${veth}
+            ${ip} link set ${peer} up
+            ${ipNetns} link set ${veth} up
+            ${ip} link set ${peer} master nb-br
             ${ipNetns} route add default via ${bridgeIp}
             ${netnsIptables} -w -P INPUT DROP
             ${netnsIptables} -w -A INPUT -s 127.0.0.1,${bridgeIp},${v.address} -j ACCEPT
@@ -165,7 +166,7 @@ in {
           '') v.availableNetns;
           preStop = ''
             ${ip} netns delete ${netnsName}
-            ${ip} link del br-${vethName}
+            ${ip} link del ${peer}
           '';
           serviceConfig = {
             Type = "oneshot";

From 809e75485169c4761ee438807df55eacd20731de Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:05 +0200
Subject: [PATCH 15/25] netns: improve bridge setup

- Explain why we don't use option `networking.bridges`
- Make the bridge setup service part of NixOS' network-setup.service.
  This yields no noticable functional changes for now, but it's
  conceptually cleaner to finish the network setup before network.target
  becomes active.
- Add 'nb-' prefix to service name
---
 modules/netns-isolation.nix | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 1d39d40..5a19914 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -99,6 +99,7 @@ in {
     services.tor.client.socksListenAddress = "${bridgeIp}:9050";
     networking.firewall.interfaces.nb-br.allowedTCPPorts = [ 9050 ];
     boot.kernel.sysctl."net.ipv4.ip_forward" = true;
+
     security.wrappers.netns-exec = {
       source  = "${pkgs.nix-bitcoin.netns-exec}/netns-exec";
       capabilities = "cap_sys_admin=ep";
@@ -107,10 +108,18 @@ in {
     };
 
     systemd.services = {
-      netns-bridge = {
-        description = "Create bridge";
-        requiredBy = [ "tor.service" ];
-        before = [ "tor.service" ];
+      # Due to a NixOS bug we can't currently use option `networking.bridges` to
+      # setup the bridge while `networking.useDHCP` is enabled.
+      nb-netns-bridge = {
+        description = "nix-bitcoin netns bridge";
+        wantedBy = [ "network-setup.service" ];
+        partOf = [ "network-setup.service" ];
+        before = [ "network-setup.service" ];
+        after = [ "network-pre.target" ];
+        serviceConfig = {
+          Type = "oneshot";
+          RemainAfterExit = "yes";
+        };
         script = ''
           ${ip} link add name nb-br type bridge
           ${ip} link set nb-br up
@@ -121,10 +130,6 @@ in {
           ${iptables} -w -t nat -D POSTROUTING -s 169.254.${toString cfg.addressblock}.0/24 -j MASQUERADE
           ${ip} link del nb-br
         '';
-        serviceConfig = {
-          Type = "oneshot";
-          RemainAfterExit = "yes";
-        };
       };
     } //
     (let
@@ -138,8 +143,8 @@ in {
         "${n}".serviceConfig.NetworkNamespacePath = "/var/run/netns/${netnsName}";
 
         "netns-${n}" = rec {
-          requires = [ "netns-bridge.service" ];
-          after = [ "netns-bridge.service" ];
+          requires = [ "nb-netns-bridge.service" ];
+          after = [ "nb-netns-bridge.service" ];
           bindsTo = [ "${n}.service" ];
           requiredBy = bindsTo;
           before = bindsTo;

From 91ebc2d517bdee91f3909d8abe2c1288105de223 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:06 +0200
Subject: [PATCH 16/25] netns-exec: simplify installation

---
 modules/netns-isolation.nix | 2 +-
 pkgs/netns-exec/default.nix | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 5a19914..81acfc5 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -101,7 +101,7 @@ in {
     boot.kernel.sysctl."net.ipv4.ip_forward" = true;
 
     security.wrappers.netns-exec = {
-      source  = "${pkgs.nix-bitcoin.netns-exec}/netns-exec";
+      source = pkgs.nix-bitcoin.netns-exec;
       capabilities = "cap_sys_admin=ep";
       owner = cfg.allowedUser;
       permissions = "u+rx,g+rx,o-rwx";
diff --git a/pkgs/netns-exec/default.nix b/pkgs/netns-exec/default.nix
index bafd516..5998549 100644
--- a/pkgs/netns-exec/default.nix
+++ b/pkgs/netns-exec/default.nix
@@ -5,7 +5,6 @@ stdenv.mkDerivation {
   buildInputs = [ pkgs.libcap ];
   src = ./src;
   installPhase = ''
-    mkdir -p $out
-    cp main $out/netns-exec
+    cp main $out
   '';
 }

From ed73627e0256c69cbaa1e1234b8a248edd7cea36 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:07 +0200
Subject: [PATCH 17/25] netns-exec: minor style fixes

- Use inline variable declarations
- Improve messages
- Fix naming: available -> allowed
- Simplify intro comment
---
 pkgs/netns-exec/src/main.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/pkgs/netns-exec/src/main.c b/pkgs/netns-exec/src/main.c
index 3271fc0..60cc85a 100644
--- a/pkgs/netns-exec/src/main.c
+++ b/pkgs/netns-exec/src/main.c
@@ -1,7 +1,4 @@
-/* This program must be run with CAP_SYS_ADMIN. This can be achieved for example
- * with
- *         # setcap CAP_SYS_ADMIN+ep ./main
- */
+/* This program requires CAP_SYS_ADMIN */
 
 #define _GNU_SOURCE
 #include <sched.h>
@@ -12,18 +9,17 @@
 #include <fcntl.h>
 #include <sys/capability.h>
 
-static char *available_netns[] = {
+static char *allowed_netns[] = {
     "nb-lnd",
     "nb-lightning-loop",
     "nb-bitcoind",
     "nb-liquidd"
 };
 
-int check_netns(char *netns) {
-    int i;
-    int n_available_netns = sizeof(available_netns) / sizeof(available_netns[0]);
-    for (i = 0; i < n_available_netns; i++) {
-        if (strcmp(available_netns[i], netns) == 0) {
+int is_netns_allowed(char *netns) {
+    int n_allowed_netns = sizeof(allowed_netns) / sizeof(allowed_netns[0]);
+    for (int i = 0; i < n_allowed_netns; i++) {
+        if (strcmp(allowed_netns[i], netns) == 0) {
             return 1;
         }
     }
@@ -35,6 +31,7 @@ void print_capabilities() {
     printf("Capabilities: %s\n", cap_to_text(caps, NULL));
     cap_free(caps);
 }
+
 void drop_capabilities() {
     cap_t caps = cap_get_proc();
     cap_clear(caps);
@@ -43,25 +40,24 @@ void drop_capabilities() {
 }
 
 int main(int argc, char **argv) {
-    int fd;
     char netns_path[256];
 
     if (argc < 3) {
-        printf("usage: %s <netns> <command to execute>\n", argv[0]);
+        printf("usage: %s <netns> <command>\n", argv[0]);
         return 1;
     }
 
-    if (!check_netns(argv[1])) {
-        printf("Failed checking %s against available netns.\n", argv[1]);
+    if (!is_netns_allowed(argv[1])) {
+        printf("%s is not an allowed netns.\n", argv[1]);
         return 1;
     }
 
     if(snprintf(netns_path, sizeof(netns_path), "/var/run/netns/%s", argv[1]) < 0) {
-        printf("Failed concatenating %s to the netns path.\n", argv[1]);
+        printf("Path length exceeded for netns %s.\n", argv[1]);
         return 1;
     }
 
-    fd = open(netns_path, O_RDONLY);
+    int fd = open(netns_path, O_RDONLY);
     if (fd < 0) {
         printf("Failed opening netns %s: %d, %s \n", netns_path, errno, strerror(errno));
         return 1;
@@ -84,4 +80,3 @@ int main(int argc, char **argv) {
     execvp(argv[2], &argv[2]);
     return 0;
 }
-

From 9237e5dc3df54036da5ee055c3b40025bf4bb155 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:08 +0200
Subject: [PATCH 18/25] test: use pydoc docstring

---
 test/scenarios/lib.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/test/scenarios/lib.py b/test/scenarios/lib.py
index bffd4bb..1de7039 100644
--- a/test/scenarios/lib.py
+++ b/test/scenarios/lib.py
@@ -33,11 +33,12 @@ def assert_running(unit):
 if "is_interactive" in vars():
     raise Exception()
 
-### Tests
 
-# The argument extra_tests is a dictionary from strings to functions. The string
-# determines at which point of run_tests the corresponding function is executed.
 def run_tests(extra_tests):
+    """
+    :param extra_tests: Test functions that hook into the testing code below
+    :type extra_tests: Dict[str, Callable[]]
+    """
     test_security()
 
     assert_running("bitcoind")

From 0f56ea6ad1f04f281dcb2cd2c88215c7b599aa71 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:09 +0200
Subject: [PATCH 19/25] test: include scenario in test name

---
 test/test.nix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test.nix b/test/test.nix
index 27155bf..807c66e 100644
--- a/test/test.nix
+++ b/test/test.nix
@@ -8,7 +8,7 @@ let
 in
 
 import ./make-test.nix rec {
-  name = "nix-bitcoin";
+  name = "nix-bitcoin-${scenario}";
 
   hardened = {
     imports = [ <nixpkgs/nixos/modules/profiles/hardened.nix> ];

From 9b4cd7bd1ccb98a3cc9d0ad116dd61094c5166e1 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:10 +0200
Subject: [PATCH 20/25] test: simplify scenario handling

We can switch to a more sophisticated scheme later when adding more scenarios
---
 test/test.nix | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/test/test.nix b/test/test.nix
index 807c66e..fbc24c9 100644
--- a/test/test.nix
+++ b/test/test.nix
@@ -2,11 +2,6 @@
 
 { scenario ? "default" }:
 
-let
-  netns-isolation = builtins.getAttr scenario { default = false; withnetns = true; };
-  testScriptFilename = builtins.getAttr scenario { default = ./scenarios/default.py; withnetns = ./scenarios/withnetns.py; };
-in
-
 import ./make-test.nix rec {
   name = "nix-bitcoin-${scenario}";
 
@@ -23,7 +18,7 @@ import ./make-test.nix rec {
       # hardened
     ];
 
-    nix-bitcoin.netns-isolation.enable = mkForce netns-isolation;
+    nix-bitcoin.netns-isolation.enable = (scenario == "withnetns");
 
     services.bitcoind.extraConfig = mkForce "connect=0";
 
@@ -61,5 +56,7 @@ import ./make-test.nix rec {
       install -o nobody -g nogroup -m777 <(:) /secrets/dummy
     '';
   };
-  testScript = builtins.readFile ./scenarios/lib.py + "\n\n" + builtins.readFile testScriptFilename;
+  testScript =
+    builtins.readFile ./scenarios/lib.py + "\n\n" +
+    builtins.readFile "${./.}/scenarios/${scenario}.py";
 }

From 80da0a41bc21f3297e8223614217a76595bb9d58 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:11 +0200
Subject: [PATCH 21/25] test: load complete test environment in debug mode

Stop just before executing actual tests.
This makes all test functions accessible in debug mode.
---
 test/scenarios/lib.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/test/scenarios/lib.py b/test/scenarios/lib.py
index 1de7039..7df7108 100644
--- a/test/scenarios/lib.py
+++ b/test/scenarios/lib.py
@@ -1,3 +1,6 @@
+is_interactive = "is_interactive" in vars()
+
+
 def succeed(*cmds):
     """Returns the concatenated output of all cmds"""
     return machine.succeed(*cmds)
@@ -29,16 +32,15 @@ def assert_running(unit):
     assert_no_failure(unit)
 
 
-# Don't execute the following test suite when this script is running in interactive mode
-if "is_interactive" in vars():
-    raise Exception()
-
-
 def run_tests(extra_tests):
     """
     :param extra_tests: Test functions that hook into the testing code below
     :type extra_tests: Dict[str, Callable[]]
     """
+    # Don't execute the following test suite when this script is running in interactive mode
+    if is_interactive:
+        raise Exception()
+
     test_security()
 
     assert_running("bitcoind")

From 28236691aa1d8323a5fb2b0fa62a79f9d88c5328 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:12 +0200
Subject: [PATCH 22/25] test: rename scenarios/lib.py -> base.py

This file isn't a scenario, it's also not a lib because it contains
the main share of actual tests.
---
 test/{scenarios/lib.py => base.py} | 0
 test/test.nix                      | 3 +--
 2 files changed, 1 insertion(+), 2 deletions(-)
 rename test/{scenarios/lib.py => base.py} (100%)

diff --git a/test/scenarios/lib.py b/test/base.py
similarity index 100%
rename from test/scenarios/lib.py
rename to test/base.py
diff --git a/test/test.nix b/test/test.nix
index fbc24c9..c54312e 100644
--- a/test/test.nix
+++ b/test/test.nix
@@ -57,6 +57,5 @@ import ./make-test.nix rec {
     '';
   };
   testScript =
-    builtins.readFile ./scenarios/lib.py + "\n\n" +
-    builtins.readFile "${./.}/scenarios/${scenario}.py";
+    builtins.readFile ./base.py + "\n\n" + builtins.readFile "${./.}/scenarios/${scenario}.py";
 }

From 91697b1427d4deb6d41dccd5561e3939e076814d Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:13 +0200
Subject: [PATCH 23/25] test: allow for testing all scenarios

Test all scenarios by default when running 'build' (which happens
when the script is called without arguments).

Default to scenario 'default' in other test commands like 'debug'.
---
 test/run-tests.sh | 61 ++++++++++++++++++++++-------------------------
 test/test.nix     |  1 +
 2 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/test/run-tests.sh b/test/run-tests.sh
index def34be..9099d09 100755
--- a/test/run-tests.sh
+++ b/test/run-tests.sh
@@ -4,51 +4,34 @@
 # The test (./test.nix) uses the NixOS testing framework and is executed in a VM.
 #
 # Usage:
-#   Run test
+#   Run all tests
+#   ./run-tests.sh
+#
+#   Test specific scenario
 #   ./run-tests.sh --scenario <scenario>
 #
 #   Run test and save result to avoid garbage collection
-#   ./run-tests.sh --scenario <scenario> build --out-link /tmp/nix-bitcoin-test
+#   ./run-tests.sh [--scenario <scenario>] build --out-link /tmp/nix-bitcoin-test
 #
 #   Run interactive test debugging
-#   ./run-tests.sh --scenario <scenario> debug
+#   ./run-tests.sh [--scenario <scenario>] debug
 #
 #   This starts the testing VM and drops you into a Python REPL where you can
 #   manually execute the tests from ./test-script.py
 
 set -eo pipefail
 
-die() {
-    printf '%s\n' "$1" >&2
-    exit 1
-}
-
-# Initialize all the option variables.
-# This ensures we are not contaminated by variables from the environment.
 scenario=
 
-while :; do
-    case $1 in
-	--scenario)
-	    if [ "$2" ]; then
-		scenario=$2
+if [[ $1 == --scenario ]]; then
+    if [[ $2 ]]; then
+        scenario=$2
 		shift
-	    else
-		die 'ERROR: "--scenario" requires a non-empty option argument.'
-	    fi
-	    ;;
-	-?*)
-	    printf 'WARN: Unknown option (ignored): %s\n' "$1" >&2
-	    ;;
-	*)
-	    break
-    esac
-
-    shift
-done
-
-if [[ -z $scenario ]]; then
-    die 'ERROR: "--scenario" is required'
+        shift
+    else
+        >&2 echo 'Error: "--scenario" requires an argument.'
+        exit 1
+    fi
 fi
 
 numCPUs=${numCPUs:-$(nproc)}
@@ -108,7 +91,7 @@ debug() {
 }
 
 # Run the test by building the test derivation
-build() {
+buildTest() {
     vmTestNixExpr | nix-build --no-out-link "$@" -
 }
 
@@ -137,4 +120,18 @@ vmTestNixExpr() {
 EOF
 }
 
+build() {
+    if [[ $scenario ]]; then
+        buildTest "$@"
+    else
+        scenario=default buildTest "$@"
+        scenario=withnetns buildTest "$@"
+    fi
+}
+
+# Set default scenario for all actions other than 'build'
+if [[ $1 && $1 != build ]]; then
+    : ${scenario:=default}
+fi
+
 eval "${@:-build}"
diff --git a/test/test.nix b/test/test.nix
index c54312e..5de981b 100644
--- a/test/test.nix
+++ b/test/test.nix
@@ -1,5 +1,6 @@
 # Integration test, can be run without internet access.
 
+# Make sure to update build() in ./run-tests.sh when adding new scenarios
 { scenario ? "default" }:
 
 import ./make-test.nix rec {

From df790f67661cf9e83c39701f7c8e72de0e829d8e Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:14 +0200
Subject: [PATCH 24/25] run-tests: allow linking test build results for all
 scenarios

---
 test/run-tests.sh | 49 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/test/run-tests.sh b/test/run-tests.sh
index 9099d09..52d7609 100755
--- a/test/run-tests.sh
+++ b/test/run-tests.sh
@@ -10,8 +10,8 @@
 #   Test specific scenario
 #   ./run-tests.sh --scenario <scenario>
 #
-#   Run test and save result to avoid garbage collection
-#   ./run-tests.sh [--scenario <scenario>] build --out-link /tmp/nix-bitcoin-test
+#   Run test and link results to avoid garbage collection
+#   ./run-tests.sh [--scenario <scenario>] --out-link-prefix /tmp/nix-bitcoin-test build
 #
 #   Run interactive test debugging
 #   ./run-tests.sh [--scenario <scenario>] debug
@@ -22,17 +22,33 @@
 set -eo pipefail
 
 scenario=
-
-if [[ $1 == --scenario ]]; then
-    if [[ $2 ]]; then
-        scenario=$2
-		shift
-        shift
-    else
-        >&2 echo 'Error: "--scenario" requires an argument.'
-        exit 1
-    fi
-fi
+outLinkPrefix=
+while :; do
+    case $1 in
+        --scenario|-s)
+            if [[ $2 ]]; then
+                scenario=$2
+                shift
+                shift
+            else
+                >&2 echo 'Error: "$1" requires an argument.'
+                exit 1
+            fi
+            ;;
+        --out-link-prefix|-o)
+            if [[ $2 ]]; then
+                outLinkPrefix=$2
+                shift
+                shift
+            else
+                >&2 echo 'Error: "$1" requires an argument.'
+                exit 1
+            fi
+            ;;
+        *)
+            break
+    esac
+done
 
 numCPUs=${numCPUs:-$(nproc)}
 # Min. 800 MiB needed to avoid 'out of memory' errors
@@ -92,7 +108,12 @@ debug() {
 
 # Run the test by building the test derivation
 buildTest() {
-    vmTestNixExpr | nix-build --no-out-link "$@" -
+    if [[ $outLinkPrefix ]]; then
+        buildArgs="--out-link $outLinkPrefix-$scenario"
+    else
+        buildArgs=--no-out-link
+    fi
+    vmTestNixExpr | nix-build $buildArgs "$@" -
 }
 
 # On continuous integration nodes there are few other processes running alongside the

From e5fb3f6a7fb0f32ec036d2eebfc239528b0345af Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Fri, 21 Aug 2020 22:36:15 +0200
Subject: [PATCH 25/25] run-tests: document how to pass extra build args

---
 test/run-tests.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/run-tests.sh b/test/run-tests.sh
index 52d7609..7e42465 100755
--- a/test/run-tests.sh
+++ b/test/run-tests.sh
@@ -13,6 +13,9 @@
 #   Run test and link results to avoid garbage collection
 #   ./run-tests.sh [--scenario <scenario>] --out-link-prefix /tmp/nix-bitcoin-test build
 #
+#   Pass extra args to nix-build
+#   ./run-tests.sh build --builders 'ssh://mybuildhost - - 15'
+#
 #   Run interactive test debugging
 #   ./run-tests.sh [--scenario <scenario>] debug
 #