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

Ask a question about incrementProposerPriority func #3042

Closed
james-ray opened this issue Dec 18, 2018 · 5 comments
Closed

Ask a question about incrementProposerPriority func #3042

james-ray opened this issue Dec 18, 2018 · 5 comments
Labels
C:consensus Component: Consensus T:question Type: Question

Comments

@james-ray
Copy link
Contributor

james-ray commented Dec 18, 2018

func (vals *ValidatorSet) incrementProposerPriority(subAvg bool) *Validator {
	for _, val := range vals.Validators {
		// Check for overflow for sum.
		val.ProposerPriority = safeAddClip(val.ProposerPriority, val.VotingPower)
	}

	validatorsHeap := cmn.NewHeap()
	if subAvg { // shift by avg ProposerPriority
		vals.shiftByAvgProposerPriority(validatorsHeap)
	} else { // just update the heap
		for _, val := range vals.Validators {
			validatorsHeap.PushComparable(val, proposerPriorityComparable{val})
		}
	}

	// Decrement the validator with most ProposerPriority:
	mostest := validatorsHeap.Peek().(*Validator)
	// mind underflow
	mostest.ProposerPriority = safeSubClip(mostest.ProposerPriority, vals.TotalVotingPower())

	return mostest
}

Before, we just let the mostest sub totalVotingPower. Now, we do one more step shiftByAvgProposerPriority. In this func, each validator sub avgPower. So I think the whole validatorSet sub twice totalPower.

Does that make sense? It will become negative and smaller and smaller.
Am I misunderstanding something? Please, anybody, tell me? Thanks!

@melekes melekes added T:question Type: Question C:consensus Component: Consensus labels Dec 18, 2018
@james-ray
Copy link
Contributor Author

james-ray commented Dec 18, 2018

I have looked into #2718 and #2785 . not fully understood.. still trying to get the answer.

Seems like the totalPower can be substracted to negative, so the avg can be negative too, so sub the avgPower will not lead to become smaller and smaller but close to 0.

@james-ray james-ray reopened this Dec 29, 2018
@james-ray
Copy link
Contributor Author

james-ray commented Dec 29, 2018

In this func IncrementProposerPriority:

``
func (vals *ValidatorSet) IncrementProposerPriority(times int) {
if times <= 0 {
panic("Cannot call IncrementProposerPriority with non-positive times")
}

const shiftEveryNthIter = 10
var proposer *Validator
// call IncrementProposerPriority(1) times times:
for i := 0; i < times; i++ {
	shiftByAvgProposerPriority := i%shiftEveryNthIter == 0
	proposer = vals.incrementProposerPriority(shiftByAvgProposerPriority)
}
isShiftedAvgOnLastIter := (times-1)%shiftEveryNthIter == 0
if !isShiftedAvgOnLastIter {
	validatorsHeap := cmn.NewHeap()
	vals.shiftByAvgProposerPriority(validatorsHeap)
}
vals.Proposer = proposer

}
``
we have a local const shiftEveryNthIter=10, How about the consensus goes well and we finish each height at round 0, so the times here is always 1, so the shiftByAvgProposerPriority will not get executed because isShiftedAvgOnLastIter := (times-1)%shiftEveryNthIter == 0 is true.

I sugguest we can add a variable incrCount in validatorSet, we increase the count each time, and execute shiftByAvgProposerPriority when it reaches 10, then reset it to 0 again.

@melekes
Copy link
Contributor

melekes commented Feb 14, 2019

Proposer selection algorithm has undergone a few changes in the past releases. @james-ray do you still have questions? Have you looked at the new implementation and formal specification?

@ebuchman
Copy link
Contributor

@james-ray can we close this?

@james-ray
Copy link
Contributor Author

Yes, I see the code there is completely different now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:consensus Component: Consensus T:question Type: Question
Projects
None yet
Development

No branches or pull requests

3 participants