EN-10574 Get block by round #3430

mariusmihaic · 2021-09-09T14:27:14Z

Added /by-round/:round route to get block by round with optional withTxs query param
Created a new static storer which stores: key = round; value = nonce, also see function putRoundByNonce in (hr *historyRepository) RecordBlock
When asked to retrieve block data (withTxs) from key = round, we get its corresponding value = nonce and call existing function GetBlockByNonce(nonce, withTxs)
Added tests everrywhere, except in two places where there was no test file created and the functionality was pretty trivial.

codecov · 2021-09-10T15:26:34Z

Codecov Report

Merging #3430 (968815e) into development (b573b38) will increase coverage by 0.04%.
The diff coverage is 79.64%.

@@               Coverage Diff               @@
##           development    #3430      +/-   ##
===============================================
+ Coverage        73.85%   73.89%   +0.04%     
===============================================
  Files              581      581              
  Lines            73907    74278     +371     
===============================================
+ Hits             54583    54888     +305     
- Misses           14945    15003      +58     
- Partials          4379     4387       +8

Impacted Files	Coverage Δ
dataRetriever/interface.go	`0.00% <ø> (ø)`
node/nodeRunner.go	`0.00% <0.00%> (ø)`
process/economics/testEconomicsData.go	`0.00% <0.00%> (ø)`
storage/factory/pruningStorerFactory.go	`0.00% <0.00%> (ø)`
genesis/process/shardGenesisBlockCreator.go	`43.09% <66.66%> (+0.15%)`	⬆️
process/economics/economicsData.go	`71.17% <69.29%> (-3.91%)`	⬇️
process/block/preprocess/smartContractResults.go	`69.75% <70.00%> (+1.00%)`	⬆️
process/coordinator/process.go	`75.06% <74.46%> (-0.07%)`	⬇️
process/block/preprocess/transactions.go	`55.69% <76.00%> (+2.55%)`	⬆️
node/blockAPI/baseBlock.go	`72.41% <77.77%> (+0.61%)`	⬆️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1229c24...968815e. Read the comment docs.

miiu96 · 2021-09-20T13:36:49Z

dblookupext/factory/historyRepositoryFactory_test.go

