switch to current block randomness for ordering transactions #5683
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## rc/v1.7.0 #5683 +/- ##
=============================================
+ Coverage 80.31% 80.35% +0.04%
=============================================
Files 708 709 +1
Lines 93961 93973 +12
=============================================
+ Hits 75462 75513 +51
+ Misses 13219 13181 -38
+ Partials 5280 5279 -1 ☔ View full report in Codecov by Sentry. |
process/block/metablock.go
Outdated
| @@ -928,7 +928,12 @@ func (mp *metaProcessor) createBlockBody(metaBlock data.HeaderHandler, haveTime | |||
| "nonce", metaBlock.GetNonce(), | |||
| ) | |||
|
|
|||
| miniBlocks, err := mp.createMiniBlocks(haveTime, metaBlock.GetPrevRandSeed()) | |||
| randomness := metaBlock.GetPrevRandSeed() | |||
| if mp.enableEpochsHandler.IsFlagEnabled(common.CurrentRandomnessOnSortingFlag) { | |||
There was a problem hiding this comment.
L932-L934 can be a common function in baseProcess.go, something like:
func (bp *baseProcessor) computeRandomnessForTxSorting(hdr HeaderHandler) []byte {
if bp.enableEpochsHandler.IsFlagEnabled(common.CurrentRandomnessOnSortingFlag){
return hdr.GetRandSeed()
}
return hdr.GetPrevRandSeed()
}this function can be then reused in shardblock.go
Or better, an exported "helper" function in the process/block package so it can be reused in preprocess/transactions.go
There was a problem hiding this comment.
added a helper function
| @@ -683,6 +683,12 @@ func (handler *enableEpochsHandler) createAllFlagsMap() { | |||
| }, | |||
| activationEpoch: handler.enableEpochsConfig.NFTStopCreateEnableEpoch, | |||
| }, | |||
| common.CurrentRandomnessOnSortingFlag: { | |||
There was a problem hiding this comment.
missing test asserts in tomlConfig_test.go
missing tests in enableEpochsHandler_test.go
process/block/metablock.go
Outdated
| @@ -928,7 +928,12 @@ func (mp *metaProcessor) createBlockBody(metaBlock data.HeaderHandler, haveTime | |||
| "nonce", metaBlock.GetNonce(), | |||
| ) | |||
|
|
|||
| miniBlocks, err := mp.createMiniBlocks(haveTime, metaBlock.GetPrevRandSeed()) | |||
| randomness := metaBlock.GetPrevRandSeed() | |||
There was a problem hiding this comment.
missing flag definition in checkProcessorParameters, L518
| @@ -332,7 +332,11 @@ func (txs *transactions) ProcessBlockTransactions( | |||
| } | |||
|
|
|||
| if txs.isBodyFromMe(body) { | |||
| return txs.processTxsFromMe(body, haveTime, header.GetPrevRandSeed()) | |||
| randomness := header.GetPrevRandSeed() | |||
There was a problem hiding this comment.
missing flag definition in checkProcessorParameters, L143
| @@ -0,0 +1,15 @@ | |||
| package helpers | |||
There was a problem hiding this comment.
this file could have been placed directly on process/block package so we could have avoid the helper package
There was a problem hiding this comment.
since this needs to be imported from different packages, it's better to be in a separate package to avoid circular dependencies
config/tomlConfig_test.go
Outdated
| @@ -824,6 +824,9 @@ func TestEnableEpochConfig(t *testing.T) { | |||
| # NFTStopCreateEnableEpoch represents the epoch when NFT stop create feature is enabled | |||
| NFTStopCreateEnableEpoch = 89 | |||
|
|
|||
| # CurrentRandomnessOnSortingEnableEpoch represents the epoch when the current randomness on sorting is enabled | |||
There was a problem hiding this comment.
spaces vs tab, fixed
There was a problem hiding this comment.
same issue for the comment
| @@ -26,6 +27,8 @@ func TestExecuteBlocksWithTransactionsAndCheckRewards(t *testing.T) { | |||
| t.Skip("this is not a short test") | |||
| } | |||
|
|
|||
| _ = logger.SetLogLevel("process:TRACE") | |||
There was a problem hiding this comment.
is this still needed?
# Conflicts: # cmd/node/config/enableEpochs.toml # common/constants.go # common/enablers/enableEpochsHandler.go # common/enablers/enableEpochsHandler_test.go # config/epochConfig.go # config/tomlConfig_test.go
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?