From 6b51da64a4c705d1c4dffae46c18fe572608e5e2 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 8 Mar 2024 10:35:58 -0400 Subject: [PATCH 1/7] feat: place slot calculation in `types.Header` --- dot/state/epoch.go | 36 ++++-------------------------------- dot/types/header.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/dot/state/epoch.go b/dot/state/epoch.go index 6d1d0880d5..70fc31316b 100644 --- a/dot/state/epoch.go +++ b/dot/state/epoch.go @@ -22,7 +22,6 @@ var ( errHashNotInMemory = errors.New("hash not found in memory map") errEpochNotInDatabase = errors.New("epoch data not found in the database") errHashNotPersisted = errors.New("hash with next epoch not found in database") - errNoPreRuntimeDigest = errors.New("header does not contain pre-runtime digest") ) var ( @@ -197,39 +196,12 @@ func (s *EpochState) GetEpochForBlock(header *types.Header) (uint64, error) { return 0, err } - for _, d := range header.Digest { - digestValue, err := d.Value() - if err != nil { - continue - } - predigest, ok := digestValue.(types.PreRuntimeDigest) - if !ok { - continue - } - - digest, err := types.DecodeBabePreDigest(predigest.Data) - if err != nil { - return 0, fmt.Errorf("failed to decode babe header: %w", err) - } - - var slotNumber uint64 - switch d := digest.(type) { - case types.BabePrimaryPreDigest: - slotNumber = d.SlotNumber - case types.BabeSecondaryVRFPreDigest: - slotNumber = d.SlotNumber - case types.BabeSecondaryPlainPreDigest: - slotNumber = d.SlotNumber - } - - if slotNumber < firstSlot { - return 0, nil - } - - return (slotNumber - firstSlot) / s.epochLength, nil + slotNumber, err := header.SlotNumber() + if err != nil { + return 0, fmt.Errorf("getting slot number: %w", err) } - return 0, errNoPreRuntimeDigest + return (slotNumber - firstSlot) / s.epochLength, nil } // SetEpochDataRaw sets the epoch data raw for a given epoch diff --git a/dot/types/header.go b/dot/types/header.go index 59344641e5..b59dc6a57c 100644 --- a/dot/types/header.go +++ b/dot/types/header.go @@ -4,12 +4,15 @@ package types import ( + "errors" "fmt" "github.com/ChainSafe/gossamer/lib/common" "github.com/ChainSafe/gossamer/pkg/scale" ) +var ErrNoPreRuntimeDigest = errors.New("header does not contain pre-runtime digest") + // Header is a state block header type Header struct { ParentHash common.Hash `json:"parentHash"` @@ -96,3 +99,32 @@ func (bh *Header) Hash() common.Hash { return bh.hash } + +func (bh *Header) SlotNumber() (uint64, error) { + for _, d := range bh.Digest { + digestValue, err := d.Value() + if err != nil { + continue + } + predigest, ok := digestValue.(PreRuntimeDigest) + if !ok { + continue + } + + digest, err := DecodeBabePreDigest(predigest.Data) + if err != nil { + return 0, fmt.Errorf("failed to decode babe header: %w", err) + } + + switch d := digest.(type) { + case BabePrimaryPreDigest: + return d.SlotNumber, nil + case BabeSecondaryVRFPreDigest: + return d.SlotNumber, nil + case BabeSecondaryPlainPreDigest: + return d.SlotNumber, nil + } + } + + return 0, ErrNoPreRuntimeDigest +} From a61ba289bb08cc3bbb8c1b3423600cfc673aed32 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Wed, 13 Mar 2024 19:57:56 -0400 Subject: [PATCH 2/7] chore: improve `GetHashesByNumber` state method --- dot/state/block.go | 34 ++++++++++++--------------------- lib/blocktree/blocktree.go | 20 +++++++++---------- lib/blocktree/blocktree_test.go | 8 +++++--- lib/blocktree/node.go | 21 ++++++++++++++++++++ 4 files changed, 48 insertions(+), 35 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 5abf31897c..3bc7d0ca29 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -19,7 +19,6 @@ import ( "github.com/ChainSafe/gossamer/pkg/scale" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "golang.org/x/exp/slices" rtstorage "github.com/ChainSafe/gossamer/lib/runtime/storage" wazero_runtime "github.com/ChainSafe/gossamer/lib/runtime/wazero" @@ -253,19 +252,20 @@ func (bs *BlockState) GetHashByNumber(num uint) (common.Hash, error) { // GetHashesByNumber returns the block hashes with the given number func (bs *BlockState) GetHashesByNumber(blockNumber uint) ([]common.Hash, error) { - block, err := bs.GetBlockByNumber(blockNumber) - if err != nil { - return nil, fmt.Errorf("getting block by number: %w", err) - } - - blockHashes := bs.bt.GetAllBlocksAtNumber(block.Header.ParentHash) + inMemoryBlockHashes := bs.bt.GetHashesAtNumber(blockNumber) + if len(inMemoryBlockHashes) == 0 { + bh, err := bs.db.Get(headerHashKey(uint64(blockNumber))) + if err != nil { + if errors.Is(err, database.ErrNotFound) { + return []common.Hash{}, nil + } + return []common.Hash{}, fmt.Errorf("cannot get block by its number %d: %w", blockNumber, err) + } - hash := block.Header.Hash() - if !slices.Contains(blockHashes, hash) { - blockHashes = append(blockHashes, hash) + return []common.Hash{common.NewHash(bh)}, nil } - return blockHashes, nil + return inMemoryBlockHashes, nil } // GetAllDescendants gets all the descendants for a given block hash (including itself), by first checking in memory @@ -494,17 +494,7 @@ func (bs *BlockState) AddBlockWithArrivalTime(block *types.Block, arrivalTime ti // GetAllBlocksAtNumber returns all unfinalised blocks with the given number func (bs *BlockState) GetAllBlocksAtNumber(num uint) ([]common.Hash, error) { - header, err := bs.GetHeaderByNumber(num) - if err != nil { - return nil, err - } - - return bs.GetAllBlocksAtDepth(header.ParentHash), nil -} - -// GetAllBlocksAtDepth returns all hashes with the depth of the given hash plus one -func (bs *BlockState) GetAllBlocksAtDepth(hash common.Hash) []common.Hash { - return bs.bt.GetAllBlocksAtNumber(hash) + return bs.bt.GetHashesAtNumber(num), nil } func (bs *BlockState) isBlockOnCurrentChain(header *types.Header) (bool, error) { diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 0f1faf21cc..0fa4e4d06e 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -120,24 +120,24 @@ func (bt *BlockTree) AddBlock(header *types.Header, arrivalTime time.Time) (err return nil } -// GetAllBlocksAtNumber will return all blocks hashes with the number of the given hash plus one. +// GetHashesAtNumber will return all blocks hashes that contains the number of the given hash plus one. // To find all blocks at a number matching a certain block, pass in that block's parent hash -func (bt *BlockTree) GetAllBlocksAtNumber(hash common.Hash) (hashes []common.Hash) { +func (bt *BlockTree) GetHashesAtNumber(number uint) (hashes []common.Hash) { bt.RLock() defer bt.RUnlock() - if bt.getNode(hash) == nil { - return hashes + if number < bt.root.number { + return []common.Hash{} } - number := bt.getNode(hash).number + 1 - - if bt.root.number == number { - hashes = append(hashes, bt.root.hash) - return hashes + bestLeave := bt.leaves.bestBlock() + if number > bestLeave.number { + return []common.Hash{} } - return bt.root.getNodesWithNumber(number, hashes) + possibleNumOfBlocks := len(bt.leaves.nodes()) + hashes = make([]common.Hash, 0, possibleNumOfBlocks) + return bt.root.hashesAtNumber(number, hashes) } var ErrStartGreaterThanEnd = errors.New("start greater than end") diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index f7833eeec5..bca3a93e16 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -139,9 +139,10 @@ func Test_BlockTree_GetNode(t *testing.T) { require.NotNil(t, block) } -func Test_BlockTree_GetAllBlocksAtNumber(t *testing.T) { +func Test_BlockTree_GetHashesAtNumber(t *testing.T) { bt, _ := createTestBlockTree(t, testHeader, 8) - hashes := bt.root.getNodesWithNumber(10, []common.Hash{}) + hashes := make([]common.Hash, 0) + hashes = bt.root.hashesAtNumber(10, hashes) require.Empty(t, hashes) @@ -194,7 +195,8 @@ func Test_BlockTree_GetAllBlocksAtNumber(t *testing.T) { } } - hashes = bt.root.getNodesWithNumber(desiredNumber, []common.Hash{}) + hashes = make([]common.Hash, 0, 100) + hashes = bt.root.hashesAtNumber(desiredNumber, hashes) require.Equal(t, expected, hashes) } diff --git a/lib/blocktree/node.go b/lib/blocktree/node.go index 557f805066..1640e2786f 100644 --- a/lib/blocktree/node.go +++ b/lib/blocktree/node.go @@ -210,3 +210,24 @@ func (n *node) primaryAncestorCount(count int) int { return n.parent.primaryAncestorCount(count) } + +func (n *node) hashesAtNumber(number uint, hashes []common.Hash) []common.Hash { + // there is no need to go furthen in the node's children + // since they have a greater number at least + if number == n.number { + hashes = append(hashes, n.hash) + return hashes + } + + // if the number is greater than current node, + // then search among its children + if number > n.number { + for _, children := range n.children { + hashes = children.hashesAtNumber(number, hashes) + } + + return hashes + } + + return hashes +} From de01dee01ad23bbfdeb4e630ef76b2f5e5b97fc4 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Wed, 13 Mar 2024 20:01:36 -0400 Subject: [PATCH 3/7] chore: remove unneeded func --- lib/blocktree/node.go | 46 +++++++++++++------------------------------ 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/lib/blocktree/node.go b/lib/blocktree/node.go index 1640e2786f..5eb4556860 100644 --- a/lib/blocktree/node.go +++ b/lib/blocktree/node.go @@ -59,20 +59,23 @@ func (n *node) getNode(h common.Hash) *node { return nil } -// getNodesWithNumber returns all descendent nodes with the desired number -func (n *node) getNodesWithNumber(number uint, hashes []common.Hash) []common.Hash { - for _, child := range n.children { - // number matches - if child.number == number { - hashes = append(hashes, child.hash) - } +// hashesAtNumber returns all nodes in the chain that contains the desired number +func (n *node) hashesAtNumber(number uint, hashes []common.Hash) []common.Hash { + // there is no need to go furthen in the node's children + // since they have a greater number at least + if number == n.number { + hashes = append(hashes, n.hash) + return hashes + } - // are deeper than desired number, return - if child.number > number { - return hashes + // if the number is greater than current node, + // then search among its children + if number > n.number { + for _, children := range n.children { + hashes = children.hashesAtNumber(number, hashes) } - hashes = child.getNodesWithNumber(number, hashes) + return hashes } return hashes @@ -210,24 +213,3 @@ func (n *node) primaryAncestorCount(count int) int { return n.parent.primaryAncestorCount(count) } - -func (n *node) hashesAtNumber(number uint, hashes []common.Hash) []common.Hash { - // there is no need to go furthen in the node's children - // since they have a greater number at least - if number == n.number { - hashes = append(hashes, n.hash) - return hashes - } - - // if the number is greater than current node, - // then search among its children - if number > n.number { - for _, children := range n.children { - hashes = children.hashesAtNumber(number, hashes) - } - - return hashes - } - - return hashes -} From 29199ea4d064ba137c5eb5d89da9766e49ae6a09 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 15 Mar 2024 15:44:59 -0400 Subject: [PATCH 4/7] chore: remove unneeded interface method --- lib/babe/state.go | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/babe/state.go b/lib/babe/state.go index 47de36a930..f4aee6f4d4 100644 --- a/lib/babe/state.go +++ b/lib/babe/state.go @@ -24,7 +24,6 @@ type BlockState interface { BestBlockHash() common.Hash BestBlockHeader() (*types.Header, error) AddBlock(*types.Block) error - GetAllBlocksAtDepth(hash common.Hash) []common.Hash GetHeader(common.Hash) (*types.Header, error) GetBlockByNumber(blockNumber uint) (*types.Block, error) GetBlockHashesBySlot(slot uint64) (blockHashes []common.Hash, err error) From cfb653bedc30c63690e6a12b383811f14ec4172e Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 19 Mar 2024 15:21:58 -0400 Subject: [PATCH 5/7] chore: remove `t.Parallel()` --- dot/state/grandpa_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/dot/state/grandpa_test.go b/dot/state/grandpa_test.go index 8018308c0b..f55f279ac8 100644 --- a/dot/state/grandpa_test.go +++ b/dot/state/grandpa_test.go @@ -220,8 +220,6 @@ func TestAddScheduledChangesKeepTheRightForkTree(t *testing.T) { for tname, tt := range tests { tt := tt t.Run(tname, func(t *testing.T) { - t.Parallel() - // clear the scheduledChangeRoots after the test ends // this does not cause race condition because t.Run without // t.Parallel() blocks until this function returns From c5cb08820dd89176bc21882cce46af2dfe28e9e4 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 19 Mar 2024 15:27:49 -0400 Subject: [PATCH 6/7] chore: add `nolint:tparallel` --- dot/state/grandpa_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/state/grandpa_test.go b/dot/state/grandpa_test.go index f55f279ac8..7043cac47b 100644 --- a/dot/state/grandpa_test.go +++ b/dot/state/grandpa_test.go @@ -143,7 +143,7 @@ func testBlockState(t *testing.T, db database.Database) *BlockState { return bs } -func TestAddScheduledChangesKeepTheRightForkTree(t *testing.T) { +func TestAddScheduledChangesKeepTheRightForkTree(t *testing.T) { //nolint:tparallel t.Parallel() keyring, err := keystore.NewSr25519Keyring() From ac34c8ce885277eb125af4934e1778c850f8fdc5 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 22 Mar 2024 09:54:17 -0400 Subject: [PATCH 7/7] chore: fix babe mocks --- lib/babe/mock_state_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/lib/babe/mock_state_test.go b/lib/babe/mock_state_test.go index c9b3b83da2..ab5e132028 100644 --- a/lib/babe/mock_state_test.go +++ b/lib/babe/mock_state_test.go @@ -113,20 +113,6 @@ func (mr *MockBlockStateMockRecorder) GenesisHash() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenesisHash", reflect.TypeOf((*MockBlockState)(nil).GenesisHash)) } -// GetAllBlocksAtDepth mocks base method. -func (m *MockBlockState) GetAllBlocksAtDepth(arg0 common.Hash) []common.Hash { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAllBlocksAtDepth", arg0) - ret0, _ := ret[0].([]common.Hash) - return ret0 -} - -// GetAllBlocksAtDepth indicates an expected call of GetAllBlocksAtDepth. -func (mr *MockBlockStateMockRecorder) GetAllBlocksAtDepth(arg0 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllBlocksAtDepth", reflect.TypeOf((*MockBlockState)(nil).GetAllBlocksAtDepth), arg0) -} - // GetBlockByNumber mocks base method. func (m *MockBlockState) GetBlockByNumber(arg0 uint) (*types.Block, error) { m.ctrl.T.Helper()