Fix vault paths, enable itest coverage, and display coveralls coverage badge

.github/workflows/docker-test.yml

@@ -16,3 +16,20 @@ jobs:
 
       - name: Run tests in docker container
         run:  make docker-test-all
+
+      - name: Upload unit test coverage
+        uses: shogo82148/actions-goveralls@v1
+        with:
+          path-to-profile: cover.out
+          parallel: true
+
+      - name: Upload integration test coverage
+        uses: shogo82148/actions-goveralls@v1
+        with:
+          path-to-profile: icover.out
+          parallel: true
+
+      - name: Generate coverage badge
+        uses: shogo82148/actions-goveralls@v1
+        with:
+          parallel-finished: true

Makefile

@@ -1,9 +1,11 @@
-.PHONY: docker docker-itest docker-test docker-test-all docker-check docker-shell itest test test-all
+.PHONY: cover docker docker-itest docker-test docker-test-all docker-check docker-shell itest test test-all
 
-IMG_NAME      := lndsigner-builder
+IMG_NAME    := lndsigner-builder
 
 CPLATFORM   := $(shell uname -m)
 
+GOCOVERDIR  := /tmp/lndsigner-icover-$(shell date +%s)
+
 ifeq ($(CPLATFORM), x86_64)
 	GOPLATFORM := amd64
 endif
@@ -18,9 +20,9 @@ ifeq ($(CPLATFORM), arm64)
 endif 
 
 GOVER         := 1.20.3
-LND           := v0.16.2-beta
-BITCOIND      := 24.0.1
-VAULT         := 1.12.2
+LND           := v0.16.4-beta
+BITCOIND      := 25.0
+VAULT         := 1.12.3
 
 # docker builds a builder image for the host platform if one isn't cached.
 docker:
@@ -55,11 +57,18 @@ docker-shell: docker
 		--mount type=bind,source=$(CURDIR),target=/app $(IMG_NAME):latest \
 		bash -l 
 
-itest:
-	go install -race -buildvcs=false ./cmd/... && go test -v -count=1 -race -tags=itest -cover ./itest
+cover:
+	mkdir $(GOCOVERDIR) && mkdir $(GOCOVERDIR)/lndsignerd && \
+		mkdir $(GOCOVERDIR)/plugin && mkdir $(GOCOVERDIR)/combined
+
+itest: cover
+	go install -race -cover -buildvcs=false ./cmd/... && \
+		GOCOVERDIR=$(GOCOVERDIR) go test -v -count=1 -race -tags=itest ./itest && \
+		go tool covdata merge -pcombine -i=$(GOCOVERDIR)/lndsignerd,$(GOCOVERDIR)/plugin -o=$(GOCOVERDIR)/combined && \
+		go tool covdata textfmt -i=$(GOCOVERDIR)/combined -o icover.out && \
+		rm -rf $(GOCOVERDIR)
 
 test:
-	go test -v -count=1 -race -cover ./...
+	go test -v -count=1 -race -coverprofile cover.out ./...
 
-test-all:
-	go install -race -buildvcs=false ./cmd/... && go test -v -count=1 -race -tags=itest -cover ./...
+test-all: test itest

README.md

