Skip to content

Add 5th period for protocol "Adoption" and reduce voting period to 5 cycle

Tomáš Zemanovič requested to merge tomas/adoption-period into proto-proposal

This MR can be split in multiple part:

Feature : Add a fifth voting period "adoption"

agora post why it's needed

In order to allow the developer/baker time to adapt their code/infrastructure when a new protocol have been voted an new voting period "Adoption" is introduce.

implements #26 (closed)

commits :

b2559ccd * Proto: add the 5th period - Activation_delay ef33e568 * Client: Update for the new period 1616d1a7 * Tests/OCaml: update voting tests ea531b5b * Tests/Sandbox: update for Activation_delay period 0445d061 * Doc: Update voting section with the new 5th period

Issue

voting period is dependent of the first version of proto alpha

Because the voting period index is monotonic from the first block of the first version of proto alpha, the index and voting period position are currently determined from the level since the first protocol after genesis:

(* level of first protocol activated after genesis; 001 in mainnet
    and alpha in sandbox.
    This value is stored in the context
    *)
let first_level

(* level_position = current_level - 1 in mainnet *)
let level_position = current_level - first_level

let voting_period = level_position / blocks_per_voting_period

let voting_period_position = level_position mod blocks_per_voting_period

Changing the value of blocks_per_voting_period would break the monotonic of voting_period index, which means that there would be a jump from e.g. voting period index 50 to 70 when the constant is reduced.

kind is changed on the last block whereas the index changes on the first block

The voting period changes when the last block of a voting period is finalized with this call (in apply.finalize_application):

let last_of_a_voting_period ctxt l =
  Compare.Int32.(
    Int32.succ l.Level.voting_period_position
    = Constants.blocks_per_voting_period ctxt)

let may_start_new_voting_period ctxt =
  let level = Level.current ctxt in
  if last_of_a_voting_period ctxt level then start_new_voting_period ctxt
  else return ctxt

And the voting period index is calculated from the current block level, resulting in the index being only incremented on the first block of a new voting period (see calc above).

This introduced an issue where on the last block of the voting period we change period kind but it's only on the next block that the index is incremented.

a detailed example of that issue can be found here

proposed fix

The approach taken here to tackle all the previously mentioned issues is to decouple the voting period from level and store the extra information about the current voting period.

We store in Storage.Votes.current_period the current index, kind and starting position.

type voting_period = {
    index : int32; (* index since the first voting period of the first activated protocol*)
    kind : kind; (* kind of the voting period :Proposal, ..., Adoption *)
    start_position : Int32.t (* level at witch this voting period started *)
    }

The start_position needs to be saved or else we would rely on level and blocks_per_voting_period to calculate the voting period position and fall in the same problem as for the index monotonicity.

While there may be simpler approach taken in terms of brevity, this patch allows easier further changes (such as !109) and fixes the offset issues by changing the voting period index together with the voting period kind together.

Because these 2 issue are heavily inter-dependent it was not possible to separate the commit for each issue :

commits :

  • 1f70b89c * proto: re-order Voting_period module
  • 02205444 * proto: replace Voting.t by Voting.Index.t
  • 0cb54930 * Proto: Update Voting_period type, storage and module interface
  • 8ca54e84 * Proto: Remove voting period fields from Level.t type
  • 4d93e618 * Proto: Update the amendment to use the new Voting_period interface
  • f68180f5 * Proto/RPC: Add endpoint for the current voting period
  • 9bdd07b1 * Proto/RPC: Update the helpers endpoint for compatibility
  • f81e8292 * Migration: Migrate to new Voting_period storage
  • 0584ed7f * Baker|bench: Remove unnecessary import
  • aae1481c * Client: Use explicit voting period offsets
  • 5d33ebaf * Client: Use the new voting services RPC endpoint for current period
  • 6afe2090 * Proto/Test: Update tests Voting_period usages
  • 6e50d3e0 * proto/test: verify new duration for period logic
  • 6f66ba49 * Test/Python: Update tests Voting_period usages
  • cd0ac3bd * tezt/encoding: update regression test

Issue : The output context is ready for creation of the next block

The context to apply operation is prepared on finalization of the previous block. This is problematic for the voting period because that means that we need to change voting period on the last block of a voting period. The context of the last block of a voting period will thus contain the voting period of the next block.

For example the returned context of the last block of a voting period at level n is :

voting_period : {
    index = i;
    kind = X;
    start_position = n + 1;
}

If we query with the RPC the voting period position and remaining blocks information, we will obtain (see above calc):

position  = -1
remaining = blocks_per_voting_cycle

Proposed fix

The desired behaviour would be to have in storage the voting period that just finished. A proper way to fix that would be to prepare the correct context when starting the creation of a block (i.e. before starting to apply operation), but this should be carefully done, because the voting period listing depends on the rolls and it might break some invariant.

Instead, a fix to have the correct value in the RPC is implemented in this MR. The value is calculated with the function voting_period_storage.get_rpc_fixed_current_info. This will allow to fix this issue in a future proposal without breaking the RPC for the user.

commits:

  • 4785cdeb * proto/RPC: fix offset -1 in voting period service

Feature Reduce size of voting_period duration to 5 cycle

Because of the above change change updating the parameter blocks_per_voting_period is easy and has no side-effect

commits:

  • 658c0ac6 * Migration: Migrate to new blocks_per_voting_period constant
  • 2577ea08 * Parameters: change the default blocks_factor_per_voting

fix #109 (closed)

Edited by Adrian Brink

Merge request reports