Merge pull request #38 from nitronit/threshold_small_pr

Threshold to main

Makefile

@@ -18,7 +18,7 @@ build-linux:
 	@GOOS=linux GOARCH=amd64 go build --mod readonly $(BUILD_FLAGS) -o ./build/horcrux ./cmd/horcrux
 
 test:
-	@go test -timeout 20m -mod readonly -v ./...
+	@go test -race -timeout 20m -mod readonly -v ./...
 
 test-short:
 	@go test -mod readonly -run TestDownedSigners2of3 -v ./... 

client/address.go

@@ -0,0 +1,30 @@
+package client
+
+import (
+	"fmt"
+	"net/url"
+	"strings"
+)
+
+func SanitizeAddress(address string) (string, error) {
+	u, err := url.Parse(address)
+	if err != nil {
+		return "", fmt.Errorf("error parsing peer URL: %w", err)
+	}
+
+	return u.Host, nil
+}
+
+func MultiAddress(addresses []string) (string, error) {
+	grpcAddresses := make([]string, len(addresses))
+
+	for i, addr := range addresses {
+		peerAddr, err := SanitizeAddress(addr)
+		if err != nil {
+			return "", err
+		}
+		grpcAddresses[i] = peerAddr
+	}
+
+	return fmt.Sprintf("multi:///%s", strings.Join(grpcAddresses, ",")), nil
+}

client/address_test.go

