From 151071a8c4c109c9824532ccd6d716834697aa83 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 5 Dec 2022 07:46:22 -0400 Subject: [PATCH 01/19] feat: introduce Range method --- dot/state/block.go | 121 ++++++++++++++++++++++++++++++++++++- lib/blocktree/blocktree.go | 51 +++++++++++++++- 2 files changed, 169 insertions(+), 3 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index d12e9b5011..818cefd957 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -38,6 +38,7 @@ var ( messageQueuePrefix = []byte("mqp") // messageQueuePrefix + hash -> message queue justificationPrefix = []byte("jcp") // justificationPrefix + hash -> justification + errNilBlockTree = errors.New("blocktree is nil") errNilBlockBody = errors.New("block body is nil") syncedBlocksGauge = promauto.NewGauge(prometheus.GaugeOpts{ @@ -542,10 +543,126 @@ func (bs *BlockState) GetSlotForBlock(hash common.Hash) (uint64, error) { return types.GetSlotFromHeader(header) } +var ( + errNotInDatabase = errors.New("not in database") + errStartHashIsEmpty = errors.New("start hash is empty") + errEndHashIsEmpty = errors.New("end hash is empty") +) + +func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header, err error) { + startHeaderData, err := bs.db.Get(headerKey(hash)) + if err != nil { + return nil, fmt.Errorf("querying database: %w", err) + } + + header = types.NewEmptyHeader() + err = scale.Unmarshal(startHeaderData, header) + if err != nil { + return nil, fmt.Errorf("unmarshaling start header: %w", err) + } + + if header.Empty() { + return nil, fmt.Errorf("%w: %s", chaindb.ErrKeyNotFound, hash) + } + + return header, nil +} + +func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Hash, err error) { + if bs.bt == nil { + return nil, errNilBlockTree + } + + endHeader, err := bs.loadHeaderFromDisk(endHash) + if errors.Is(err, chaindb.ErrKeyNotFound) { + // end hash is not in the disk so we should lookup + // block that could be in memory and in the disk as well + return bs.retrieveRange(startHash, endHash) + } else if err != nil { + return nil, fmt.Errorf("getting range end hash: %w", err) + } + + // end hash was found in the disk, that means all the blocks + // between start and end can be found in the disk + return bs.retrieveRangeFromDisk(startHash, endHeader) +} + +func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []common.Hash, err error) { + inMemoryHashes, err := bs.bt.RetrieveRange(startHash, endHash) + if err != nil { + return nil, fmt.Errorf("retrieving range from in-memory blocktree: %w", err) + } + + // if the first item is equal to the startHash that means we got the range + // from the in-memory blocktree + if inMemoryHashes[0] == startHash { + return inMemoryHashes, nil + } + + // since we got as many blocks as we could from + // the block tree but still missing blocks to + // fullfill the range that means we should lookup in the + // disk we should lookup in the disk for the remaining ones + // the first item in the hashes array is the block tree root + // that is also persisted in the disk so we will start from + // its parent since it is already in the array + blockTreeRootHeader, err := bs.loadHeaderFromDisk(inMemoryHashes[0]) + if err != nil { + return nil, fmt.Errorf("loading block tree root from disk: %w", err) + } + + startingAtParentHeader, err := bs.loadHeaderFromDisk(blockTreeRootHeader.ParentHash) + if err != nil { + return nil, fmt.Errorf("range end should be in database: %w", err) + } + + inDiskHashes, err := bs.retrieveRangeFromDisk(startHash, startingAtParentHeader) + if err != nil { + return nil, fmt.Errorf("retrieving range from disk: %w", err) + } + + hashes = make([]common.Hash, 0, len(inMemoryHashes)+len(inDiskHashes)) + hashes = append(inDiskHashes, inMemoryHashes...) + + return hashes, nil +} + +func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, endHeader *types.Header) (hashes []common.Hash, err error) { + startHeader, err := bs.loadHeaderFromDisk(startHash) + if err != nil { + return nil, fmt.Errorf("range start should be in database: %w", err) + } + + blocksInRange := endHeader.Number - startHeader.Number + hashes = make([]common.Hash, blocksInRange) + + lastPosition := blocksInRange - 1 + hashes[0] = startHash + hashes[lastPosition] = endHeader.Hash() + + return retrieveHashAt(bs.loadHeaderFromDisk, hashes, endHeader.ParentHash, lastPosition-1) +} + +func retrieveHashAt[ + T func(common.Hash) (*types.Header, error)]( + loader T, hashes []common.Hash, hashToLookup common.Hash, pos uint) ([]common.Hash, error) { + if pos == 0 { + return hashes, nil + } + + respectiveHeader, err := loader(hashToLookup) + if err != nil { + return nil, fmt.Errorf("expected to be in disk: %w", err) + } + + hashes[pos] = hashToLookup + return retrieveHashAt(loader, hashes, respectiveHeader.ParentHash, pos-1) +} + // SubChain returns the sub-blockchain between the starting hash and the ending hash using the block tree func (bs *BlockState) SubChain(start, end common.Hash) ([]common.Hash, error) { if bs.bt == nil { - return nil, fmt.Errorf("blocktree is nil") + return nil, errNilBlockTree } return bs.bt.SubBlockchain(start, end) @@ -555,7 +672,7 @@ func (bs *BlockState) SubChain(start, end common.Hash) ([]common.Hash, error) { // it returns an error if parent or child are not in the blocktree. func (bs *BlockState) IsDescendantOf(parent, child common.Hash) (bool, error) { if bs.bt == nil { - return false, fmt.Errorf("blocktree is nil") + return false, errNilBlockTree } return bs.bt.IsDescendantOf(parent, child) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 5396f1b00a..f956fb7dff 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -130,6 +130,56 @@ func (bt *BlockTree) GetAllBlocksAtNumber(hash common.Hash) (hashes []common.Has return bt.root.getNodesWithNumber(number, hashes) } +var ErrNilBlockInRange = errors.New("nil block in range") + +func (bt *BlockTree) RetrieveRange(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { + endNode := bt.getNode(endHash) + if endNode == nil { + return nil, fmt.Errorf("%w: %s", ErrNodeNotFound, endHash) + } + + // if we don't find the start hash in the blocktree + // that means it should be in the disk, so we retrieve + // as many nodes as we can, in other words we get all the + // block from the end hash till the bt.root inclusive + startNode := bt.getNode(startHash) + if startNode == nil { + startNode = bt.root + } + + blocksInRange := endNode.number - startNode.number + hashes, err = accumulateHashesInDescedingOrder(blocksInRange, endNode, startNode.hash) + if err != nil { + return nil, fmt.Errorf("getting blocks in range: %w", err) + } + + return hashes, nil +} + +func accumulateHashesInDescedingOrder(blocksInRange uint, startPoint *node, + startHash common.Hash) (hashes []common.Hash, err error) { + hashes = make([]common.Hash, blocksInRange) + hashes[0] = startHash + + for position := blocksInRange - 1; position >= 0; position-- { + respectiveHash := startPoint.hash + + // we ensure we complete the range hitting the start hash + if position == 0 && respectiveHash == startHash { + break + } + + hashes[position] = respectiveHash + startPoint = startPoint.parent + + if startPoint == nil { + return nil, ErrNilBlockInRange + } + } + + return hashes, nil +} + // getNode finds and returns a node based on its Hash. Returns nil if not found. func (bt *BlockTree) getNode(h Hash) (ret *node) { if bt.root.hash == h { @@ -237,7 +287,6 @@ func (bt *BlockTree) SubBlockchain(start, end Hash) ([]Hash, error) { bc = append(bc, node.hash) } return bc, nil - } // best returns the best node in the block tree using the fork choice rule. From 3c7c465a2e684ce8bbb34e57e89b9f9778600705 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 5 Dec 2022 07:47:26 -0400 Subject: [PATCH 02/19] chore: improve `fmt.Errorf` context message --- 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 818cefd957..1e0c9099c6 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -579,7 +579,7 @@ func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Has // block that could be in memory and in the disk as well return bs.retrieveRange(startHash, endHash) } else if err != nil { - return nil, fmt.Errorf("getting range end hash: %w", err) + return nil, fmt.Errorf("retrieving end hash from disk: %w", err) } // end hash was found in the disk, that means all the blocks From f9949a50aaf40cfb103aa8aa11ce7875ecace91a Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Thu, 8 Dec 2022 16:52:08 -0400 Subject: [PATCH 03/19] chore: add tests to `blockstate.Range` method --- dot/state/block.go | 62 ++++++++++++++++--------- dot/state/block_test.go | 94 ++++++++++++++++++++++++++++++++++++++ lib/blocktree/blocktree.go | 38 +++++++++------ 3 files changed, 157 insertions(+), 37 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 1e0c9099c6..74428eaece 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -543,12 +543,6 @@ func (bs *BlockState) GetSlotForBlock(hash common.Hash) (uint64, error) { return types.GetSlotFromHeader(header) } -var ( - errNotInDatabase = errors.New("not in database") - errStartHashIsEmpty = errors.New("start hash is empty") - errEndHashIsEmpty = errors.New("end hash is empty") -) - func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header, err error) { startHeaderData, err := bs.db.Get(headerKey(hash)) if err != nil { @@ -568,6 +562,8 @@ func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header return header, nil } +// Range returns the sub-blockchain between the starting hash and the +// ending hash using both block tree and disk func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Hash, err error) { if bs.bt == nil { return nil, errNilBlockTree @@ -593,20 +589,21 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co return nil, fmt.Errorf("retrieving range from in-memory blocktree: %w", err) } + firstItem := inMemoryHashes[0] + // if the first item is equal to the startHash that means we got the range // from the in-memory blocktree - if inMemoryHashes[0] == startHash { + if firstItem == startHash { return inMemoryHashes, nil } // since we got as many blocks as we could from // the block tree but still missing blocks to - // fullfill the range that means we should lookup in the - // disk we should lookup in the disk for the remaining ones - // the first item in the hashes array is the block tree root - // that is also persisted in the disk so we will start from - // its parent since it is already in the array - blockTreeRootHeader, err := bs.loadHeaderFromDisk(inMemoryHashes[0]) + // fulfil the range we should lookup in the + // disk for the remaining ones, the first item in the hashes array + // must be the block tree root that is also placed in the disk + // so we will start from its parent since it is already in the array + blockTreeRootHeader, err := bs.loadHeaderFromDisk(firstItem) if err != nil { return nil, fmt.Errorf("loading block tree root from disk: %w", err) } @@ -627,26 +624,47 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co return hashes, nil } -func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, endHeader *types.Header) (hashes []common.Hash, err error) { +// retrieveRangeFromDisk takes the start and the end and will retrieve all block in between +// where all blocks (start and end inclusive) are supposed to be placed at disk +func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, + endHeader *types.Header) (hashes []common.Hash, err error) { startHeader, err := bs.loadHeaderFromDisk(startHash) if err != nil { return nil, fmt.Errorf("range start should be in database: %w", err) } - blocksInRange := endHeader.Number - startHeader.Number + // blocksInRange is the difference between the end number to start number + // but the difference don't includes the start item that is why we add 1 + blocksInRange := (endHeader.Number - startHeader.Number) + 1 + hashes = make([]common.Hash, blocksInRange) lastPosition := blocksInRange - 1 hashes[0] = startHash hashes[lastPosition] = endHeader.Hash() - return retrieveHashAt(bs.loadHeaderFromDisk, hashes, endHeader.ParentHash, lastPosition-1) + return recursiveRetrieveFromDisk(bs.loadHeaderFromDisk, endHeader.ParentHash, hashes, lastPosition-1) } -func retrieveHashAt[ - T func(common.Hash) (*types.Header, error)]( - loader T, hashes []common.Hash, hashToLookup common.Hash, pos uint) ([]common.Hash, error) { - if pos == 0 { +var ErrStartHashMismatch = errors.New("start hash mismatch") + +type loadHeaderFromDiskFunc func(common.Hash) (*types.Header, error) + +// recursiveRetrieveFromDisk takes the function that loads a respective hash from disk +// retrieve from disk and stores it in the hashes at position given by currentPosition argument +// and recursivelly calls it self giving the parent hash and decreasing the current position by 1 +// once it achieves the position 0 it returns the slice with hashes to the caller +func recursiveRetrieveFromDisk(loader loadHeaderFromDiskFunc, hashToLookup common.Hash, + hashes []common.Hash, currentPosition uint) ([]common.Hash, error) { + if currentPosition == 0 { + // at position 0 we must ensure that all the recursive calls ended + // up in the same start hash as the one the caller is searching for + alreadyStoredStartHash := hashes[0] + if hashToLookup != alreadyStoredStartHash { + return nil, fmt.Errorf("%w: expected %s, got %s", + ErrStartHashMismatch, alreadyStoredStartHash, hashToLookup) + } + return hashes, nil } @@ -655,8 +673,8 @@ func retrieveHashAt[ return nil, fmt.Errorf("expected to be in disk: %w", err) } - hashes[pos] = hashToLookup - return retrieveHashAt(loader, hashes, respectiveHeader.ParentHash, pos-1) + hashes[currentPosition] = hashToLookup + return recursiveRetrieveFromDisk(loader, respectiveHeader.ParentHash, hashes, currentPosition-1) } // SubChain returns the sub-blockchain between the starting hash and the ending hash using the block tree diff --git a/dot/state/block_test.go b/dot/state/block_test.go index e788427957..64e6576344 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -599,3 +599,97 @@ func TestNumberIsFinalised(t *testing.T) { require.NoError(t, err) require.False(t, fin) } + +func TestRange(t *testing.T) { + t.Parallel() + + testcases := map[string]struct { + blocksToCreate int + blocksToPersistAtDisk int + + wantErr error + }{ + "all_blocks_stored_in_disk": { + blocksToCreate: 128, + blocksToPersistAtDisk: 128, + }, + + "all_blocks_persisted_in_blocktree": { + blocksToCreate: 128, + blocksToPersistAtDisk: 0, + }, + + "half_blocks_placed_in_blocktree_half_stored_in_disk": { + blocksToCreate: 128, + blocksToPersistAtDisk: 64, + }, + } + + for tname, tt := range testcases { + tt := tt + + t.Run(tname, func(t *testing.T) { + t.Parallel() + + if tt.blocksToCreate < tt.blocksToPersistAtDisk { + require.Fail(t, "blocksToPersistAtDisk should be lower or equal blocksToCreate") + } + + ctrl := gomock.NewController(t) + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + genesisHeader := &types.Header{ + Number: 0, + StateRoot: trie.EmptyHash, + Digest: types.NewDigest(), + } + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + + testBlockBody := *types.NewBody( + []types.Extrinsic{[]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}}) + hashes := make([]common.Hash, 0, tt.blocksToCreate) + + previousHeaderHash := genesisHeader.Hash() + for blockNumber := 1; blockNumber <= tt.blocksToCreate; blockNumber++ { + currentHeader := &types.Header{ + Number: uint(blockNumber), + Digest: createPrimaryBABEDigest(t), + ParentHash: previousHeaderHash, + } + + block := &types.Block{ + Header: *currentHeader, + Body: testBlockBody, + } + + err := blockState.AddBlock(block) + require.NoError(t, err) + + hashes = append(hashes, currentHeader.Hash()) + previousHeaderHash = currentHeader.Hash() + } + + if tt.blocksToPersistAtDisk > 0 { + hashIndexToSetAsFinalized := tt.blocksToPersistAtDisk - 1 + selectedHash := hashes[hashIndexToSetAsFinalized] + + err := blockState.SetFinalisedHash(selectedHash, 0, 0) + require.NoError(t, err) + } + + // execute the Range call. All the values returned must + // match the hashes we previsouly created + startHash := hashes[0] + endHash := hashes[len(hashes)-1] + + hashesInRange, err := blockState.Range(startHash, endHash) + require.ErrorIs(t, err, tt.wantErr) + require.Equal(t, hashes, hashesInRange) + }) + } +} diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 9ab0bb28fb..886316b2e8 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -132,6 +132,13 @@ func (bt *BlockTree) GetAllBlocksAtNumber(hash common.Hash) (hashes []common.Has var ErrNilBlockInRange = errors.New("nil block in range") +// RetrieveRange will return all the blocks between the start and +// end hash inclusive. +// If the end hash does not exist in the blocktree than an error +// will be returned. +// If the start hash does not exists in the blocktree +// then we will return all blocks between the end and the blocktree +// root inclusive func (bt *BlockTree) RetrieveRange(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { endNode := bt.getNode(endHash) if endNode == nil { @@ -147,8 +154,7 @@ func (bt *BlockTree) RetrieveRange(startHash common.Hash, endHash common.Hash) ( startNode = bt.root } - blocksInRange := endNode.number - startNode.number - hashes, err = accumulateHashesInDescedingOrder(blocksInRange, endNode, startNode.hash) + hashes, err = accumulateHashesInDescedingOrder(endNode, startNode) if err != nil { return nil, fmt.Errorf("getting blocks in range: %w", err) } @@ -156,24 +162,26 @@ func (bt *BlockTree) RetrieveRange(startHash common.Hash, endHash common.Hash) ( return hashes, nil } -func accumulateHashesInDescedingOrder(blocksInRange uint, startPoint *node, - startHash common.Hash) (hashes []common.Hash, err error) { +func accumulateHashesInDescedingOrder(endNode, startNode *node) ( + hashes []common.Hash, err error) { + + // blocksInRange is the difference between the end number to start number + // but the difference don't includes the start item that is why we add 1 + blocksInRange := (endNode.number - startNode.number) + 1 hashes = make([]common.Hash, blocksInRange) - hashes[0] = startHash - for position := blocksInRange - 1; position >= 0; position-- { - respectiveHash := startPoint.hash + lastPosition := blocksInRange - 1 + hashes[0] = startNode.hash - // we ensure we complete the range hitting the start hash - if position == 0 && respectiveHash == startHash { - break - } + for position := lastPosition; position > 0; position-- { + currentNodeHash := endNode.hash + hashes[position] = currentNodeHash - hashes[position] = respectiveHash - startPoint = startPoint.parent + endNode = endNode.parent - if startPoint == nil { - return nil, ErrNilBlockInRange + if endNode == nil { + return nil, fmt.Errorf("%w: missing parent of %s", + ErrNilBlockInRange, currentNodeHash) } } From d4e99115ca0ee3b6d6528f0933bd75eb9a1fb8ec Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Thu, 8 Dec 2022 17:50:04 -0400 Subject: [PATCH 04/19] chore: resolving an lint warn --- dot/state/block.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 74428eaece..27ec61075f 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -618,9 +618,7 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co return nil, fmt.Errorf("retrieving range from disk: %w", err) } - hashes = make([]common.Hash, 0, len(inMemoryHashes)+len(inDiskHashes)) hashes = append(inDiskHashes, inMemoryHashes...) - return hashes, nil } From 4fcb51b2561ba4bcffb6a271ed8f3dd61b73cc38 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 9 Dec 2022 20:48:56 -0400 Subject: [PATCH 05/19] chore: add more tests --- dot/state/block_test.go | 88 ++++++++++++-- dot/state/mocks_database_test.go | 193 +++++++++++++++++++++++++++++++ dot/state/mocks_generate_test.go | 1 + 3 files changed, 271 insertions(+), 11 deletions(-) create mode 100644 dot/state/mocks_database_test.go diff --git a/dot/state/block_test.go b/dot/state/block_test.go index 64e6576344..e1a8bd2b6f 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -4,6 +4,7 @@ package state import ( + "errors" "testing" "time" @@ -603,25 +604,92 @@ func TestNumberIsFinalised(t *testing.T) { func TestRange(t *testing.T) { t.Parallel() + loadHeaderFromDiskErr := errors.New("[mocked] cannot read, database closed ex.") testcases := map[string]struct { blocksToCreate int blocksToPersistAtDisk int - wantErr error + newBlockState func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState + wantErr error + stringErr string }{ "all_blocks_stored_in_disk": { blocksToCreate: 128, blocksToPersistAtDisk: 128, + newBlockState: func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState { + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + + return blockState + }, }, "all_blocks_persisted_in_blocktree": { blocksToCreate: 128, blocksToPersistAtDisk: 0, + newBlockState: func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState { + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + + return blockState + }, }, "half_blocks_placed_in_blocktree_half_stored_in_disk": { blocksToCreate: 128, blocksToPersistAtDisk: 64, + newBlockState: func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState { + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + + return blockState + }, + }, + + "error_while_loading_header_from_disk": { + blocksToCreate: 1, + blocksToPersistAtDisk: 0, + wantErr: loadHeaderFromDiskErr, + stringErr: "retrieving end hash from disk: " + + "querying database: [mocked] cannot read, database closed ex.", + newBlockState: func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState { + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + + mockedDb := NewMockDatabase(ctrl) + // cannot assert the exact hash type since the block header + // hash is generate by the running test case + mockedDb.EXPECT().Get(gomock.AssignableToTypeOf([]byte{})). + Return(nil, loadHeaderFromDiskErr) + + blockState.db = mockedDb + + require.NoError(t, err) + return blockState + }, }, } @@ -636,24 +704,16 @@ func TestRange(t *testing.T) { } ctrl := gomock.NewController(t) - telemetryMock := NewMockClient(ctrl) - telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() - - db := NewInMemoryDB(t) - genesisHeader := &types.Header{ Number: 0, StateRoot: trie.EmptyHash, Digest: types.NewDigest(), } - blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) - require.NoError(t, err) + blockState := tt.newBlockState(t, ctrl, genesisHeader) - testBlockBody := *types.NewBody( - []types.Extrinsic{[]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}}) + testBlockBody := *types.NewBody([]types.Extrinsic{[]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}}) hashes := make([]common.Hash, 0, tt.blocksToCreate) - previousHeaderHash := genesisHeader.Hash() for blockNumber := 1; blockNumber <= tt.blocksToCreate; blockNumber++ { currentHeader := &types.Header{ @@ -689,6 +749,12 @@ func TestRange(t *testing.T) { hashesInRange, err := blockState.Range(startHash, endHash) require.ErrorIs(t, err, tt.wantErr) + if tt.stringErr != "" { + require.EqualError(t, err, tt.stringErr) + require.Empty(t, hashesInRange) + return + } + require.Equal(t, hashes, hashesInRange) }) } diff --git a/dot/state/mocks_database_test.go b/dot/state/mocks_database_test.go new file mode 100644 index 0000000000..a25cdb39f6 --- /dev/null +++ b/dot/state/mocks_database_test.go @@ -0,0 +1,193 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/ChainSafe/chaindb (interfaces: Database) + +// Package state is a generated GoMock package. +package state + +import ( + context "context" + reflect "reflect" + + chaindb "github.com/ChainSafe/chaindb" + pb "github.com/dgraph-io/badger/v2/pb" + gomock "github.com/golang/mock/gomock" +) + +// MockDatabase is a mock of Database interface. +type MockDatabase struct { + ctrl *gomock.Controller + recorder *MockDatabaseMockRecorder +} + +// MockDatabaseMockRecorder is the mock recorder for MockDatabase. +type MockDatabaseMockRecorder struct { + mock *MockDatabase +} + +// NewMockDatabase creates a new mock instance. +func NewMockDatabase(ctrl *gomock.Controller) *MockDatabase { + mock := &MockDatabase{ctrl: ctrl} + mock.recorder = &MockDatabaseMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockDatabase) EXPECT() *MockDatabaseMockRecorder { + return m.recorder +} + +// ClearAll mocks base method. +func (m *MockDatabase) ClearAll() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ClearAll") + ret0, _ := ret[0].(error) + return ret0 +} + +// ClearAll indicates an expected call of ClearAll. +func (mr *MockDatabaseMockRecorder) ClearAll() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClearAll", reflect.TypeOf((*MockDatabase)(nil).ClearAll)) +} + +// Close mocks base method. +func (m *MockDatabase) Close() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Close") + ret0, _ := ret[0].(error) + return ret0 +} + +// Close indicates an expected call of Close. +func (mr *MockDatabaseMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockDatabase)(nil).Close)) +} + +// Del mocks base method. +func (m *MockDatabase) Del(arg0 []byte) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Del", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// Del indicates an expected call of Del. +func (mr *MockDatabaseMockRecorder) Del(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Del", reflect.TypeOf((*MockDatabase)(nil).Del), arg0) +} + +// Flush mocks base method. +func (m *MockDatabase) Flush() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Flush") + ret0, _ := ret[0].(error) + return ret0 +} + +// Flush indicates an expected call of Flush. +func (mr *MockDatabaseMockRecorder) Flush() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Flush", reflect.TypeOf((*MockDatabase)(nil).Flush)) +} + +// Get mocks base method. +func (m *MockDatabase) Get(arg0 []byte) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", arg0) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockDatabaseMockRecorder) Get(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockDatabase)(nil).Get), arg0) +} + +// Has mocks base method. +func (m *MockDatabase) Has(arg0 []byte) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Has", arg0) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Has indicates an expected call of Has. +func (mr *MockDatabaseMockRecorder) Has(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Has", reflect.TypeOf((*MockDatabase)(nil).Has), arg0) +} + +// NewBatch mocks base method. +func (m *MockDatabase) NewBatch() chaindb.Batch { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewBatch") + ret0, _ := ret[0].(chaindb.Batch) + return ret0 +} + +// NewBatch indicates an expected call of NewBatch. +func (mr *MockDatabaseMockRecorder) NewBatch() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewBatch", reflect.TypeOf((*MockDatabase)(nil).NewBatch)) +} + +// NewIterator mocks base method. +func (m *MockDatabase) NewIterator() chaindb.Iterator { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewIterator") + ret0, _ := ret[0].(chaindb.Iterator) + return ret0 +} + +// NewIterator indicates an expected call of NewIterator. +func (mr *MockDatabaseMockRecorder) NewIterator() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewIterator", reflect.TypeOf((*MockDatabase)(nil).NewIterator)) +} + +// Path mocks base method. +func (m *MockDatabase) Path() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Path") + ret0, _ := ret[0].(string) + return ret0 +} + +// Path indicates an expected call of Path. +func (mr *MockDatabaseMockRecorder) Path() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Path", reflect.TypeOf((*MockDatabase)(nil).Path)) +} + +// Put mocks base method. +func (m *MockDatabase) Put(arg0, arg1 []byte) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Put", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// Put indicates an expected call of Put. +func (mr *MockDatabaseMockRecorder) Put(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Put", reflect.TypeOf((*MockDatabase)(nil).Put), arg0, arg1) +} + +// Subscribe mocks base method. +func (m *MockDatabase) Subscribe(arg0 context.Context, arg1 func(*pb.KVList) error, arg2 []byte) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Subscribe", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// Subscribe indicates an expected call of Subscribe. +func (mr *MockDatabaseMockRecorder) Subscribe(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Subscribe", reflect.TypeOf((*MockDatabase)(nil).Subscribe), arg0, arg1, arg2) +} diff --git a/dot/state/mocks_generate_test.go b/dot/state/mocks_generate_test.go index bde3f7cf99..f0ed459969 100644 --- a/dot/state/mocks_generate_test.go +++ b/dot/state/mocks_generate_test.go @@ -4,3 +4,4 @@ package state //go:generate mockgen -destination=mock_telemetry_test.go -package $GOPACKAGE github.com/ChainSafe/gossamer/dot/telemetry Client +//go:generate mockgen -destination=mocks_database_test.go -package $GOPACKAGE github.com/ChainSafe/chaindb Database From 2e50d10ac4f88dbffd6ba4bc01e7ca2b8a44216c Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 09:55:28 -0400 Subject: [PATCH 06/19] chore: solving a lint problem --- dot/state/block_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/state/block_test.go b/dot/state/block_test.go index e1a8bd2b6f..d24e539a27 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -604,7 +604,7 @@ func TestNumberIsFinalised(t *testing.T) { func TestRange(t *testing.T) { t.Parallel() - loadHeaderFromDiskErr := errors.New("[mocked] cannot read, database closed ex.") + loadHeaderFromDiskErr := errors.New("[mocked] cannot read, database closed ex") testcases := map[string]struct { blocksToCreate int blocksToPersistAtDisk int From bd10b2d60b0e31bae995be5a7241c7d7329903cc Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 11:27:22 -0400 Subject: [PATCH 07/19] chore: doing benchamarks and changed the `retrieveRangeFromDisk` function --- dot/state/block.go | 40 +++++++++++++++------------------------ dot/state/block_test.go | 2 +- dot/state/test_helpers.go | 2 +- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 28713686d6..9c05be086e 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -622,6 +622,8 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co return hashes, nil } +var ErrStartHashMismatch = errors.New("start hash mismatch") + // retrieveRangeFromDisk takes the start and the end and will retrieve all block in between // where all blocks (start and end inclusive) are supposed to be placed at disk func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, @@ -638,41 +640,29 @@ func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, hashes = make([]common.Hash, blocksInRange) lastPosition := blocksInRange - 1 + hashes[0] = startHash hashes[lastPosition] = endHeader.Hash() - return recursiveRetrieveFromDisk(bs.loadHeaderFromDisk, endHeader.ParentHash, hashes, lastPosition-1) -} - -var ErrStartHashMismatch = errors.New("start hash mismatch") + inLoopHash := endHeader.ParentHash + for currentPosition := lastPosition - 1; currentPosition > 0; currentPosition-- { + hashes[currentPosition] = inLoopHash -type loadHeaderFromDiskFunc func(common.Hash) (*types.Header, error) - -// recursiveRetrieveFromDisk takes the function that loads a respective hash from disk -// retrieve from disk and stores it in the hashes at position given by currentPosition argument -// and recursivelly calls it self giving the parent hash and decreasing the current position by 1 -// once it achieves the position 0 it returns the slice with hashes to the caller -func recursiveRetrieveFromDisk(loader loadHeaderFromDiskFunc, hashToLookup common.Hash, - hashes []common.Hash, currentPosition uint) ([]common.Hash, error) { - if currentPosition == 0 { - // at position 0 we must ensure that all the recursive calls ended - // up in the same start hash as the one the caller is searching for - alreadyStoredStartHash := hashes[0] - if hashToLookup != alreadyStoredStartHash { - return nil, fmt.Errorf("%w: expected %s, got %s", - ErrStartHashMismatch, alreadyStoredStartHash, hashToLookup) + inLoopHeader, err := bs.loadHeaderFromDisk(inLoopHash) + if err != nil { + return nil, fmt.Errorf("retrieving hash %s from disk: %w", inLoopHash.Short(), err) } - return hashes, nil + inLoopHash = inLoopHeader.ParentHash } - respectiveHeader, err := loader(hashToLookup) - if err != nil { - return nil, fmt.Errorf("expected to be in disk: %w", err) + // here we ensure that we finished up the loop + // with the same hash as the startHash + if inLoopHash != startHash { + return nil, fmt.Errorf("%w: expecting %s, found: %s", ErrStartHashMismatch, startHash.Short(), inLoopHash.Short()) } - hashes[currentPosition] = hashToLookup - return recursiveRetrieveFromDisk(loader, respectiveHeader.ParentHash, hashes, currentPosition-1) + return hashes, err } // SubChain returns the sub-blockchain between the starting hash and the ending hash using the block tree diff --git a/dot/state/block_test.go b/dot/state/block_test.go index d24e539a27..9a555f3b4c 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -670,7 +670,7 @@ func TestRange(t *testing.T) { blocksToPersistAtDisk: 0, wantErr: loadHeaderFromDiskErr, stringErr: "retrieving end hash from disk: " + - "querying database: [mocked] cannot read, database closed ex.", + "querying database: [mocked] cannot read, database closed ex", newBlockState: func(t *testing.T, ctrl *gomock.Controller, genesisHeader *types.Header) *BlockState { telemetryMock := NewMockClient(ctrl) diff --git a/dot/state/test_helpers.go b/dot/state/test_helpers.go index 4066343e05..6dfed8f905 100644 --- a/dot/state/test_helpers.go +++ b/dot/state/test_helpers.go @@ -36,7 +36,7 @@ func NewInMemoryDB(t *testing.T) chaindb.Database { return db } -func createPrimaryBABEDigest(t *testing.T) scale.VaryingDataTypeSlice { +func createPrimaryBABEDigest(t testing.TB) scale.VaryingDataTypeSlice { babeDigest := types.NewBabeDigest() err := babeDigest.Set(types.BabePrimaryPreDigest{AuthorityIndex: 0}) require.NoError(t, err) From e79c5d68d049b11f133511f6b412abcaeff03f53 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 11:59:33 -0400 Subject: [PATCH 08/19] chore: replacing `SubBlockchain` with improved version --- dot/state/block.go | 2 +- lib/blocktree/blocktree.go | 33 ++---------------- lib/blocktree/blocktree_test.go | 59 +++++++++++++++++---------------- 3 files changed, 33 insertions(+), 61 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 9c05be086e..1ff4fc7f90 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -584,7 +584,7 @@ func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Has } func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []common.Hash, err error) { - inMemoryHashes, err := bs.bt.RetrieveRange(startHash, endHash) + inMemoryHashes, err := bs.bt.SubBlockchain(startHash, endHash) if err != nil { return nil, fmt.Errorf("retrieving range from in-memory blocktree: %w", err) } diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 886316b2e8..dbc2a6a347 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -132,14 +132,14 @@ func (bt *BlockTree) GetAllBlocksAtNumber(hash common.Hash) (hashes []common.Has var ErrNilBlockInRange = errors.New("nil block in range") -// RetrieveRange will return all the blocks between the start and +// SubBlockchain will return all the blocks between the start and // end hash inclusive. // If the end hash does not exist in the blocktree than an error // will be returned. // If the start hash does not exists in the blocktree // then we will return all blocks between the end and the blocktree // root inclusive -func (bt *BlockTree) RetrieveRange(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { +func (bt *BlockTree) SubBlockchain(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { endNode := bt.getNode(endHash) if endNode == nil { return nil, fmt.Errorf("%w: %s", ErrNodeNotFound, endHash) @@ -268,35 +268,6 @@ func (bt *BlockTree) String() string { return fmt.Sprintf("%s\n%s\n", metadata, tree.Print()) } -// subChain returns the path from the node with Hash start to the node with Hash end -func (bt *BlockTree) subChain(start, end Hash) ([]*node, error) { - sn := bt.getNode(start) - if sn == nil { - return nil, fmt.Errorf("%w: %s", ErrStartNodeNotFound, start) - } - en := bt.getNode(end) - if en == nil { - return nil, fmt.Errorf("%w: %s", ErrEndNodeNotFound, end) - } - return sn.subChain(en) -} - -// SubBlockchain returns the path from the node with Hash start to the node with Hash end -func (bt *BlockTree) SubBlockchain(start, end Hash) ([]Hash, error) { - bt.RLock() - defer bt.RUnlock() - - sc, err := bt.subChain(start, end) - if err != nil { - return nil, err - } - var bc []Hash - for _, node := range sc { - bc = append(bc, node.hash) - } - return bc, nil -} - // best returns the best node in the block tree using the fork choice rule. func (bt *BlockTree) best() *node { return bt.leaves.bestBlock() diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index f25be6adb0..b327b78457 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -37,7 +37,7 @@ func newBlockTreeFromNode(root *node) *BlockTree { } } -func createPrimaryBABEDigest(t *testing.T) scale.VaryingDataTypeSlice { +func createPrimaryBABEDigest(t testing.TB) scale.VaryingDataTypeSlice { babeDigest := types.NewBabeDigest() err := babeDigest.Set(types.BabePrimaryPreDigest{AuthorityIndex: 0}) require.NoError(t, err) @@ -116,7 +116,7 @@ func createTestBlockTree(t *testing.T, header *types.Header, number uint) (*Bloc return bt, branches } -func createFlatTree(t *testing.T, number uint) (*BlockTree, []common.Hash) { +func createFlatTree(t testing.TB, number uint) (*BlockTree, []common.Hash) { rootHeader := &types.Header{ ParentHash: zeroHash, Digest: createPrimaryBABEDigest(t), @@ -219,33 +219,6 @@ func Test_Node_isDecendantOf(t *testing.T) { } } -func Test_BlockTree_Subchain(t *testing.T) { - bt, hashes := createFlatTree(t, 4) - expectedPath := hashes[1:] - - // Insert a block to create a competing path - extraBlock := &types.Header{ - ParentHash: hashes[0], - Number: 1, - Digest: createPrimaryBABEDigest(t), - } - - extraBlock.Hash() - err := bt.AddBlock(extraBlock, time.Unix(0, 0)) - require.NotNil(t, err) - - subChain, err := bt.subChain(hashes[1], hashes[3]) - if err != nil { - t.Fatal(err) - } - - for i, n := range subChain { - if n.hash != expectedPath[i] { - t.Errorf("expected Hash: 0x%X got: 0x%X\n", expectedPath[i], n.hash) - } - } -} - func Test_BlockTree_Best_AllPrimary(t *testing.T) { arrivalTime := int64(256) var expected Hash @@ -823,3 +796,31 @@ func Test_BlockTree_best(t *testing.T) { bt.leaves.store(bt.root.children[2].hash, bt.root.children[2]) require.Equal(t, bt.root.children[2].hash, bt.BestBlockHash()) } + +func BenchmarkBlockTreeSubBlockchain(b *testing.B) { + testInputs := []struct { + input int + }{ + {input: 100}, + {input: 1000}, + {input: 10000}, + } + + for _, tt := range testInputs { + tt := tt + + bt, expectedHashes := createFlatTree(b, uint(tt.input)) + + firstHash := expectedHashes[0] + endHash := expectedHashes[len(expectedHashes)-1] + + b.Run(fmt.Sprintf("input_len_%d", tt.input), func(b *testing.B) { + for i := 0; i < b.N; i++ { + retrieved, err := bt.SubBlockchain(firstHash, endHash) + require.NoError(b, err) + require.Equal(b, expectedHashes, retrieved) + } + }) + } + +} From 70b3a745a79ec28e873e20991bf70b28eed1cb9c Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 12:15:36 -0400 Subject: [PATCH 09/19] chore: solving lint warns --- lib/blocktree/node.go | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/lib/blocktree/node.go b/lib/blocktree/node.go index cacc940197..557f805066 100644 --- a/lib/blocktree/node.go +++ b/lib/blocktree/node.go @@ -78,29 +78,6 @@ func (n *node) getNodesWithNumber(number uint, hashes []common.Hash) []common.Ha return hashes } -// subChain searches for a chain with head n and descendant going from child -> parent -func (n *node) subChain(descendant *node) ([]*node, error) { - if descendant == nil { - return nil, ErrNilDescendant - } - - var path []*node - - if n.hash == descendant.hash { - path = append(path, n) - return path, nil - } - - for curr := descendant; curr != nil; curr = curr.parent { - path = append([]*node{curr}, path...) - if curr.hash == n.hash { - return path, nil - } - } - - return nil, ErrDescendantNotFound -} - // isDescendantOf traverses the tree following all possible paths until it determines if n is a descendant of parent func (n *node) isDescendantOf(parent *node) bool { if parent == nil || n == nil { From 0f244c5a6274cf1f0673f8b5765ff8185dd9ab86 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 17:44:50 -0400 Subject: [PATCH 10/19] chore: improve tests --- dot/state/block.go | 12 +- dot/state/block_test.go | 233 ++++++++++++++++++++++++++++++++++--- lib/blocktree/blocktree.go | 38 +++++- 3 files changed, 262 insertions(+), 21 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 1ff4fc7f90..bd8232ada7 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -569,6 +569,11 @@ func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Has return nil, errNilBlockTree } + if startHash == endHash { + hashes = []common.Hash{startHash} + return hashes, nil + } + endHeader, err := bs.loadHeaderFromDisk(endHash) if errors.Is(err, chaindb.ErrKeyNotFound) { // end hash is not in the disk so we should lookup @@ -584,7 +589,7 @@ func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Has } func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []common.Hash, err error) { - inMemoryHashes, err := bs.bt.SubBlockchain(startHash, endHash) + inMemoryHashes, err := bs.bt.Range(startHash, endHash) if err != nil { return nil, fmt.Errorf("retrieving range from in-memory blocktree: %w", err) } @@ -623,6 +628,7 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co } var ErrStartHashMismatch = errors.New("start hash mismatch") +var ErrStartGreaterThanEnd = errors.New("start greater than end") // retrieveRangeFromDisk takes the start and the end and will retrieve all block in between // where all blocks (start and end inclusive) are supposed to be placed at disk @@ -633,6 +639,10 @@ func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, return nil, fmt.Errorf("range start should be in database: %w", err) } + if startHeader.Number > endHeader.Number { + return nil, fmt.Errorf("%w", ErrStartGreaterThanEnd) + } + // blocksInRange is the difference between the end number to start number // but the difference don't includes the start item that is why we add 1 blocksInRange := (endHeader.Number - startHeader.Number) + 1 diff --git a/dot/state/block_test.go b/dot/state/block_test.go index 9a555f3b4c..08cf00d18d 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -8,7 +8,9 @@ import ( "testing" "time" + "github.com/ChainSafe/chaindb" "github.com/ChainSafe/gossamer/dot/types" + "github.com/ChainSafe/gossamer/lib/blocktree" "github.com/ChainSafe/gossamer/lib/common" "github.com/ChainSafe/gossamer/lib/trie" "github.com/ChainSafe/gossamer/pkg/scale" @@ -613,6 +615,10 @@ func TestRange(t *testing.T) { genesisHeader *types.Header) *BlockState wantErr error stringErr string + + expectedHashes func(hashesCreated []common.Hash) (expected []common.Hash) + executeRangeCall func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) }{ "all_blocks_stored_in_disk": { blocksToCreate: 128, @@ -629,6 +635,19 @@ func TestRange(t *testing.T) { return blockState }, + + // execute the Range call. All the values returned must + // match the hashes we previsouly created + expectedHashes: func(hashesCreated []common.Hash) (expected []common.Hash) { + return hashesCreated + }, + executeRangeCall: func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) { + startHash := hashesCreated[0] + endHash := hashesCreated[len(hashesCreated)-1] + + return blockState.Range(startHash, endHash) + }, }, "all_blocks_persisted_in_blocktree": { @@ -646,6 +665,19 @@ func TestRange(t *testing.T) { return blockState }, + + // execute the Range call. All the values returned must + // match the hashes we previsouly created + expectedHashes: func(hashesCreated []common.Hash) (expected []common.Hash) { + return hashesCreated + }, + executeRangeCall: func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) { + startHash := hashesCreated[0] + endHash := hashesCreated[len(hashesCreated)-1] + + return blockState.Range(startHash, endHash) + }, }, "half_blocks_placed_in_blocktree_half_stored_in_disk": { @@ -663,10 +695,22 @@ func TestRange(t *testing.T) { return blockState }, + // execute the Range call. All the values returned must + // match the hashes we previsouly created + expectedHashes: func(hashesCreated []common.Hash) (expected []common.Hash) { + return hashesCreated + }, + executeRangeCall: func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) { + startHash := hashesCreated[0] + endHash := hashesCreated[len(hashesCreated)-1] + + return blockState.Range(startHash, endHash) + }, }, "error_while_loading_header_from_disk": { - blocksToCreate: 1, + blocksToCreate: 2, blocksToPersistAtDisk: 0, wantErr: loadHeaderFromDiskErr, stringErr: "retrieving end hash from disk: " + @@ -684,12 +728,154 @@ func TestRange(t *testing.T) { // hash is generate by the running test case mockedDb.EXPECT().Get(gomock.AssignableToTypeOf([]byte{})). Return(nil, loadHeaderFromDiskErr) - blockState.db = mockedDb require.NoError(t, err) return blockState }, + // execute the Range call. All the values returned must + // match the hashes we previsouly created + expectedHashes: func(hashesCreated []common.Hash) (expected []common.Hash) { + return nil + }, + executeRangeCall: func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) { + startHash := hashesCreated[0] + endHash := hashesCreated[len(hashesCreated)-1] + + return blockState.Range(startHash, endHash) + }, + }, + + "using_same_hash_as_parameters": { + blocksToCreate: 128, + blocksToPersistAtDisk: 0, + newBlockState: func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState { + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + + return blockState + }, + + // execute the Range call. All the values returned must + // match the hashes we previsouly created + expectedHashes: func(hashesCreated []common.Hash) (expected []common.Hash) { + return []common.Hash{hashesCreated[0]} + }, + executeRangeCall: func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) { + startHash := hashesCreated[0] + endHash := hashesCreated[0] + + return blockState.Range(startHash, endHash) + }, + }, + + "start_hash_greater_than_end_hash_in_database": { + blocksToCreate: 128, + blocksToPersistAtDisk: 128, + wantErr: ErrStartGreaterThanEnd, + stringErr: "start greater than end", + newBlockState: func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState { + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + + return blockState + }, + + // execute the Range call. All the values returned must + // match the hashes we previsouly created + expectedHashes: func(hashesCreated []common.Hash) (expected []common.Hash) { + return nil + }, + executeRangeCall: func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) { + startHash := hashesCreated[10] + endHash := hashesCreated[0] + + return blockState.Range(startHash, endHash) + }, + }, + + "start_hash_greater_than_end_hash_in_memory": { + blocksToCreate: 128, + blocksToPersistAtDisk: 0, + wantErr: blocktree.ErrStartGreaterThanEnd, + stringErr: "retrieving range from in-memory blocktree: " + + "getting blocks in range: start greater than end", + newBlockState: func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState { + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + + return blockState + }, + + // execute the Range call. All the values returned must + // match the hashes we previsouly created + expectedHashes: func(hashesCreated []common.Hash) (expected []common.Hash) { + return nil + }, + executeRangeCall: func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) { + startHash := hashesCreated[10] + endHash := hashesCreated[0] + + return blockState.Range(startHash, endHash) + }, + }, + + "start_hash_in_memory_while_end_hash_in_database": { + blocksToCreate: 128, + blocksToPersistAtDisk: 64, + wantErr: chaindb.ErrKeyNotFound, + stringErr: "range start should be in database: " + + "querying database: Key not found", + newBlockState: func(t *testing.T, ctrl *gomock.Controller, + genesisHeader *types.Header) *BlockState { + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + + return blockState + }, + + // execute the Range call. All the values returned must + // match the hashes we previsouly created + expectedHashes: func(hashesCreated []common.Hash) (expected []common.Hash) { + return nil + }, + executeRangeCall: func(blockState *BlockState, + hashesCreated []common.Hash) (retrievedHashes []common.Hash, err error) { + startHash := hashesCreated[len(hashesCreated)-1] + // since we finalized 64 of 128 blocks the end hash is one of + // those blocks persisted at database, while start hash is + // one of those blocks that keeps in memory + endHash := hashesCreated[0] + + return blockState.Range(startHash, endHash) + }, }, } @@ -699,9 +885,8 @@ func TestRange(t *testing.T) { t.Run(tname, func(t *testing.T) { t.Parallel() - if tt.blocksToCreate < tt.blocksToPersistAtDisk { - require.Fail(t, "blocksToPersistAtDisk should be lower or equal blocksToCreate") - } + require.LessOrEqualf(t, tt.blocksToPersistAtDisk, tt.blocksToCreate, + "blocksToPersistAtDisk should be lower or equal blocksToCreate") ctrl := gomock.NewController(t) genesisHeader := &types.Header{ @@ -713,7 +898,7 @@ func TestRange(t *testing.T) { blockState := tt.newBlockState(t, ctrl, genesisHeader) testBlockBody := *types.NewBody([]types.Extrinsic{[]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}}) - hashes := make([]common.Hash, 0, tt.blocksToCreate) + hashesCreated := make([]common.Hash, 0, tt.blocksToCreate) previousHeaderHash := genesisHeader.Hash() for blockNumber := 1; blockNumber <= tt.blocksToCreate; blockNumber++ { currentHeader := &types.Header{ @@ -730,32 +915,46 @@ func TestRange(t *testing.T) { err := blockState.AddBlock(block) require.NoError(t, err) - hashes = append(hashes, currentHeader.Hash()) + hashesCreated = append(hashesCreated, currentHeader.Hash()) previousHeaderHash = currentHeader.Hash() } if tt.blocksToPersistAtDisk > 0 { hashIndexToSetAsFinalized := tt.blocksToPersistAtDisk - 1 - selectedHash := hashes[hashIndexToSetAsFinalized] + selectedHash := hashesCreated[hashIndexToSetAsFinalized] err := blockState.SetFinalisedHash(selectedHash, 0, 0) require.NoError(t, err) } - // execute the Range call. All the values returned must - // match the hashes we previsouly created - startHash := hashes[0] - endHash := hashes[len(hashes)-1] - - hashesInRange, err := blockState.Range(startHash, endHash) + expectedHashes := tt.expectedHashes(hashesCreated) + retrievedHashes, err := tt.executeRangeCall(blockState, hashesCreated) require.ErrorIs(t, err, tt.wantErr) if tt.stringErr != "" { require.EqualError(t, err, tt.stringErr) - require.Empty(t, hashesInRange) - return } - require.Equal(t, hashes, hashesInRange) + require.Equal(t, expectedHashes, retrievedHashes) }) } } + +func Test_loadHeaderFromDisk_WithGenesisBlock(t *testing.T) { + ctrl := gomock.NewController(t) + + telemetryMock := NewMockClient(ctrl) + telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes() + + db := NewInMemoryDB(t) + + genesisHeader := &types.Header{ + Number: 0, + StateRoot: trie.EmptyHash, + Digest: types.NewDigest(), + } + + blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + header, err := blockState.loadHeaderFromDisk(genesisHeader.Hash()) + require.NoError(t, err) + require.Equal(t, genesisHeader.Hash(), header.Hash()) +} diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index dbc2a6a347..baead45316 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -130,19 +130,20 @@ func (bt *BlockTree) GetAllBlocksAtNumber(hash common.Hash) (hashes []common.Has return bt.root.getNodesWithNumber(number, hashes) } +var ErrStartGreaterThanEnd = errors.New("start greater than end") var ErrNilBlockInRange = errors.New("nil block in range") -// SubBlockchain will return all the blocks between the start and +// Range will return all the blocks between the start and // end hash inclusive. // If the end hash does not exist in the blocktree than an error // will be returned. // If the start hash does not exists in the blocktree // then we will return all blocks between the end and the blocktree // root inclusive -func (bt *BlockTree) SubBlockchain(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { +func (bt *BlockTree) Range(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { endNode := bt.getNode(endHash) if endNode == nil { - return nil, fmt.Errorf("%w: %s", ErrNodeNotFound, endHash) + return nil, fmt.Errorf("%w: %s", ErrEndNodeNotFound, endHash) } // if we don't find the start hash in the blocktree @@ -162,9 +163,40 @@ func (bt *BlockTree) SubBlockchain(startHash common.Hash, endHash common.Hash) ( return hashes, nil } +func (bt *BlockTree) SubBlockchain(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { + endNode := bt.getNode(endHash) + if endNode == nil { + return nil, fmt.Errorf("%w: %s", ErrEndNodeNotFound, endHash) + } + + // if we don't find the start hash in the blocktree + // that means it should be in the disk, so we retrieve + // as many nodes as we can, in other words we get all the + // block from the end hash till the bt.root inclusive + startNode := bt.getNode(startHash) + if startNode == nil { + return nil, fmt.Errorf("%w: %s", ErrStartNodeNotFound, endHash) + } + + if startNode.number > endNode.number { + return nil, fmt.Errorf("%w", ErrStartGreaterThanEnd) + } + + hashes, err = accumulateHashesInDescedingOrder(endNode, startNode) + if err != nil { + return nil, fmt.Errorf("getting blocks in range: %w", err) + } + + return hashes, nil +} + func accumulateHashesInDescedingOrder(endNode, startNode *node) ( hashes []common.Hash, err error) { + if startNode.number > endNode.number { + return nil, fmt.Errorf("%w", ErrStartGreaterThanEnd) + } + // blocksInRange is the difference between the end number to start number // but the difference don't includes the start item that is why we add 1 blocksInRange := (endNode.number - startNode.number) + 1 From 4e0e85c0a78b9c9052158e5d830156c0de830285 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 17:46:37 -0400 Subject: [PATCH 11/19] chore: wrapping `errNilBlockTree` error --- dot/state/block.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index bd8232ada7..359e4036b3 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -566,7 +566,7 @@ func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header // ending hash using both block tree and disk func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Hash, err error) { if bs.bt == nil { - return nil, errNilBlockTree + return nil, fmt.Errorf("%w", errNilBlockTree) } if startHash == endHash { @@ -678,7 +678,7 @@ func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, // SubChain returns the sub-blockchain between the starting hash and the ending hash using the block tree func (bs *BlockState) SubChain(start, end common.Hash) ([]common.Hash, error) { if bs.bt == nil { - return nil, errNilBlockTree + return nil, fmt.Errorf("%w", errNilBlockTree) } return bs.bt.SubBlockchain(start, end) @@ -688,7 +688,7 @@ func (bs *BlockState) SubChain(start, end common.Hash) ([]common.Hash, error) { // it returns an error if parent or child are not in the blocktree. func (bs *BlockState) IsDescendantOf(parent, child common.Hash) (bool, error) { if bs.bt == nil { - return false, errNilBlockTree + return false, fmt.Errorf("%w", errNilBlockTree) } return bs.bt.IsDescendantOf(parent, child) From e7c496a6defb774a80361bf0db05b5f0c5d4ac12 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 17:47:27 -0400 Subject: [PATCH 12/19] chore: rename to `inDiskHashes` --- dot/state/block.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 359e4036b3..5715311002 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -618,12 +618,12 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co return nil, fmt.Errorf("range end should be in database: %w", err) } - inDiskHashes, err := bs.retrieveRangeFromDisk(startHash, startingAtParentHeader) + inDatabaseHashes, err := bs.retrieveRangeFromDisk(startHash, startingAtParentHeader) if err != nil { return nil, fmt.Errorf("retrieving range from disk: %w", err) } - hashes = append(inDiskHashes, inMemoryHashes...) + hashes = append(inDatabaseHashes, inMemoryHashes...) return hashes, nil } From 2be1952d71752c38ea89cff65b00744864a468b5 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 17:49:25 -0400 Subject: [PATCH 13/19] chore: remove unneeded paren --- dot/state/block.go | 6 +++--- lib/blocktree/blocktree.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index 5715311002..e32b8c72f7 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -615,7 +615,7 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co startingAtParentHeader, err := bs.loadHeaderFromDisk(blockTreeRootHeader.ParentHash) if err != nil { - return nil, fmt.Errorf("range end should be in database: %w", err) + return nil, fmt.Errorf("loading header of parent of the root from database: %w", err) } inDatabaseHashes, err := bs.retrieveRangeFromDisk(startHash, startingAtParentHeader) @@ -645,7 +645,7 @@ func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, // blocksInRange is the difference between the end number to start number // but the difference don't includes the start item that is why we add 1 - blocksInRange := (endHeader.Number - startHeader.Number) + 1 + blocksInRange := endHeader.Number - startHeader.Number + 1 hashes = make([]common.Hash, blocksInRange) @@ -672,7 +672,7 @@ func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, return nil, fmt.Errorf("%w: expecting %s, found: %s", ErrStartHashMismatch, startHash.Short(), inLoopHash.Short()) } - return hashes, err + return hashes, nil } // SubChain returns the sub-blockchain between the starting hash and the ending hash using the block tree diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index baead45316..fa743dc09b 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -199,7 +199,7 @@ func accumulateHashesInDescedingOrder(endNode, startNode *node) ( // blocksInRange is the difference between the end number to start number // but the difference don't includes the start item that is why we add 1 - blocksInRange := (endNode.number - startNode.number) + 1 + blocksInRange := endNode.number - startNode.number + 1 hashes = make([]common.Hash, blocksInRange) lastPosition := blocksInRange - 1 From b75101fd75a1d0a926952f33ebfc9eff79ee4be6 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 12 Dec 2022 17:51:53 -0400 Subject: [PATCH 14/19] chore: addressing comments --- 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 fa743dc09b..1a0a566086 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -135,9 +135,9 @@ var ErrNilBlockInRange = errors.New("nil block in range") // Range will return all the blocks between the start and // end hash inclusive. -// If the end hash does not exist in the blocktree than an error -// will be returned. -// If the start hash does not exists in the blocktree +// If the end hash does not exist in the blocktree then an error +// is be returned. +// If the start hash does not exist in the blocktree // then we will return all blocks between the end and the blocktree // root inclusive func (bt *BlockTree) Range(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { From dc7b6678149fb89ee4889eb9b752ec46489a0506 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 13 Dec 2022 10:49:18 -0400 Subject: [PATCH 15/19] chore: addressing comments --- dot/state/block.go | 13 ++++++------- lib/blocktree/blocktree.go | 6 ++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index e32b8c72f7..c2b38ead6d 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -543,6 +543,8 @@ func (bs *BlockState) GetSlotForBlock(hash common.Hash) (uint64, error) { return types.GetSlotFromHeader(header) } +var ErrEmptyHeader = errors.New("empty header") + func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header, err error) { startHeaderData, err := bs.db.Get(headerKey(hash)) if err != nil { @@ -556,7 +558,7 @@ func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header } if header.Empty() { - return nil, fmt.Errorf("%w: %s", chaindb.ErrKeyNotFound, hash) + return nil, fmt.Errorf("%w: %s", ErrEmptyHeader, hash) } return header, nil @@ -565,18 +567,15 @@ func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header // Range returns the sub-blockchain between the starting hash and the // ending hash using both block tree and disk func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Hash, err error) { - if bs.bt == nil { - return nil, fmt.Errorf("%w", errNilBlockTree) - } - if startHash == endHash { hashes = []common.Hash{startHash} return hashes, nil } endHeader, err := bs.loadHeaderFromDisk(endHash) - if errors.Is(err, chaindb.ErrKeyNotFound) { - // end hash is not in the disk so we should lookup + if errors.Is(err, chaindb.ErrKeyNotFound) || + errors.Is(err, ErrEmptyHeader) { + // end hash is not in the disk so we should lookup the // block that could be in memory and in the disk as well return bs.retrieveRange(startHash, endHash) } else if err != nil { diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 1a0a566086..d3392d4e92 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -141,6 +141,9 @@ var ErrNilBlockInRange = errors.New("nil block in range") // then we will return all blocks between the end and the blocktree // root inclusive func (bt *BlockTree) Range(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { + bt.Lock() + defer bt.Unlock() + endNode := bt.getNode(endHash) if endNode == nil { return nil, fmt.Errorf("%w: %s", ErrEndNodeNotFound, endHash) @@ -164,6 +167,9 @@ func (bt *BlockTree) Range(startHash common.Hash, endHash common.Hash) (hashes [ } func (bt *BlockTree) SubBlockchain(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { + bt.Lock() + defer bt.Unlock() + endNode := bt.getNode(endHash) if endNode == nil { return nil, fmt.Errorf("%w: %s", ErrEndNodeNotFound, endHash) From 10a14b912698344b5706fbf01fd1c7b93b141407 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 13 Dec 2022 10:51:22 -0400 Subject: [PATCH 16/19] chore: replacing all `disk` occurrences for `database` --- dot/state/block.go | 44 +++++++++++++++++++------------------- dot/state/block_test.go | 2 +- lib/blocktree/blocktree.go | 4 ++-- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/dot/state/block.go b/dot/state/block.go index c2b38ead6d..6c5f941c07 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -545,7 +545,7 @@ func (bs *BlockState) GetSlotForBlock(hash common.Hash) (uint64, error) { var ErrEmptyHeader = errors.New("empty header") -func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header, err error) { +func (bs *BlockState) loadHeaderFromDatabase(hash common.Hash) (header *types.Header, err error) { startHeaderData, err := bs.db.Get(headerKey(hash)) if err != nil { return nil, fmt.Errorf("querying database: %w", err) @@ -565,26 +565,26 @@ func (bs *BlockState) loadHeaderFromDisk(hash common.Hash) (header *types.Header } // Range returns the sub-blockchain between the starting hash and the -// ending hash using both block tree and disk +// ending hash using both block tree and database func (bs *BlockState) Range(startHash, endHash common.Hash) (hashes []common.Hash, err error) { if startHash == endHash { hashes = []common.Hash{startHash} return hashes, nil } - endHeader, err := bs.loadHeaderFromDisk(endHash) + endHeader, err := bs.loadHeaderFromDatabase(endHash) if errors.Is(err, chaindb.ErrKeyNotFound) || errors.Is(err, ErrEmptyHeader) { - // end hash is not in the disk so we should lookup the - // block that could be in memory and in the disk as well + // end hash is not in the database so we should lookup the + // block that could be in memory and in the database as well return bs.retrieveRange(startHash, endHash) } else if err != nil { - return nil, fmt.Errorf("retrieving end hash from disk: %w", err) + return nil, fmt.Errorf("retrieving end hash from database: %w", err) } - // end hash was found in the disk, that means all the blocks - // between start and end can be found in the disk - return bs.retrieveRangeFromDisk(startHash, endHeader) + // end hash was found in the database, that means all the blocks + // between start and end can be found in the database + return bs.retrieveRangeFromDatabase(startHash, endHeader) } func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []common.Hash, err error) { @@ -604,22 +604,22 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co // since we got as many blocks as we could from // the block tree but still missing blocks to // fulfil the range we should lookup in the - // disk for the remaining ones, the first item in the hashes array - // must be the block tree root that is also placed in the disk + // database for the remaining ones, the first item in the hashes array + // must be the block tree root that is also placed in the database // so we will start from its parent since it is already in the array - blockTreeRootHeader, err := bs.loadHeaderFromDisk(firstItem) + blockTreeRootHeader, err := bs.loadHeaderFromDatabase(firstItem) if err != nil { - return nil, fmt.Errorf("loading block tree root from disk: %w", err) + return nil, fmt.Errorf("loading block tree root from database: %w", err) } - startingAtParentHeader, err := bs.loadHeaderFromDisk(blockTreeRootHeader.ParentHash) + startingAtParentHeader, err := bs.loadHeaderFromDatabase(blockTreeRootHeader.ParentHash) if err != nil { return nil, fmt.Errorf("loading header of parent of the root from database: %w", err) } - inDatabaseHashes, err := bs.retrieveRangeFromDisk(startHash, startingAtParentHeader) + inDatabaseHashes, err := bs.retrieveRangeFromDatabase(startHash, startingAtParentHeader) if err != nil { - return nil, fmt.Errorf("retrieving range from disk: %w", err) + return nil, fmt.Errorf("retrieving range from database: %w", err) } hashes = append(inDatabaseHashes, inMemoryHashes...) @@ -629,11 +629,11 @@ func (bs *BlockState) retrieveRange(startHash, endHash common.Hash) (hashes []co var ErrStartHashMismatch = errors.New("start hash mismatch") var ErrStartGreaterThanEnd = errors.New("start greater than end") -// retrieveRangeFromDisk takes the start and the end and will retrieve all block in between -// where all blocks (start and end inclusive) are supposed to be placed at disk -func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, +// retrieveRangeFromDatabase takes the start and the end and will retrieve all block in between +// where all blocks (start and end inclusive) are supposed to be placed at database +func (bs *BlockState) retrieveRangeFromDatabase(startHash common.Hash, endHeader *types.Header) (hashes []common.Hash, err error) { - startHeader, err := bs.loadHeaderFromDisk(startHash) + startHeader, err := bs.loadHeaderFromDatabase(startHash) if err != nil { return nil, fmt.Errorf("range start should be in database: %w", err) } @@ -657,9 +657,9 @@ func (bs *BlockState) retrieveRangeFromDisk(startHash common.Hash, for currentPosition := lastPosition - 1; currentPosition > 0; currentPosition-- { hashes[currentPosition] = inLoopHash - inLoopHeader, err := bs.loadHeaderFromDisk(inLoopHash) + inLoopHeader, err := bs.loadHeaderFromDatabase(inLoopHash) if err != nil { - return nil, fmt.Errorf("retrieving hash %s from disk: %w", inLoopHash.Short(), err) + return nil, fmt.Errorf("retrieving hash %s from database: %w", inLoopHash.Short(), err) } inLoopHash = inLoopHeader.ParentHash diff --git a/dot/state/block_test.go b/dot/state/block_test.go index 08cf00d18d..1573a47099 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -954,7 +954,7 @@ func Test_loadHeaderFromDisk_WithGenesisBlock(t *testing.T) { } blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) - header, err := blockState.loadHeaderFromDisk(genesisHeader.Hash()) + header, err := blockState.loadHeaderFromDatabase(genesisHeader.Hash()) require.NoError(t, err) require.Equal(t, genesisHeader.Hash(), header.Hash()) } diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index d3392d4e92..f058542727 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -150,7 +150,7 @@ func (bt *BlockTree) Range(startHash common.Hash, endHash common.Hash) (hashes [ } // if we don't find the start hash in the blocktree - // that means it should be in the disk, so we retrieve + // that means it should be in the database, so we retrieve // as many nodes as we can, in other words we get all the // block from the end hash till the bt.root inclusive startNode := bt.getNode(startHash) @@ -176,7 +176,7 @@ func (bt *BlockTree) SubBlockchain(startHash common.Hash, endHash common.Hash) ( } // if we don't find the start hash in the blocktree - // that means it should be in the disk, so we retrieve + // that means it should be in the database, so we retrieve // as many nodes as we can, in other words we get all the // block from the end hash till the bt.root inclusive startNode := bt.getNode(startHash) From fa9fcb210c08b9eb8e15f1dc1b6e5f6e609f40b1 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 13 Dec 2022 11:07:57 -0400 Subject: [PATCH 17/19] chore: addressing comments --- dot/state/block.go | 2 +- dot/state/{mocks_database_test.go => mocks_chaindb_test.go} | 0 dot/state/mocks_generate_test.go | 2 +- lib/blocktree/blocktree.go | 4 ++-- lib/blocktree/blocktree_test.go | 6 +----- 5 files changed, 5 insertions(+), 9 deletions(-) rename dot/state/{mocks_database_test.go => mocks_chaindb_test.go} (100%) diff --git a/dot/state/block.go b/dot/state/block.go index 6c5f941c07..b57774f13b 100644 --- a/dot/state/block.go +++ b/dot/state/block.go @@ -643,7 +643,7 @@ func (bs *BlockState) retrieveRangeFromDatabase(startHash common.Hash, } // blocksInRange is the difference between the end number to start number - // but the difference don't includes the start item that is why we add 1 + // but the difference doesn't include the start item so we add 1 blocksInRange := endHeader.Number - startHeader.Number + 1 hashes = make([]common.Hash, blocksInRange) diff --git a/dot/state/mocks_database_test.go b/dot/state/mocks_chaindb_test.go similarity index 100% rename from dot/state/mocks_database_test.go rename to dot/state/mocks_chaindb_test.go diff --git a/dot/state/mocks_generate_test.go b/dot/state/mocks_generate_test.go index f0ed459969..c4b130a82f 100644 --- a/dot/state/mocks_generate_test.go +++ b/dot/state/mocks_generate_test.go @@ -4,4 +4,4 @@ package state //go:generate mockgen -destination=mock_telemetry_test.go -package $GOPACKAGE github.com/ChainSafe/gossamer/dot/telemetry Client -//go:generate mockgen -destination=mocks_database_test.go -package $GOPACKAGE github.com/ChainSafe/chaindb Database +//go:generate mockgen -destination=mocks_chaindb_test.go -package $GOPACKAGE github.com/ChainSafe/chaindb Database diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index f058542727..ee91acf73b 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -152,7 +152,7 @@ func (bt *BlockTree) Range(startHash common.Hash, endHash common.Hash) (hashes [ // if we don't find the start hash in the blocktree // that means it should be in the database, so we retrieve // as many nodes as we can, in other words we get all the - // block from the end hash till the bt.root inclusive + // blocks from the end hash till the bt.root inclusive startNode := bt.getNode(startHash) if startNode == nil { startNode = bt.root @@ -178,7 +178,7 @@ func (bt *BlockTree) SubBlockchain(startHash common.Hash, endHash common.Hash) ( // if we don't find the start hash in the blocktree // that means it should be in the database, so we retrieve // as many nodes as we can, in other words we get all the - // block from the end hash till the bt.root inclusive + // blocks from the end hash till the bt.root inclusive startNode := bt.getNode(startHash) if startNode == nil { return nil, fmt.Errorf("%w: %s", ErrStartNodeNotFound, endHash) diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index b327b78457..0acf2637cf 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -807,8 +807,6 @@ func BenchmarkBlockTreeSubBlockchain(b *testing.B) { } for _, tt := range testInputs { - tt := tt - bt, expectedHashes := createFlatTree(b, uint(tt.input)) firstHash := expectedHashes[0] @@ -816,9 +814,7 @@ func BenchmarkBlockTreeSubBlockchain(b *testing.B) { b.Run(fmt.Sprintf("input_len_%d", tt.input), func(b *testing.B) { for i := 0; i < b.N; i++ { - retrieved, err := bt.SubBlockchain(firstHash, endHash) - require.NoError(b, err) - require.Equal(b, expectedHashes, retrieved) + bt.SubBlockchain(firstHash, endHash) } }) } From 3cb4c754ecd99cc7d3967717a4657297bf8ac96c Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 13 Dec 2022 11:26:58 -0400 Subject: [PATCH 18/19] chore: addressing ci warns --- dot/state/block_test.go | 2 ++ lib/blocktree/blocktree.go | 1 + 2 files changed, 3 insertions(+) diff --git a/dot/state/block_test.go b/dot/state/block_test.go index 1573a47099..9ffa24946b 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -954,6 +954,8 @@ func Test_loadHeaderFromDisk_WithGenesisBlock(t *testing.T) { } blockState, err := NewBlockStateFromGenesis(db, newTriesEmpty(), genesisHeader, telemetryMock) + require.NoError(t, err) + header, err := blockState.loadHeaderFromDatabase(genesisHeader.Hash()) require.NoError(t, err) require.Equal(t, genesisHeader.Hash(), header.Hash()) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index ee91acf73b..a9ae5443a2 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -166,6 +166,7 @@ func (bt *BlockTree) Range(startHash common.Hash, endHash common.Hash) (hashes [ return hashes, nil } +// SubBlockchain returns the path from the node with Hash start to the node with Hash end func (bt *BlockTree) SubBlockchain(startHash common.Hash, endHash common.Hash) (hashes []common.Hash, err error) { bt.Lock() defer bt.Unlock() From 4b632d8d4b7d3aff13ac15a8a3558374b2a85b9d Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 13 Dec 2022 14:51:56 -0400 Subject: [PATCH 19/19] chore: solving tests --- dot/state/block_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/state/block_test.go b/dot/state/block_test.go index 9ffa24946b..1764183f85 100644 --- a/dot/state/block_test.go +++ b/dot/state/block_test.go @@ -713,7 +713,7 @@ func TestRange(t *testing.T) { blocksToCreate: 2, blocksToPersistAtDisk: 0, wantErr: loadHeaderFromDiskErr, - stringErr: "retrieving end hash from disk: " + + stringErr: "retrieving end hash from database: " + "querying database: [mocked] cannot read, database closed ex", newBlockState: func(t *testing.T, ctrl *gomock.Controller, genesisHeader *types.Header) *BlockState {