Feat/two keepers #90
Feat/two keepers #90
Conversation
| needed_keepers = math.ceil( | ||
| self.synchronized_data.nb_participants / 2 | ||
| ) # half or 1 | ||
|
|
||
| # Check if we need random selection | ||
| if len(relevant_set) <= needed_keepers: | ||
| keeper_addresses = list(relevant_set) | ||
| self.context.logger.info(f"Selected new keepers: {keeper_addresses}.") | ||
| return keeper_addresses | ||
|
|
||
| # Random seeding and shuffling of the set | ||
| random.seed(self.synchronized_data.keeper_randomness) | ||
| random.shuffle(relevant_set) | ||
|
|
||
| # If the keeper is not set yet, pick the first address | ||
| keeper_addresses = relevant_set[0:2] | ||
|
|
||
| # If the keepers have been already set, select the next ones. | ||
| if ( | ||
| self.synchronized_data.are_keepers_set | ||
| and len(self.synchronized_data.participants) > 2 | ||
| ): | ||
| old_keeper_index = relevant_set.index( | ||
| self.synchronized_data.most_voted_keeper_addresses[0] | ||
| ) | ||
| keeper_addresses = [ | ||
| relevant_set[ | ||
| (old_keeper_index + 2) % len(relevant_set) | ||
| ], # skip the previous 2 | ||
| relevant_set[(old_keeper_index + 3) % len(relevant_set)], | ||
| ] |
There was a problem hiding this comment.
This is the part I've modified
| keeper_addresses = relevant_set[0:2] | ||
|
|
||
| # If the keepers have been already set, select the next ones. | ||
| if ( | ||
| self.synchronized_data.are_keepers_set | ||
| and len(self.synchronized_data.participants) > 2 | ||
| ): | ||
| old_keeper_index = relevant_set.index( | ||
| self.synchronized_data.most_voted_keeper_addresses[0] | ||
| ) | ||
| keeper_addresses = [ | ||
| relevant_set[ | ||
| (old_keeper_index + 2) % len(relevant_set) | ||
| ], # skip the previous 2 | ||
| relevant_set[(old_keeper_index + 3) % len(relevant_set)], | ||
| ] |
There was a problem hiding this comment.
This looks a bit odd to me, we're slicing the keepers from the set of relevant keepers and after that we are checking if the keepers are set and using the index of the existing keepers to select the next ones. The problem here is that we're shuffling the keepers which means the relevant_set is none deterministic and we cannot use indexes to select different keepers. I would suggest we loop through the relevant set and select keepers which are not in self.synchronized_data.most_voted_keeper_addresses
There was a problem hiding this comment.
This follows the current implementation for single keeper selection, which is tested code, but I'd like to have @Adamantios opinion as well since he implemented the original code IIRC.
There was a problem hiding this comment.
This is deterministic across the agents because they are using the same seed when shuffling.
This was implemented in this manner to ensure that we consistently choose all the agents as keepers.
Proposed changes
Fixes
n/a
Types of changes
What types of changes does your code introduce? (A breaking change is a fix or feature that would cause existing functionality and APIs to not work as expected.)
Put an
xin the box that appliesChecklist
Put an
xin the boxes that apply.mainbranch (left side). Also you should start your branch off ourmain.Further comments
n/a