@@ -9,6 +9,8 @@ import (
 	"github.com/ElrondNetwork/elrond-go/config"
 	"github.com/ElrondNetwork/elrond-go/dataRetriever"
 	"github.com/ElrondNetwork/elrond-go/dblookupext/factory"
+	"github.com/ElrondNetwork/elrond-go/process"
+	mock2 "github.com/ElrondNetwork/elrond-go/process/mock"


Consider replacing mock2 with processorMock alias

Renamed it to processMock

miiu96 · 2021-09-20T13:40:19Z

node/blockAPI/metaBlock.go

@@ -65,6 +65,21 @@ func (mbp *metaAPIBlockProcessor) GetBlockByHash(hash []byte, withTxs bool) (*ap
 	return mbp.computeStatusAndPutInBlock(blockAPI, dataRetriever.MetaHdrNonceHashDataUnit)
 }

+// GetBlockByRound will return a shard APIBlock by round


// GetBlockByRound will return a meta APIBlock by round

Yes, fixed it here and for GetBlockByHash as well

api/block/routes.go

iulianpascalau · 2021-09-22T10:09:17Z

dblookupext/factory/historyRepositoryFactory_test.go

@@ -9,6 +9,8 @@ import (
 	"github.com/ElrondNetwork/elrond-go/config"
 	"github.com/ElrondNetwork/elrond-go/dataRetriever"
 	"github.com/ElrondNetwork/elrond-go/dblookupext/factory"
+	"github.com/ElrondNetwork/elrond-go/process"
+	mock2 "github.com/ElrondNetwork/elrond-go/process/mock"


Consider replacing mock2 with processorMock alias

dblookupext/historyRepository.go

dblookupext/historyRepository_test.go

node/blockAPI/shardBlock.go

iulianpascalau · 2021-09-22T10:20:57Z

node/nodeBlocks.go

@@ -35,6 +35,15 @@ func (n *Node) GetBlockByNonce(nonce uint64, withTxs bool) (*api.Block, error) {
 	return apiBlockProcessor.GetBlockByNonce(nonce, withTxs)
 }

+func (n *Node) GetBlockByRound(round uint64, withTxs bool) (*api.Block, error) {
+	apiBlockProcessor, err := n.createAPIBlockProcessor()


@miiu96 please check this. Why do we need to create an apiBlockProcessor for each request? We should definitely change this.

# Conflicts: # api/groups/blockGroup.go # api/groups/blockGroup_test.go # api/mock/facade.go # facade/disabled/disabledNodeFacade_test.go

iulianpascalau · 2021-09-29T07:13:19Z

dataRetriever/interface.go

+	// MetaHdrRoundHashDataUnit is the meta header round-hash storage data pair unit identifier
+	MetaHdrRoundHashDataUnit UnitType = 49
+	// ShardHdrRoundHashDataUnit is the shard round-hash storage data unit identifier
+	ShardHdrRoundHashDataUnit UnitType = 50


let's put this on the 200 value (or even better ~400 as to allow the creation of 256 shards) and place it after the ShardHdrNonceHashDataUnit definition. MetaHdrRoundHashDataUnit can be 19. Adjust the comment below to include the ShardHdrNonceHashDataUnit.

iulianpascalau · 2021-09-29T07:15:59Z

dblookupext/historyRepository_test.go

@@ -94,12 +94,12 @@ func TestNewHistoryRepository(t *testing.T) {
 	require.NotNil(t, repo)
 }

-func TestHistoryRepository_RecordBlock_InvalidBlockRoundByNonceStorer_ExpectError(t *testing.T) {
+func TestHistoryRepository_RecordBlock_InvalidBlockRoundByHashStorer_ExpectError(t *testing.T) {


too many _ in name. To align with the rest of the project, please rename this to:

TestHistoryRepository_RecordBlockInvalidBlockRoundByHashStorerExpectError

Renamed it to be consistent with the rest of the tests

iulianpascalau · 2021-09-29T07:24:51Z

storage/factory/pruningStorerFactory.go

-	createdStorers = append(createdStorers, blockRoundByNonceUnit)
-	chainStorer.AddStorer(dataRetriever.RoundNonceUnit, blockRoundByNonceUnit)
+	createdStorers = append(createdStorers, blockHashByRoundUnit)
+	chainStorer.AddStorer(hashByRoundUnitData, blockHashByRoundUnit)


Ok, one question here: why do we need a different value for blockHashByRoundUnit (according to the shard in which the node is operating) but we always open the same storer from the same path? This can be simplified by stating a hardcoded value in dataretrieve/interface.go for this unit type (not added to the shard ID value), something like 19 or 20 or something.

LucianMincu

System tests passed.

FEAT: Add initial version

6fed29c

mariusmihaic closed this Sep 9, 2021

mariusmihaic reopened this Sep 9, 2021

mariusmihaic closed this Sep 9, 2021

mariusmihaic reopened this Sep 9, 2021

mariusmihaic changed the title ~~FEAT: Add initial version~~ EN-10574 Get block by round Sep 9, 2021

mariusmihaic closed this Sep 9, 2021

mariusmihaic deleted the EN-10574-Get_block_by_round branch September 9, 2021 14:32

mariusmihaic restored the EN-10574-Get_block_by_round branch September 9, 2021 14:32

mariusmihaic reopened this Sep 9, 2021

mariusmihaic added 8 commits September 9, 2021 17:35

FEAT: Add GetBlockByRound in facade

47c9a57

FEAT: Add unit tests for routes

18edb17

FEAT: Add tests for shardBlock

227dd98

FEAT: Add tests for metaBlock

8b032f0

FEAT: Fix bug, add historyRepository test

afa8e65

FEAT: Add one more test in history repo

f33de7d

FEAT: Add tests for nodeBlocks

972e4ec

FIX: One more bug

0b79866

bogdan-rosianu marked this pull request as draft September 13, 2021 08:05

mariusmihaic added 2 commits September 13, 2021 11:44

FIX: Some bugs, add more tests

4e29d0e

FIX: One more bug

43568a1

mariusmihaic self-assigned this Sep 15, 2021

mariusmihaic marked this pull request as ready for review September 16, 2021 12:44

Merge branch 'development' into EN-10574-Get_block_by_round

2d620cc

miiu96 reviewed Sep 20, 2021

View reviewed changes

iulianpascalau self-requested a review September 22, 2021 09:58

iulianpascalau requested changes Sep 22, 2021

View reviewed changes

mariusmihaic added 2 commits September 27, 2021 13:39

Merge branch 'development' into EN-10574-Get_block_by_round

85edbfb

# Conflicts: # api/groups/blockGroup.go # api/groups/blockGroup_test.go # api/mock/facade.go # facade/disabled/disabledNodeFacade_test.go

FIX: Merge errors

23ff43b

mariusmihaic added 3 commits September 27, 2021 14:11

FIX: More errors

50b8a4e

FIX: Review findings

e4ae2bc

FEAT: Save block hash by round instead of block round by nonce

e952a15

mariusmihaic requested review from miiu96 and iulianpascalau September 28, 2021 12:21

iulianpascalau requested changes Sep 29, 2021

View reviewed changes

mariusmihaic added 2 commits September 29, 2021 11:42

Merge branch 'development' into EN-10574-Get_block_by_round

9a4e6b2

FIX: Review findings, storage units

196cc87

mariusmihaic requested a review from iulianpascalau September 29, 2021 12:04

iulianpascalau approved these changes Sep 29, 2021

View reviewed changes

miiu96 approved these changes Sep 30, 2021

View reviewed changes

mariusmihaic and others added 4 commits October 1, 2021 11:47

Merge branch 'development' into EN-10574-Get_block_by_round

711fd7b

Merge branch 'development' into EN-10574-Get_block_by_round

c96ffca

Merge branch 'development' into EN-10574-Get_block_by_round

15eee8a

Merge branch 'development' into EN-10574-Get_block_by_round

968815e

LucianMincu approved these changes Oct 8, 2021

View reviewed changes

LucianMincu merged commit cc555fb into development Oct 8, 2021

LucianMincu deleted the EN-10574-Get_block_by_round branch October 8, 2021 10:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EN-10574 Get block by round #3430

EN-10574 Get block by round #3430

mariusmihaic commented Sep 9, 2021 •

edited

codecov bot commented Sep 10, 2021 •

edited

miiu96 Sep 20, 2021

iulianpascalau Sep 22, 2021

mariusmihaic Sep 27, 2021

miiu96 Sep 20, 2021

mariusmihaic Sep 27, 2021

iulianpascalau Sep 22, 2021

iulianpascalau Sep 22, 2021

iulianpascalau Sep 29, 2021

iulianpascalau Sep 29, 2021

mariusmihaic Sep 29, 2021

iulianpascalau Sep 29, 2021

LucianMincu left a comment

EN-10574 Get block by round #3430

EN-10574 Get block by round #3430

Conversation

mariusmihaic commented Sep 9, 2021 • edited

codecov bot commented Sep 10, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LucianMincu left a comment

Choose a reason for hiding this comment

mariusmihaic commented Sep 9, 2021 •

edited

codecov bot commented Sep 10, 2021 •

edited