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

BEP-341: Validators can produce consecutive blocks #2482

Merged

Conversation

NathanBSC
Copy link
Contributor

@NathanBSC NathanBSC commented May 20, 2024

Description

BEP-341: Validators can produce consecutive blocks
related change in system contracts

Rationale

tell us why we need these changes...

Example

add an example CLI or API response...

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

@NathanBSC NathanBSC changed the base branch from master to develop May 20, 2024 11:01
@NathanBSC NathanBSC force-pushed the BEP-341-produce-consecutive-blocks branch 8 times, most recently from 3193cf0 to 6f8423c Compare May 22, 2024 11:38
@buddh0 buddh0 changed the title BEP-341: Validators can produce consecutive blocks feat: make validators to produce consecutive blocks defined by BEP-341 May 22, 2024
@NathanBSC NathanBSC force-pushed the BEP-341-produce-consecutive-blocks branch 14 times, most recently from 417a19f to bb16185 Compare May 30, 2024 03:53
@buddh0 buddh0 marked this pull request as ready for review May 31, 2024 09:06
@buddh0 buddh0 force-pushed the BEP-341-produce-consecutive-blocks branch 3 times, most recently from e705707 to e1723e1 Compare June 4, 2024 11:28
consensus/parlia/parlia.go Show resolved Hide resolved
consensus/parlia/parlia.go Outdated Show resolved Hide resolved
consensus/parlia/parlia.go Outdated Show resolved Hide resolved
consensus/parlia/snapshot.go Outdated Show resolved Hide resolved
@bnb-chain bnb-chain deleted a comment from NathanBSC Jun 5, 2024
@NathanBSC NathanBSC force-pushed the BEP-341-produce-consecutive-blocks branch from 356d364 to f513d20 Compare July 8, 2024 04:20
@NathanBSC NathanBSC force-pushed the BEP-341-produce-consecutive-blocks branch from 9f663bf to 6589d44 Compare July 12, 2024 08:36
galaio
galaio previously approved these changes Jul 22, 2024
}
snap.Recents[number] = validator
snap.RecentForkHashes[number] = hex.EncodeToString(header.Extra[extraVanity-nextForkHashSize : extraVanity])
snap.updateAttestation(header, chainConfig, s.config)
// change validator set
if number > 0 && number%s.config.Epoch == uint64(len(snap.Validators)/2) {
if number > 0 && number%s.config.Epoch == snap.minerHistoryCheckLen() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not snap.minerHistoryCheckLen() + 1 to change validator set?

Copy link
Collaborator

Choose a reason for hiding this comment

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

why not snap.minerHistoryCheckLen() + 1 to change validator set?

when turnLenght==1, snap.minerHistoryCheckLen() == uint64(len(snap.Validators)/2), exactly the same as before.

why not +1 here?
for example when turnLenght ==1,
[200, 210] contains 11 validators, is a majority of validators, so can do the change at 210, not wait to 211.

snap.Validators = newVals
if chainConfig.IsLuban(header.Number) {
validators := snap.validators()
for idx, val := range validators {
snap.Validators[val].Index = idx + 1 // offset by 1
}
}
for i := snap.versionHistoryCheckLen(); i < oldVersionsLen; i++ {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this apply(...) is not code review friendly...

Copy link
Collaborator

@buddh0 buddh0 Jul 23, 2024

Choose a reason for hiding this comment

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

we can compare to the old logic about deleting RecentForkHashes,
the effect is the same as before, but the code is much more simplified

}

// inturnValidator returns the validator at a given block height.
func (s *Snapshot) inturnValidator() common.Address {
validators := s.validators()
offset := (s.Number + 1) % uint64(len(validators))
offset := (s.Number + 1) / uint64(s.TurnLength) % uint64(len(validators))
Copy link
Collaborator

Choose a reason for hiding this comment

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

(s.Number + s.TurnLength) / uint64(s.TurnLength)?

Copy link
Collaborator

@buddh0 buddh0 Jul 23, 2024

Choose a reason for hiding this comment

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

(s.Number + s.TurnLength) / uint64(s.TurnLength)?

when at height s.Number + 1 , we can get snapshot with s.Number.
the function name is not quite clear. I will update the comments for the function

Copy link
Collaborator

Choose a reason for hiding this comment

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

(s.Number + s.TurnLength) == (s.Number + 1 + (s.TurnLength -1))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(s.Number + s.TurnLength) == (s.Number + 1 + (s.TurnLength -1))

not enable the first validator after switch go generate s.TurnLength blocks

@unclezoro unclezoro merged commit 17e0e45 into bnb-chain:develop Jul 23, 2024
7 checks passed
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.

8 participants