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

consensus epoch number fix #1832

Merged
merged 17 commits into from
Jun 1, 2020

Conversation

raduchis
Copy link
Contributor

@raduchis raduchis commented May 31, 2020

computation of the consensus group for the start of epoch block was computed with the current epoch not the previous epoch

  • fixed consensus group computation for start of epoch with previous epoch both on shards and meta
  • added unit tests for UpdateState method

@raduchis raduchis marked this pull request as ready for review June 1, 2020 10:56
@AdoAdoAdo AdoAdoAdo self-requested a review June 1, 2020 14:07
@SebastianMarian SebastianMarian self-requested a review June 1, 2020 14:07
AdoAdoAdo
AdoAdoAdo previously approved these changes Jun 1, 2020
}

// DecreaseLeaderSuccessRate -
func (p *PeerAccountHandlerMock) DecreaseLeaderSuccessRate(val uint32) {
if p.DecreaseLeaderSuccessRateCalled != nil {
p.DecreaseLeaderSuccessRateCalled(val)
}
p.DecreaseLeaderSuccessRateValue += val
}

// IncreaseValidatorSuccessRate -
func (p *PeerAccountHandlerMock) IncreaseValidatorSuccessRate(val uint32) {
if p.IncreaseValidatorSuccessRateCalled != nil {
p.IncreaseValidatorSuccessRateCalled(val)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be added return ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, no need for the += afterwards

}
p.IncreaseLeaderSuccessRateValue += val
}

// DecreaseLeaderSuccessRate -
func (p *PeerAccountHandlerMock) DecreaseLeaderSuccessRate(val uint32) {
if p.DecreaseLeaderSuccessRateCalled != nil {
p.DecreaseLeaderSuccessRateCalled(val)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be added return ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}

// IncreaseValidatorSuccessRate -
func (p *PeerAccountHandlerMock) IncreaseValidatorSuccessRate(val uint32) {
if p.IncreaseValidatorSuccessRateCalled != nil {
p.IncreaseValidatorSuccessRateCalled(val)
}
p.IncreaseValidatorSuccessRateValue += val
}

// DecreaseValidatorSuccessRate -
func (p *PeerAccountHandlerMock) DecreaseValidatorSuccessRate(val uint32) {
if p.DecreaseValidatorSuccessRateCalled != nil {
p.DecreaseValidatorSuccessRateCalled(val)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be added return ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

// TODO: change if start of epoch block needs to be validated by the new epoch nodes
// previous block was proposed by the consensus group of the previous epoch
epoch := header.GetEpoch()
if (header.IsStartOfEpochBlock()) && epoch > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for open-close ()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

if !ok {
return fmt.Errorf("%w - updateShardDataPeerState header from cache - hash: %s, round: %v, nonce: %v",
process.ErrMissingHeader,
hex.EncodeToString(h.GetPrevHash()),
Copy link
Contributor

Choose a reason for hiding this comment

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

h.HeaderHash ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -596,6 +598,535 @@ func TestValidatorStatisticsProcessor_UpdatePeerStateCallsIncrease(t *testing.T)
assert.True(t, increaseValidatorCalled)
}

func TestValidatorStatisticsProcessor_UpdatePeerState_IncreasesConsensusPreviousMetaBlock_SameEpoch(t *testing.T) {
t.Parallel()
Copy link
Contributor

Choose a reason for hiding this comment

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

Add empty line after each t.Parallel() in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

AdoAdoAdo
AdoAdoAdo previously approved these changes Jun 1, 2020
AdoAdoAdo
AdoAdoAdo previously approved these changes Jun 1, 2020
SebastianMarian
SebastianMarian previously approved these changes Jun 1, 2020
@raduchis raduchis changed the base branch from development to release-candidate June 1, 2020 15:36
@raduchis raduchis dismissed stale reviews from SebastianMarian and AdoAdoAdo June 1, 2020 15:36

The base branch was changed.

Copy link
Contributor

@LucianMincu LucianMincu left a comment

Choose a reason for hiding this comment

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

System tests passed.

@LucianMincu LucianMincu merged commit d199354 into release-candidate Jun 1, 2020
@LucianMincu LucianMincu deleted the EN-6511-consensus-epoch-incorrect branch June 1, 2020 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants