Merge #621: Trustedcoin fixes

4e83697 dev: add trustedcoin (Erik Arvstedt) ac59f93 trustedcoin: add regtest support, reenable test (Erik Arvstedt) aa41886 tests/trustedcoin: extract fn (Erik Arvstedt) 6de5029 tests/trustedcoin: refactor (Erik Arvstedt) 53ea447 trustedcoin: add option `tor.proxy` (Erik Arvstedt) 31b76f1 clightning: add option `useBcliPlugin` (Erik Arvstedt) f3e9c64 clightning-plugins/trustedcoin: improve docs (Erik Arvstedt) 7640c69 tests/clightning-plugins: enable active plugin test for `trustedcoin` (Erik Arvstedt) 57455eb tests/clightning-plugins: set actual `clboss` pkg path (Erik Arvstedt) dca6813 tests/clightning-plugins: extract variable (Erik Arvstedt) 858ab1c clightning: revert whitespace changes (Erik Arvstedt) Pull request description: ACKs for top commit: jonasnick: ACK 4e83697 Tree-SHA512: 4abf6f2e6ba51b39351f8c2ef97c6afc7c2e686da41b7a6bb4a8d6e89bc9f829ca53109efb800d7b26d360319271bcc8beafc5ddfe133668cf412790d645682b
fort-nix · Aug 5, 2023 · 9d99256 · 9d99256
2 parents 2e53fd2 + 4e83697
commit 9d99256
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 50 deletions.
diff --git a/README.md b/README.md
@@ -79,7 +79,7 @@ NixOS modules ([src](modules/modules.nix))
     * [prometheus](https://github.com/lightningd/plugins/tree/master/prometheus): lightning node exporter for the prometheus timeseries server
     * [rebalance](https://github.com/lightningd/plugins/tree/master/rebalance): keeps your channels balanced
     * [summary](https://github.com/lightningd/plugins/tree/master/summary): print a nice summary of the node status
-    * [trustedcoin](https://github.com/nbd-wtf/trustedcoin) [[experimental](docs/services.md#trustedcoin-hints)]: replaces bitcoind with trusted public explorers
+    * [trustedcoin](https://github.com/nbd-wtf/trustedcoin) ([experimental](docs/services.md#trustedcoin)): replaces bitcoind with trusted public explorers
     * [zmq](https://github.com/lightningd/plugins/tree/master/zmq): publishes notifications via ZeroMQ to configured endpoints
   * [clightning-rest](https://github.com/Ride-The-Lightning/c-lightning-REST): REST server for clightning
   * [lnd](https://github.com/lightningnetwork/lnd) with support for announcing an onion service and [static channel backups](https://github.com/lightningnetwork/lnd/blob/master/docs/recovery.md)

diff --git a/dev/dev-features.sh b/dev/dev-features.sh
@@ -279,6 +279,15 @@ c curl 127.0.0.1:62601
 nix run --inputs-from . nixpkgs#lynx -- --dump $ip:62601
 c curl -s 127.0.0.1:62601 | grep -i "orders found"
 
+#―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
+# trustedcoin
+run-tests.sh -s trustedcoin-online container
+
+c systemctl start clightning
+c journalctl -u clightning -f
+# This should show log msgs like
+# plugin-trustedcoin returning block 801409, 0000000000000000000482ddc4…, 1483968 bytes
+
 #―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
 # rtl
 # see ./topics/rtl.sh
diff --git a/dev/dev-scenarios.nix b/dev/dev-scenarios.nix
@@ -75,4 +75,23 @@ with lib;
     };
     nix-bitcoin.nodeinfo.enable = true;
   };
+
+  trustedcoin-online = {
+    services.clightning = {
+      enable = true;
+      tor.proxy = true;
+      plugins.trustedcoin.enable = true;
+      plugins.trustedcoin.tor.proxy = false;
+    };
+
+    # Don't run clightning on startup.
+    # This breaks the follwing dependency cycle:
+    #   clightning
+    #   -> network (trustedcoin fails and exits clightning without network access)
+    #   -> multi-user.target (NixOS containers only gain network access after multi-user.target has completed)
+    #   -> clightning
+    systemd.services.clightning.wantedBy = mkForce [];
+
+    test.container.enableWAN = true;
+  };
 }
diff --git a/docs/services.md b/docs/services.md
@@ -579,26 +579,18 @@ services.clightning = {
 
 Please have a look at the module for a plugin (e.g. [prometheus.nix](../modules/clightning-plugins/prometheus.nix)) to learn its configuration options.
 
-### Trustedcoin hints
-The [trustedcoin](https://github.com/nbd-wtf/trustedcoin) plugin use a Tor
-proxy for all of its external connections by default. That's why you can
-sometimes face issues with your connections to esploras getting blocked.
-
-An example of clightning log error output in a case your connections are getting blocked:
+### Trustedcoin
+When `services.clightning.tor.proxy` is enabled, [trustedcoin](https://github.com/nbd-wtf/trustedcoin)
+also uses Tor for all external connections by default.
+In this case, connections to block explorers can sometimes get blocked.
 
+An example of clightning log error output when connections are getting blocked:
 ```
 lightningd[5138]: plugin-trustedcoin estimatefees error: https://blockstream.info/api error: 403 Forbidden
+lightningd[4933]: plugin-trustedcoin getblock error: got something that isn't a block hash: <html><head>...
 ```
 
-```
-lightningd[4933]: plugin-trustedcoin getblock error: got something that isn't a block hash: <html><head>
-lightningd[4933]: <meta http-equiv="content-type" content="text/html;
-```
-
-If you face these issues and you still need to use trustedcoin, use can disable
-clightning's tor hardening by setting this option in your `configuration.nix`
-file:
-
-```
-services.clightning.tor.enforce = false;
+To work around this and connect via clearnet instead, set this option:
+```nix
+services.clightning.plugins.trustedcoin.tor.proxy = false;
 ```
diff --git a/modules/clightning-plugins/trustedcoin.nix b/modules/clightning-plugins/trustedcoin.nix
@@ -5,24 +5,36 @@ let cfg = config.services.clightning.plugins.trustedcoin; in
 {
   options.services.clightning.plugins.trustedcoin = {
     enable = mkEnableOption "Trustedcoin (clightning plugin)";
+
     package = mkOption {
       type = types.package;
       default = config.nix-bitcoin.pkgs.trustedcoin;
       defaultText = "config.nix-bitcoin.pkgs.trustedcoin";
       description = mdDoc "The package providing trustedcoin binaries.";
     };
+
+    tor.proxy = mkOption {
+      type = types.bool;
+      default = config.services.clightning.tor.proxy;
+      description = mdDoc "Whether to proxy outgoing connections with Tor.";
+    };
   };
 
   config = mkIf cfg.enable {
-    services.clightning.extraConfig = ''
-      plugin=${cfg.package}/bin/trustedcoin
-      disable-plugin=bcli
-    '';
+    services.clightning = {
+      useBcliPlugin = false;
+      extraConfig = ''
+        plugin=${cfg.package}/bin/trustedcoin
+      '';
+      tor.enforce = mkIf (!cfg.tor.proxy) false;
+    };
 
-    # Trustedcoin does not honor the clightning's proxy configuration.
-    # Ref.: https://github.com/nbd-wtf/trustedcoin/pull/19
-    systemd.services.clightning.environment = mkIf (config.services.clightning.proxy != null) {
-      HTTPS_PROXY = "socks5://${config.services.clightning.proxy}";
+    systemd.services.clightning.environment = mkIf (cfg.tor.proxy) {
+      HTTPS_PROXY = let
+        clnProxy = config.services.clightning.proxy;
+        proxy = if clnProxy != null then clnProxy else config.nix-bitcoin.torClientAddressWithPort;
+      in
+        "socks5://${proxy}";
     };
   };
 }
diff --git a/modules/clightning.nix b/modules/clightning.nix
@@ -49,6 +49,15 @@ let
         parameters, as fully qualified data source name.
       '';
     };
+    useBcliPlugin = mkOption {
+      type = types.bool;
+      default = true;
+      description = ''
+        Use bitcoind (via plugin `bcli`) for getting block data.
+        This option is disabled by plugins that use other sources for
+        fetching block data, like `trustedcoin`.
+      '';
+    };
     extraConfig = mkOption {
       type = types.lines;
       default = "";
@@ -107,15 +116,19 @@ let
   network = bitcoind.makeNetworkName "bitcoin" "regtest";
   configFile = pkgs.writeText "config" ''
     network=${network}
-    ${optionalString (!cfg.plugins.trustedcoin.enable) "bitcoin-datadir=${bitcoind.dataDir}"}
+    ${
+      if cfg.useBcliPlugin then ''
+        bitcoin-datadir=${config.services.bitcoind.dataDir}
+      '' else ''
+        disable-plugin=bcli
+      ''
+    }
     ${optionalString (cfg.proxy != null) "proxy=${cfg.proxy}"}
     always-use-proxy=${boolToString cfg.always-use-proxy}
     bind-addr=${cfg.address}:${toString cfg.port}
-
     bitcoin-rpcconnect=${nbLib.address bitcoind.rpc.address}
     bitcoin-rpcport=${toString bitcoind.rpc.port}
     bitcoin-rpcuser=${bitcoind.rpc.users.public.name}
-
     rpc-file-mode=0660
     log-timestamps=false
     ${optionalString (cfg.wallet != null) "wallet=${cfg.wallet}"}
@@ -163,7 +176,6 @@ in {
         {
           cat ${configFile}
           echo "bitcoin-rpcpassword=$(cat ${config.nix-bitcoin.secretsDir}/bitcoin-rpcpassword-public)"
-
           ${optionalString (cfg.getPublicAddressCmd != "") ''
             echo "announce-addr=$(${cfg.getPublicAddressCmd}):${toString publicPort}"
           ''}

diff --git a/pkgs/trustedcoin/default.nix b/pkgs/trustedcoin/default.nix
@@ -1,15 +1,25 @@
-{ lib, buildGoModule, fetchFromGitHub }:
+{ lib, buildGoModule, fetchFromGitHub, fetchpatch }:
 
 buildGoModule rec {
   pname = "trustedcoin";
   version = "0.6.1";
+
   src = fetchFromGitHub {
     owner = "nbd-wtf";
     repo = pname;
     rev = "v${version}";
-    sha256 = "sha256-UNQjxhAT0mK1In7vUtIoMoMNBV+0wkrwbDmm7m+0R3o=";
+    hash = "sha256-UNQjxhAT0mK1In7vUtIoMoMNBV+0wkrwbDmm7m+0R3o=";
   };
 
+  patches = [
+    # https://github.com/nbd-wtf/trustedcoin/pull/22 required for regtest
+    (fetchpatch {
+      name = "add-regtest-support";
+      url = "https://github.com/nbd-wtf/trustedcoin/commit/aba05c55ccbfc50785328f556be8a5bd46e76beb.patch";
+      hash = "sha256-24mYyXjUMVSlr9IlaqaTVAPE6bxxScNgR8Bb3x2t90Y=";
+    })
+  ];
+
   vendorSha256 = "sha256-xvkK9rMQlXTnNyOMd79qxVSvhgPobcBk9cq4/YWbupY=";
 
   subPackages = [ "." ];

diff --git a/test/run-tests.sh b/test/run-tests.sh
@@ -295,11 +295,7 @@ buildable=(
     clightning-replication
     lndPruned
     wireguard-lndconnect
-
-    # TODO-EXTERNAL:
-    # Reenable when this has been fixed:
-    # https://github.com/nbd-wtf/trustedcoin/issues/21
-    # trustedcoin
+    trustedcoin
 )
 buildable() { buildTests buildable "$@"; }
 

diff --git a/test/tests.nix b/test/tests.nix
@@ -39,18 +39,23 @@ let
 
       tests.clightning = cfg.clightning.enable;
       test.data.clightning-replication = cfg.clightning.replication.enable;
+      tests.trustedcoin = cfg.clightning.plugins.trustedcoin.enable;
 
       # TODO-EXTERNAL:
       # When WAN is disabled, DNS bootstrapping slows down service startup by ~15 s.
       services.clightning.extraConfig = mkIf config.test.noConnections "disable-dns";
       test.data.clightning-plugins = let
         plugins = config.services.clightning.plugins;
-        removed = [ "commando" "trustedcoin" ];
-        enabled = builtins.filter (plugin: plugins.${plugin}.enable)
-                                  (subtractLists removed (builtins.attrNames plugins));
+        removed = [
+          # Only defined via `obsolete-options.nix`
+          "commando"
+        ];
+        available = subtractLists removed (builtins.attrNames plugins);
+        enabled = builtins.filter (plugin: plugins.${plugin}.enable) available;
         nbPkgs = config.nix-bitcoin.pkgs;
         pluginPkgs = nbPkgs.clightning-plugins // {
-          clboss.path = "${nbPkgs.clboss}/bin/clboss";
+          clboss.path = "${plugins.clboss.package}/bin/clboss";
+          trustedcoin.path = "${plugins.trustedcoin.package}/bin/trustedcoin";
         };
       in map (plugin: pluginPkgs.${plugin}.path) enabled;
 
@@ -313,9 +318,9 @@ let
       services.bitcoind.prune = 1000;
     };
 
-    # Test the special clightning setup where trustedcoin plugin is used
     trustedcoin = {
-      tests.trustedcoin = true;
+      imports = [ scenarios.regtestBase ];
+
       services.clightning = {
         enable = true;
         plugins.trustedcoin.enable = true;

diff --git a/test/tests.py b/test/tests.py
@@ -429,14 +429,16 @@ def get_block_height(ip, port):
 
 @test("trustedcoin")
 def _():
-    machine.wait_for_unit("bitcoind")
-    machine.wait_for_unit("clightning")
+    def expect_clightning_log(str):
+        machine.wait_until_succeeds(log_has_string("clightning", str))
 
-    # Let's check the trustedcoin plugin was correctly initialized
-    machine.wait_until_succeeds(log_has_string("clightning", "plugin-trustedcoin[^^]\[0m\s+initialized plugin"))
-    machine.wait_until_succeeds(log_has_string("clightning", "plugin-trustedcoin[^^]\[0m\s+bitcoind RPC working"))
-    machine.wait_until_succeeds(log_has_string("clightning", "plugin-trustedcoin[^^]\[0m\s+tip: 0"))
-    machine.wait_until_succeeds(log_has_string("clightning", "plugin-trustedcoin[^^]\[0m\s+estimatefees error: none of the esploras returned usable responses"))
+    machine.wait_for_unit("clightning")
+    expect_clightning_log("plugin-trustedcoin[^^]\[0m\s+bitcoind RPC working")
+    expect_clightning_log("plugin-trustedcoin[^^]\[0m\s+estimatefees error: none of the esploras returned usable responses")
+    if "regtest" in enabled_tests:
+        num_blocks = test_data["num_blocks"]
+        expect_clightning_log(f"plugin-trustedcoin[^^]\[0m\s+tip: {num_blocks}")
+        expect_clightning_log("plugin-trustedcoin[^^]\[0m\s+returning block")
 
 
 if "netns-isolation" in enabled_tests: