From a86dcb3066bda5bfe6e47b139582ac165062d0ee Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Mon, 23 Jan 2023 20:02:00 +0530 Subject: [PATCH 01/24] Fixing descendant search If blocktree fails to get highest finalised hash (which is stored in memory), load it from disk. Fixes #3053 --- dot/state/block.go | 30 +++++++++++++++++++++++++++++- lib/blocktree/blocktree.go | 7 ++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 311b7c7e81..f7dedaedef 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -250,6 +250,28 @@ func (bs *BlockState) GetHashByNumber(num uint) (common.Hash, error) { return common.NewHash(bh), nil } +func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) { + allDescendants := []common.Hash{} + for _, h := range bs.bt.GetAllBlocksAtNumber(hash) { + ifDescendant, err := bs.bt.IsDescendantOf(hash, h) + if err != nil { + return nil, fmt.Errorf("failed to check if block %s is descendant of block %s: %w", h, hash, err) + } + if !ifDescendant { + continue + } + + descendants, err := bs.bt.GetAllDescendants(h) + if err != nil { + return nil, fmt.Errorf("failed to get descendants: %w", err) + } + + allDescendants = append(allDescendants, descendants...) + } + + return allDescendants, nil +} + // GetBlockHashesBySlot gets all block hashes that were produced in the given slot. func (bs *BlockState) GetBlockHashesBySlot(slotNum uint64) ([]common.Hash, error) { highestFinalisedHash, err := bs.GetHighestFinalisedHash() @@ -258,7 +280,13 @@ func (bs *BlockState) GetBlockHashesBySlot(slotNum uint64) ([]common.Hash, error } descendants, err := bs.bt.GetAllDescendants(highestFinalisedHash) - if err != nil { + if errors.Is(err, blocktree.ErrNodeNotFound) { + var err2 error + descendants, err2 = bs.GetAllDescendants(highestFinalisedHash) + if err2 != nil { + return nil, fmt.Errorf("failed to get descendants: %w", err) + } + } else if err != nil { return nil, fmt.Errorf("failed to get descendants: %w", err) } diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index b03e059c96..745b51c09b 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -10,6 +10,7 @@ import ( "time" "github.com/ChainSafe/gossamer/dot/types" + "github.com/ChainSafe/gossamer/lib/common" "github.com/disiqueira/gotree" "github.com/prometheus/client_golang/prometheus" @@ -111,15 +112,15 @@ func (bt *BlockTree) AddBlock(header *types.Header, arrivalTime time.Time) (err // GetAllBlocksAtNumber will return all blocks hashes with 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) GetAllBlocksAtNumber(parent common.Hash) (hashes []common.Hash) { bt.RLock() defer bt.RUnlock() - if bt.getNode(hash) == nil { + if bt.getNode(parent) == nil { return hashes } - number := bt.getNode(hash).number + 1 + number := bt.getNode(parent).number + 1 if bt.root.number == number { hashes = append(hashes, bt.root.hash) From b20a8e2940a3a024316cedc3a64f115d61cd493c Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Mon, 23 Jan 2023 22:04:26 +0530 Subject: [PATCH 02/24] fix lint --- dot/state/block.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dot/state/block.go b/dot/state/block.go index f7dedaedef..41a99dc43b 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -250,9 +250,11 @@ func (bs *BlockState) GetHashByNumber(num uint) (common.Hash, error) { return common.NewHash(bh), nil } +// GetAllDescendants gets all the descendants even if hash is not stored in memory. func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) { allDescendants := []common.Hash{} for _, h := range bs.bt.GetAllBlocksAtNumber(hash) { + allDescendants = append(allDescendants, h) ifDescendant, err := bs.bt.IsDescendantOf(hash, h) if err != nil { return nil, fmt.Errorf("failed to check if block %s is descendant of block %s: %w", h, hash, err) From 9e7be4d1c6d520fb73fa22db92fe0feb66cf3e59 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Mon, 23 Jan 2023 22:08:22 +0530 Subject: [PATCH 03/24] parent -> hash --- lib/blocktree/blocktree.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 745b51c09b..6e67afaafc 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -112,15 +112,15 @@ func (bt *BlockTree) AddBlock(header *types.Header, arrivalTime time.Time) (err // GetAllBlocksAtNumber will return all blocks hashes with 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(parent common.Hash) (hashes []common.Hash) { +func (bt *BlockTree) GetAllBlocksAtNumber(hash common.Hash) (hashes []common.Hash) { bt.RLock() defer bt.RUnlock() - if bt.getNode(parent) == nil { + if bt.getNode(hash) == nil { return hashes } - number := bt.getNode(parent).number + 1 + number := bt.getNode(hash).number + 1 if bt.root.number == number { hashes = append(hashes, bt.root.hash) From d0aa5f81c48d4fbed11eaeef4bf51319acd4beb5 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Tue, 24 Jan 2023 20:37:22 +0530 Subject: [PATCH 04/24] different approach for getting descendants --- dot/state/block.go | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 41a99dc43b..9ba57876f0 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -252,25 +252,27 @@ func (bs *BlockState) GetHashByNumber(num uint) (common.Hash, error) { // GetAllDescendants gets all the descendants even if hash is not stored in memory. func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) { - allDescendants := []common.Hash{} - for _, h := range bs.bt.GetAllBlocksAtNumber(hash) { - allDescendants = append(allDescendants, h) - ifDescendant, err := bs.bt.IsDescendantOf(hash, h) - if err != nil { - return nil, fmt.Errorf("failed to check if block %s is descendant of block %s: %w", h, hash, err) - } - if !ifDescendant { - continue - } - - descendants, err := bs.bt.GetAllDescendants(h) - if err != nil { - return nil, fmt.Errorf("failed to get descendants: %w", err) - } + header, err := bs.GetHeader(hash) + if err != nil { + return nil, fmt.Errorf("getting header from hash %s: %w", hash, err) + } + // TODO: Use GetBlocksByNumber after https://github.com/ChainSafe/gossamer/issues/2748 is done + nextBlock, err := bs.GetBlockByNumber(header.Number + 1) + if err != nil { + return nil, fmt.Errorf("getting block by number %d: %w", header.Number+1, err) + } - allDescendants = append(allDescendants, descendants...) + ifDescendant, err := bs.bt.IsDescendantOf(hash, nextBlock.Header.Hash()) + if err != nil { + return nil, fmt.Errorf("failed to check if block %s is descendant of block %s: %w", nextBlock.Header.Hash(), hash, err) + } + if !ifDescendant { + return nil, nil } + allDescendants := []common.Hash{nextBlock.Header.Hash()} + allDescendants = append(allDescendants, bs.bt.GetAllBlocksAtNumber(nextBlock.Header.Hash())...) + return allDescendants, nil } @@ -286,7 +288,7 @@ func (bs *BlockState) GetBlockHashesBySlot(slotNum uint64) ([]common.Hash, error var err2 error descendants, err2 = bs.GetAllDescendants(highestFinalisedHash) if err2 != nil { - return nil, fmt.Errorf("failed to get descendants: %w", err) + return nil, fmt.Errorf("failed to get descendants: %w", err2) } } else if err != nil { return nil, fmt.Errorf("failed to get descendants: %w", err) From 32db55ad79ec0432b47ecb6934c00e9b330f37ca Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Wed, 25 Jan 2023 22:37:21 +0530 Subject: [PATCH 05/24] different approach for getting descendants --- dot/state/block.go | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 9ba57876f0..de773426b1 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -252,26 +252,48 @@ func (bs *BlockState) GetHashByNumber(num uint) (common.Hash, error) { // GetAllDescendants gets all the descendants even if hash is not stored in memory. func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) { + allDescendants, err := bs.bt.GetAllDescendants(hash) + if err != nil && !errors.Is(err, blocktree.ErrNodeNotFound) { + return nil, err + } + + if err == nil { + return allDescendants, nil + } + header, err := bs.GetHeader(hash) if err != nil { return nil, fmt.Errorf("getting header from hash %s: %w", hash, err) } + // TODO: Use GetBlocksByNumber after https://github.com/ChainSafe/gossamer/issues/2748 is done nextBlock, err := bs.GetBlockByNumber(header.Number + 1) if err != nil { return nil, fmt.Errorf("getting block by number %d: %w", header.Number+1, err) } - ifDescendant, err := bs.bt.IsDescendantOf(hash, nextBlock.Header.Hash()) - if err != nil { - return nil, fmt.Errorf("failed to check if block %s is descendant of block %s: %w", nextBlock.Header.Hash(), hash, err) - } - if !ifDescendant { + // next block number is not descendant of hash + if nextBlock.Header.ParentHash != hash { return nil, nil } - allDescendants := []common.Hash{nextBlock.Header.Hash()} - allDescendants = append(allDescendants, bs.bt.GetAllBlocksAtNumber(nextBlock.Header.Hash())...) + allDescendants = append(allDescendants, nextBlock.Header.Hash()) + + nextDescendants, err := bs.bt.GetAllDescendants(nextBlock.Header.Hash()) + if err != nil && !errors.Is(err, blocktree.ErrNodeNotFound) { + return nil, fmt.Errorf("failed to get descendants: %w", err) + } + if err == nil { + allDescendants = append(allDescendants, nextDescendants...) + return allDescendants, nil + } + + nextDescendants, err = bs.GetAllDescendants(nextBlock.Header.Hash()) + if err != nil { + return nil, err + } + + allDescendants = append(allDescendants, nextDescendants...) return allDescendants, nil } From 0f761555e1ba33aa88b1879cf67c5e06fa59de90 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Thu, 26 Jan 2023 22:20:07 +0530 Subject: [PATCH 06/24] fixed the use of bs.GetAllDescendants --- dot/state/block.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index de773426b1..653ab86da7 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -305,14 +305,8 @@ func (bs *BlockState) GetBlockHashesBySlot(slotNum uint64) ([]common.Hash, error return nil, fmt.Errorf("failed to get highest finalised hash: %w", err) } - descendants, err := bs.bt.GetAllDescendants(highestFinalisedHash) - if errors.Is(err, blocktree.ErrNodeNotFound) { - var err2 error - descendants, err2 = bs.GetAllDescendants(highestFinalisedHash) - if err2 != nil { - return nil, fmt.Errorf("failed to get descendants: %w", err2) - } - } else if err != nil { + descendants, err := bs.GetAllDescendants(highestFinalisedHash) + if err != nil { return nil, fmt.Errorf("failed to get descendants: %w", err) } From d552579cd482a8ed405596dd1b70a19901c072f8 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 30 Jan 2023 11:17:50 +0530 Subject: [PATCH 07/24] Update dot/state/block.go Co-authored-by: Quentin McGaw --- dot/state/block.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/state/block.go b/dot/state/block.go index 653ab86da7..89083fd333 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -263,7 +263,7 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) header, err := bs.GetHeader(hash) if err != nil { - return nil, fmt.Errorf("getting header from hash %s: %w", hash, err) + return nil, fmt.Errorf("getting header: %w", err) } // TODO: Use GetBlocksByNumber after https://github.com/ChainSafe/gossamer/issues/2748 is done From 0a9e2afa0329a84b550fceaa5826d07d76ff92ea Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 30 Jan 2023 11:18:10 +0530 Subject: [PATCH 08/24] Update dot/state/block.go Co-authored-by: Quentin McGaw --- dot/state/block.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/state/block.go b/dot/state/block.go index 89083fd333..fd81943995 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -269,7 +269,7 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) // TODO: Use GetBlocksByNumber after https://github.com/ChainSafe/gossamer/issues/2748 is done nextBlock, err := bs.GetBlockByNumber(header.Number + 1) if err != nil { - return nil, fmt.Errorf("getting block by number %d: %w", header.Number+1, err) + return nil, fmt.Errorf("getting block by number: %w", err) } // next block number is not descendant of hash From 254951ada2789222f05e687d0ff2c726f603ec91 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 30 Jan 2023 12:49:06 +0530 Subject: [PATCH 09/24] Update dot/state/block.go Co-authored-by: Quentin McGaw --- dot/state/block.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/state/block.go b/dot/state/block.go index fd81943995..348bcb3b79 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -272,7 +272,7 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) return nil, fmt.Errorf("getting block by number: %w", err) } - // next block number is not descendant of hash + // next block is not a descendant of the block for the given hash if nextBlock.Header.ParentHash != hash { return nil, nil } From 0ef5508c829a17216da686419fe320f29ddcbf2a Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 30 Jan 2023 12:49:39 +0530 Subject: [PATCH 10/24] Update dot/state/block.go Co-authored-by: Quentin McGaw --- dot/state/block.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/state/block.go b/dot/state/block.go index 348bcb3b79..616704df29 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -281,7 +281,7 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) nextDescendants, err := bs.bt.GetAllDescendants(nextBlock.Header.Hash()) if err != nil && !errors.Is(err, blocktree.ErrNodeNotFound) { - return nil, fmt.Errorf("failed to get descendants: %w", err) + return nil, fmt.Errorf("getting all descendants: %w", err) } if err == nil { allDescendants = append(allDescendants, nextDescendants...) From a2b189316112688b0bb1d824f1619b0b444a925b Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 30 Jan 2023 12:50:44 +0530 Subject: [PATCH 11/24] Update dot/state/block.go Co-authored-by: Quentin McGaw --- dot/state/block.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dot/state/block.go b/dot/state/block.go index 616704df29..92f50d6789 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -250,7 +250,8 @@ func (bs *BlockState) GetHashByNumber(num uint) (common.Hash, error) { return common.NewHash(bh), nil } -// GetAllDescendants gets all the descendants even if hash is not stored in memory. +// GetAllDescendants gets all the descendants for a given block hash, by first checking in memory +// and, if not found, reading from the block state database. func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) { allDescendants, err := bs.bt.GetAllDescendants(hash) if err != nil && !errors.Is(err, blocktree.ErrNodeNotFound) { From 3529a36220387aaa18e4662dd765a0e00b09b461 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Mon, 30 Jan 2023 16:14:12 +0530 Subject: [PATCH 12/24] temp --- lib/blocktree/blocktree_test.go | 123 -------------------------------- 1 file changed, 123 deletions(-) diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index 28254b5303..6b479d9130 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -6,30 +6,15 @@ package blocktree import ( "bytes" "fmt" - "math/rand" "testing" "time" "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/common" - "github.com/ChainSafe/gossamer/pkg/scale" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -var zeroHash, _ = common.HexToHash("0x00") -var testHeader = &types.Header{ - ParentHash: zeroHash, - Number: 0, - Digest: types.NewDigest(), -} - -type testBranch struct { - hash Hash - number uint - arrivalTime int64 -} - func newBlockTreeFromNode(root *node) *BlockTree { return &BlockTree{ root: root, @@ -37,114 +22,6 @@ func newBlockTreeFromNode(root *node) *BlockTree { } } -func createPrimaryBABEDigest(t testing.TB) scale.VaryingDataTypeSlice { - babeDigest := types.NewBabeDigest() - err := babeDigest.Set(types.BabePrimaryPreDigest{AuthorityIndex: 0}) - require.NoError(t, err) - - bdEnc, err := scale.Marshal(babeDigest) - require.NoError(t, err) - - digest := types.NewDigest() - err = digest.Add(types.PreRuntimeDigest{ - ConsensusEngineID: types.BabeEngineID, - Data: bdEnc, - }) - require.NoError(t, err) - return digest -} - -func createTestBlockTree(t *testing.T, header *types.Header, number uint) (*BlockTree, []testBranch) { - bt := NewBlockTreeFromRoot(header) - previousHash := header.Hash() - - // branch tree randomly - var branches []testBranch - r := rand.New(rand.NewSource(time.Now().UnixNano())) //skipcq: GSC-G404 - - at := int64(0) - - // create base tree - for i := uint(1); i <= number; i++ { - header := &types.Header{ - ParentHash: previousHash, - Number: i, - Digest: createPrimaryBABEDigest(t), - } - - hash := header.Hash() - err := bt.AddBlock(header, time.Unix(0, at)) - require.NoError(t, err) - - previousHash = hash - - isBranch := r.Intn(2) - if isBranch == 1 { - branches = append(branches, testBranch{ - hash: hash, - number: bt.getNode(hash).number, - arrivalTime: at, - }) - } - - at += int64(r.Intn(8)) - } - - // create tree branches - for _, branch := range branches { - at := branch.arrivalTime - previousHash = branch.hash - - for i := branch.number; i <= number; i++ { - header := &types.Header{ - ParentHash: previousHash, - Number: i + 1, - StateRoot: common.Hash{0x1}, - Digest: createPrimaryBABEDigest(t), - } - - hash := header.Hash() - err := bt.AddBlock(header, time.Unix(0, at)) - require.NoError(t, err) - - previousHash = hash - at += int64(r.Intn(8)) - - } - } - - return bt, branches -} - -func createFlatTree(t testing.TB, number uint) (*BlockTree, []common.Hash) { - rootHeader := &types.Header{ - ParentHash: zeroHash, - Digest: createPrimaryBABEDigest(t), - } - - bt := NewBlockTreeFromRoot(rootHeader) - require.NotNil(t, bt) - previousHash := bt.root.hash - - hashes := []common.Hash{bt.root.hash} - for i := uint(1); i <= number; i++ { - header := &types.Header{ - ParentHash: previousHash, - Number: i, - Digest: createPrimaryBABEDigest(t), - } - - hash := header.Hash() - hashes = append(hashes, hash) - - err := bt.AddBlock(header, time.Unix(0, 0)) - require.NoError(t, err) - previousHash = hash - } - - return bt, hashes -} - func Test_NewBlockTreeFromNode(t *testing.T) { var bt *BlockTree var branches []testBranch From 57f5eb80d542ad8c5a8336503cea7fc71a222135 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Mon, 30 Jan 2023 16:26:28 +0530 Subject: [PATCH 13/24] temo --- lib/blocktree/test_helpers.go | 133 ++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 lib/blocktree/test_helpers.go diff --git a/lib/blocktree/test_helpers.go b/lib/blocktree/test_helpers.go new file mode 100644 index 0000000000..be2efa8180 --- /dev/null +++ b/lib/blocktree/test_helpers.go @@ -0,0 +1,133 @@ +package blocktree + +import ( + "math/rand" + "testing" + "time" + + "github.com/ChainSafe/gossamer/dot/types" + "github.com/ChainSafe/gossamer/lib/common" + "github.com/ChainSafe/gossamer/pkg/scale" + "github.com/stretchr/testify/require" +) + +var zeroHash, _ = common.HexToHash("0x00") +var testHeader = &types.Header{ + ParentHash: zeroHash, + Number: 0, + Digest: types.NewDigest(), +} + +type testBranch struct { + hash Hash + number uint + arrivalTime int64 +} + +func createPrimaryBABEDigest(t testing.TB) scale.VaryingDataTypeSlice { + babeDigest := types.NewBabeDigest() + err := babeDigest.Set(types.BabePrimaryPreDigest{AuthorityIndex: 0}) + require.NoError(t, err) + + bdEnc, err := scale.Marshal(babeDigest) + require.NoError(t, err) + + digest := types.NewDigest() + err = digest.Add(types.PreRuntimeDigest{ + ConsensusEngineID: types.BabeEngineID, + Data: bdEnc, + }) + require.NoError(t, err) + return digest +} + +func createTestBlockTree(t *testing.T, header *types.Header, number uint) (*BlockTree, []testBranch) { + bt := NewBlockTreeFromRoot(header) + previousHash := header.Hash() + + // branch tree randomly + var branches []testBranch + r := rand.New(rand.NewSource(time.Now().UnixNano())) //skipcq: GSC-G404 + + at := int64(0) + + // create base tree + for i := uint(1); i <= number; i++ { + header := &types.Header{ + ParentHash: previousHash, + Number: i, + Digest: createPrimaryBABEDigest(t), + } + + hash := header.Hash() + err := bt.AddBlock(header, time.Unix(0, at)) + require.NoError(t, err) + + previousHash = hash + + isBranch := r.Intn(2) + if isBranch == 1 { + branches = append(branches, testBranch{ + hash: hash, + number: bt.getNode(hash).number, + arrivalTime: at, + }) + } + + at += int64(r.Intn(8)) + } + + // create tree branches + for _, branch := range branches { + at := branch.arrivalTime + previousHash = branch.hash + + for i := branch.number; i <= number; i++ { + header := &types.Header{ + ParentHash: previousHash, + Number: i + 1, + StateRoot: common.Hash{0x1}, + Digest: createPrimaryBABEDigest(t), + } + + hash := header.Hash() + err := bt.AddBlock(header, time.Unix(0, at)) + require.NoError(t, err) + + previousHash = hash + at += int64(r.Intn(8)) + + } + } + + return bt, branches +} + +func createFlatTree(t testing.TB, number uint) (*BlockTree, []common.Hash) { + rootHeader := &types.Header{ + ParentHash: zeroHash, + Digest: createPrimaryBABEDigest(t), + } + + bt := NewBlockTreeFromRoot(rootHeader) + require.NotNil(t, bt) + previousHash := bt.root.hash + + hashes := []common.Hash{bt.root.hash} + for i := uint(1); i <= number; i++ { + header := &types.Header{ + ParentHash: previousHash, + Number: i, + Digest: createPrimaryBABEDigest(t), + } + + hash := header.Hash() + hashes = append(hashes, hash) + + err := bt.AddBlock(header, time.Unix(0, 0)) + require.NoError(t, err) + previousHash = hash + } + + return bt, hashes +} From 382592f1b8bd19de684854c6eaa9af49df2cd29b Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Wed, 1 Feb 2023 22:12:35 +0530 Subject: [PATCH 14/24] added a test for GetAllDescendants --- dot/state/block.go | 5 ++-- dot/state/block_test.go | 62 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 653ab86da7..901953debf 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -261,6 +261,9 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) return allDescendants, nil } + // adding itself as descendant to keep this method similar to blocktree.GetAllDescendants + allDescendants = []common.Hash{hash} + header, err := bs.GetHeader(hash) if err != nil { return nil, fmt.Errorf("getting header from hash %s: %w", hash, err) @@ -277,8 +280,6 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) return nil, nil } - allDescendants = append(allDescendants, nextBlock.Header.Hash()) - nextDescendants, err := bs.bt.GetAllDescendants(nextBlock.Header.Hash()) if err != nil && !errors.Is(err, blocktree.ErrNodeNotFound) { return nil, fmt.Errorf("failed to get descendants: %w", err) diff --git a/dot/state/block_test.go b/dot/state/block_test.go index 47972b2e13..01ea936c0d 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -198,6 +198,68 @@ func TestGetSlotForBlock(t *testing.T) { require.Equal(t, expectedSlot, res) } +func TestGetAllDescendants(t *testing.T) { + t.Parallel() + + bs := newTestBlockState(t, newTriesEmpty()) + slot := uint64(77) + + babeHeader := types.NewBabeDigest() + err := babeHeader.Set(*types.NewBabePrimaryPreDigest(0, slot, [32]byte{}, [64]byte{})) + require.NoError(t, err) + data, err := scale.Marshal(babeHeader) + require.NoError(t, err) + preDigest := types.NewBABEPreRuntimeDigest(data) + + digest := types.NewDigest() + err = digest.Add(*preDigest) + require.NoError(t, err) + block := &types.Block{ + Header: types.Header{ + ParentHash: testGenesisHeader.Hash(), + Number: 1, + Digest: digest, + }, + Body: sampleBlockBody, + } + + err = bs.AddBlockWithArrivalTime(block, time.Now()) + require.NoError(t, err) + + babeHeader2 := types.NewBabeDigest() + err = babeHeader2.Set(*types.NewBabePrimaryPreDigest(1, slot+1, [32]byte{}, [64]byte{})) + require.NoError(t, err) + data2, err := scale.Marshal(babeHeader2) + require.NoError(t, err) + preDigest2 := types.NewBABEPreRuntimeDigest(data2) + + digest2 := types.NewDigest() + err = digest2.Add(*preDigest2) + require.NoError(t, err) + block2 := &types.Block{ + Header: types.Header{ + ParentHash: block.Header.Hash(), + Number: 2, + Digest: digest2, + }, + Body: sampleBlockBody, + } + err = bs.AddBlockWithArrivalTime(block2, time.Now()) + require.NoError(t, err) + + err = bs.SetFinalisedHash(block2.Header.Hash(), 1, 1) + require.NoError(t, err) + + // can't fetch given block's descendants since the given block get removed from memory after being finalised, using blocktree.GetAllDescendants + _, err = bs.bt.GetAllDescendants(block.Header.Hash()) + require.ErrorIs(t, err, blocktree.ErrNodeNotFound) + + // can fetch given finalised block's descendants using disk, using using blockstate.GetAllDescendants + blockHashes, err := bs.GetAllDescendants(block.Header.Hash()) + require.NoError(t, err) + require.ElementsMatch(t, blockHashes, []common.Hash{block.Header.Hash(), block2.Header.Hash()}) +} + func TestGetBlockHashesBySlot(t *testing.T) { t.Parallel() From 8f0075fbde670db6576c8a13291f9f4ca41d1df3 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Wed, 1 Feb 2023 22:34:50 +0530 Subject: [PATCH 15/24] fix lint --- dot/state/block_test.go | 3 ++- lib/blocktree/test_helpers.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dot/state/block_test.go b/dot/state/block_test.go index 01ea936c0d..47f694ee7d 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -250,7 +250,8 @@ func TestGetAllDescendants(t *testing.T) { err = bs.SetFinalisedHash(block2.Header.Hash(), 1, 1) require.NoError(t, err) - // can't fetch given block's descendants since the given block get removed from memory after being finalised, using blocktree.GetAllDescendants + // can't fetch given block's descendants since the given block get removed from memory after + // being finalised, using blocktree.GetAllDescendants _, err = bs.bt.GetAllDescendants(block.Header.Hash()) require.ErrorIs(t, err, blocktree.ErrNodeNotFound) diff --git a/lib/blocktree/test_helpers.go b/lib/blocktree/test_helpers.go index be2efa8180..67d177f4da 100644 --- a/lib/blocktree/test_helpers.go +++ b/lib/blocktree/test_helpers.go @@ -47,7 +47,7 @@ func createTestBlockTree(t *testing.T, header *types.Header, number uint) (*Bloc // branch tree randomly var branches []testBranch - r := rand.New(rand.NewSource(time.Now().UnixNano())) //skipcq: GSC-G404 + r := rand.New(rand.NewSource(time.Now().UnixNano())) //nolint at := int64(0) From 166d7cfe0370416ea07e7100472a2d8d33b9b398 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Thu, 2 Feb 2023 11:35:29 +0530 Subject: [PATCH 16/24] addressed a review comment --- dot/state/block.go | 3 +-- lib/blocktree/blocktree.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 3f512f5dfe..21f5a1bc8c 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -250,7 +250,7 @@ func (bs *BlockState) GetHashByNumber(num uint) (common.Hash, error) { return common.NewHash(bh), nil } -// GetAllDescendants gets all the descendants for a given block hash, by first checking in memory +// GetAllDescendants gets all the descendants for a given block hash (including itself), by first checking in memory // and, if not found, reading from the block state database. func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) { allDescendants, err := bs.bt.GetAllDescendants(hash) @@ -262,7 +262,6 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) return allDescendants, nil } - // adding itself as descendant to keep this method similar to blocktree.GetAllDescendants allDescendants = []common.Hash{hash} header, err := bs.GetHeader(hash) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 9885f1ae9a..e7a9d5f912 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -417,7 +417,7 @@ func (bt *BlockTree) GetAllBlocks() []Hash { return bt.root.getAllDescendants(nil) } -// GetAllDescendants returns all block hashes that are descendants of the given block hash. +// GetAllDescendants returns all block hashes that are descendants of the given block hash (including itself). func (bt *BlockTree) GetAllDescendants(hash common.Hash) ([]Hash, error) { bt.RLock() defer bt.RUnlock() From a8c026d1d0d648fb003822df41eaac448cabe290 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Mon, 6 Feb 2023 13:07:42 +0530 Subject: [PATCH 17/24] fix copyright --- lib/blocktree/test_helpers.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/blocktree/test_helpers.go b/lib/blocktree/test_helpers.go index 67d177f4da..170f845c5c 100644 --- a/lib/blocktree/test_helpers.go +++ b/lib/blocktree/test_helpers.go @@ -1,3 +1,6 @@ +// Copyright 2023 ChainSafe Systems (ON) +// SPDX-License-Identifier: LGPL-3.0-only + package blocktree import ( From 4bc59c1e5e62c5faf63308427dd3638df07b7f48 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Fri, 10 Feb 2023 21:03:13 +0530 Subject: [PATCH 18/24] tackle the scenario when multiple blocks have same block number --- dot/state/block.go | 46 ++++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 8a4f7a72cd..fb22509cb6 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -287,32 +287,38 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) return nil, fmt.Errorf("getting header: %w", err) } - // TODO: Use GetBlocksByNumber after https://github.com/ChainSafe/gossamer/issues/2748 is done - nextBlock, err := bs.GetBlockByNumber(header.Number + 1) + nextBlockHashes, err := bs.GetHashesByNumber(header.Number + 1) if err != nil { - return nil, fmt.Errorf("getting block by number: %w", err) + return nil, fmt.Errorf("getting hashes by number: %w", err) } - // next block is not a descendant of the block for the given hash - if nextBlock.Header.ParentHash != hash { - return nil, nil - } + for _, nextBlockHash := range nextBlockHashes { - nextDescendants, err := bs.bt.GetAllDescendants(nextBlock.Header.Hash()) - if err != nil && !errors.Is(err, blocktree.ErrNodeNotFound) { - return nil, fmt.Errorf("getting all descendants: %w", err) - } - if err == nil { - allDescendants = append(allDescendants, nextDescendants...) - return allDescendants, nil - } + nextHeader, err := bs.GetHeader(nextBlockHash) + if err != nil { + return nil, fmt.Errorf("getting header from block hash %s: %w", nextBlockHash, err) + } + // next block is not a descendant of the block for the given hash + if nextHeader.ParentHash != hash { + return nil, nil + } - nextDescendants, err = bs.GetAllDescendants(nextBlock.Header.Hash()) - if err != nil { - return nil, err - } + nextDescendants, err := bs.bt.GetAllDescendants(nextBlockHash) + if err != nil && !errors.Is(err, blocktree.ErrNodeNotFound) { + return nil, fmt.Errorf("getting all descendants: %w", err) + } + if err == nil { + allDescendants = append(allDescendants, nextDescendants...) + return allDescendants, nil + } - allDescendants = append(allDescendants, nextDescendants...) + nextDescendants, err = bs.GetAllDescendants(nextBlockHash) + if err != nil { + return nil, err + } + + allDescendants = append(allDescendants, nextDescendants...) + } return allDescendants, nil } From 73cb1b33113628ed88ecb116fdee4fce4f77c997 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Tue, 14 Feb 2023 12:35:32 +0530 Subject: [PATCH 19/24] suppress deepsouce random warning --- lib/blocktree/test_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/blocktree/test_helpers.go b/lib/blocktree/test_helpers.go index 170f845c5c..25553631d0 100644 --- a/lib/blocktree/test_helpers.go +++ b/lib/blocktree/test_helpers.go @@ -50,7 +50,7 @@ func createTestBlockTree(t *testing.T, header *types.Header, number uint) (*Bloc // branch tree randomly var branches []testBranch - r := rand.New(rand.NewSource(time.Now().UnixNano())) //nolint + r := rand.New(rand.NewSource(time.Now().UnixNano())) // skipcq at := int64(0) From 0f80ba42b2022334bd00da73292122ab8933d24e Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Tue, 14 Feb 2023 21:00:52 +0530 Subject: [PATCH 20/24] try --- lib/blocktree/test_helpers.go | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/blocktree/test_helpers.go b/lib/blocktree/test_helpers.go index 25553631d0..bca04c8163 100644 --- a/lib/blocktree/test_helpers.go +++ b/lib/blocktree/test_helpers.go @@ -50,6 +50,7 @@ func createTestBlockTree(t *testing.T, header *types.Header, number uint) (*Bloc // branch tree randomly var branches []testBranch + //nolint r := rand.New(rand.NewSource(time.Now().UnixNano())) // skipcq at := int64(0) From 396f9133dcc84e17f40a4e638728df1fb1dca9d8 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 14 Feb 2023 22:26:09 +0530 Subject: [PATCH 21/24] Update dot/state/block.go Co-authored-by: Quentin McGaw --- dot/state/block.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/state/block.go b/dot/state/block.go index fb22509cb6..7a4776a5f9 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -300,7 +300,7 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) } // next block is not a descendant of the block for the given hash if nextHeader.ParentHash != hash { - return nil, nil + return []common.Hash{hash}, nil } nextDescendants, err := bs.bt.GetAllDescendants(nextBlockHash) From 72a758a4605e6689de8d3617432e85814109999c Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 14 Feb 2023 22:26:27 +0530 Subject: [PATCH 22/24] Update dot/state/block.go Co-authored-by: Quentin McGaw --- dot/state/block.go | 1 - 1 file changed, 1 deletion(-) diff --git a/dot/state/block.go b/dot/state/block.go index 7a4776a5f9..3736502601 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -293,7 +293,6 @@ func (bs *BlockState) GetAllDescendants(hash common.Hash) ([]common.Hash, error) } for _, nextBlockHash := range nextBlockHashes { - nextHeader, err := bs.GetHeader(nextBlockHash) if err != nil { return nil, fmt.Errorf("getting header from block hash %s: %w", nextBlockHash, err) From 2cf149b6c0574cea2791148e04c0f7980f9389f2 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 14 Feb 2023 22:27:07 +0530 Subject: [PATCH 23/24] Update lib/blocktree/test_helpers.go Co-authored-by: Quentin McGaw --- lib/blocktree/test_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/blocktree/test_helpers.go b/lib/blocktree/test_helpers.go index bca04c8163..d2a6e9a080 100644 --- a/lib/blocktree/test_helpers.go +++ b/lib/blocktree/test_helpers.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -var zeroHash, _ = common.HexToHash("0x00") +var zeroHash = common.MustHexToHash("0x00") var testHeader = &types.Header{ ParentHash: zeroHash, Number: 0, From 8e8756a435e3d509c5e026bec2aeb304db3a372e Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Tue, 14 Feb 2023 22:46:35 +0530 Subject: [PATCH 24/24] rename test_helpers to helpers_test --- lib/blocktree/{test_helpers.go => helpers_test.go} | 1 - 1 file changed, 1 deletion(-) rename lib/blocktree/{test_helpers.go => helpers_test.go} (99%) diff --git a/lib/blocktree/test_helpers.go b/lib/blocktree/helpers_test.go similarity index 99% rename from lib/blocktree/test_helpers.go rename to lib/blocktree/helpers_test.go index bca04c8163..25553631d0 100644 --- a/lib/blocktree/test_helpers.go +++ b/lib/blocktree/helpers_test.go @@ -50,7 +50,6 @@ func createTestBlockTree(t *testing.T, header *types.Header, number uint) (*Bloc // branch tree randomly var branches []testBranch - //nolint r := rand.New(rand.NewSource(time.Now().UnixNano())) // skipcq at := int64(0)