interfaces defined where it is used.

cmd/horcrux/cmd/threshold.go

@@ -24,8 +24,8 @@ func NewThresholdValidator(
 	}
 
 	thresholdCfg := config.Config.ThresholdModeConfig
-
-	remoteCosigners := make([]pcosigner.ICosigner, 0, len(thresholdCfg.Cosigners)-1)
+	// NOTE: Shouldnt this be a list of concrete type instead of interface type?
+	remoteCosigners := make([]node.ICosigner, 0, len(thresholdCfg.Cosigners)-1)
 
 	var p2pListen string
 

pkg/multiresolver/multi_test.go

@@ -9,8 +9,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/strangelove-ventures/horcrux/pkg/pcosigner"
-
 	"github.com/strangelove-ventures/horcrux/pkg/node"
 
 	grpcretry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
@@ -37,7 +35,7 @@ func createListener(nodeID string, homedir string) (string, func(), error) {
 		nil)
 
 	// Need to set pointers to avoid nil pointers.
-	var cosigners []pcosigner.ICosigner
+	var cosigners []node.ICosigner
 	var timeDuration time.Duration
 	thresholdvalidator := node.NewThresholdValidator(nil, nil, 0, timeDuration, 0, nil, cosigners, nil)
 	s.SetThresholdValidator(thresholdvalidator)

pkg/node/grpc_server.go

