Merge fort-nix/nix-bitcoin#621: Trustedcoin fixes

4e8369705d dev: add trustedcoin (Erik Arvstedt) ac59f93e63 trustedcoin: add regtest support, reenable test (Erik Arvstedt) aa418869b3 tests/trustedcoin: extract fn (Erik Arvstedt) 6de5029c49 tests/trustedcoin: refactor (Erik Arvstedt) 53ea447ab7 trustedcoin: add option `tor.proxy` (Erik Arvstedt) 31b76f1ffe clightning: add option `useBcliPlugin` (Erik Arvstedt) f3e9c644e3 clightning-plugins/trustedcoin: improve docs (Erik Arvstedt) 7640c69d79 tests/clightning-plugins: enable active plugin test for `trustedcoin` (Erik Arvstedt) 57455eb897 tests/clightning-plugins: set actual `clboss` pkg path (Erik Arvstedt) dca6813d6b tests/clightning-plugins: extract variable (Erik Arvstedt) 858ab1cfd6 clightning: revert whitespace changes (Erik Arvstedt) Pull request description: ACKs for top commit: jonasnick: ACK 4e8369705d Tree-SHA512: 4abf6f2e6ba51b39351f8c2ef97c6afc7c2e686da41b7a6bb4a8d6e89bc9f829ca53109efb800d7b26d360319271bcc8beafc5ddfe133668cf412790d645682b
2023-08-05 20:09:27 +00:00
parent 2e53fd2e51 4e8369705d
commit 9d9925621a
10 changed files with 107 additions and 50 deletions
--- 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)
--- 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
--- 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;
+  };
 }
--- 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;
 ```
--- 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}";
    };
  };
 }
--- 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}"
          ''}
--- 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 = [ "." ];
--- 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 "$@"; }

--- 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;
--- a/test/tests.py
+++ b/test/tests.py
@@ -429,14 +429,16 @@ def _():

@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: