IF: Update new proposer policy algorithm

[heifner]

set_proposed_producers[_ex] host functions after Savanna activation check to see if the policy to be proposed is different than the last proposal policy in the queue (not the currently active one). If it is the same, it avoids adding the policy to the queue (it acts like a no-op).

The host functions now return uint32_t max as the version number if the proposed policy is valid. If it is invalid (e.g. empty producers in the policy) it returns -1; this is the only time is returns -1. This means that if the caller of the host function proposes a policy that is identical to the one it last proposed, the host function will still return successfully (not with a -1 return value, but with a uint32_t max).

The diff in the block extension is relative to the previous policy proposed and represented in a block extension (as a diff) in the history of the chain. (diff is begin worked under #5).

Resolves #6

[heifner]

It seems inconsistent. I'm not quite understanding why we used proposer_policies.end()) before?

That was an error, corrected in this PR.

[heifner]

Are we returning a policy which will become active later? If that's the case, even if the one proposed in the current block is identical, shouldn't we take that one into account rather than making it a no-op?

The same block is handled in the set_proposed_producers method.

[heifner]

Will if (itr != parent.proposer_policies.begin()) { ever be true? What are the cases where we have more than one entry in this map?

round A [1,2,..12], next_round B [1,2,..12], next_next_round C [1,2,..12], D [1,2,..12]

If set in A1, A2, .. A12 becomes active in C1
If set in B1, B2, .. B12 becomes active in D1

e.g. If one is set in A2 and another in B2 then there can be 2 in the map at B3.

[heifner]

Added additional test cases and added a fix. Ultimately, it is rather complicated to calculate what the version will be ahead of time. If you can think of additional test cases please let me know.

Change to algorithm.

[greg7mdp]

No need to change if you don't want to, but I kinda like this more functional style where the lambda returns the new proposer_policy (if any) , and we use the return value directly without an intermediate variable.

         // Any proposer policy?
         auto process_new_proposer_policy = [&](auto&) -> std::unique_ptr<proposer_policy> {
            std::unique_ptr<proposer_policy> new_proposer_policy;
            const auto& gpo = db.get<global_property_object>();
            if (gpo.proposed_schedule_block_num) {

               auto [version, should_propose] = pending->get_next_proposer_schedule_version(gpo.proposed_schedule.producers);
               if (should_propose) {
                  new_proposer_policy                    = std::make_unique<proposer_policy>();
                  new_proposer_policy->active_time       = detail::get_next_next_round_block_time(bb.timestamp());
                  new_proposer_policy->proposer_schedule = producer_authority_schedule::from_shared(gpo.proposed_schedule);
                  new_proposer_policy->proposer_schedule.version = version;
                  ilog("Scheduling proposer schedule change at ${t}: ${s}",
                       ("t", new_proposer_policy->active_time)("s", new_proposer_policy->proposer_schedule));
               }

               db.modify( gpo, [&]( auto& gp ) {
                  gp.proposed_schedule_block_num = std::optional<block_num_type>();
                  gp.proposed_schedule.version = 0;
                  gp.proposed_schedule.producers.clear();
               });
            }
            return new_proposer_policy;
         };

         auto assembled_block =
            bb.assemble_block(thread_pool.get_executor(), protocol_features.get_protocol_feature_set(), fork_db,
                              apply_s<std::unique_ptr<proposer_policy>>(chain_head, process_new_proposer_policy),
                              validating, std::move(validating_qc_data), validating_bsp);

[greg7mdp]

This could return std::optional<uint32_t>, i.e. return a new version if one is needed.

[ericpassmore]

Note:start
group: IF
category: INTERNALS
summary: Avoid adding new proposer policy if there is no change.
Note:end