Merge #217: Concurrent LN

e650df30d5 bitcoind: bump rpcthread count (nixbitcoin)
46e15ee9cc tests: make lnd & clightning tests run concurrently (nixbitcoin)
ac96fd59db assertions: make lnd.enable depend on !clightning.enable or port != 9735 (nixbitcoin)
3ed564ea06 lnd: make listen IP address only (nixbitcoin)
716e98789c lnd: add listenPort option (nixbitcoin)
43da15557d clightning: refactor bind-addr to be IP address only (nixbitcoin)
d99ccc8445 clightning: add bindport option (nixbitcoin)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK e650df30d5

Tree-SHA512: 5c8c2cdd41cd57c60fc91d5752190b7ff905041b09cda32d60d1790960321a86ea5e9e1f7b4519198bcb28372034f86362778d1b960369a23d24c29d0c2ecccf

examples/configuration.nix

@@ -44,8 +44,10 @@
   # services.clightning.announce-tor = true;
 
   ### LND
-  # Disable clightning and uncomment the following line in order to enable lnd,
+  # Uncomment the following line in order to enable lnd, a lightning
-  # a lightning implementation written in Go.
+  # implementation written in Go. In order to avoid collisions with clightning
+  # you must disable clightning or change the services.clightning.bindport or
+  # services.lnd.listenPort to a port other than 9735.
   # services.lnd.enable = true;
   # Enable this option to announce our Tor Hidden Service. By default lnd
   # offers outgoing functionality, but doesn't announce the Tor Hidden Service

modules/bitcoind.nix

@@ -27,6 +27,7 @@ let
     ${lib.concatMapStrings (node: "addnode=${node}\n") cfg.addnodes}
 
     # RPC server options
