From 66c637ac564985f780f8306f91241e164b719a80 Mon Sep 17 00:00:00 2001 From: Diego Date: Thu, 4 May 2023 12:43:21 -0300 Subject: [PATCH 1/3] fix(babe): Add support for versioned NextConfigData decoding --- dot/digest/digest.go | 25 ++++++++----- dot/digest/digest_integration_test.go | 19 +++++++--- dot/digest/interfaces.go | 2 +- dot/digest/mock_epoch_state_test.go | 2 +- dot/state/epoch.go | 14 ++++---- dot/state/epoch_test.go | 16 ++++----- dot/types/consensus_digest.go | 52 +++++++++++++++++++++++---- dot/types/consensus_digest_test.go | 23 ++++++++++++ lib/babe/verify_integration_test.go | 12 ++++--- 9 files changed, 126 insertions(+), 39 deletions(-) diff --git a/dot/digest/digest.go b/dot/digest/digest.go index a0fac2c4f6..2f77cc4f5d 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -201,17 +201,26 @@ func (h *Handler) handleBabeConsensusDigest(digest scale.VaryingDataType, header case types.BABEOnDisabled: return nil - case types.NextConfigData: - currEpoch, err := h.epochState.GetEpochForBlock(header) + case types.VersionedNextConfigData: + nextVersionedConfigData := digestValue.(types.VersionedNextConfigData) + nextConfigDataVersion, err := nextVersionedConfigData.Value() if err != nil { - return fmt.Errorf("cannot get epoch for block %d (%s): %w", - header.Number, headerHash, err) + return fmt.Errorf("getting digest value: %w", err) } - nextEpoch := currEpoch + 1 - h.epochState.StoreBABENextConfigData(nextEpoch, headerHash, val) - h.logger.Debugf("stored BABENextConfigData data: %v for hash: %s to epoch: %d", digest, headerHash, nextEpoch) - return nil + switch nextConfigData := nextConfigDataVersion.(type) { + case types.NextConfigDataV1: + currEpoch, err := h.epochState.GetEpochForBlock(header) + if err != nil { + return fmt.Errorf("cannot get epoch for block %d (%s): %w", header.Number, headerHash, err) + } + nextEpoch := currEpoch + 1 + h.epochState.StoreBABENextConfigData(nextEpoch, headerHash, nextConfigData) + h.logger.Debugf("stored BABENextConfigData data: %v for hash: %s to epoch: %d", digest, headerHash, nextEpoch) + return nil + default: + return fmt.Errorf("next config data version not supported: %T", nextConfigDataVersion) + } } return errors.New("invalid consensus digest data") diff --git a/dot/digest/digest_integration_test.go b/dot/digest/digest_integration_test.go index 402014417f..c9941960af 100644 --- a/dot/digest/digest_integration_test.go +++ b/dot/digest/digest_integration_test.go @@ -381,13 +381,16 @@ func TestHandler_HandleNextEpochData(t *testing.T) { func TestHandler_HandleNextConfigData(t *testing.T) { var digest = types.NewBabeConsensusDigest() - nextConfigData := types.NextConfigData{ + nextConfigData := types.NextConfigDataV1{ C1: 1, C2: 8, SecondarySlots: 1, } - err := digest.Set(nextConfigData) + VersionedNextConfigData := types.NewVersionedNextConfigData() + VersionedNextConfigData.Set(nextConfigData) + + err := digest.Set(VersionedNextConfigData) require.NoError(t, err) data, err := scale.Marshal(digest) @@ -428,12 +431,20 @@ func TestHandler_HandleNextConfigData(t *testing.T) { digestValue, err := digest.Value() require.NoError(t, err) - act, ok := digestValue.(types.NextConfigData) + nextVersionedConfigData, ok := digestValue.(types.VersionedNextConfigData) + if !ok { + t.Fatal() + } + + decodedNextConfigData, err := nextVersionedConfigData.Value() + require.NoError(t, err) + + decodedNextConfigDataV1, ok := decodedNextConfigData.(types.NextConfigDataV1) if !ok { t.Fatal() } stored, err := handler.epochState.(*state.EpochState).GetConfigData(targetEpoch, nil) require.NoError(t, err) - require.Equal(t, act.ToConfigData(), stored) + require.Equal(t, decodedNextConfigDataV1.ToConfigData(), stored) } diff --git a/dot/digest/interfaces.go b/dot/digest/interfaces.go index f114af7107..151dff1c36 100644 --- a/dot/digest/interfaces.go +++ b/dot/digest/interfaces.go @@ -23,7 +23,7 @@ type BlockState interface { type EpochState interface { GetEpochForBlock(header *types.Header) (uint64, error) StoreBABENextEpochData(epoch uint64, hash common.Hash, nextEpochData types.NextEpochData) - StoreBABENextConfigData(epoch uint64, hash common.Hash, nextEpochData types.NextConfigData) + StoreBABENextConfigData(epoch uint64, hash common.Hash, nextEpochData types.NextConfigDataV1) FinalizeBABENextEpochData(finalizedHeader *types.Header) error FinalizeBABENextConfigData(finalizedHeader *types.Header) error } diff --git a/dot/digest/mock_epoch_state_test.go b/dot/digest/mock_epoch_state_test.go index ee839078b6..dc1f49c9a9 100644 --- a/dot/digest/mock_epoch_state_test.go +++ b/dot/digest/mock_epoch_state_test.go @@ -79,7 +79,7 @@ func (mr *MockEpochStateMockRecorder) GetEpochForBlock(arg0 interface{}) *gomock } // StoreBABENextConfigData mocks base method. -func (m *MockEpochState) StoreBABENextConfigData(arg0 uint64, arg1 common.Hash, arg2 types.NextConfigData) { +func (m *MockEpochState) StoreBABENextConfigData(arg0 uint64, arg1 common.Hash, arg2 types.NextConfigDataV1) { m.ctrl.T.Helper() m.ctrl.Call(m, "StoreBABENextConfigData", arg0, arg1, arg2) } diff --git a/dot/state/epoch.go b/dot/state/epoch.go index 0aa3decf69..ca7c8abcce 100644 --- a/dot/state/epoch.go +++ b/dot/state/epoch.go @@ -63,7 +63,7 @@ type EpochState struct { nextConfigDataLock sync.RWMutex // nextConfigData follows the format map[epoch]map[block hash]next config data - nextConfigData nextEpochMap[types.NextConfigData] + nextConfigData nextEpochMap[types.NextConfigDataV1] } // NewEpochStateFromGenesis returns a new EpochState given information for the first epoch, fetched from the runtime @@ -92,7 +92,7 @@ func NewEpochStateFromGenesis(db *chaindb.BadgerDB, blockState *BlockState, db: epochDB, epochLength: genesisConfig.EpochLength, nextEpochData: make(nextEpochMap[types.NextEpochData]), - nextConfigData: make(nextEpochMap[types.NextConfigData]), + nextConfigData: make(nextEpochMap[types.NextConfigDataV1]), } auths, err := types.BABEAuthorityRawToAuthority(genesisConfig.GenesisAuthorities) @@ -153,7 +153,7 @@ func NewEpochState(db *chaindb.BadgerDB, blockState *BlockState) (*EpochState, e epochLength: epochLength, skipToEpoch: skipToEpoch, nextEpochData: make(nextEpochMap[types.NextEpochData]), - nextConfigData: make(nextEpochMap[types.NextConfigData]), + nextConfigData: make(nextEpochMap[types.NextConfigDataV1]), }, nil } @@ -383,7 +383,7 @@ func (s *EpochState) getConfigDataFromDatabase(epoch uint64) (*types.ConfigData, return info, nil } -type nextEpochMap[T types.NextEpochData | types.NextConfigData] map[uint64]map[common.Hash]T +type nextEpochMap[T types.NextEpochData | types.NextConfigDataV1] map[uint64]map[common.Hash]T func (nem nextEpochMap[T]) Retrieve(blockState *BlockState, epoch uint64, header *types.Header) (*T, error) { atEpoch, has := nem[epoch] @@ -505,13 +505,13 @@ func (s *EpochState) StoreBABENextEpochData(epoch uint64, hash common.Hash, next } // StoreBABENextConfigData stores the types.NextConfigData under epoch and hash keys -func (s *EpochState) StoreBABENextConfigData(epoch uint64, hash common.Hash, nextConfigData types.NextConfigData) { +func (s *EpochState) StoreBABENextConfigData(epoch uint64, hash common.Hash, nextConfigData types.NextConfigDataV1) { s.nextConfigDataLock.Lock() defer s.nextConfigDataLock.Unlock() _, has := s.nextConfigData[epoch] if !has { - s.nextConfigData[epoch] = make(map[common.Hash]types.NextConfigData) + s.nextConfigData[epoch] = make(map[common.Hash]types.NextConfigDataV1) } s.nextConfigData[epoch][hash] = nextConfigData } @@ -641,7 +641,7 @@ func (s *EpochState) FinalizeBABENextConfigData(finalizedHeader *types.Header) e // findFinalizedHeaderForEpoch given a specific epoch (the key) will go through the hashes looking // for a database persisted hash (belonging to the finalized chain) // which contains the right configuration or data to be persisted and safely used -func findFinalizedHeaderForEpoch[T types.NextConfigData | types.NextEpochData]( +func findFinalizedHeaderForEpoch[T types.NextConfigDataV1 | types.NextEpochData]( nextEpochMap map[uint64]map[common.Hash]T, es *EpochState, epoch uint64) (next *T, err error) { hashes, has := nextEpochMap[epoch] if !has { diff --git a/dot/state/epoch_test.go b/dot/state/epoch_test.go index aa0a0903e5..4addc8e278 100644 --- a/dot/state/epoch_test.go +++ b/dot/state/epoch_test.go @@ -446,7 +446,7 @@ func TestStoreAndFinalizeBabeNextConfigData(t *testing.T) { tests := map[string]struct { finalizedHeader *types.Header - inMemoryEpoch []inMemoryBABEData[types.NextConfigData] + inMemoryEpoch []inMemoryBABEData[types.NextConfigDataV1] finalizedEpoch uint64 expectErr error shouldRemainInMemory int @@ -455,7 +455,7 @@ func TestStoreAndFinalizeBabeNextConfigData(t *testing.T) { shouldRemainInMemory: 1, finalizedEpoch: 2, finalizedHeader: finalizedHeader, - inMemoryEpoch: []inMemoryBABEData[types.NextConfigData]{ + inMemoryEpoch: []inMemoryBABEData[types.NextConfigDataV1]{ { epoch: 1, hashes: []common.Hash{ @@ -463,7 +463,7 @@ func TestStoreAndFinalizeBabeNextConfigData(t *testing.T) { common.MustHexToHash("0x91b171bb158e2d3848fa23a9f1c25182fb8e20313b2c1eb49219da7a70ce90c3"), common.MustHexToHash("0xc0096358534ec8d21d01d34b836eed476a1c343f8724fa2153dc0725ad797a90"), }, - nextData: []types.NextConfigData{ + nextData: []types.NextConfigDataV1{ { C1: 1, C2: 2, @@ -488,7 +488,7 @@ func TestStoreAndFinalizeBabeNextConfigData(t *testing.T) { common.MustHexToHash("0xd380bee22de487a707cbda65dd9d4e2188f736908c42cf390c8919d4f7fc547c"), finalizedHeaderHash, }, - nextData: []types.NextConfigData{ + nextData: []types.NextConfigDataV1{ { C1: 1, C2: 2, @@ -511,7 +511,7 @@ func TestStoreAndFinalizeBabeNextConfigData(t *testing.T) { hashes: []common.Hash{ common.MustHexToHash("0xab5c9230a7dde8bb90a6728ba4a0165423294dac14336b1443f865b796ff682c"), }, - nextData: []types.NextConfigData{ + nextData: []types.NextConfigDataV1{ { C1: 1, C2: 2, @@ -526,7 +526,7 @@ func TestStoreAndFinalizeBabeNextConfigData(t *testing.T) { finalizedEpoch: 2, finalizedHeader: finalizedHeader, // finalize when the hash does not exist expectErr: errHashNotPersisted, - inMemoryEpoch: []inMemoryBABEData[types.NextConfigData]{ + inMemoryEpoch: []inMemoryBABEData[types.NextConfigDataV1]{ { epoch: 2, hashes: []common.Hash{ @@ -534,7 +534,7 @@ func TestStoreAndFinalizeBabeNextConfigData(t *testing.T) { common.MustHexToHash("0x91b171bb158e2d3848fa23a9f1c25182fb8e20313b2c1eb49219da7a70ce90c3"), common.MustHexToHash("0xc0096358534ec8d21d01d34b836eed476a1c343f8724fa2153dc0725ad797a90"), }, - nextData: []types.NextConfigData{ + nextData: []types.NextConfigDataV1{ { C1: 1, C2: 2, @@ -558,7 +558,7 @@ func TestStoreAndFinalizeBabeNextConfigData(t *testing.T) { shouldRemainInMemory: 0, finalizedEpoch: 1, // try to finalize an epoch that does not exist finalizedHeader: finalizedHeader, - inMemoryEpoch: []inMemoryBABEData[types.NextConfigData]{}, + inMemoryEpoch: []inMemoryBABEData[types.NextConfigDataV1]{}, }, } diff --git a/dot/types/consensus_digest.go b/dot/types/consensus_digest.go index 9ea043782a..9cb061e325 100644 --- a/dot/types/consensus_digest.go +++ b/dot/types/consensus_digest.go @@ -11,7 +11,7 @@ import ( // NewBabeConsensusDigest constructs a vdt representing a babe consensus digest func NewBabeConsensusDigest() scale.VaryingDataType { - return scale.MustNewVaryingDataType(NextEpochData{}, BABEOnDisabled{}, NextConfigData{}) + return scale.MustNewVaryingDataType(NextEpochData{}, BABEOnDisabled{}, NewVersionedNextConfigData()) } // NewGrandpaConsensusDigest constructs a vdt representing a grandpa consensus digest @@ -126,27 +126,67 @@ func (b BABEOnDisabled) String() string { return fmt.Sprintf("BABEOnDisabled{ID=%d}", b.ID) } -// NextConfigData is the digest that contains changes to the BABE configuration. +// NextConfigDataV1 is the digest that contains changes to the BABE configuration. // It is potentially included in the first block of an epoch to describe the next epoch. -type NextConfigData struct { +type NextConfigDataV1 struct { C1 uint64 C2 uint64 SecondarySlots byte } // Index returns VDT index -func (NextConfigData) Index() uint { return 3 } //skipcq: GO-W1029 +func (NextConfigDataV1) Index() uint { return 1 } //skipcq: GO-W1029 -func (d NextConfigData) String() string { //skipcq: GO-W1029 +func (d NextConfigDataV1) String() string { //skipcq: GO-W1029 return fmt.Sprintf("NextConfigData{C1=%d, C2=%d, SecondarySlots=%d}", d.C1, d.C2, d.SecondarySlots) } // ToConfigData returns the NextConfigData as ConfigData -func (d *NextConfigData) ToConfigData() *ConfigData { //skipcq: GO-W1029 +func (d *NextConfigDataV1) ToConfigData() *ConfigData { //skipcq: GO-W1029 return &ConfigData{ C1: d.C1, C2: d.C2, SecondarySlots: d.SecondarySlots, } } + +// VersionedNextConfigData represents the enum of next config data consensus digest messages +type VersionedNextConfigData scale.VaryingDataType + +// Index returns VDT index +func (VersionedNextConfigData) Index() uint { return 3 } + +// Value returns the current VDT value +func (vncd *VersionedNextConfigData) Value() (val scale.VaryingDataTypeValue, err error) { + vdt := scale.VaryingDataType(*vncd) + return vdt.Value() +} + +// Set updates the current VDT value to be `val` +func (vncd *VersionedNextConfigData) Set(val scale.VaryingDataTypeValue) (err error) { + vdt := scale.VaryingDataType(*vncd) + err = vdt.Set(val) + if err != nil { + return fmt.Errorf("setting varying data type value: %w", err) + } + *vncd = VersionedNextConfigData(vdt) + return nil +} + +// String returns the string representation for the current VDT value +func (vncd VersionedNextConfigData) String() string { + val, err := vncd.Value() + if err != nil { + return "VersionedNextConfigData()" + } + + return fmt.Sprintf("VersionedNextConfigData(%s)", val) +} + +// NewVersionedNextConfigData creates a new VersionedNextConfigData instance +func NewVersionedNextConfigData() VersionedNextConfigData { + vdt := scale.MustNewVaryingDataType(NextConfigDataV1{}) + + return VersionedNextConfigData(vdt) +} diff --git a/dot/types/consensus_digest_test.go b/dot/types/consensus_digest_test.go index 475bc2edbe..ecd9e5f2c8 100644 --- a/dot/types/consensus_digest_test.go +++ b/dot/types/consensus_digest_test.go @@ -46,3 +46,26 @@ func TestBabeEncodeAndDecode(t *testing.T) { require.NoError(t, err) require.Equal(t, d, dec) } + +func TestBabeDecodeVersionedNextConfigData(t *testing.T) { + // Block #5608275 NextConfigData digest + enc := common.MustHexToBytes("0x03010100000000000000040000000000000002") + + var dec = NewBabeConsensusDigest() + err := scale.Unmarshal(enc, &dec) + require.NoError(t, err) + + decValue, err := dec.Value() + require.NoError(t, err) + + nextVersionedConfigData := decValue.(VersionedNextConfigData) + + nextConfigData, err := nextVersionedConfigData.Value() + require.NoError(t, err) + + nextConfigDataV1 := nextConfigData.(NextConfigDataV1) + + require.GreaterOrEqual(t, 1, int(nextConfigDataV1.C1)) + require.GreaterOrEqual(t, 4, int(nextConfigDataV1.C2)) + require.GreaterOrEqual(t, 2, int(nextConfigDataV1.SecondarySlots)) +} diff --git a/lib/babe/verify_integration_test.go b/lib/babe/verify_integration_test.go index bb0f2f8cbc..12db82b9b3 100644 --- a/lib/babe/verify_integration_test.go +++ b/lib/babe/verify_integration_test.go @@ -544,7 +544,7 @@ func TestVerifyForkBlocksWithRespectiveEpochData(t *testing.T) { aliceBlockNextEpoch := types.NextEpochData{ Authorities: authorities[3:], } - aliceBlockNextConfigData := types.NextConfigData{ + aliceBlockNextConfigData := types.NextConfigDataV1{ C1: 9, C2: 10, SecondarySlots: 1, @@ -555,7 +555,7 @@ func TestVerifyForkBlocksWithRespectiveEpochData(t *testing.T) { bobBlockNextEpoch := types.NextEpochData{ Authorities: authorities[6:], } - bobBlockNextConfigData := types.NextConfigData{ + bobBlockNextConfigData := types.NextConfigDataV1{ C1: 3, C2: 8, SecondarySlots: 1, @@ -672,7 +672,7 @@ func TestVerifyForkBlocksWithRespectiveEpochData(t *testing.T) { // blocks that contains different consensus messages digests func issueConsensusDigestsBlockFromGenesis(t *testing.T, genesisHeader *types.Header, kp *sr25519.Keypair, stateService *state.Service, - nextEpoch types.NextEpochData, nextConfig types.NextConfigData) *types.Header { + nextEpoch types.NextEpochData, nextConfig types.NextConfigDataV1) *types.Header { t.Helper() output, proof, err := kp.VrfSign(makeTranscript(Randomness{}, uint64(0), 0)) @@ -691,7 +691,11 @@ func issueConsensusDigestsBlockFromGenesis(t *testing.T, genesisHeader *types.He require.NoError(t, babeConsensusDigestNextEpoch.Set(nextEpoch)) babeConsensusDigestNextConfigData := types.NewBabeConsensusDigest() - require.NoError(t, babeConsensusDigestNextConfigData.Set(nextConfig)) + + versionedNextConfigData := types.NewVersionedNextConfigData() + versionedNextConfigData.Set(nextConfig) + + require.NoError(t, babeConsensusDigestNextConfigData.Set(versionedNextConfigData)) nextEpochData, err := scale.Marshal(babeConsensusDigestNextEpoch) require.NoError(t, err) From bc2b89dfd0ab5ba19a8b24d2428e99938e801783 Mon Sep 17 00:00:00 2001 From: Diego Date: Fri, 5 May 2023 11:23:02 -0300 Subject: [PATCH 2/3] fix(babe): Change error messages --- dot/digest/digest.go | 4 ++-- dot/digest/digest_integration_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dot/digest/digest.go b/dot/digest/digest.go index 2f77cc4f5d..e64d545117 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -189,7 +189,7 @@ func (h *Handler) handleBabeConsensusDigest(digest scale.VaryingDataType, header case types.NextEpochData: currEpoch, err := h.epochState.GetEpochForBlock(header) if err != nil { - return fmt.Errorf("cannot get epoch for block %d (%s): %w", + return fmt.Errorf("getting epoch for block %d (%s): %w", header.Number, headerHash, err) } @@ -212,7 +212,7 @@ func (h *Handler) handleBabeConsensusDigest(digest scale.VaryingDataType, header case types.NextConfigDataV1: currEpoch, err := h.epochState.GetEpochForBlock(header) if err != nil { - return fmt.Errorf("cannot get epoch for block %d (%s): %w", header.Number, headerHash, err) + return fmt.Errorf("getting epoch for block %d (%s): %w", header.Number, headerHash, err) } nextEpoch := currEpoch + 1 h.epochState.StoreBABENextConfigData(nextEpoch, headerHash, nextConfigData) diff --git a/dot/digest/digest_integration_test.go b/dot/digest/digest_integration_test.go index c9941960af..6d4c295c3b 100644 --- a/dot/digest/digest_integration_test.go +++ b/dot/digest/digest_integration_test.go @@ -387,10 +387,10 @@ func TestHandler_HandleNextConfigData(t *testing.T) { SecondarySlots: 1, } - VersionedNextConfigData := types.NewVersionedNextConfigData() - VersionedNextConfigData.Set(nextConfigData) + versionedNextConfigData := types.NewVersionedNextConfigData() + versionedNextConfigData.Set(nextConfigData) - err := digest.Set(VersionedNextConfigData) + err := digest.Set(versionedNextConfigData) require.NoError(t, err) data, err := scale.Marshal(digest) From fc0d0a6cb659fa44d4cd263570bf2afc1fb3c150 Mon Sep 17 00:00:00 2001 From: Diego Date: Fri, 5 May 2023 14:47:47 -0300 Subject: [PATCH 3/3] fix(babe): Refactor --- dot/digest/digest.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dot/digest/digest.go b/dot/digest/digest.go index e64d545117..8015cad053 100644 --- a/dot/digest/digest.go +++ b/dot/digest/digest.go @@ -202,8 +202,7 @@ func (h *Handler) handleBabeConsensusDigest(digest scale.VaryingDataType, header return nil case types.VersionedNextConfigData: - nextVersionedConfigData := digestValue.(types.VersionedNextConfigData) - nextConfigDataVersion, err := nextVersionedConfigData.Value() + nextConfigDataVersion, err := val.Value() if err != nil { return fmt.Errorf("getting digest value: %w", err) }