@@ -1,3 +1,5 @@
+[![Coverage Status](https://coveralls.io/repos/github/NYDIG-OSS/lndsigner/badge.svg?branch=main)](https://coveralls.io/github/NYDIG-OSS/lndsigner?branch=main)
+
 # lndsigner
 `lndsigner` is a [remote signer](https://github.com/lightningnetwork/lnd/blob/master/docs/remote-signing.md) for [lnd](https://github.com/lightningnetwork/lnd). Currently, it can do the following:
 - [x] store seeds for multiple nodes in [Hashicorp Vault](https://github.com/hashicorp/vault/)
@@ -105,7 +107,7 @@ Next, get the account list for the node (this works on Linux with `jq` installed
 
 ```
 ~/.lnd-watchonly$ VAULT_ADDR=http://127.0.0.1:8200 VAULT_TOKEN=root \
-   vault read lndsigner/lnd-nodes/accounts node=*pubkey* | \
+   vault read lndsigner/lnd-nodes/*pubkey*/accounts | \
    tail -n 1 | sed s/acctList\\s*// | jq > accounts.json
 ```
 

itest/itest_context_test.go

@@ -35,6 +35,7 @@ type testContext struct {
 	vaultPort   string
 	vaultCmd    *exec.Cmd
 	vaultClient *api.Logical
+	vaultSys    *api.Sys
 
 	bitcoinDir     string
 	bitcoinRPC     string
@@ -137,6 +138,11 @@ func newTestContext(t *testing.T) *testContext {
 		"-dev-root-token-id=root", "-dev-plugin-dir="+pluginDir,
 		"-dev-listen-address=127.0.0.1:"+tctx.vaultPort)
 
+	tctx.vaultCmd.Env = append(tctx.vaultCmd.Env,
+		"PATH="+os.Getenv("PATH"),
+		"GOCOVERDIR="+path.Join(os.Getenv("GOCOVERDIR"), "plugin"),
+	)
+
 	go waitProc(tctx.vaultCmd)
 
 	vaultClientConf := api.DefaultConfig()
@@ -148,9 +154,9 @@ func newTestContext(t *testing.T) *testContext {
 	vaultClient.SetToken("root")
 
 	tctx.vaultClient = vaultClient.Logical()
+	tctx.vaultSys = vaultClient.Sys()
 
-	vaultSys := vaultClient.Sys()
-	err = vaultSys.Mount("lndsigner", &api.MountInput{
+	err = tctx.vaultSys.Mount("lndsigner", &api.MountInput{
 		Type: "vault-plugin-lndsigner",
 	})
 	require.NoError(t, err)
@@ -213,6 +219,7 @@ func (tctx *testContext) Close() {
 		lnd.Close()
 	}
 
+	_ = tctx.vaultSys.Unmount("lndsigner")
 	_ = tctx.bitcoinCli("stop")
 	_ = tctx.vaultCmd.Process.Signal(os.Interrupt)
 

itest/itest_lndharness_test.go

@@ -14,8 +14,6 @@ import (
 	"encoding/json"
 	"encoding/pem"
 	"fmt"
-	"github.com/nydig-oss/lndsigner"
-	"github.com/nydig-oss/lndsigner/itest"
 	"io/fs"
 	"math/big"
 	"net"
@@ -25,6 +23,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/nydig-oss/lndsigner"
+	"github.com/nydig-oss/lndsigner/itest"
+
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
@@ -95,16 +96,18 @@ func (l *lndHarness) Start() {
 	l.lndSignerCmd.Env = append(l.lndSignerCmd.Env,
 		"VAULT_ADDR=http://127.0.0.1:"+l.tctx.vaultPort,
 		"VAULT_TOKEN=root",
+		"GOCOVERDIR="+path.Join(os.Getenv("GOCOVERDIR"), "lndsignerd"),
 	)
 
+	l.lndSignerCmd.Cancel = func() error {
+		return l.lndSignerCmd.Process.Signal(os.Interrupt)
+	}
+
 	go waitProc(l.lndSignerCmd)
 
 	// Start lnd.
-	acctsResp, err := l.tctx.vaultClient.ReadWithData(
-		"lndsigner/lnd-nodes/accounts",
-		map[string][]string{
-			"node": []string{l.idPubKey},
-		},
+	acctsResp, err := l.tctx.vaultClient.Read(
+		"lndsigner/lnd-nodes/" + l.idPubKey + "/accounts",
 	)
 	require.NoError(l.tctx.t, err)
 
@@ -268,7 +271,9 @@ func waitFile(t *testing.T, file, waitStr string) {
 // exit error, the program's entire stderr and stdout are logged.
 func waitProc(cmd *exec.Cmd) {
 	output, err := cmd.CombinedOutput()
-	if err != nil && err.Error() != "signal: killed" {
+	if err != nil && err.Error() != "signal: killed" &&
+		err != context.Canceled {
+
 		config := zap.NewDevelopmentConfig()
 		config.EncoderConfig.EncodeLevel = zapcore.CapitalColorLevelEncoder
 		config.EncoderConfig.EncodeCaller = nil

itest/lndsigner_test.go

@@ -58,6 +58,8 @@ func TestIntegration(t *testing.T) {
 
 	tctx.waitForSync()
 
+	t.Run("check vault for parameter path override", tctx.testPathOverride)
+
 	t.Run("fund each lnd with a p2tr address", tctx.testFundLnds)
 
 	tctx.mine(1)
@@ -196,3 +198,36 @@ func (tctx *testContext) testEachSignVerifyEachOther(t *testing.T) {
 		tctx.log.Info(message)
 	})
 }
+
+// testPathOverride attempts to get accounts from the vault for each node by
+// calling with a path including node ID
+// "000000000000000000000000000000000000000000000000000000000000000000"
+// and the node's pubkey. This shouldn't work, according to
+// https://github.com/hashicorp/vault/blob/339b314f20fbd5deaeb5524f0a0db7df66f6524e/sdk/framework/path.go#L74
+func (tctx *testContext) testPathOverride(t *testing.T) {
+	tctx.testEach(func(lnd *lndHarness) {
+		// Attempt to read accounts for the node by overriding an
+		// invalid node ID in the path with a query parameter specifying
+		// the real node ID.
+		_, err := tctx.vaultClient.ReadWithData(
+			"lndsigner/lnd-nodes/00000000000000000000000000000000"+
+				"0000000000000000000000000000000000/accounts",
+			map[string][]string{
+				"node": []string{lnd.idPubKey},
+			},
+		)
+		require.ErrorContains(tctx.t, err, "node not found")
+
+		// Attempt to read accounts for the node by overriding an
+		// invalid-length node ID in the path with a query parameter
+		// specifying the real node ID. We should get nothing at all.
+		x, err := tctx.vaultClient.ReadWithData(
+			"lndsigner/lnd-nodes/0000000000000000000000/accounts",
+			map[string][]string{
+				"node": []string{lnd.idPubKey},
+			},
+		)
+		require.NoError(tctx.t, err)
+		require.Nil(tctx.t, x)
+	})
+}

keyring/keyring.go

@@ -112,7 +112,6 @@ func (k *KeyRing) ECDH(keyDesc KeyDescriptor, pub *btcec.PublicKey) ([32]byte,
 	error) {
 
 	reqData := map[string]interface{}{
-		"node": k.node,
 		"path": []int{
 			int(vault.Bip0043purpose +
 				hdkeychain.HardenedKeyStart),
@@ -133,7 +132,7 @@ func (k *KeyRing) ECDH(keyDesc KeyDescriptor, pub *btcec.PublicKey) ([32]byte,
 	log.Debugf("Sending data %+v for shared key request", reqData)
 
 	sharedKeyResp, err := k.client.Write(
-		"lndsigner/lnd-nodes/ecdh",
+		"lndsigner/lnd-nodes/"+k.node+"/ecdh",
 		reqData,
 	)
 	if err != nil {
@@ -176,7 +175,6 @@ func (k *KeyRing) SignMessage(keyLoc KeyLocator, msg []byte, doubleHash bool,
 	}
 
 	reqData := map[string]interface{}{
-		"node": k.node,
 		"path": []int{
 			int(vault.Bip0043purpose + hdkeychain.HardenedKeyStart),
 			int(k.coin + hdkeychain.HardenedKeyStart),
@@ -195,7 +193,7 @@ func (k *KeyRing) SignMessage(keyLoc KeyLocator, msg []byte, doubleHash bool,
 	log.Debugf("Sending data %+v for signing request", reqData)
 
 	signResp, err := k.client.Write(
-		"lndsigner/lnd-nodes/sign",
+		"lndsigner/lnd-nodes/"+k.node+"/sign",
 		reqData,
 	)
 	if err != nil {
@@ -226,7 +224,6 @@ func (k *KeyRing) SignMessageSchnorr(keyLoc KeyLocator, msg []byte,
 	}
 
 	reqData := map[string]interface{}{
-		"node": k.node,
 		"path": []int{
 			int(vault.Bip0043purpose + hdkeychain.HardenedKeyStart),
 			int(k.coin + hdkeychain.HardenedKeyStart),
@@ -245,7 +242,7 @@ func (k *KeyRing) SignMessageSchnorr(keyLoc KeyLocator, msg []byte,
 	log.Debugf("Sending data %+v for signing request", reqData)
 
 	signResp, err := k.client.Write(
-		"lndsigner/lnd-nodes/sign",
+		"lndsigner/lnd-nodes/"+k.node+"/sign",
 		reqData,
 	)
 	if err != nil {
@@ -395,7 +392,6 @@ func (k *KeyRing) signSegWitV0(in *psbt.PInput, tx *wire.MsgTx,
 		in.Unknowns)
 
 	reqData := map[string]interface{}{
-		"node":   k.node,
 		"path":   sliceUint32ToInt(in.Bip32Derivation[0].Bip32Path),
 		"method": "ecdsa",
 		"digest": hex.EncodeToString(digest),
@@ -406,7 +402,7 @@ func (k *KeyRing) signSegWitV0(in *psbt.PInput, tx *wire.MsgTx,
 	log.Debugf("Sending data %+v for signing request", reqData)
 
 	signResp, err := k.client.Write(
-		"lndsigner/lnd-nodes/sign",
+		"lndsigner/lnd-nodes/"+k.node+"/sign",
 		reqData,
 	)
 	if err != nil {
@@ -465,7 +461,6 @@ func (k *KeyRing) signSegWitV1KeySpend(in *psbt.PInput, tx *wire.MsgTx,
 	}
 
 	reqData := map[string]interface{}{
-		"node":     k.node,
 		"path":     sliceUint32ToInt(in.Bip32Derivation[0].Bip32Path),
 		"method":   "schnorr",
 		"digest":   hex.EncodeToString(digest),
@@ -477,7 +472,7 @@ func (k *KeyRing) signSegWitV1KeySpend(in *psbt.PInput, tx *wire.MsgTx,
 	log.Debugf("Sending data %+v for signing request", reqData)
 
 	signResp, err := k.client.Write(
-		"lndsigner/lnd-nodes/sign",
+		"lndsigner/lnd-nodes/"+k.node+"/sign",
 		reqData,
 	)
 	if err != nil {
@@ -523,7 +518,6 @@ func (k *KeyRing) signSegWitV1ScriptSpend(in *psbt.PInput, tx *wire.MsgTx,
 	}
 
 	reqData := map[string]interface{}{
-		"node":   k.node,
 		"path":   sliceUint32ToInt(in.Bip32Derivation[0].Bip32Path),
 		"method": "schnorr",
 		"digest": hex.EncodeToString(digest),
@@ -534,7 +528,7 @@ func (k *KeyRing) signSegWitV1ScriptSpend(in *psbt.PInput, tx *wire.MsgTx,
 	log.Debugf("Sending data %+v for signing request", reqData)
 
 	signResp, err := k.client.Write(
-		"lndsigner/lnd-nodes/sign",
+		"lndsigner/lnd-nodes/"+k.node+"/sign",
 		reqData,
 	)
 	if err != nil {