+    ${optionalString (cfg.rpcthreads != null) "rpcthreads=${toString cfg.rpcthreads}"}
     rpcport=${toString cfg.rpc.port}
     rpcwhitelistdefault=0
     ${concatMapStringsSep  "\n"
@@ -66,7 +67,6 @@ in {
         default = "";
         example = ''
           par=16
-          rpcthreads=16
           logips=1
         '';
         description = "Additional configurations to be appended to <filename>bitcoin.conf</filename>.";
@@ -140,6 +140,11 @@ in {
           '';
         };
       };
+      rpcthreads = mkOption {
+        type = types.nullOr types.ints.u16;
+        default = null;
+        description = "Set the number of threads to service RPC calls";
+      };
       rpcbind = mkOption {
         type = types.listOf types.str;
         default = [ "127.0.0.1" ];

modules/clightning.nix

@@ -11,7 +11,7 @@ let
     bitcoin-datadir=${config.services.bitcoind.dataDir}
     ${optionalString (cfg.proxy != null) "proxy=${cfg.proxy}"}
     always-use-proxy=${if cfg.always-use-proxy then "true" else "false"}
-    ${optionalString (cfg.bind-addr != null) "bind-addr=${cfg.bind-addr}"}
+    ${optionalString (cfg.bind-addr != null) "bind-addr=${cfg.bind-addr}:${toString cfg.bindport}"}
     ${optionalString (cfg.bitcoin-rpcconnect != null) "bitcoin-rpcconnect=${cfg.bitcoin-rpcconnect}"}
     bitcoin-rpcuser=${config.services.bitcoind.rpc.users.public.name}
     rpc-file-mode=0660
@@ -46,10 +46,15 @@ in {
       '';
     };
     bind-addr = mkOption {
-      type = types.nullOr types.str;
+      type = types.addCheck types.str (s: builtins.length (builtins.split ":" s) == 1);
-      default = null;
+      default = "127.0.0.1";
       description = "Set an IP address or UNIX domain socket to listen to";
     };
+    bindport = mkOption {
+      type = types.port;
+      default = 9735;
+      description = "Set a Port to listen to locally";
+    };
     announce-tor = mkOption {
       type = types.bool;
       default = false;

modules/lnd.nix

@@ -14,7 +14,7 @@ let
     tlscertpath=${secretsDir}/lnd-cert
     tlskeypath=${secretsDir}/lnd-key
 
-    listen=${toString cfg.listen}
+    listen=${toString cfg.listen}:${toString cfg.listenPort}
     ${lib.concatMapStrings (rpclisten: "rpclisten=${rpclisten}:${toString cfg.rpcPort}\n") cfg.rpclisten}
     ${lib.concatMapStrings (restlisten: "restlisten=${restlisten}:${toString cfg.restPort}\n") cfg.restlisten}
 
@@ -47,10 +47,15 @@ in {
       description = "The data directory for LND.";
     };
     listen = mkOption {
-      type = types.str;
+      type = types.addCheck types.str (s: builtins.length (builtins.split ":" s) == 1);
       default = "localhost";
       description = "Bind to given address to listen to peer connections";
     };
+    listenPort = mkOption {
+      type = types.port;
+      default = 9735;
+      description = "Bind to given port to listen to peer connections";
+    };
     rpclisten = mkOption {
       type = types.listOf types.str;
       default = [ "localhost" ];

modules/modules.nix

@@ -30,10 +30,11 @@
 
   config = {
     assertions = [
-      # lnd.wantedBy == [] needed for `test/tests.nix` in which both clightning and lnd are enabled
+      { assertion = (config.services.lnd.enable -> ( !config.services.clightning.enable || config.services.clightning.bindport != config.services.lnd.listenPort));
-      { assertion = config.services.lnd.enable -> (!config.services.clightning.enable || config.systemd.services.lnd.wantedBy == []);
         message = ''
-          LND and clightning can't be run in parallel because they both bind to lightning port 9735.
+          LND and clightning can't both bind to lightning port 9735. Either
+          disable LND/clightning or change services.clightning.bindPort or
+          services.lnd.listenPort to a port other than 9735.
         '';
       }
     ];

modules/netns-isolation.nix

@@ -70,8 +70,7 @@ in {
     };
   };
 
-  config = mkMerge [
+  config = mkIf cfg.enable {
-    (mkIf cfg.enable {
     # Prerequisites
     networking.dhcpcd.denyInterfaces = [ "br0" "br-nb*" "nb-veth*" ];
     services.tor.client.socksListenAddress = "${bridgeIp}:9050";
@@ -227,7 +226,7 @@ in {
     # clightning: Custom netns configs
     services.clightning = mkIf config.services.clightning.enable {
       bitcoin-rpcconnect = netns.bitcoind.address;
-        bind-addr = "${netns.clightning.address}:${toString config.services.clightning.onionport}";
+      bind-addr = netns.clightning.address;
     };
 
     # lnd: Custom netns configs
@@ -303,11 +302,5 @@ in {
         netns-exec nb-lightning-loop sudo -u lnd ${config.services.lightning-loop.package}/bin/loop "$@"
       '';
     };
-    })
+  };
-    # Custom netns config option values if netns-isolation not enabled
-    (mkIf (!cfg.enable) {
-      # clightning
-      services.clightning.bind-addr = "127.0.0.1:${toString config.services.clightning.onionport}";
-    })
-  ];
 }

modules/presets/secure-node.nix

@@ -73,6 +73,9 @@ in {
       discover = false;
       addresstype = "bech32";
       dbCache = 1000;
+      # higher rpcthread count due to reports that lightning implementations fail
+      # under high bitcoind rpc load
+      rpcthreads = 16;
       rpc.users.privileged = {
         name = "bitcoinrpc";
         # Placeholder to be sed'd out by bitcoind preStart
@@ -152,14 +155,18 @@ in {
       enforceTor = true;
       always-use-proxy = true;
     };
-    services.tor.hiddenServices.clightning = mkIf cfg.clightning.enable (mkHiddenService { port = cfg.clightning.onionport; toHost = (builtins.head (builtins.split ":" cfg.clightning.bind-addr)); });
+    services.tor.hiddenServices.clightning = mkIf cfg.clightning.enable (mkHiddenService {
+      port = cfg.clightning.onionport;
+      toHost = cfg.clightning.bind-addr;
+      toPort = cfg.clightning.bindport;
+    });
 
     # lnd
     services.lnd = {
       tor-socks = cfg.tor.client.socksListenAddress;
       enforceTor = true;
     };
-    services.tor.hiddenServices.lnd = mkIf cfg.lnd.enable (mkHiddenService { port = cfg.lnd.onionport; toHost = cfg.lnd.listen; });
+    services.tor.hiddenServices.lnd = mkIf cfg.lnd.enable (mkHiddenService { port = cfg.lnd.onionport; toHost = cfg.lnd.listen; toPort = cfg.lnd.listenPort; });
 
     # lightning-loop
     services.lightning-loop = {

test/scenarios/default.py

@@ -25,7 +25,7 @@ def web_index():
     assert_matches("curl -L localhost/store", "tshirt")
 
 
-def post_clightning():
+def final():
     pass
 
 
@@ -35,7 +35,7 @@ extra_tests = {
     "lightning-charge": lightning_charge,
     "nanopos": nanopos,
     "web-index": web_index,
-    "post-clightning": post_clightning,
+    "final": final,
 }
 
 run_tests(extra_tests)

test/scenarios/lib.py

@@ -70,6 +70,18 @@ def run_tests(extra_tests):
     assert_running("clightning")
     assert_matches("su operator -c 'lightning-cli getinfo' | jq", '"id"')
 
+    assert_running("lnd")
+    assert_matches("su operator -c 'lncli getinfo' | jq", '"version"')
+    assert_no_failure("lnd")
+
+    succeed("systemctl start lightning-loop")
+    assert_matches("su operator -c 'loop --version'", "version")
+    # Check that lightning-loop fails with the right error, making sure
+    # lightning-loop can connect to lnd
+    machine.wait_until_succeeds(
+        log_has_string("lightning-loop", "chain notifier RPC isstill in the process of starting")
+    )
+
     assert_running("spark-wallet")
     extra_tests.pop("spark-wallet")()
 
@@ -104,7 +116,9 @@ def run_tests(extra_tests):
     pre_restart = succeed("date +%s.%6N").rstrip()
 
     # Sanity-check system by restarting all services
-    succeed("systemctl restart bitcoind clightning spark-wallet lightning-charge nanopos liquidd")
+    succeed(
+        "systemctl restart bitcoind clightning lnd lightning-loop spark-wallet lightning-charge nanopos liquidd"
+    )
 
     # Now that the bitcoind restart triggered a banlist import restart, check that
     # re-importing already banned addresses works
@@ -113,25 +127,7 @@ def run_tests(extra_tests):
     )
     assert_no_failure("bitcoind-import-banlist")
 
-    extra_tests.pop("post-clightning")()
+    extra_tests.pop("final")()
-
-    ### Test lnd
-
-    stopped_services = "nanopos lightning-charge spark-wallet clightning"
-    succeed("systemctl stop " + stopped_services)
-    succeed("systemctl start lnd")
-    assert_matches("su operator -c 'lncli getinfo' | jq", '"version"')
-    assert_no_failure("lnd")
-
-    ### Test loopd
-
-    succeed("systemctl start lightning-loop")
-    assert_matches("su operator -c 'loop --version'", "version")
-    # Check that lightning-loop fails with the right error, making sure
-    # lightning-loop can connect to lnd
-    machine.wait_until_succeeds(
-        log_has_string("lightning-loop", "chain notifier RPC isstill in the process of starting")
-    )
 
     ### Check that all extra_tests have been run
     assert len(extra_tests) == 0

test/scenarios/withnetns.py

@@ -9,6 +9,7 @@ lightningcharge_ip = "169.254.1.18"
 nanopos_ip = "169.254.1.19"
 recurringdonations_ip = "169.254.1.20"
 nginx_ip = "169.254.1.21"
+lightningloop_ip = "169.254.1.22"
 
 
 def electrs():
@@ -46,7 +47,7 @@ def web_index():
     assert_matches("ip netns exec nb-nginx curl -L localhost/store", "tshirt")
 
 
-def post_clightning():
+def final():
     ping_bitcoind = "ip netns exec nb-bitcoind ping -c 1 -w 1"
     ping_nanopos = "ip netns exec nb-nanopos ping -c 1 -w 1"
 
@@ -54,6 +55,7 @@ def post_clightning():
     machine.succeed(
         "%s %s &&" % (ping_bitcoind, bitcoind_ip)
         + "%s %s &&" % (ping_bitcoind, clightning_ip)
+        + "%s %s &&" % (ping_bitcoind, lnd_ip)
         + "%s %s &&" % (ping_bitcoind, liquidd_ip)
         + "%s %s &&" % (ping_nanopos, lightningcharge_ip)
         + "%s %s &&" % (ping_nanopos, nanopos_ip)
@@ -63,6 +65,7 @@ def post_clightning():
     # Negative ping tests (non-exhaustive)
     machine.fail(
         "%s %s ||" % (ping_bitcoind, sparkwallet_ip)
+        + "%s %s ||" % (ping_bitcoind, lightningloop_ip)
         + "%s %s ||" % (ping_bitcoind, lightningcharge_ip)
         + "%s %s ||" % (ping_bitcoind, nanopos_ip)
         + "%s %s ||" % (ping_bitcoind, recurringdonations_ip)
@@ -70,6 +73,7 @@ def post_clightning():
         + "%s %s ||" % (ping_nanopos, bitcoind_ip)
         + "%s %s ||" % (ping_nanopos, clightning_ip)
         + "%s %s ||" % (ping_nanopos, lnd_ip)
+        + "%s %s ||" % (ping_nanopos, lightningloop_ip)
         + "%s %s ||" % (ping_nanopos, liquidd_ip)
         + "%s %s ||" % (ping_nanopos, electrs_ip)
         + "%s %s ||" % (ping_nanopos, sparkwallet_ip)
@@ -94,7 +98,7 @@ extra_tests = {
     "lightning-charge": lightning_charge,
     "nanopos": nanopos,
     "web-index": web_index,
-    "post-clightning": post_clightning,
+    "final": final,
 }
 
 run_tests(extra_tests)

test/test.nix

@@ -33,8 +33,10 @@ import ./make-test.nix rec {
     services.nanopos.enable = true;
 
     services.lnd.enable = true;
-    systemd.services.lnd.wantedBy = mkForce [];
+    services.lnd.listenPort = 9736;
     services.lightning-loop.enable = true;
+    # needed because we must control when lightning-loop starts so it doesn't
+    # fail before we run commands in the nb-lightning-loop netns
     systemd.services.lightning-loop.wantedBy = mkForce [];
 
     services.electrs.enable = true;