core: refactor UnsignedData to interface

core/consensus/component.go

@@ -75,7 +75,11 @@ func New(tcpNode host.Host, sender *p2p.Sender, peers []p2p.Peer, p2pKey *ecdsa.
 
 		// Decide sends consensus output to subscribers.
 		Decide: func(ctx context.Context, duty core.Duty, _ [32]byte, qcommit []qbft.Msg[core.Duty, [32]byte]) {
-			set := core.UnsignedDataSetFromProto(qcommit[0].(msg).msg.Value)
+			set, err := core.UnsignedDataSetFromProto(duty.Type, qcommit[0].(msg).msg.Value)
+			if err != nil {
+				log.Error(ctx, "Unmarshal decided value", err)
+				return
+			}
 			for _, sub := range c.subs {
 				if err := sub(ctx, duty, set); err != nil {
 					log.Warn(ctx, "Subscriber error", err)
@@ -146,7 +150,11 @@ func (c *Component) Propose(ctx context.Context, duty core.Duty, data core.Unsig
 	log.Debug(ctx, "Starting qbft consensus instance", z.Any("duty", duty))
 
 	// Hash the proposed data, since qbft ony supports simple comparable values.
-	value := core.UnsignedDataSetToProto(data)
+	value, err := core.UnsignedDataSetToProto(data)
+	if err != nil {
+		return err
+	}
+
 	hash, err := hashProto(value)
 	if err != nil {
 		return err

core/consensus/component_test.go

@@ -106,7 +106,7 @@ func TestComponent(t *testing.T) {
 			runErrs <- c.Propose(
 				log.WithCtx(ctx, z.Int("node", i)),
 				core.Duty{Type: core.DutyAttester},
-				core.UnsignedDataSet{pubkey: core.UnsignedData{byte(i)}},
+				core.UnsignedDataSet{pubkey: testutil.RandomCoreAttestationData(t)},
 			)
 		}(ctx, i, c)
 	}
@@ -118,7 +118,6 @@ func TestComponent(t *testing.T) {
 	for {
 		select {
 		case err := <-runErrs:
-			log.Error(ctx, "", err)
 			require.NoError(t, err)
 		case res := <-results:
 			t.Logf("Got result: %#v", res)

core/consensus/msg_internal_test.go

@@ -35,12 +35,13 @@ func TestHashProto(t *testing.T) {
 	set := testutil.RandomUnsignedDataSet(t)
 	testutil.RequireGoldenJSON(t, set)
 
-	setPB := core.UnsignedDataSetToProto(set)
+	setPB, err := core.UnsignedDataSetToProto(set)
+	require.NoError(t, err)
 	hash, err := hashProto(setPB)
 	require.NoError(t, err)
 
 	require.Equal(t,
-		"2629f0aaf0f78c37ad7aeae4cc3ee0ff05741a9b341e0002c03b257d62b2e237",
+		"39933362de95b6dabf0b6512bc19a43826debf8cb71936d99e251b053ad8846d",
 		hex.EncodeToString(hash[:]),
 	)
 }
@@ -49,14 +50,19 @@ func TestSigning(t *testing.T) {
 	privkey, err := crypto.GenerateKey()
 	require.NoError(t, err)
 
+	v, err := core.UnsignedDataSetToProto(testutil.RandomUnsignedDataSet(t))
+	require.NoError(t, err)
+	pv, err := core.UnsignedDataSetToProto(testutil.RandomUnsignedDataSet(t))
+	require.NoError(t, err)
+
 	msg := &pbv1.QBFTMsg{
 		Type:          rand.Int63(),
 		Duty:          core.DutyToProto(core.Duty{Type: core.DutyType(rand.Int()), Slot: rand.Int63()}),
 		PeerIdx:       rand.Int63(),
 		Round:         rand.Int63(),
-		Value:         core.UnsignedDataSetToProto(testutil.RandomUnsignedDataSet(t)),
+		Value:         v,
 		PreparedRound: rand.Int63(),
-		PreparedValue: core.UnsignedDataSetToProto(testutil.RandomUnsignedDataSet(t)),
+		PreparedValue: pv,
 		Signature:     nil,
 	}
 

core/consensus/testdata/TestHashProto.golden

@@ -1,3 +1,26 @@
 {
- "0xabdb58472a254161accbb62d14ecf92951cac6b37ad08b9ca4c36635fd7c88f803a08e7f3e8940d6b68d0ba7175a4f22": "eyJEYXRhIjp7InNsb3QiOiIxMTg5MzM1Nzc2OTI0NzkwMTg3MSIsImluZGV4IjoiMTc3NDkzMjg5MTI4Njk4MDE1MyIsImJlYWNvbl9ibG9ja19yb290IjoiMHg3ODg5MmVlMjg1ZWNlMTUxMTQ1NTc4MDg3NWQ2NGVlMmQzZDBkMGRlNmJmOGY5YjQ0Y2U4NWZmMDQ0YzZiMWY4Iiwic291cmNlIjp7ImVwb2NoIjoiMTYxMjMxNTcyODU4NTI5NjMxIiwicm9vdCI6IjB4NWU3YTgxYmZkZTI3YzM1NGYzZWRlMmQ2YmVjYzRlYTNhZTVlODg1MjZhOWY0YTU3OGJjYjllZjJkNGE2NTMxNCJ9LCJ0YXJnZXQiOnsiZXBvY2giOiIxMTg1MDQxMDc3NzEzOTU4MjU3NSIsInJvb3QiOiIweDYwN2JlMDYzNzEwNDVjM2YwMDBmOGE3OTZiY2U2YzUxMmMzODAxYWFjYWVlZGZhZDViNTA2NjY0ZThjMGU0YTcifX0sIkR1dHkiOnsicHVia2V5IjoiMHg5NGZmMDY5MTBkNTA1MGM0ZTAxZDY5NDZkMGY4ZWE1MGFlMDc0NmM5YjRhODE0ZDkyZjhkYjAwZGIyZjU0MjI5ZDAzOWI0YTQ2YzcyZjM2ZjVlMThiMjQzNzBmMjdlMDEiLCJzbG90IjoiMjI1OTQwNDExNzcwNDM5MzE1MiIsInZhbGlkYXRvcl9pbmRleCI6IjYwNTAxMjg2NzM4MDI5OTU4MjciLCJjb21taXR0ZWVfaW5kZXgiOiI5NzI0NjA1NDg3MzkzOTczNjAyIiwiY29tbWl0dGVlX2xlbmd0aCI6IjI1NiIsImNvbW1pdHRlZXNfYXRfc2xvdCI6IjI1NiIsInZhbGlkYXRvcl9jb21taXR0ZWVfaW5kZXgiOiIyNTEifX0="
+ "0x94ff06910d5050c4e01d6946d0f8ea50ae0746c9b4a814d92f8db00db2f54229d039b4a46c72f36f5e18b24370f27e01": {
+  "attestation_data": {
+   "slot": "1774932891286980153",
+   "index": "15267744271532198264",
+   "beacon_block_root": "0x511455780875d64ee2d3d0d0de6bf8f9b44ce85ff044c6b1f83b8e883bbf857a",
+   "source": {
+    "epoch": "16482847956365694147",
+    "root": "0x67cfe242cf3ccc4ea3ae5e88526a9f4a578bcb9ef2d4a65314768d6d299761ea"
+   },
+   "target": {
+    "epoch": "6303220950515014660",
+    "root": "0xddcdd01d756bce6c512c3801aacaeedfad5b506664e8c0e4a771ece0b8b7c196"
+   }
+  },
+  "attestation_duty": {
+   "pubkey": "0x8af7b15d89bb85ee4d8de40b0d811eb821ea31e406c84d0cb4387994a0cfbcd0ed27da8fcd309abfde5902543119abbc",
+   "slot": "6050128673802995827",
+   "validator_index": "9724605487393973602",
+   "committee_index": "12613765599614152010",
+   "committee_length": "256",
+   "committees_at_slot": "256",
+   "validator_committee_index": "224"
+  }
+ }
 }

core/dutydb/memory.go

@@ -16,7 +16,6 @@
 package dutydb
 
 import (
-	"bytes"
 	"context"
 	"sync"
 
@@ -155,11 +154,16 @@ func (db *MemDB) PubKeyByAttestation(_ context.Context, slot, commIdx, valCommId
 
 // storeAttestationUnsafe stores the unsigned attestation. It is unsafe since it assumes the lock is held.
 func (db *MemDB) storeAttestationUnsafe(pubkey core.PubKey, unsignedData core.UnsignedData) error {
-	attData, err := core.DecodeAttesterUnsignedData(unsignedData)
+	cloned, err := unsignedData.Clone() // Clone before storing.
 	if err != nil {
 		return err
 	}
 
+	attData, ok := cloned.(core.AttestationData)
+	if !ok {
+		return errors.New("invalid unsigned attestation data")
+	}
+
 	// Store key and value for PubKeyByAttestation
 	pKey := pkKey{
 		Slot:       int64(attData.Data.Slot),
@@ -193,27 +197,37 @@ func (db *MemDB) storeAttestationUnsafe(pubkey core.PubKey, unsignedData core.Un
 
 // storeBeaconBlockUnsafe stores the unsigned BeaconBlock. It is unsafe since it assumes the lock is held.
 func (db *MemDB) storeBeaconBlockUnsafe(unsignedData core.UnsignedData) error {
-	block, err := core.DecodeProposerUnsignedData(unsignedData)
+	cloned, err := unsignedData.Clone() // Clone before storing.
 	if err != nil {
 		return err
 	}
 
+	block, ok := cloned.(core.VersionedBeaconBlock)
+	if !ok {
+		return errors.New("invalid unsigned block")
+	}
+
 	slot, err := block.Slot()
 	if err != nil {
 		return err
 	}
 
-	if actualBlock, ok := db.proDuties[int64(slot)]; ok {
-		actualData, err := core.EncodeProposerUnsignedData(actualBlock)
+	if existing, ok := db.proDuties[int64(slot)]; ok {
+		existingRoot, err := existing.Root()
+		if err != nil {
+			return errors.Wrap(err, "block root")
+		}
+
+		providedRoot, err := block.Root()
 		if err != nil {
-			return errors.Wrap(err, "marshalling block")
+			return errors.Wrap(err, "block root")
 		}
 
-		if !bytes.Equal(actualData, unsignedData) {
+		if existingRoot != providedRoot {
 			return errors.New("clashing blocks")
 		}
 	} else {
-		db.proDuties[int64(slot)] = block
+		db.proDuties[int64(slot)] = &block.VersionedBeaconBlock
 	}
 
 	return nil

core/dutydb/memory_test.go

@@ -95,24 +95,22 @@ func TestMemDB(t *testing.T) {
 	duty := core.Duty{Slot: slot, Type: core.DutyAttester}
 
 	// The two validators have similar unsigned data, just the ValidatorCommitteeIndex is different.
-	unsignedA, err := core.EncodeAttesterUnsignedData(&core.AttestationData{
+	unsignedA := core.AttestationData{
 		Data: attData,
 		Duty: eth2v1.AttesterDuty{
 			CommitteeLength:         commLen,
 			ValidatorCommitteeIndex: valCommIdxA,
 			CommitteesAtSlot:        notZero,
 		},
-	})
-	require.NoError(t, err)
-	unsignedB, err := core.EncodeAttesterUnsignedData(&core.AttestationData{
+	}
+	unsignedB := core.AttestationData{
 		Data: attData,
 		Duty: eth2v1.AttesterDuty{
 			CommitteeLength:         commLen,
 			ValidatorCommitteeIndex: valCommIdxB,
 			CommitteesAtSlot:        notZero,
 		},
-	})
-	require.NoError(t, err)
+	}
 
 	// Store it
 	err = db.Store(ctx, duty, core.UnsignedDataSet{pubkeysByIdx[vIdxA]: unsignedA, pubkeysByIdx[vIdxB]: unsignedB})
@@ -172,7 +170,7 @@ func TestMemDBProposer(t *testing.T) {
 
 	// Store the Blocks
 	for i := 0; i < queries; i++ {
-		unsigned, err := core.EncodeProposerUnsignedData(blocks[i])
+		unsigned, err := core.NewVersionedBeaconBlock(blocks[i])
 		require.NoError(t, err)
 
 		duty := core.Duty{Slot: slots[i], Type: core.DutyProposer}
@@ -207,10 +205,10 @@ func TestMemDBClashingBlocks(t *testing.T) {
 	pubkey := testutil.RandomCorePubKey(t)
 
 	// Encode the Blocks
-	unsigned1, err := core.EncodeProposerUnsignedData(block1)
+	unsigned1, err := core.NewVersionedBeaconBlock(block1)
 	require.NoError(t, err)
 
-	unsigned2, err := core.EncodeProposerUnsignedData(block2)
+	unsigned2, err := core.NewVersionedBeaconBlock(block2)
 	require.NoError(t, err)
 
 	// Store the Blocks
@@ -240,7 +238,7 @@ func TestMemDBClashProposer(t *testing.T) {
 	pubkey := testutil.RandomCorePubKey(t)
 
 	// Encode the block
-	unsigned, err := core.EncodeProposerUnsignedData(block)
+	unsigned, err := core.NewVersionedBeaconBlock(block)
 	require.NoError(t, err)
 
 	// Store the Blocks
@@ -258,7 +256,7 @@ func TestMemDBClashProposer(t *testing.T) {
 
 	// Store a different block for the same slot
 	block.Phase0.ProposerIndex++
-	unsignedB, err := core.EncodeProposerUnsignedData(block)
+	unsignedB, err := core.NewVersionedBeaconBlock(block)
 	require.NoError(t, err)
 	err = db.Store(ctx, duty, core.UnsignedDataSet{
 		pubkey: unsignedB,

core/encode.go

@@ -19,7 +19,6 @@ import (
 	"encoding/json"
 
 	eth2v1 "github.com/attestantio/go-eth2-client/api/v1"
-	"github.com/attestantio/go-eth2-client/spec"
 
 	"github.com/obolnetwork/charon/app/errors"
 )
@@ -65,45 +64,3 @@ func EncodeProposerFetchArg(proDuty *eth2v1.ProposerDuty) (FetchArg, error) {
 
 	return b, nil
 }
-
-// DecodeAttesterUnsignedData returns the attestation data from the encoded UnsignedData.
-func DecodeAttesterUnsignedData(unsignedData UnsignedData) (*AttestationData, error) {
-	attData := new(AttestationData)
-	err := json.Unmarshal(unsignedData, attData)
-	if err != nil {
-		return nil, errors.Wrap(err, "unmarshal attestation data")
-	}
-
-	return attData, nil
-}
-
-// EncodeAttesterUnsignedData returns the attestation data as an encoded UnsignedData.
-func EncodeAttesterUnsignedData(attData *AttestationData) (UnsignedData, error) {
-	b, err := json.Marshal(attData)
-	if err != nil {
-		return nil, errors.Wrap(err, "marshal attestation data")
-	}
-
-	return b, nil
-}
-
-// EncodeProposerUnsignedData returns the proposer data as an encoded UnsignedData.
-func EncodeProposerUnsignedData(proData *spec.VersionedBeaconBlock) (UnsignedData, error) {
-	b, err := json.Marshal(proData)
-	if err != nil {
-		return nil, errors.Wrap(err, "marshal proposer data")
-	}
-
-	return b, nil
-}
-
-// DecodeProposerUnsignedData returns the proposer data from the encoded UnsignedData.
-func DecodeProposerUnsignedData(unsignedData UnsignedData) (*spec.VersionedBeaconBlock, error) {
-	proData := new(spec.VersionedBeaconBlock)
-	err := json.Unmarshal(unsignedData, proData)
-	if err != nil {
-		return nil, errors.Wrap(err, "unmarshal proposer data")
-	}
-
-	return proData, nil
-}

core/encode_test.go

@@ -18,7 +18,6 @@ package core_test
 import (
 	"testing"
 
-	"github.com/attestantio/go-eth2-client/spec"
 	"github.com/stretchr/testify/require"
 
 	"github.com/obolnetwork/charon/core"
@@ -41,25 +40,6 @@ func TestEncodeAttesterFetchArg(t *testing.T) {
 	require.Equal(t, arg1, arg2)
 }
 
-func TestEncodeAttesterUnsignedData(t *testing.T) {
-	attData1 := &core.AttestationData{
-		Data: *testutil.RandomAttestationData(),
-		Duty: *testutil.RandomAttestationDuty(t),
-	}
-
-	data1, err := core.EncodeAttesterUnsignedData(attData1)
-	require.NoError(t, err)
-
-	attData2, err := core.DecodeAttesterUnsignedData(data1)
-	require.NoError(t, err)
-
-	data2, err := core.EncodeAttesterUnsignedData(attData2)
-	require.NoError(t, err)
-
-	require.Equal(t, attData1, attData2)
-	require.Equal(t, data1, data2)
-}
-
 func TestEncodeProposerFetchArg(t *testing.T) {
 	proDuty1 := testutil.RandomProposerDuty(t)
 
@@ -75,22 +55,3 @@ func TestEncodeProposerFetchArg(t *testing.T) {
 	require.Equal(t, arg1, arg2)
 	require.Equal(t, proDuty1, proDuty2)
 }
-
-func TestEncodeProposerUnsignedData(t *testing.T) {
-	proData1 := &spec.VersionedBeaconBlock{
-		Version: spec.DataVersionPhase0,
-		Phase0:  testutil.RandomPhase0BeaconBlock(),
-	}
-
-	data1, err := core.EncodeProposerUnsignedData(proData1)
-	require.NoError(t, err)
-
-	proData2, err := core.DecodeProposerUnsignedData(data1)
-	require.NoError(t, err)
-
-	data2, err := core.EncodeProposerUnsignedData(proData2)
-	require.NoError(t, err)
-
-	require.Equal(t, proData1, proData2)
-	require.Equal(t, data1, data2)
-}