diff --git a/node/cmd/guardiand/node.go b/node/cmd/guardiand/node.go index 7018da5c81..ce0a9559ee 100644 --- a/node/cmd/guardiand/node.go +++ b/node/cmd/guardiand/node.go @@ -626,7 +626,7 @@ func runNode(cmd *cobra.Command, args []string) { } 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 + // being executed if at least one is specified. For example, in the case where the signer 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") diff --git a/node/pkg/adminrpc/adminserver.go b/node/pkg/adminrpc/adminserver.go index b6bcbd44c9..85e2b68b70 100644 --- a/node/pkg/adminrpc/adminserver.go +++ b/node/pkg/adminrpc/adminserver.go @@ -42,6 +42,7 @@ import ( ) const maxResetReleaseTimerDays = 7 +const ecdsaSignatureLength = 65 var ( vaaInjectionsTotal = promauto.NewCounter( @@ -1167,12 +1168,12 @@ func (s *nodePrivilegedService) SignExistingVAA(ctx context.Context, req *nodev1 panic(err) } - sigData := [65]byte{} - copy(sigData[:], sig) + signature := [ecdsaSignatureLength]byte{} + copy(signature[:], sig) newVAA.Signatures = append(v.Signatures, &vaa.Signature{ Index: uint8(localGuardianIndex), - Signature: sigData, + Signature: signature, }) // Sort VAA signatures by guardian ID diff --git a/node/pkg/adminrpc/adminserver_test.go b/node/pkg/adminrpc/adminserver_test.go index c3c345db17..f5f6ed6e98 100644 --- a/node/pkg/adminrpc/adminserver_test.go +++ b/node/pkg/adminrpc/adminserver_test.go @@ -107,7 +107,8 @@ func addrsToHexStrings(addrs []common.Address) (out []string) { return } -func generateMockVAA(gsIndex uint32, signers []guardiansigner.GuardianSigner) []byte { +func generateMockVAA(gsIndex uint32, signers []guardiansigner.GuardianSigner, t *testing.T) []byte { + t.Helper() v := &vaa.VAA{ Version: 1, GuardianSetIndex: gsIndex, @@ -123,15 +124,15 @@ func generateMockVAA(gsIndex uint32, signers []guardiansigner.GuardianSigner) [] for i, signer := range signers { sig, err := signer.Sign(v.SigningDigest().Bytes()) if err != nil { - panic(err) + require.NoError(t, err) } - sigData := [65]byte{} - copy(sigData[:], sig) + signature := [ecdsaSignatureLength]byte{} + copy(signature[:], sig) v.Signatures = append(v.Signatures, &vaa.Signature{ Index: uint8(i), - Signature: sigData, + Signature: signature, }) } @@ -182,7 +183,7 @@ func TestSignExistingVAA_NotGuardian(t *testing.T) { signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, signers) + v := generateMockVAA(0, signers, t) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ Vaa: v, @@ -196,7 +197,7 @@ func TestSignExistingVAA_InvalidVAA(t *testing.T) { signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, signers[:2]) + v := generateMockVAA(0, signers[:2], t) gsAddrs = append(gsAddrs, s.guardianAddress) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ @@ -211,7 +212,7 @@ func TestSignExistingVAA_DuplicateGuardian(t *testing.T) { signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, signers) + v := generateMockVAA(0, signers, t) gsAddrs = append(gsAddrs, s.guardianAddress) gsAddrs = append(gsAddrs, s.guardianAddress) @@ -231,7 +232,7 @@ func TestSignExistingVAA_AlreadyGuardian(t *testing.T) { guardianSetIndex: 0, } - v := generateMockVAA(0, append(signers, s.guardianSigner)) + v := generateMockVAA(0, append(signers, s.guardianSigner), t) gsAddrs = append(gsAddrs, s.guardianAddress) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ @@ -246,7 +247,7 @@ func TestSignExistingVAA_NotAFutureGuardian(t *testing.T) { signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, signers) + v := generateMockVAA(0, signers, t) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ Vaa: v, @@ -260,7 +261,7 @@ func TestSignExistingVAA_CantReachQuorum(t *testing.T) { signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, signers) + v := generateMockVAA(0, signers, t) gsAddrs = append(gsAddrs, s.guardianAddress) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ @@ -275,7 +276,7 @@ func TestSignExistingVAA_Valid(t *testing.T) { signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, signers) + v := generateMockVAA(0, signers, t) gsAddrs = append(gsAddrs, s.guardianAddress) res, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ @@ -285,7 +286,7 @@ func TestSignExistingVAA_Valid(t *testing.T) { }) require.NoError(t, err) - v2 := generateMockVAA(1, append(signers, s.guardianSigner)) + v2 := generateMockVAA(1, append(signers, s.guardianSigner), t) require.Equal(t, v2, res.Vaa) } diff --git a/node/pkg/guardiansigner/generatedsigner.go b/node/pkg/guardiansigner/generatedsigner.go index 9b0460e025..503fccd5ea 100644 --- a/node/pkg/guardiansigner/generatedsigner.go +++ b/node/pkg/guardiansigner/generatedsigner.go @@ -5,7 +5,6 @@ import ( "crypto/rand" "fmt" - "github.com/ethereum/go-ethereum/crypto" ethcrypto "github.com/ethereum/go-ethereum/crypto" ) @@ -18,7 +17,7 @@ type GeneratedSigner struct { func NewGeneratedSigner(key *ecdsa.PrivateKey) (*GeneratedSigner, error) { if key == nil { - privateKey, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + privateKey, err := ecdsa.GenerateKey(ethcrypto.S256(), rand.Reader) return &GeneratedSigner{privateKey: privateKey}, err } else { return &GeneratedSigner{privateKey: key}, nil @@ -28,10 +27,10 @@ func NewGeneratedSigner(key *ecdsa.PrivateKey) (*GeneratedSigner, error) { func (gs *GeneratedSigner) Sign(hash []byte) (sig []byte, err error) { // Sign the hash - sig, err = crypto.Sign(hash, gs.privateKey) + sig, err = ethcrypto.Sign(hash, gs.privateKey) if err != nil { - return nil, fmt.Errorf("failed to sign wormchain address: %w", err) + return nil, fmt.Errorf("failed to sign: %w", err) } return sig, nil diff --git a/node/pkg/guardiansigner/guardiansigner.go b/node/pkg/guardiansigner/guardiansigner.go index 8b2076bf57..057f9c03bf 100644 --- a/node/pkg/guardiansigner/guardiansigner.go +++ b/node/pkg/guardiansigner/guardiansigner.go @@ -41,7 +41,7 @@ func NewGuardianSignerFromUri(signerUri string, unsafeDevMode bool) (GuardianSig case FileSignerType: return NewFileSigner(unsafeDevMode, signerKeyConfig) default: - return nil, errors.New("Unsupported guardian signer type") + return nil, errors.New("unsupported guardian signer type") } } @@ -51,7 +51,7 @@ func ParseSignerUri(signerUri string) (signerType SignerType, signerKeyConfig st // This check is purely for ensuring that there is actually a path separator. if len(signerUriSplit) < 2 { - return InvalidSignerType, "", errors.New("No path separator in guardian signer URI") + return InvalidSignerType, "", errors.New("no path separator in guardian signer URI") } typeStr := signerUriSplit[0] @@ -63,6 +63,6 @@ func ParseSignerUri(signerUri string) (signerType SignerType, signerKeyConfig st case "file": return FileSignerType, keyConfig, nil default: - return InvalidSignerType, "", fmt.Errorf("Unsupported guardian signer type: %s", typeStr) + return InvalidSignerType, "", fmt.Errorf("unsupported guardian signer type: %s", typeStr) } } diff --git a/node/pkg/guardiansigner/guardiansigner_test.go b/node/pkg/guardiansigner/guardiansigner_test.go index 1ea5c38ae3..5e660357ed 100644 --- a/node/pkg/guardiansigner/guardiansigner_test.go +++ b/node/pkg/guardiansigner/guardiansigner_test.go @@ -6,6 +6,7 @@ import ( "github.com/ethereum/go-ethereum/crypto" ethcrypto "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestParseSignerUri(t *testing.T) { @@ -66,17 +67,17 @@ func TestFileSigner(t *testing.T) { // Attempt to generate signer using top-level generator fileSigner1, err := NewGuardianSignerFromUri(fileUri, true) - assert.NoError(t, err) + require.NoError(t, err) assert.NotNil(t, fileSigner1) assert.Equal(t, ethcrypto.PubkeyToAddress(fileSigner1.PublicKey()).Hex(), expectedEthAddress) // Attempt to generate signer using NewFileSigner signerType, keyPath, err := ParseSignerUri(fileUri) assert.Equal(t, signerType, FileSignerType) - assert.NoError(t, err) + require.NoError(t, err) fileSigner2, err := NewFileSigner(true, keyPath) - assert.NoError(t, err) + require.NoError(t, err) assert.NotNil(t, fileSigner2) assert.Equal(t, ethcrypto.PubkeyToAddress(fileSigner2.PublicKey()).Hex(), expectedEthAddress)