apply pr reviews

node/cmd/guardiand/node.go

@@ -268,7 +268,7 @@ func init() {
 
 	dataDir = NodeCmd.Flags().String("dataDir", "", "Data directory")
 
-	guardianKeyPath = NodeCmd.Flags().String("guardianKey", "", "Path to guardian key (required)")
+	guardianKeyPath = NodeCmd.Flags().String("guardianKey", "", "Path to guardian key")
 	guardianSignerUri = NodeCmd.Flags().String("guardianSignerUri", "", "Guardian signer URI")
 	solanaContract = NodeCmd.Flags().String("solanaContract", "", "Address of the Solana program (required)")
 
@@ -624,8 +624,13 @@ func runNode(cmd *cobra.Command, args []string) {
 	if *nodeKeyPath == "" && env != common.UnsafeDevNet { // In devnet mode, keys are deterministically generated.
 		logger.Fatal("Please specify --nodeKey")
 	}
-	if *guardianKeyPath == "" && *guardianSignerUri == "" {
-		logger.Fatal("Please specify --guardianKey or --guardianSignerUri")
+	if *guardianKeyPath == "" {
+		// This if-statement is nested, since checking if both are empty at once will always result in the else-branch
+		// being executed if at least one is specified. For example, in the case where the singer URI is specified and
+		// the guardianKeyPath not, then the else-statement will create an empty `file://` URI.
+		if *guardianSignerUri == "" {
+			logger.Fatal("Please specify --guardianKey or --guardianSignerUri")
+		}
 	} else {
 		// If guardianKeyPath is set, set guardianSignerUri to the file signer URI, pointing to guardianKeyPath.
 		// This ensures that the signer-abstracted guardian has backwards compatibility with guardians that would

node/pkg/accountant/accountant.go

@@ -120,7 +120,7 @@ func NewAccountant(
 	enforceFlag bool, // whether or not accountant should be enforced
 	nttContract string, // the address of the NTT smart contract on wormchain
 	nttWormchainConn AccountantWormchainConn, // used for communicating with the NTT smart contract
-	guardianSigner guardiansigner.GuardianSigner, // the guardian key used for signing observation requests
+	guardianSigner guardiansigner.GuardianSigner, // the guardian signer used for signing observation requests
 	gst *common.GuardianSetState, // used to get the current guardian set index when sending observation requests
 	msgChan chan<- *common.MessagePublication, // the channel where transfers received by the accountant runnable should be published
 	env common.Environment, // Controls the set of token bridges to be monitored

node/pkg/guardiansigner/filesigner.go

@@ -73,7 +73,7 @@ func (fs *FileSigner) Sign(hash []byte) ([]byte, error) {
 	sig, err := crypto.Sign(hash, fs.privateKey)
 
 	if err != nil {
-		return nil, fmt.Errorf("failed to sign wormchain address: %w", err)
+		return nil, fmt.Errorf("failed to sign hash: %w", err)
 	}
 
 	return sig, nil

node/pkg/guardiansigner/guardiansigner.go

@@ -44,16 +44,15 @@ func ParseSignerUri(signerUri string) (signerType SignerType, signerKeyConfig st
 	// Split the URI using the standard "://" scheme separator
 	signerUriSplit := strings.Split(signerUri, "://")
 
-	// Length smaller than 2 implies that there is no path following the scheme separator,
-	// which is invalid.
+	// This check is purely for ensuring that there is actually a path separator.
 	if len(signerUriSplit) < 2 {
 		return InvalidSignerType, ""
 	}
 
 	typeStr := signerUriSplit[0]
 	// Rejoin the remainder of the split URI as the configuration for the guardian signer
-	// implementation.
-	keyConfig := strings.Join(signerUriSplit[1:], "")
+	// implementation. The remainder of the split is joined using the URI scheme separator.
+	keyConfig := strings.Join(signerUriSplit[1:], "://")
 
 	switch typeStr {
 	case "file":

node/pkg/p2p/run_params.go

@@ -244,7 +244,7 @@ func (p *RunParams) verify() error {
 	}
 	if p.nodeName != "" { // Heartbeating is enabled.
 		if p.guardianSigner == nil {
-			return errors.New("if heart beating is enabled, vs may not be nil")
+			return errors.New("if heart beating is enabled, guardianSigner may not be nil")
 		}
 	}
 	if p.obsvReqSendC != nil {