@@ -16,26 +16,28 @@ import (
 var _ proto.ICosignerGRPCServerServer = &GRPCServer{}
 
 type GRPCServer struct {
-	cosigner           *pcosigner.LocalCosigner
-	thresholdValidator *ThresholdValidator
-	raftStore          *RaftStore
-	// Promoted Fields
+	cosigner           *pcosigner.LocalCosigner // The "node's" LocalCosigner
+	thresholdValidator *ThresholdValidator      // The "node's" ThresholdValidator
+	raftStore          *RaftStore               // The "node's" RaftStore
+	// Promoted Fields is embedded to have forward compatiblitity
 	proto.UnimplementedICosignerGRPCServerServer
 }
 
+// NewGRPCServer returns a new GRPCServer.
 func NewGRPCServer(
 	cosigner *pcosigner.LocalCosigner,
 	thresholdValidator *ThresholdValidator,
 	raftStore *RaftStore,
 ) *GRPCServer {
 	return &GRPCServer{
+		// TODO: This is a hack to get around the fact that the cosigner is not a?
 		cosigner:           cosigner,
 		thresholdValidator: thresholdValidator,
 		raftStore:          raftStore,
 	}
 }
 
-// SignBlock implements the CosignerGRPCServer interface.
+// SignBlock "pseudo-implements" the ICosignerGRPCServer interface in pkg/proto/cosigner_grpc_server_grpc.pb.go
 func (rpc *GRPCServer) SignBlock(
 	_ context.Context,
 	req *proto.CosignerGRPCSignBlockRequest,
@@ -47,6 +49,7 @@ func (rpc *GRPCServer) SignBlock(
 		SignBytes: req.Block.GetSignBytes(),
 		Timestamp: time.Unix(0, req.Block.GetTimestamp()),
 	}
+	// this
 	res, _, err := rpc.thresholdValidator.SignBlock(req.ChainID, block)
 	if err != nil {
 		return nil, err
@@ -60,12 +63,13 @@ func (rpc *GRPCServer) SetNoncesAndSign(
 	_ context.Context,
 	req *proto.CosignerGRPCSetNoncesAndSignRequest,
 ) (*proto.CosignerGRPCSetNoncesAndSignResponse, error) {
-	res, err := rpc.cosigner.SetNoncesAndSign(pcosigner.CosignerSetNoncesAndSignRequest{
-		ChainID:   req.ChainID,
-		Nonces:    pcosigner.CosignerNoncesFromProto(req.GetNonces()),
-		HRST:      types.HRSTKeyFromProto(req.GetHrst()),
-		SignBytes: req.GetSignBytes(),
-	})
+	res, err := rpc.cosigner.SetNoncesAndSign(
+		pcosigner.CosignerSetNoncesAndSignRequest{
+			ChainID:   req.ChainID,
+			Nonces:    pcosigner.CosignerNoncesFromProto(req.GetNonces()),
+			HRST:      types.HRSTKeyFromProto(req.GetHrst()),
+			SignBytes: req.GetSignBytes(),
+		})
 	if err != nil {
 		rpc.raftStore.logger.Error(
 			"Failed to sign with shard",
@@ -91,7 +95,7 @@ func (rpc *GRPCServer) SetNoncesAndSign(
 	}, nil
 }
 
-// GetNonces implements the UnimplementedCosignerGRPCServer interface.
+// GetNonces implements the ICosignerGRPCServer interface.
 func (rpc *GRPCServer) GetNonces(
 	_ context.Context,
 	req *proto.CosignerGRPCGetNoncesRequest,
@@ -108,6 +112,7 @@ func (rpc *GRPCServer) GetNonces(
 	}, nil
 }
 
+// TransferLeadership pseudo-implements the ICosignerGRPCServer interface in pkg/proto/cosigner_grpc_server_grpc.pb.go
 func (rpc *GRPCServer) TransferLeadership(
 	_ context.Context,
 	req *proto.CosignerGRPCTransferLeadershipRequest,
@@ -132,6 +137,8 @@ func (rpc *GRPCServer) TransferLeadership(
 	return &proto.CosignerGRPCTransferLeadershipResponse{}, nil
 }
 
+// GetLeader pseudo-implements the ICosignerGRPCServer interface in pkg/proto/cosigner_grpc_server_grpc.pb.go
+// GetLeader gets the current raft cluster leader and send it as respons.
 func (rpc *GRPCServer) GetLeader(
 	context.Context,
 	*proto.CosignerGRPCGetLeaderRequest,

pkg/node/icosigner.go

@@ -0,0 +1,30 @@
+package node
+
+import (
+	cometcrypto "github.com/cometbft/cometbft/crypto"
+	"github.com/strangelove-ventures/horcrux/pkg/pcosigner"
+	"github.com/strangelove-ventures/horcrux/pkg/types"
+)
+
+// ICosigner interface is a set of methods for an m-of-n threshold signature.
+// This interface abstracts the underlying key storage and management
+type ICosigner interface {
+	// GetID should return the id number of the cosigner
+	// The ID is the shamir index: 1, 2, etc...
+	GetID() int
+
+	// GetAddress gets the P2P URL (GRPC and Raft)
+	GetAddress() string
+
+	// GetPubKey gets the combined public key (permament)
+	// Not used by Remote Cosigner
+	GetPubKey(chainID string) (cometcrypto.PubKey, error)
+
+	VerifySignature(chainID string, payload, signature []byte) bool
+
+	// GetNonces requests nonce frpm the peer cosigners
+	GetNonces(chainID string, hrst types.HRSTKey) (*pcosigner.CosignerNoncesResponse, error)
+
+	// Sign the requested bytes
+	SetNoncesAndSign(req pcosigner.CosignerSetNoncesAndSignRequest) (*pcosigner.CosignerSignResponse, error)
+}

pkg/node/raft_store.go

@@ -53,15 +53,14 @@ type RaftStore struct {
 	RaftDir     string
 	RaftBind    string
 	RaftTimeout time.Duration
-	// Cosigners   []Cosigner
 
 	mu sync.Mutex
 	m  map[string]string // The key-value store for the system.
 
 	raft *raft.Raft // The consensus mechanism
 
 	logger log.Logger
-	// cosigner           *LocalCosigner
+
 	thresholdValidator *ThresholdValidator
 }
 

pkg/node/threshold_validator.go

@@ -43,7 +43,7 @@ type ThresholdValidator struct {
 	myCosigner *pcosigner.LocalCosigner
 
 	// peer cosigners
-	peerCosigners []pcosigner.ICosigner
+	peerCosigners []ICosigner
 
 	leader ILeader
 
@@ -73,7 +73,7 @@ func NewThresholdValidator(
 	grpcTimeout time.Duration,
 	maxWaitForSameBlockAttempts int,
 	myCosigner *pcosigner.LocalCosigner,
-	peerCosigners []pcosigner.ICosigner,
+	peerCosigners []ICosigner,
 	leader ILeader,
 ) *ThresholdValidator {
 	return &ThresholdValidator{
@@ -369,10 +369,10 @@ func newSameBlockError(chainID string, hrs types.HRSKey) *SameBlockError {
 
 func (pv *ThresholdValidator) waitForPeerNonces(
 	chainID string,
-	peer pcosigner.ICosigner,
+	peer ICosigner,
 	hrst types.HRSTKey,
 	wg *sync.WaitGroup,
-	nonces map[pcosigner.ICosigner][]pcosigner.CosignerNonce,
+	nonces map[ICosigner][]pcosigner.CosignerNonce,
 	thresholdPeersMutex *sync.Mutex,
 ) {
 	peerStartTime := time.Now()
@@ -398,9 +398,9 @@ func (pv *ThresholdValidator) waitForPeerNonces(
 }
 func (pv *ThresholdValidator) waitForPeerSetNoncesAndSign(
 	chainID string,
-	peer pcosigner.ICosigner,
+	peer ICosigner,
 	hrst types.HRSTKey,
-	noncesMap map[pcosigner.ICosigner][]pcosigner.CosignerNonce,
+	noncesMap map[ICosigner][]pcosigner.CosignerNonce,
 	signBytes []byte,
 	shareSignatures *[][]byte,
 	shareSignaturesMutex *sync.Mutex,
@@ -649,14 +649,18 @@ func (pv *ThresholdValidator) SignBlock(chainID string, block *Block) ([]byte, t
 	// Used to track how close we are to threshold
 
 	// Here the actual signing process starts from a cryptological perspective
-	nonces := make(map[pcosigner.ICosigner][]pcosigner.CosignerNonce)
+	nonces := make(map[ICosigner][]pcosigner.CosignerNonce)
 	thresholdPeersMutex := sync.Mutex{}
 
+	// From each cosigner peer we are requesting the nonce.
+	// This is done asynchronously.
+	// pv.waitForPeersNonces uses GRPC to get the nonce from the peer.
 	for _, c := range pv.peerCosigners {
 		go pv.waitForPeerNonces(chainID, c, hrst, &getEphemeralWaitGroup,
 			nonces, &thresholdPeersMutex)
 	}
 
+	// Requesting a nonce from our own cosigner (a.k.a. the local cosigner)
 	myNonces, err := pv.myCosigner.GetNonces(chainID, hrst)
 	if err != nil {
 		pv.notifyBlockSignError(chainID, block.HRSKey())
@@ -738,7 +742,7 @@ func (pv *ThresholdValidator) SignBlock(chainID string, block *Block) ([]byte, t
 		return nil, stamp, errors.New("not enough co-signers")
 	}
 
-	// assemble into final signature
+	// assemble the partial signatures into a valid signature
 	signature, err := pv.myCosigner.CombineSignatures(chainID, shareSigs)
 	if err != nil {
 		pv.notifyBlockSignError(chainID, block.HRSKey())

pkg/node/threshold_validator_test.go

@@ -73,7 +73,7 @@ func loadKeyForLocalCosigner(
 func testThresholdValidator(t *testing.T, threshold, total uint8) {
 	cosigners, pubKey := getTestLocalCosigners(t, threshold, total)
 
-	thresholdCosigners := make([]pcosigner.ICosigner, 0, threshold-1)
+	thresholdCosigners := make([]ICosigner, 0, threshold-1)
 
 	for i, cosigner := range cosigners {
 		require.Equal(t, i+1, cosigner.GetID())
@@ -279,6 +279,7 @@ func getTestLocalCosigners(t *testing.T, threshold, total uint8) ([]*pcosigner.L
 
 	privateKey := cometcryptoed25519.GenPrivKey()
 	privKeyBytes := privateKey[:]
+	// DealShares splits the secret by using Shamir Secret Sharing (Note its not verifiable secret sharing)
 	privShards := tsed25519.DealShares(tsed25519.ExpandSecret(privKeyBytes[:32]), threshold, total)
 
 	tmpDir := t.TempDir()
@@ -340,7 +341,7 @@ func testThresholdValidatorLeaderElection(t *testing.T, threshold, total uint8)
 	var leader *ThresholdValidator
 	leaders := make([]*MockLeader, total)
 	for i, cosigner := range cosigners {
-		peers := make([]pcosigner.ICosigner, 0, len(cosigners)-1)
+		peers := make([]ICosigner, 0, len(cosigners)-1)
 		for j, otherCosigner := range cosigners {
 			if i != j {
 				peers = append(peers, otherCosigner)

pkg/pcosigner/cosigner.go

@@ -5,32 +5,9 @@ import (
 
 	"github.com/strangelove-ventures/horcrux/pkg/types"
 
-	cometcrypto "github.com/cometbft/cometbft/crypto"
 	"github.com/strangelove-ventures/horcrux/pkg/proto"
 )
 
-// ICosigner interface is a set of methods for an m-of-n threshold signature.
-// This interface abstracts the underlying key storage and management
-type ICosigner interface {
-	// Get the ID of the cosigner
-	// The ID is the shamir index: 1, 2, etc...
-	GetID() int
-
-	// GetAddress gets the P2P URL (GRPC and Raft)
-	GetAddress() string
-
-	// Get the combined public key
-	GetPubKey(chainID string) (cometcrypto.PubKey, error)
-
-	VerifySignature(chainID string, payload, signature []byte) bool
-
-	// Get nonces for all cosigner shards
-	GetNonces(chainID string, hrst types.HRSTKey) (*CosignerNoncesResponse, error)
-
-	// Sign the requested bytes
-	SetNoncesAndSign(req CosignerSetNoncesAndSignRequest) (*CosignerSignResponse, error)
-}
-
 type CosignerSignBlockResponse struct {
 	Signature []byte
 }

pkg/pcosigner/cosigner_key_shares.go

@@ -24,8 +24,10 @@ func CreateCosignerEd25519ShardsFromFile(priv string, threshold, shards uint8) (
 }
 
 // CreateCosignerEd25519Shards creates CosignerEd25519Key objects from a privval.FilePVKey
+// by splitting the secret using Shamir secret sharing.
 func CreateCosignerEd25519Shards(pv privval.FilePVKey, threshold, shards uint8) []CosignerEd25519Key {
-	privShards := tsed25519.DealShares(tsed25519.ExpandSecret(pv.PrivKey.Bytes()[:32]), threshold, shards)
+	// tsed25519.DealShares splits the secret using Shamir Secret Sharing (Note its: no verifiable secret sharing)
+	privShards := tsed25519.DealShares(tsed25519.ExpandSecret(pv.PrivKey.Bytes()[:32]), threshold, shards) // privshards is shamir shares
 	out := make([]CosignerEd25519Key, shards)
 	for i, shard := range privShards {
 		out[i] = CosignerEd25519Key{

pkg/pcosigner/cosigner_signer.go

@@ -1,14 +1,15 @@
 package pcosigner
 
-// Interface for the local signer whether it's a soft sign or HSM
+// IThresholdSigner is interface for the cosigner_signer whether it's a soft sign or HSM
 type IThresholdSigner interface {
-	// GetPubKey returns the public key bytes for the combination of all cosigners.
+	// GetPubKey returns the persistent public key
 	GetPubKey() []byte
 
 	// GenerateNonces deals nonces for all cosigners.
 	GenerateNonces() (Nonces, error)
 
-	// Sign signs a byte payload with the provided nonces.
+	// Sign signs a byte payload using the a list of nonces.
+	// Sign returns the partial signature and an error if any.
 	Sign(nonces []Nonce, payload []byte) ([]byte, error)
 
 	// CombineSignatures combines multiple partial signatures to a full signature.
@@ -17,7 +18,11 @@ type IThresholdSigner interface {
 
 // Nonces contains the ephemeral information generated by one cosigner for all other cosigners.
 type Nonces struct {
+	// PubKey is the public key for the generated nounces by cosigner
 	PubKey []byte
+
+	// Shares is the list nonces of size n generated by the cosigner.
+	// Shares[i] is the nonce for cosigner i.
 	Shares [][]byte
 }
 
@@ -28,7 +33,7 @@ type Nonce struct {
 	PubKey []byte
 }
 
-// PartialSignature contains the signature and identifier for a piece of the combined signature.
+// PartialSignature contains the partial signature and identifier (shamir id) of the signer.
 type PartialSignature struct {
 	ID        int
 	Signature []byte

pkg/pcosigner/cosigner_signer_soft.go

@@ -12,11 +12,13 @@ import (
 
 var _ IThresholdSigner = &ThresholdSignerSoft{}
 
+// ThresholdSignerSoft is a soft implementation of the IThresholdSigner interface.
 type ThresholdSignerSoft struct {
-	privateKeyShard []byte
-	pubKey          []byte
-	threshold       uint8
-	total           uint8
+	privateKeyShard []byte // privateKeyShard is our persistent private key shard
+	pubKey          []byte // pubKey is our persistent public key
+	threshold       uint8  // threshold is the number of t cosigners required to sign, t
+	total           uint8  // total is the total number n of cosigners
+
 }
 
 func NewThresholdSignerSoft(config *RuntimeConfig, id int, chainID string) (*ThresholdSignerSoft, error) {
@@ -44,6 +46,7 @@ func NewThresholdSignerSoft(config *RuntimeConfig, id int, chainID string) (*Thr
 	return &s, nil
 }
 
+// GetPubkey implements the IThresholdSigner interface
 func (s *ThresholdSignerSoft) GetPubKey() []byte {
 	return s.pubKey
 }
@@ -59,6 +62,7 @@ func (s *ThresholdSignerSoft) Sign(nonces []Nonce, payload []byte) ([]byte, erro
 	return append(noncePub, sig...), nil
 }
 
+// Note: sumNonces sums the "working secret"
 func (s *ThresholdSignerSoft) sumNonces(nonces []Nonce) (tsed25519.Scalar, tsed25519.Element, error) {
 	shareParts := make([]tsed25519.Scalar, len(nonces))
 	publicKeys := make([]tsed25519.Element, len(nonces))
@@ -85,7 +89,7 @@ func (s *ThresholdSignerSoft) sumNonces(nonces []Nonce) (tsed25519.Scalar, tsed2
 	return nonceShare, noncePub, nil
 }
 
-// GenerateNonces deals nonces (A Pubkey and t of n shares) for all cosigners.
+// GenerateNonces deals nonces (A Pubkey and t of n shares) for all the cosigners.
 func (s *ThresholdSignerSoft) GenerateNonces() (Nonces, error) {
 	secret := make([]byte, 32)
 	if _, err := rand.Read(secret); err != nil {
@@ -97,6 +101,7 @@ func (s *ThresholdSignerSoft) GenerateNonces() (Nonces, error) {
 		Shares: make([][]byte, s.total),
 	}
 
+	// Splits the secret with Shamir secret sharing
 	shares := tsed25519.DealShares(secret, s.threshold, s.total)
 
 	for i, s := range shares {