@@ -0,0 +1,55 @@
+package client_test
+
+import (
+	"testing"
+
+	"github.com/strangelove-ventures/horcrux/client"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLeaderElectionMultiAddressDomain(t *testing.T) {
+	addresses := []string{
+		"tcp://signer-1:2222",
+		"tcp://signer-2:2222",
+		"tcp://signer-3:2222",
+	}
+
+	multiAddress, err := client.MultiAddress(addresses)
+	require.NoError(t, err, "failed to assemble fqdn multi address")
+
+	require.Equal(t, "multi:///signer-1:2222,signer-2:2222,signer-3:2222", multiAddress)
+}
+
+func TestLeaderElectionMultiAddressIPv4(t *testing.T) {
+	addresses := []string{
+		"tcp://10.0.0.1:2222",
+		"tcp://10.0.0.2:2222",
+		"tcp://10.0.0.3:2222",
+	}
+
+	multiAddress, err := client.MultiAddress(addresses)
+	require.NoError(t, err, "failed to assemble ipv4 multi address")
+
+	require.Equal(t, "multi:///10.0.0.1:2222,10.0.0.2:2222,10.0.0.3:2222", multiAddress)
+}
+
+func TestLeaderElectionMultiAddressIPv6(t *testing.T) {
+	addresses := []string{
+		"tcp://[2001:db8:3333:4444:5555:6666:7777:8888]:2222",
+		"tcp://[::]:2222",
+		"tcp://[::1234:5678]:2222",
+		"tcp://[2001:db8::]:2222",
+		"tcp://[2001:db8::1234:5678]:2222",
+	}
+
+	multiAddress, err := client.MultiAddress(addresses)
+	require.NoError(t, err, "failed to assemble ipv6 multi address")
+
+	const expected = "multi:///" +
+		"[2001:db8:3333:4444:5555:6666:7777:8888]:2222" +
+		",[::]:2222,[::1234:5678]:2222" +
+		",[2001:db8::]:2222" +
+		",[2001:db8::1234:5678]:2222"
+
+	require.Equal(t, expected, multiAddress)
+}

cmd/horcrux/cmd/config.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"github.com/strangelove-ventures/horcrux/client"
 	"github.com/strangelove-ventures/horcrux/signer"
 	tmlog "github.com/tendermint/tendermint/libs/log"
 	"gopkg.in/yaml.v2"
@@ -75,7 +76,9 @@ func initCmd() *cobra.Command {
 			if keyFileFlag != "" {
 				keyFile = &keyFileFlag
 			}
+			debugAddr, _ := cmdFlags.GetString("debug-addr")
 			if cs {
+				// Cosigner Config
 				p, _ := cmdFlags.GetString("peers")
 				threshold, _ := cmdFlags.GetUint8("threshold")
 				timeout, _ := cmdFlags.GetString("timeout")
@@ -111,18 +114,21 @@ func initCmd() *cobra.Command {
 						Timeout:   timeout,
 					},
 					ChainNodes: cn,
+					DebugAddr:  debugAddr,
 				}
 				if err = validateCosignerConfig(cfg); err != nil {
 					return err
 				}
 			} else {
+				// Single Signer Config
 				if len(cn) == 0 {
 					return fmt.Errorf("must input at least one node")
 				}
 				cfg = DiskConfig{
 					PrivValKeyFile: keyFile,
 					ChainID:        cid,
 					ChainNodes:     cn,
+					DebugAddr:      debugAddr,
 				}
 				if err = validateSingleSignerConfig(cfg); err != nil {
 					return err
@@ -163,6 +169,7 @@ func initCmd() *cobra.Command {
 		"(i.e. \"tcp://node-1:2222|2,tcp://node-2:2222|3\")")
 	cmd.Flags().Uint8P("threshold", "t", 0, "indicate number of signatures required for threshold signature")
 	cmd.Flags().StringP("listen", "l", "", "listen address of the signer")
+	cmd.Flags().StringP("debug-addr", "d", "", "listen address for Debug and Prometheus metrics in format localhost:8543")
 	cmd.Flags().StringP("keyfile", "k", "",
 		"priv val key file path (full key for single signer, or key share for cosigner)")
 	cmd.Flags().String("timeout", "1500ms", "configure cosigner rpc server timeout value, \n"+
@@ -191,9 +198,14 @@ func validateCosignerConfig(cfg DiskConfig) error {
 	if cfg.CosignerConfig == nil {
 		return fmt.Errorf("cosigner config can't be empty")
 	}
-	if float32(len(cfg.CosignerConfig.Peers))/float32(2) >= float32(cfg.CosignerConfig.Threshold) {
-		return fmt.Errorf("the threshold, t = (%d) must be greater than, 'peers/2' = (%.1f)",
-			cfg.CosignerConfig.Threshold, float32(len(cfg.CosignerConfig.Peers))/2)
+
+	if cfg.CosignerConfig.Threshold <= cfg.CosignerConfig.Shares/2 {
+		return fmt.Errorf("threshold (%d) must be greater than number of shares (%d) / 2",
+			cfg.CosignerConfig.Threshold, cfg.CosignerConfig.Shares)
+	}
+	if cfg.CosignerConfig.Shares < cfg.CosignerConfig.Threshold {
+		return fmt.Errorf("number of shares (%d) must be greater or equal to threshold (%d)",
+			cfg.CosignerConfig.Shares, cfg.CosignerConfig.Threshold)
 	}
 
 	_, err := time.ParseDuration(cfg.CosignerConfig.Timeout)
@@ -329,6 +341,7 @@ func addPeersCmd() *cobra.Command {
 				return errors.New("no new peer nodes in args")
 			}
 			diff = append(config.Config.CosignerConfig.Peers, diff...)
+			config.Config.CosignerConfig.Shares = len(diff) + 1
 			if err := validateCosignerPeers(diff, config.Config.CosignerConfig.Shares); err != nil {
 				return err
 			}
@@ -372,6 +385,8 @@ func removePeersCmd() *cobra.Command {
 			if len(diff) == 0 {
 				return errors.New("cannot remove all peer nodes from config, please leave at least one")
 			}
+
+			config.Config.CosignerConfig.Shares = len(diff) + 1
 			// If none of the peer nodes in the args are listed in the config, just continue
 			// without throwing an error, as the peer nodes in the config remain untouched.
 			if err := validateCosignerPeers(diff, config.Config.CosignerConfig.Shares); err != nil {
@@ -487,6 +502,7 @@ type DiskConfig struct {
 	ChainID        string          `json:"chain-id" yaml:"chain-id"`
 	CosignerConfig *CosignerConfig `json:"cosigner,omitempty" yaml:"cosigner,omitempty"`
 	ChainNodes     []ChainNode     `json:"chain-nodes,omitempty" yaml:"chain-nodes,omitempty"`
+	DebugAddr      string          `json:"debug-addr,omitempty" yaml:"debug-addr,omitempty"`
 }
 
 func (c *DiskConfig) Nodes() []signer.NodeConfig {
@@ -544,6 +560,15 @@ type CosignerConfig struct {
 	SignerType string         `json:"signer-type" yaml:"signer-type"`
 }
 
+func (cfg *CosignerConfig) LeaderElectMultiAddress() (string, error) {
+	addresses := make([]string, 1+len(cfg.Peers))
+	addresses[0] = cfg.P2PListen
+	for i, peer := range cfg.Peers {
+		addresses[i+1] = peer.P2PAddr
+	}
+	return client.MultiAddress(addresses)
+}
+
 func (c *DiskConfig) CosignerPeers() (out []signer.CosignerConfig) {
 	for _, p := range c.CosignerConfig.Peers {
 		out = append(out, signer.CosignerConfig{ID: p.ShareID, Address: p.P2PAddr})
@@ -599,11 +624,12 @@ func validateCosignerPeers(peers []CosignerPeer, shares uint8) error {
 		}
 	}
 
-	// Check that no more than {num-shares}-1 peers are in the peer list, assuming
+	// Check that exactly {num-shares}-1 peers are in the peer list, assuming
 	// the remaining peer ID is the ID the local node is configured with.
-	if len(peers) == int(shares) {
-		return fmt.Errorf("too many peers (%v+local node = %v) for the specified number of key shares (%v)",
-			len(peers), len(peers)+1, shares)
+
+	if len(peers) != shares-1 {
+		return fmt.Errorf("incorrect number of peers. expected (%d shares - local node = %d peers)",
+			shares, shares-1)
 	}
 	return nil
 }