From a806de0287f366b18e2f2314e6800a5f5850291c Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 20 Mar 2023 15:31:38 -0400 Subject: [PATCH 1/3] chore: use `bs.latestFinalised` field instead of retrieve from database remove from `setHighestRoundAndSetID` the verification of the given round and set id compared to the current round and set id. --- dot/state/block_finalisation.go | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/dot/state/block_finalisation.go b/dot/state/block_finalisation.go index 9b8e1f5e0c..fa9b98afbe 100644 --- a/dot/state/block_finalisation.go +++ b/dot/state/block_finalisation.go @@ -63,16 +63,6 @@ func (bs *BlockState) GetFinalisedHash(round, setID uint64) (common.Hash, error) } func (bs *BlockState) setHighestRoundAndSetID(round, setID uint64) error { - currRound, currSetID, err := bs.GetHighestRoundAndSetID() - if err != nil { - return err - } - - // higher setID takes precedence over round - if setID < currSetID || setID == currSetID && round <= currRound { - return nil - } - return bs.db.Put(highestRoundAndSetIDKey, roundAndSetIDToBytes(round, setID)) } @@ -206,16 +196,7 @@ func (bs *BlockState) handleFinalisedBlock(curr common.Hash) error { return nil } - prev, err := bs.GetHighestFinalisedHash() - if err != nil { - return fmt.Errorf("failed to get highest finalised hash: %w", err) - } - - if prev == curr { - return nil - } - - subchain, err := bs.RangeInMemory(prev, curr) + subchain, err := bs.RangeInMemory(bs.lastFinalised, curr) if err != nil { return err } From 8d34c39bb3d505a16ce27774a303ba148ad19b75 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 28 Mar 2023 15:11:08 -0400 Subject: [PATCH 2/3] chore: add verification for set id --- dot/state/block_finalisation.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dot/state/block_finalisation.go b/dot/state/block_finalisation.go index fa9b98afbe..0666c72f9e 100644 --- a/dot/state/block_finalisation.go +++ b/dot/state/block_finalisation.go @@ -5,6 +5,7 @@ package state import ( "encoding/binary" + "errors" "fmt" "github.com/ChainSafe/gossamer/dot/telemetry" @@ -12,6 +13,7 @@ import ( "github.com/ChainSafe/gossamer/lib/common" ) +var errSetIDLowerThanHighest = errors.New("set id lower than highest") var highestRoundAndSetIDKey = []byte("hrs") // finalisedHashKey = FinalizedBlockHashKey + round + setID (LE encoded) @@ -63,6 +65,15 @@ func (bs *BlockState) GetFinalisedHash(round, setID uint64) (common.Hash, error) } func (bs *BlockState) setHighestRoundAndSetID(round, setID uint64) error { + _, highestSetID, err := bs.GetHighestRoundAndSetID() + if err != nil { + return err + } + + if setID < highestSetID { + return fmt.Errorf("%w: %d should be greater or equal %d", errSetIDLowerThanHighest, setID, highestSetID) + } + return bs.db.Put(highestRoundAndSetIDKey, roundAndSetIDToBytes(round, setID)) } From 3e090c7e4918a4269450729733910e09b55f9d1a Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 28 Mar 2023 15:16:49 -0400 Subject: [PATCH 3/3] chore: fix `TestHighestRoundAndSetID` test --- dot/state/block_finalisation_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dot/state/block_finalisation_test.go b/dot/state/block_finalisation_test.go index aa62f591a4..0834529bf0 100644 --- a/dot/state/block_finalisation_test.go +++ b/dot/state/block_finalisation_test.go @@ -36,12 +36,14 @@ func TestHighestRoundAndSetID(t *testing.T) { require.Equal(t, uint64(10), round) require.Equal(t, uint64(0), setID) + // is possible to have a lower round number + // in the same set ID: https://github.com/ChainSafe/gossamer/issues/3150 err = bs.setHighestRoundAndSetID(9, 0) require.NoError(t, err) round, setID, err = bs.GetHighestRoundAndSetID() require.NoError(t, err) - require.Equal(t, uint64(10), round) + require.Equal(t, uint64(9), round) require.Equal(t, uint64(0), setID) err = bs.setHighestRoundAndSetID(0, 1) @@ -53,7 +55,9 @@ func TestHighestRoundAndSetID(t *testing.T) { require.Equal(t, uint64(1), setID) err = bs.setHighestRoundAndSetID(100000, 0) - require.NoError(t, err) + require.ErrorIs(t, err, errSetIDLowerThanHighest) + const expectedErrorMessage = "set id lower than highest: 0 should be greater or equal 1" + require.EqualError(t, err, expectedErrorMessage) round, setID, err = bs.GetHighestRoundAndSetID() require.NoError(t, err)