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

Add prioritized validators. Filter prioritized validators when updating validator set. #15

Merged
merged 6 commits into from
Sep 22, 2022

Conversation

nxqbao
Copy link
Contributor

@nxqbao nxqbao commented Sep 15, 2022

Description

  • Add prioritized validators
  • Add functions to update priority threshold, prioritized validators
  • Filter prioritized validators when updating the validator set: prioritized validators are picked first (up to the priority threshold); the remaining prioritized and non-prioritized ones are picked after
  • Add test case

TODO

  • Double check the case: the remaining prioritized and non-prioritized ones are picked by the highest-to-lowest voting power
  • Measuring gas

Checklist

  • I have clearly commented on all the main functions followed the NatSpec Format
  • The box that allows repo maintainers to update this PR is checked
  • I tested locally to make sure this feature/fix works

@nxqbao nxqbao marked this pull request as ready for review September 15, 2022 15:15
Copy link
Collaborator

@phuctd95 phuctd95 left a comment

Choose a reason for hiding this comment

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

Replace setPrioritizedValidators function with add/remove functions. Each time, a validator is added to or removed from the whitelist.

The _arrangeValidatorCandidates function can be optimized. We only need 1 for loop. (We do not need to fix it now. I only put a note here, let's do this later).

contracts/ronin-validator/RoninValidatorSet.sol Outdated Show resolved Hide resolved
contracts/ronin-validator/RoninValidatorSet.sol Outdated Show resolved Hide resolved
contracts/ronin-validator/RoninValidatorSet.sol Outdated Show resolved Hide resolved
contracts/ronin-validator/RoninValidatorSet.sol Outdated Show resolved Hide resolved
@nxqbao nxqbao force-pushed the feat/whitelisting-validators branch from 97dbb30 to 0db9b0f Compare September 21, 2022 09:44
contracts/mocks/MockRoninValidatorSetArrangeValidators.sol Outdated Show resolved Hide resolved
contracts/interfaces/IRoninValidatorSet.sol Outdated Show resolved Hide resolved
contracts/libraries/Math.sol Outdated Show resolved Hide resolved
contracts/validator/RoninValidatorSet.sol Outdated Show resolved Hide resolved
Comment on lines 453 to 458
function _arrangeValidatorCandidates(address[] memory _candidates, uint _newValidatorCount)
internal
view
returns (address[] memory _arrangedValidators)
{
_arrangedValidators = new address[](_newValidatorCount);
Copy link
Contributor

Choose a reason for hiding this comment

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

The array _candidates is a memory variable, we may want to remove _arrangedValidators for gas saving.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can increase the weight for the whitelisted address (e.g adding to a fixed number)

@nxqbao nxqbao merged commit 55a02b3 into main Sep 22, 2022
@nxqbao nxqbao deleted the feat/whitelisting-validators branch September 22, 2022 04:39
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.

3 participants