Skip to content

Conversation

@heifner
Copy link
Contributor

@heifner heifner commented Apr 24, 2024

  • Avoid adding new proposer policy if there is no change.
  • Returns the same version from host function if set proposer policy is ignored because there is no diff from the active proposer policy or the next proposer policy.

Resolves #6

@heifner heifner requested review from greg7mdp and linh2931 April 24, 2024 22:31
@heifner heifner added the OCI Work exclusive to OCI team label Apr 24, 2024
@heifner heifner linked an issue Apr 24, 2024 that may be closed by this pull request
if (auto itr = parent.proposer_policies.find(active_time); itr != parent.proposer_policies.cend()) {
return itr->second->proposer_schedule.version; // will replace so return same version
// returns the next proposer schedule version and true if different
// if producers is not different then returns the current schedule version (or next schedule version)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the leading spaces.


const producer_authority_schedule& lhs = get_next_sched();

if (std::ranges::equal(lhs.producers, producers)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, any reason why you use std::ranges::equal() instead of operator==() on the vectors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only because that is what legacy does. Should be the same.

@heifner heifner merged commit 29396aa into savanna Apr 25, 2024
@heifner heifner deleted the GH-6-proposer-policy branch April 25, 2024 19:43
@greg7mdp
Copy link
Contributor

greg7mdp commented Apr 25, 2024

Hum, I kept looking at it some more and I don't think it behaves as the previous code did.
For example, if the proposed one active_time is the same as one already proposed, we compare with the previously proposed one in the block ( return (--itr)->second->proposer_schedule;).
Suppose the comparison shows they are the same, then we do the quick exit:

if (!diff)
      return version;

and we don't remove any existing proposed_schedule set earlier in this block (with db.modify()).

So am I correct that we have not removed the one set earlier in the block, and should have been superseded by the new one which is identical to the previously active one?

@heifner heifner restored the GH-6-proposer-policy branch April 25, 2024 20:01
@heifner heifner deleted the GH-6-proposer-policy branch April 25, 2024 20:08
@ericpassmore
Copy link
Contributor

Note:start
group: IF
category: INTERNALS
summary: Check and test to ensure proposer policy is only set when there is a change.
Note:end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCI Work exclusive to OCI team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid adding new proposer policy if there is no change

5 participants