Merge #2321 #2334

2321: Add GitHub Action CI Workflow r=mergify[bot] a=bonomat

Resolves #1407 

This PR introduces GitHub Action CI: 

Within this PR we execute the following targets: 
* `make build` on MacOS
* `make build` on Ubuntu
* `make test` on Ubuntu
* `make e2e` on Ubuntu

Note: 
I've added a new make target `ci_gha` (CI GitHubActions) so that we can keep circleCI running in parallel for some time. Once we decide to completely get rid of CircleCI we can remove/rename this field. I'll create a follow-up ticket on this

Not in this PR:
* `make test` on MacOS as GitHub Action MacOS does not provide Docker. Aperantly this will be provided in the future though. If so, we can run tests on MacOS as well (see here: actions/runner-images#17 (comment))
* `make build/test` on Windows as we do not have an implementation for `lnd_default_dir()` for windows (see here: https://github.com/comit-network/comit-rs/blob/93f58a1762c7c7a8da038a1ec2f22dccea1a21cb/cnd/src/lib.rs#L108-L118). I will track this in another ticket once this PR is merged.

Open TODOs:
* create ticket to fix windows build for github actions:
* create ticket to remove unneeded make target

2334: Ensure default is applied if lnd settings are not present r=D4nte a=D4nte

The `Settings` struct need to always have a the lightning settings set even if not present in the file to allow the initialisation of a lnd connector in `main.rs`.

Also, we use `reqwest` to target the lnd REST API so best to have a URL in the file to avoid conversion later down the stream.

Co-authored-by: Philipp Hoenisch <[email protected]>
Co-authored-by: Franck Royer <[email protected]>

.github/workflows/ci.yml

@@ -0,0 +1,128 @@
+name: CI
+
+on:
+  push:
+    branches-ignore:
+      - 'staging.tmp'
+      - 'trying.tmp'
+
+jobs:
+  static_analysis:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout sources
+        uses: actions/checkout@v1
+
+      - name: Extract toolchain version from rust-toolchain
+        run: echo "::set-env name=RUST_TOOLCHAIN::$(cat rust-toolchain)"
+
+      - name: Install ${{ env.RUST_TOOLCHAIN }} toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: ${{ env.RUST_TOOLCHAIN }}
+          override: true
+
+      - name: Cache ~/.cargo/bin directory
+        uses: actions/cache@v1
+        with:
+          path: ~/.cargo/bin
+          key: ubuntu-rust-${{ env.RUST_TOOLCHAIN }}-cargo-bin-directory
+
+      - name: Check formatting
+        run: make check_format
+
+      - name: Run linter
+        run: make clippy
+
+  build:
+    strategy:
+      matrix:
+        os: [macos-latest, ubuntu-latest]
+        include:
+          - os: ubuntu-latest
+            binary-suffix: ""
+          - os: macos-latest
+            binary-suffix: ""
+    runs-on: ${{ matrix.os }}
+    steps:
+      - name: Checkout sources
+        uses: actions/checkout@v1
+
+      - name: Extract toolchain version from rust-toolchain
+        run: echo "::set-env name=RUST_TOOLCHAIN::$(cat rust-toolchain)"
+
+      - name: Install ${{ env.RUST_TOOLCHAIN }} toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: ${{ env.RUST_TOOLCHAIN }}
+          override: true
+
+      - name: Cache target directory
+        uses: actions/cache@v1
+        with:
+          path: target
+          key: ${{ matrix.os }}-rust-${{ env.RUST_TOOLCHAIN }}-target-directory-${{ hashFiles('Cargo.lock') }}
+
+      - name: Cache ~/.cargo/registry directory
+        uses: actions/cache@v1
+        with:
+          path: ~/.cargo/registry
+          key: ${{ matrix.os }}-rust-${{ env.RUST_TOOLCHAIN }}-cargo-registry-directory-${{ hashFiles('Cargo.lock') }}
+
+      - name: Build ${{ matrix.os }} binary
+        run: make build
+
+      # Ignore tests on macos due to missing docker
+      - name: Run unit tests
+        if: matrix.os != 'macos-latest'
+        run: make test
+
+      - name: Upload cnd ${{ matrix.os }} binary
+        if: matrix.os == 'ubuntu-latest'
+        uses: actions/upload-artifact@v1
+        with:
+          name: cnd
+          path: target/debug/cnd
+
+
+  e2e_test:
+    runs-on: ubuntu-latest
+    needs: build
+    steps:
+      - name: Checkout sources
+        uses: actions/checkout@v1
+
+      - name: Download cnd binary
+        uses: actions/download-artifact@v1
+        with:
+          name: cnd
+          path: target/debug/
+
+      - name: Fix missing executable permission
+        run: |
+          chmod a+x target/debug/cnd
+
+      - name: Install NodeJS 12.x
+        uses: actions/setup-node@v1
+        with:
+          node-version: '12.x'
+
+      - name: Install Go 1.13.3.
+        uses: actions/setup-go@v1
+        with:
+          go-version: '1.13.3'
+
+      - name: Install LND v0.9.0-beta
+        run: |
+          go get -d github.com/lightningnetwork/lnd
+          cd ~/go/src/github.com/lightningnetwork/lnd
+          git checkout v0.9.0-beta
+          make tags=invoicesrpc
+          make tags=invoicesrpc install
+
+      - name: Run e2e tests
+        run: |
+          export PATH=$HOME/.cargo/bin:$HOME/.local/bin:$HOME/go/bin:$PATH
+          make ci_gha

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## Changed
+
+- Ensure that lnd parameters are defaulted if not present.
+
 ## [0.7.2] - 2020-03-26
 
 ## [0.7.1] - 2020-03-12

Makefile

@@ -85,6 +85,9 @@ doc:
 e2e: download_bc_nodes build
 	(cd ./api_tests; yarn install; yarn test)
 
+ci_gha: download_bc_nodes
+	(cd ./api_tests; yarn install; yarn ci)
+
 check_format: check_rust_format check_toml_format check_ts_format
 
 MODIFIED_FILES = $(shell git status --untracked-files=no --short)

api_tests/package.json

@@ -9,6 +9,7 @@
         "dry": "jest --config jest.config-dry.js --maxWorkers=4",
         "e2e": "yarn jest --config jest.config-e2e.js --runInBand --forceExit --bail",
         "test": "yarn dry && yarn e2e",
+        "ci": "tsc && yarn dry && yarn e2e",
         "fix": "tslint --project . --fix && prettier --write '**/*.{ts,js,json,yml}'"
     },
     "engines": {

cnd/src/config.rs

@@ -5,17 +5,14 @@ pub mod validation;
 
 use crate::swap_protocols::ledger::ethereum;
 use libp2p::Multiaddr;
+use reqwest::Url;
 use serde::{Deserialize, Serialize};
-use std::{
-    net::{IpAddr, Ipv4Addr, SocketAddr},
-    path::PathBuf,
-};
+use std::path::PathBuf;
 
 pub use self::{file::File, settings::Settings};
-use reqwest::Url;
 
 lazy_static::lazy_static! {
-    pub static ref LND_SOCKET: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080);
+    pub static ref LND_URL: Url = Url::parse("https://localhost:8080").expect("static string to be a valid url");
 }
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
@@ -37,7 +34,7 @@ pub struct Bitcoin {
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct Bitcoind {
-    pub node_url: reqwest::Url,
+    pub node_url: Url,
 }
 
 impl Default for Bitcoin {
@@ -90,35 +87,44 @@ impl Default for Ethereum {
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct Parity {
-    pub node_url: reqwest::Url,
+    pub node_url: Url,
 }
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct Lightning {
     pub network: bitcoin::Network,
-    pub lnd: Option<Lnd>,
+    pub lnd: Lnd,
 }
 
 impl Default for Lightning {
     fn default() -> Self {
         Self {
             network: bitcoin::Network::Regtest,
-            lnd: Some(Lnd::default()),
+            lnd: Lnd::default(),
+        }
+    }
+}
+
+impl From<Lightning> for file::Lightning {
+    fn from(lightning: Lightning) -> Self {
+        file::Lightning {
+            lnd: Some(lightning.lnd),
+            network: lightning.network,
         }
     }
 }
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct Lnd {
-    pub rest_api_socket: Option<SocketAddr>,
-    pub dir: Option<PathBuf>,
+    pub rest_api_url: Url,
+    pub dir: PathBuf,
 }
 
 impl Default for Lnd {
     fn default() -> Self {
         Self {
-            rest_api_socket: Some(*LND_SOCKET),
-            dir: Some(default_lnd_dir()),
+            rest_api_url: LND_URL.clone(),
+            dir: default_lnd_dir(),
         }
     }
 }
@@ -165,77 +171,40 @@ mod tests {
 
     #[test]
     fn lnd_deserializes_correctly() {
-        let file_contents = vec![
-            r#"
-            rest_api_socket = "127.0.0.1:8080"
-            dir = "~/.local/share/comit/lnd"
-            "#,
-            r#"
-            rest_api_socket = "127.0.0.1:8080"
-            "#,
+        let actual = toml::from_str(
             r#"
+            rest_api_url = "https://localhost:8080"
             dir = "~/.local/share/comit/lnd"
             "#,
-        ];
-
-        let expected = vec![
-            Lnd {
-                rest_api_socket: Some(*LND_SOCKET),
-                dir: Some(PathBuf::from("~/.local/share/comit/lnd")),
-            },
-            Lnd {
-                rest_api_socket: Some(*LND_SOCKET),
-                dir: None,
-            },
-            Lnd {
-                rest_api_socket: None,
-                dir: Some(PathBuf::from("~/.local/share/comit/lnd")),
-            },
-        ];
+        );
 
-        let actual = file_contents
-            .into_iter()
-            .map(toml::from_str)
-            .collect::<Result<Vec<Lnd>, toml::de::Error>>()
-            .unwrap();
+        let expected = Lnd {
+            rest_api_url: LND_URL.clone(),
+            dir: PathBuf::from("~/.local/share/comit/lnd"),
+        };
 
-        assert_eq!(actual, expected);
+        assert_eq!(actual, Ok(expected));
     }
 
     #[test]
     fn lightning_deserializes_correctly() {
-        let file_contents = vec![
-            r#"
-            network = "regtest"
-            "#,
+        let actual = toml::from_str(
             r#"
             network = "regtest"
             [lnd]
-            rest_api_socket = "127.0.0.1:8080"
+            rest_api_url = "https://localhost:8080"
             dir = "/path/to/lnd"
             "#,
-        ];
+        );
 
-        let expected = vec![
-            Lightning {
-                network: bitcoin::Network::Regtest,
-                lnd: None,
-            },
-            Lightning {
-                network: bitcoin::Network::Regtest,
-                lnd: Some(Lnd {
-                    rest_api_socket: Some(*LND_SOCKET),
-                    dir: Some(PathBuf::from("/path/to/lnd")),
-                }),
+        let expected = Lightning {
+            network: bitcoin::Network::Regtest,
+            lnd: Lnd {
+                rest_api_url: LND_URL.clone(),
+                dir: PathBuf::from("/path/to/lnd"),
             },
-        ];
-
-        let actual = file_contents
-            .into_iter()
-            .map(toml::from_str)
-            .collect::<Result<Vec<Lightning>, toml::de::Error>>()
-            .unwrap();
+        };
 
-        assert_eq!(actual, expected);
+        assert_eq!(actual, Ok(expected));
     }
 }

cnd/src/config/file.rs

@@ -1,5 +1,5 @@
 use crate::{
-    config::{Bitcoind, Data, Lightning, Network, Parity},
+    config::{Bitcoind, Data, Lnd, Network, Parity},
     swap_protocols::ledger::ethereum,
 };
 use config as config_rs;
@@ -36,6 +36,12 @@ pub struct Ethereum {
     pub parity: Option<Parity>,
 }
 
+#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
+pub struct Lightning {
+    pub network: bitcoin::Network,
+    pub lnd: Option<Lnd>,
+}
+
 impl File {
     pub fn default() -> Self {
         File {
@@ -222,7 +228,8 @@ node_url = "http://localhost:8545/"
 network = "regtest"
 
 [lightning.lnd]
-rest_api_socket = "127.0.0.1:8080"
+rest_api_url = "https://localhost:8080"
+dir = "/foo/bar"
 "#;
         let file = File {
             network: Some(Network {
@@ -255,11 +262,8 @@ rest_api_socket = "127.0.0.1:8080"
             lightning: Some(Lightning {
                 network: bitcoin::Network::Regtest,
                 lnd: Some(Lnd {
-                    rest_api_socket: Some(SocketAddr::new(
-                        IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
-                        8080,
-                    )),
-                    dir: None,
+                    rest_api_url: "https://localhost:8080".parse().unwrap(),
+                    dir: PathBuf::from("/foo/bar"),
                 }),
             }),
         };