cluster: fix SSZ BytesN issue (#1695)

Fixes the SSZ issue that didn't ensure `BytesN` are always of length N. This is only included in v1.5 onwards. 

Also removes the lock.DistributedValidator.FeeRecipient field completely, also for previous versions as this isn't used at all and we do not need to support it for older versions.

category: bug
ticket: #1689

cluster/cluster_internal_test.go

@@ -35,6 +35,19 @@ func TestDefinitionVerify(t *testing.T) {
 	secret1, op1 := randomOperator(t)
 	secret3, creator := randomCreator(t)
 
+	t.Run("verify definition v1.5 solo", func(t *testing.T) {
+		definition := randomDefinition(t, creator, Operator{}, Operator{},
+			WithVersion(v1_5),
+			WithMultiVAddrs(RandomValidatorAddresses(2)),
+		)
+
+		definition, err = signCreator(secret3, definition)
+		require.NoError(t, err)
+
+		err = definition.VerifySignatures()
+		require.NoError(t, err)
+	})
+
 	t.Run("verify definition v1.5", func(t *testing.T) {
 		definition := randomDefinition(t, creator, op0, op1,
 			WithVersion(v1_5),

cluster/distvalidator.go

@@ -29,9 +29,6 @@ type DistValidator struct {
 	// PubShares are the public keys corresponding to each node's secret key share.
 	// It can be used to verify a partial signature created by any node in the cluster.
 	PubShares [][]byte `json:"public_shares,omitempty" ssz:"CompositeList[256],Bytes48" lock_hash:"1"`
-
-	// FeeRecipientAddress Ethereum address override for this validator, defaults to definition withdrawal address.
-	FeeRecipientAddress []byte `json:"fee_recipient_address,omitempty" ssz:"Bytes20" lock_hash:"2"`
 }
 
 // PublicKey returns the validator BLS group public key.
@@ -67,9 +64,8 @@ func distValidatorsFromV1x1(distValidators []distValidatorJSONv1x1) []DistValida
 	var resp []DistValidator
 	for _, dv := range distValidators {
 		resp = append(resp, DistValidator{
-			PubKey:              dv.PubKey,
-			PubShares:           dv.PubShares,
-			FeeRecipientAddress: dv.FeeRecipientAddress,
+			PubKey:    dv.PubKey,
+			PubShares: dv.PubShares,
 		})
 	}
 
@@ -80,9 +76,8 @@ func distValidatorsToV1x1(distValidators []DistValidator) []distValidatorJSONv1x
 	var resp []distValidatorJSONv1x1
 	for _, dv := range distValidators {
 		resp = append(resp, distValidatorJSONv1x1{
-			PubKey:              dv.PubKey,
-			PubShares:           dv.PubShares,
-			FeeRecipientAddress: dv.FeeRecipientAddress,
+			PubKey:    dv.PubKey,
+			PubShares: dv.PubShares,
 		})
 	}
 
@@ -97,9 +92,8 @@ func distValidatorsFromV1x2orLater(distValidators []distValidatorJSONv1x2) []Dis
 			shares = append(shares, share)
 		}
 		resp = append(resp, DistValidator{
-			PubKey:              dv.PubKey,
-			PubShares:           shares,
-			FeeRecipientAddress: dv.FeeRecipientAddress,
+			PubKey:    dv.PubKey,
+			PubShares: shares,
 		})
 	}
 
@@ -115,9 +109,8 @@ func distValidatorsToV1x2orLater(distValidators []DistValidator) []distValidator
 		}
 
 		resp = append(resp, distValidatorJSONv1x2{
-			PubKey:              dv.PubKey,
-			PubShares:           shares,
-			FeeRecipientAddress: dv.FeeRecipientAddress,
+			PubKey:    dv.PubKey,
+			PubShares: shares,
 		})
 	}
 

cluster/examples/cluster-definition-003.json

@@ -0,0 +1,40 @@
+{
+  "name": "solo flow",
+  "creator": {
+    "address": "0x65BA46f30Ac78DeDAc801F3787263B15E4E662C8",
+    "config_signature": "0x16123625db5eeaf6de350e2812b843bb2b792f219af919046ce4e96113ffd8f5517e93d7cb934be2a2466a153a2a0a87bc7fd340b84348bc6a03b0b1ec26c08f01"
+  },
+  "operators": [
+    {
+      "address": "",
+      "enr": "",
+      "config_signature": "",
+      "enr_signature": ""
+    },
+    {
+      "address": "",
+      "enr": "",
+      "config_signature": "",
+      "enr_signature": ""
+    }
+  ],
+  "uuid": "52FDFC07-2182-654F-163F-5F0F9A621D72",
+  "version": "v1.5.0",
+  "timestamp": "2023-01-26T16:59:53+02:00",
+  "num_validators": 2,
+  "threshold": 2,
+  "validators": [
+    {
+      "fee_recipient_address": "0x8da97239e9b517df4c248ff447bfc21032fa1f82",
+      "withdrawal_address": "0xac78bb58e615c380444f2413fe990511e38010d6"
+    },
+    {
+      "fee_recipient_address": "0x85b72c6b5ee8e49f29fcba5eb6b26ce3e2840d3b",
+      "withdrawal_address": "0x354e962c2e8817ce8fb389b2fe3ef5b280cc6ddc"
+    }
+  ],
+  "dkg_algorithm": "default",
+  "fork_version": "0x90000069",
+  "config_hash": "0x7d79b11219f4145bbd317d73fa22168a9309a52ddda6fea92808c90f28570196",
+  "definition_hash": "0x277e4840485aa41af0af975c13ce39d99bc939a797589f16473f5d4aeab14e88"
+}

cluster/helpers.go

@@ -220,6 +220,38 @@ func putByteList(h ssz.HashWalker, b []byte, limit int, field string) error {
 	return nil
 }
 
+// putByteList appends b as a ssz fixed size byte array of length n.
+func putBytesN(h ssz.HashWalker, b []byte, n int) error {
+	if len(b) > n {
+		return errors.New("bytes too long", z.Int("n", n), z.Int("l", len(b)))
+	}
+
+	h.PutBytes(leftPad(b, n))
+
+	return nil
+}
+
+// putHexBytes20 appends a 20 byte fixed size byte ssz array from the 0xhex address.
+func putHexBytes20(h ssz.HashWalker, addr string) error {
+	b, err := from0xHex(addr, addressLen)
+	if err != nil {
+		return err
+	}
+
+	h.PutBytes(leftPad(b, addressLen))
+
+	return nil
+}
+
+// leftPad returns the byte slice left padded with zero to ensure a length of at least l.
+func leftPad(b []byte, l int) []byte {
+	for len(b) < l {
+		b = append([]byte{0x00}, b...)
+	}
+
+	return b
+}
+
 // to0xHex returns the bytes as a 0x prefixed hex string.
 func to0xHex(b []byte) string {
 	if len(b) == 0 {

cluster/helpers_internal_test.go

@@ -29,6 +29,14 @@ import (
 	"github.com/obolnetwork/charon/testutil"
 )
 
+func TestLeftPad(t *testing.T) {
+	b := []byte{0x01, 0x02}
+	require.Equal(t, []byte{0x01, 0x02}, leftPad(b, 1))
+	require.Equal(t, []byte{0x01, 0x02}, leftPad(b, 2))
+	require.Equal(t, []byte{0x00, 0x01, 0x02}, leftPad(b, 3))
+	require.Equal(t, []byte{0x00, 0x00, 0x01, 0x02}, leftPad(b, 4))
+}
+
 func TestVerifySig(t *testing.T) {
 	secret, err := crypto.GenerateKey()
 	require.NoError(t, err)

cluster/lock.go

@@ -212,6 +212,12 @@ func unmarshalLockV1x0or1(data []byte) (lock Lock, err error) {
 		return Lock{}, errors.Wrap(err, "unmarshal definition")
 	}
 
+	for _, validator := range lockJSON.Validators {
+		if len(validator.FeeRecipientAddress) > 0 {
+			return Lock{}, errors.New("distributed validator fee recipient not supported anymore")
+		}
+	}
+
 	lock = Lock{
 		Definition:         lockJSON.Definition,
 		Validators:         distValidatorsFromV1x1(lockJSON.Validators),
@@ -228,6 +234,12 @@ func unmarshalLockV1x2orLater(data []byte) (lock Lock, err error) {
 		return Lock{}, errors.Wrap(err, "unmarshal definition")
 	}
 
+	for _, validator := range lockJSON.Validators {
+		if len(validator.FeeRecipientAddress) > 0 {
+			return Lock{}, errors.New("distributed validator fee recipient not supported anymore")
+		}
+	}
+
 	lock = Lock{
 		Definition:         lockJSON.Definition,
 		Validators:         distValidatorsFromV1x2orLater(lockJSON.Validators),