cleanup

ObolNetwork · Sep 8, 2022 · aeae27b · aeae27b
1 parent 821adb4
commit aeae27b
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 58 deletions.
diff --git a/cluster/cluster_test.go b/cluster/cluster_test.go
@@ -83,7 +83,6 @@ func TestEncode(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, b1, b2)
-		require.Equal(t, definition, definition2)
 
 		lock := cluster.Lock{
 			Definition:         definition,
@@ -127,7 +126,6 @@ func TestEncode(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, b1, b2)
-		require.Equal(t, lock, lock2)
 	}
 }
 

diff --git a/cluster/definition.go b/cluster/definition.go
@@ -91,6 +91,12 @@ type Definition struct {
 
 	// Operators define the charon nodes in the cluster and their operators.
 	Operators []Operator
+
+	// unmarshalledConfigHash defines config hash from config_hash field from JSON string after json.Unmarshal.
+	unmarshalledConfigHash []byte
+
+	// unmarshalledDefinitionHash defines definition hash from definition_hash field from JSON string after json.Unmarshal.
+	unmarshalledDefinitionHash []byte
 }
 
 // NodeIdx returns the node index for the peer.
@@ -169,42 +175,14 @@ func (d Definition) Verify() error {
 	return nil
 }
 
-func (d Definition) VerifyHashesJSON(data []byte) error {
-	var configHashJSON, definitionHashJSON []byte
-	switch {
-	case isJSONv1x1(d.Version):
-		hashes := struct {
-			ConfigHash     []byte `json:"config_hash"`
-			DefinitionHash []byte `json:"definition_hash"`
-		}{}
-		if err := json.Unmarshal(data, &hashes); err != nil {
-			return errors.Wrap(err, "unmarshal hashes")
-		}
-
-		configHashJSON = hashes.ConfigHash
-		definitionHashJSON = hashes.DefinitionHash
-	case isJSONv1x2(d.Version):
-		hashes := struct {
-			ConfigHash     ethHex `json:"config_hash"`
-			DefinitionHash ethHex `json:"definition_hash"`
-		}{}
-		if err := json.Unmarshal(data, &hashes); err != nil {
-			return errors.Wrap(err, "unmarshal hashes")
-		}
-
-		configHashJSON = hashes.ConfigHash
-		definitionHashJSON = hashes.DefinitionHash
-	default:
-		return errors.New("unsupported version")
-	}
-
+func (d Definition) VerifyHashes() error {
 	// Verify config_hash
 	confHash, err := d.ConfigHash()
 	if err != nil {
 		return errors.Wrap(err, "config hash")
 	}
 
-	if !bytes.Equal(configHashJSON, confHash[:]) {
+	if !bytes.Equal(d.unmarshalledConfigHash, confHash[:]) {
 		return errors.New("invalid config hash")
 	}
 
@@ -214,7 +192,7 @@ func (d Definition) VerifyHashesJSON(data []byte) error {
 		return errors.Wrap(err, "definition hash")
 	}
 
-	if !bytes.Equal(definitionHashJSON, defHash[:]) {
+	if !bytes.Equal(d.unmarshalledDefinitionHash, defHash[:]) {
 		return errors.New("invalid definition hash")
 	}
 
@@ -449,17 +427,19 @@ func unmarshalDefinitionV1x1(data []byte) (Definition, error) {
 	}
 
 	return Definition{
-		Name:                defJSON.Name,
-		UUID:                defJSON.UUID,
-		Version:             defJSON.Version,
-		Timestamp:           defJSON.Timestamp,
-		NumValidators:       defJSON.NumValidators,
-		Threshold:           defJSON.Threshold,
-		FeeRecipientAddress: defJSON.FeeRecipientAddress,
-		WithdrawalAddress:   defJSON.WithdrawalAddress,
-		DKGAlgorithm:        defJSON.DKGAlgorithm,
-		ForkVersion:         defJSON.ForkVersion,
-		Operators:           operators,
+		Name:                       defJSON.Name,
+		UUID:                       defJSON.UUID,
+		Version:                    defJSON.Version,
+		Timestamp:                  defJSON.Timestamp,
+		NumValidators:              defJSON.NumValidators,
+		Threshold:                  defJSON.Threshold,
+		FeeRecipientAddress:        defJSON.FeeRecipientAddress,
+		WithdrawalAddress:          defJSON.WithdrawalAddress,
+		DKGAlgorithm:               defJSON.DKGAlgorithm,
+		ForkVersion:                defJSON.ForkVersion,
+		Operators:                  operators,
+		unmarshalledConfigHash:     defJSON.ConfigHash,
+		unmarshalledDefinitionHash: defJSON.DefinitionHash,
 	}, nil
 }
 
@@ -470,17 +450,19 @@ func unmarshalDefinitionV1x2(data []byte) (Definition, error) {
 	}
 
 	return Definition{
-		Name:                defJSON.Name,
-		UUID:                defJSON.UUID,
-		Version:             defJSON.Version,
-		Timestamp:           defJSON.Timestamp,
-		NumValidators:       defJSON.NumValidators,
-		Threshold:           defJSON.Threshold,
-		FeeRecipientAddress: defJSON.FeeRecipientAddress,
-		WithdrawalAddress:   defJSON.WithdrawalAddress,
-		DKGAlgorithm:        defJSON.DKGAlgorithm,
-		ForkVersion:         defJSON.ForkVersion,
-		Operators:           operatorsFromV1x2(defJSON.Operators),
+		Name:                       defJSON.Name,
+		UUID:                       defJSON.UUID,
+		Version:                    defJSON.Version,
+		Timestamp:                  defJSON.Timestamp,
+		NumValidators:              defJSON.NumValidators,
+		Threshold:                  defJSON.Threshold,
+		FeeRecipientAddress:        defJSON.FeeRecipientAddress,
+		WithdrawalAddress:          defJSON.WithdrawalAddress,
+		DKGAlgorithm:               defJSON.DKGAlgorithm,
+		ForkVersion:                defJSON.ForkVersion,
+		Operators:                  operatorsFromV1x2(defJSON.Operators),
+		unmarshalledConfigHash:     defJSON.ConfigHash,
+		unmarshalledDefinitionHash: defJSON.DefinitionHash,
 	}, nil
 }
 

diff --git a/dkg/disk.go b/dkg/disk.go
@@ -61,7 +61,7 @@ func loadDefinition(ctx context.Context, conf Config) (cluster.Definition, error
 	}
 
 	// Verify config hash and definition hash from json string and resultant cluster.Definition.
-	if err := res.VerifyHashesJSON(buf); err != nil && !conf.NoVerify {
+	if err := res.VerifyHashes(); err != nil && !conf.NoVerify {
 		return cluster.Definition{}, errors.Wrap(err, "cluster definition hash verification failed. Run with --no-verify to bypass verification at own risk")
 	} else if err != nil && conf.NoVerify {
 		log.Warn(ctx, "Ignoring failed cluster definition hash verification due to --no-verify flag", err)

diff --git a/dkg/disk_internal_test.go b/dkg/disk_internal_test.go
@@ -16,6 +16,7 @@
 package dkg
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -74,7 +75,7 @@ func TestFetchDefinition(t *testing.T) {
 				return
 			}
 			require.NoError(t, err)
-			require.Equal(t, tt.want, got)
+			require.True(t, compareDefinitions(t, got, tt.want))
 		})
 	}
 }
@@ -160,7 +161,19 @@ func TestLoadDefinition(t *testing.T) {
 				return
 			}
 			require.NoError(t, err)
-			require.Equal(t, tt.want, got)
+			require.True(t, compareDefinitions(t, got, tt.want))
 		})
 	}
 }
+
+func compareDefinitions(t *testing.T, a, b cluster.Definition) bool {
+	t.Helper()
+
+	b1, err := json.Marshal(a)
+	require.NoError(t, err)
+
+	b2, err := json.Marshal(b)
+	require.NoError(t, err)
+
+	return bytes.Equal(b1, b2)
+}