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

Valset tie breaking 2 #91

Closed
wants to merge 14 commits into from
Closed

Valset tie breaking 2 #91

wants to merge 14 commits into from

Conversation

maurolacy
Copy link
Contributor

@maurolacy maurolacy commented Feb 9, 2022

Closes #22.

Alternative to #81: Uses an extra index for tie-breaking.

Publishing as Draft, as this is WIP / will be finished / merged later.

@hashedone
Copy link
Collaborator

You asked if it is not too much for this simple issue. To me - not. Actually you store only one additional u64 (height) per entry in state, so it is not heavy. Also it doesn't get complicated to me. The problem I see is that we have naming colision, and solving them by adding 2 is always bad idea, so maybe it still needs reconcideration of naming.

However - I see that this is not a change you introduced, there was it alredy in repo... I dislike it, but it seems to be another issue.

@maurolacy
Copy link
Contributor Author

maurolacy commented Feb 15, 2022

You asked if it is not too much for this simple issue. To me - not. Actually you store only one additional u64 (height) per entry in state, so it is not heavy. Also it doesn't get complicated to me. The problem I see is that we have naming colision, and solving them by adding 2 is always bad idea, so maybe it still needs reconcideration of naming.

However - I see that this is not a change you introduced, there was it alredy in repo... I dislike it, but it seems to be another issue.

Thanks for the feedback. I did that 2 naming in a hurry, will change it.

That's a minor thing. What I don't like about this is that it introduces a new function list_members_by_points_tie_breaking / query msg ListMembersByPointsTieBreaking. Perhaps removing that from tg4, and having it as a free-standing helper function / query?

@maurolacy
Copy link
Contributor Author

Closing in favour of #118.

@maurolacy maurolacy closed this Mar 9, 2022
@ueco-jb ueco-jb deleted the 22-valset-tie-breaking-2 branch August 30, 2022 13:47
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.

valset: fair rule to determine active valset when equal EP/Stake
2 participants