Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong genesis block save #2191

Merged
merged 17 commits into from
Aug 7, 2020
Merged

Conversation

iulianpascalau
Copy link
Contributor

@iulianpascalau iulianpascalau commented Jul 31, 2020

  • removed a wrong save of the genesis block in hash-nonce storage

Manual testing procedure:

  • Start a testnet with nodes that do shuffling between shards.
  • Without the fix, we are able to see an error message containing the string shard header (nonce-hash) after a few epochs.
  • With the fix, the error message will no longer be logged.

@@ -577,11 +577,6 @@ func prepareGenesisBlock(args *processComponentsFactoryArgs, genesisBlocks map[u
if errNotCritical != nil {
log.Error("error storing genesis shardblock", "error", errNotCritical.Error())
}
hdrNonceHashDataUnit := dataRetriever.ShardHdrNonceHashDataUnit + dataRetriever.UnitType(genesisBlock.GetShardID())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this storage exists in each shard but only for self shard. In metachain it is created for every shard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we do not need the shard genesis block stored inside the nonce-hash unit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, in this case you can move line 565 inside first if..else statement after line 570

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

@AdoAdoAdo AdoAdoAdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add for each manual testing scenario the following:

a) Prerequisites
b) Test steps
c) Expected results
d) Actual results

If there are parameters involved in testing what are the parameters boundaries and the expected results and actual results for testing inside boundaries, on boundaries and outside boundaries.

AdoAdoAdo
AdoAdoAdo previously approved these changes Aug 7, 2020
sasurobert
sasurobert previously approved these changes Aug 7, 2020
@iulianpascalau iulianpascalau changed the base branch from master to development August 7, 2020 14:06
@iulianpascalau iulianpascalau dismissed stale reviews from sasurobert and AdoAdoAdo August 7, 2020 14:06

The base branch was changed.

@LucianMincu LucianMincu merged commit eec6285 into development Aug 7, 2020
@LucianMincu LucianMincu deleted the fix-wrong-genesis-saving branch August 7, 2020 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants