Framing e2e criteria

[l-monninger]

Summary

As an eventual follow-on to #2 and #14, the notion of e2e criteria was given additional structure under #64. However, it still somewhat unclear why we are selecting these particular e2e criteria and perhaps omitting others.

This thread is for ideating on a clearer analytical framing that will allow us to present--in discussion and documentation--clear reasoning for the extent of our checks. In particular, it seems the understanding of properties post-migration related to governance, token emissions, and validator onboarding are under-expressed.

Begin with answering the questions:

1. Why have we included checks on the properties what we've included?
2. What notable properties have we potentially omitted?
3. Why have we omitted them?

[andygolay]

Regarding governance and validators, I see #80 but agree that specific properties of governance, including validator onboarding, aren't fleshed out yet.

movementlabsxyz/movement#1214 contains semantics for governance criteria in the PR description, eg:

A special script is executed to enable partial governance voting (as would be done via governance in production).
After enabling, the test ensures that a voter cannot re-vote on a proposal they already voted on before the feature was enabled.

So, the property that a voter cannot re-vote on a proposal they already voted for, may be a criterion we've omitted. In fact I don't see any governance-related checks in this repo yet.

Also

The test confirms that voting on a non-existent proposal fails with the correct error.

This would be another good check to include, along with:

• Checking that a validator with a certain amount of MOVE staked, only has that much voting power (that is, ensuring that validators cannot vote with more MOVE than they have staked)
• Checking that validators can be onboarded into the validator set through governance
• Decide on criteria for voting a validator out of the validator set, and checking that voting validators out works correctly

As far as I can tell, we've thus far omitted checks for the above criteria because we just haven't gotten to that step yet. However, we should include them and perhaps others related to governance and validators.

[0xmovses]

To add, these would simply cover the main error messages in governance.move

Check: Minimum stake threshold enforced for proposers.
Covers EINSUFFICIENT_PROPOSER_STAKE.

Check: Lockup period long enough to span voting duration.
Covers EINSUFFICIENT_STAKE_LOCKUP.

Check: Only delegated voter can propose.
Covers ENOT_DELEGATED_VOTER.

Voting Logic
Check: Voter cannot vote more than their current voting power.
Covers EVOTING_POWER_OVERFLOW.

Check: Voting power matches the stake pool (sum of active + pending).
Uses get_voting_power().

Check: Partial voting accumulates correctly.
Check consistency of VotingRecordsV2.

Double Voting & Replay Protection
Check: Voter cannot re-vote on the same proposal (if not using partial voting).
Covers EALREADY_VOTED.

Check: Re-voting after partial governance initialization only uses remaining voting power.

Proposal Lifecycle
Check: Proposals cannot be resolved early unless threshold is met.
Uses early_resolution_vote_threshold.

Check: Resolved proposals are removed from the approved hash map.

Integration with Staking
Check: Voters must be in the validator set or their vote is rejected.
Covers ENO_VOTING_POWER.

As you can see, I can up with this list by checking against the Errors in the module. Checking they are thrown and the function call is aborted under the correct conditions should give us assurance that we are in a correct and proper governance setting.

What do you think Liam, these are very module level checks, not protocol.

[l-monninger]

I think these are good, but can you speak to the framing?

1. Why have we included checks on the properties what we've included?
2. What notable properties have we potentially omitted?
3. Why have we omitted them?

[l-monninger]

In the above, the framing seems to be "these are all the errors governance.move" can throw. In some sense, that's a good approach. At the same time, as you alluded to, it's not really talking about the behavior of the system on the whole.

Arguing why either looking for these errors is complete or otherwise discussing what might be omitted I think could illuminate some of what remains.

[l-monninger]

Below, I'll comment some things I think might be missing...

[l-monninger]

Proposal Lifecycle

Proposal executes properly; has expected effects; and is correctly validated.

Rationale: This is not covered in the errors above and would essentially provide the e2e check on the system.

Proposed framework upgrades successfully

Rationale: This is not directly covered above, and I think it's reasonable to be concerned that--by starting with the feature flags, reconfiguration publicity, and testnet features we did (particularly w.r.t. DKG)--there may be something strange which happens at the point of upgrade.

Epochs handled asynchronously (?)

Rationale: To this point, we use force_end_epoch, e.g., for gas upgrades. But, the preferred upgrade path since the DKG upgrade is the asynchronous reconfiguration. (I think this has something to do with the randomness being guessable otherwise, not sure.)

We likely want to adopt this and ensure the asynchronous upgrades are in fact working, as well that we do not trim the warnings mentioned.

[l-monninger]

Voting Power

@andygolay has stated that reasonable governance check is:

Checking that a validator with a certain amount of MOVE staked, only has that much voting power (that is, ensuring that validators cannot vote with more MOVE than they have staked)

Please use this thread to detail this check and how it should be implemented.

[andygolay]

@sebtomba has began a PR #144 for adding migrator governance criteria which we can use as a starting point.

Currently the satisfies function checks that there is a required proposer stake > 0.

This structure can be followed for other criteria, such as checking that a proposal has been voted through and the validators did not use more voting power than they had staked. So, as in the balances_equal check, we can run a checked migration (to the post-l1-merge), and then run scripts to simulate voting on a proposal, and, in the satisfies function for this criterion, ensure that the validators can't vote with more MOVE than they have staked.

@0xmovses PR https://github.com/movementlabsxyz/movement/pull/1214/files has post-l1-merge governance logic testing voting power that can be brought over into a check in the movement-migration repo, eg in the test_partial_vote function.

[l-monninger]

Validator Set Onboarding (Positive View Changes)

@andygolay has stated that reasonable governance check is:

Checking that validators can be onboarded into the validator set through governance

Please use this thread to detail this check and how it should be implemented.

[andygolay]

This could be implemented as part of the check for

Checking that a validator with a certain amount of MOVE staked, only has that much voting power (that is, ensuring that validators cannot vote with more MOVE than they have staked)
or as a separate check.

As far as I can tell, for the check that validators can only vote with their staked voting power, we'll need to onboard a validator set.

However, it will probably be clearer to have this be a separate criterion so it can be checked in isolation as desired.

So, the check would run a migration to the post-l1-merge release and then the satisfies function would then run scripts to add say 2 or 3 validators, and make assertions showing that validators were successfully added.

[l-monninger]

You're going to want to be clear about when reconfiguration is triggered. Review Aptos the reconfigure method in the framework under DKG and non-DKG upgrades.

[sebtomba]

@l-monninger, I think we need to distinguish between staking (voting on blocks) and governance (voting on proposals):

Checking that validators can be onboarded into the validator set through governance

AFAIK, the mechanism for entering a validator set is staking and not through governance. https://aptos.dev/en/network/blockchain/staking#joining-the-validator-set

movement node join-validator-set \
   --pool-address <pool-address> \
   --profile mainnet-operator

[l-monninger]

Yes, distinguishing is reasonable though technically they are part of the same reconfiguration. What are the properties of staking that should be validated?

[l-monninger]

Validator Set Participant Removal (Negative View Changes)

@andygolay has stated that reasonable governance check is:

Decide on criteria for voting a validator out of the validator set, and checking that voting validators out works correctly

Please use this thread to detail this check and how it should be implemented.

[l-monninger]

@andygolay

In what crucial conceptual and programmatic ways does this differ from Validator Set Onboarding?

[andygolay]

The check would run a migration to the post-l1-merge release and then the satisfies function would then

1. run a script to add a validator set (making assertions showing that validators were successfully added), and then
2. run a script to remove a validator (making assertions showing that the validator was successfully removed).

Programmatically they appear to be similar; you can check the validator address is (for validator set onboarding) or is not (for validator set participant removal) in the active_validators field of ValidatorSet.

[l-monninger]

1. Run a script to add a validator set (making assertions showing that validators were successfully added), and then
2. run a script to remove a validator (making assertions showing that the validator was successfully removed).

This won't be so simple. You'll need to account for differences in Testnet and Mainnet reconfiguration rules. Generally speaking, Validators are added at the epoch boundary when you're actually running with Mainnet features. But, DKG makes it slightly more complicated.

[andygolay]

Okay, I'll read up on it today.

[0xmovses]

@areshand to comment on here on using the replay aptos tool to define new criteria to check that after post-l1-merge framework upgrade, certain things (to be defined by Bo), must hold true.

[areshand]

I think there are a couple of changes happening here:
1. Move framework upgrade
2. Binary upgrade

Each of these upgrades could potentially lead to changes in the state DB. In general, the new binary should remain compatible with the old framework, but the old binary may not work with a new framework.

It seems the upgrade sequence is as follows:
First, upgrade the binary and use replay-verify to replay transactions and confirm that the transaction outputs still match. Then, upgrade the framework and then do the same replay-verify again ?

[l-monninger]

@areshand

Replication of the replay-verify check for this context would be good. We started on a replay migration, but ran into an issue with genesis described below that likely just needs some additional patience.

I recommend checking the README.md and then circling back to see where they may already be overlap.

Currently, there is an issue with genesis that prevents the replay from working as expected that still needs to be resolved or otherwise agreed acceptable. It is possible to regenesis, replay, and satisfy criteria like global-storage-includes, accounts-equal, and balances-equal, but we can't use the exact same genesis directly it seems and the state roots are different.

We have also showed in the past that historical ledger correctness holds, but have left it incomplete owing in part to the replay issue above.

The steps at the moment are basically:

1. Run pre-l1-merge upgrade.
2. Take a snapshot of the state DB. Make no modifications.
3. Start the node over the snapshot disk state.

#136 discusses what is reasoned about within this repository and what is at the moment without--which might help for an additional understanding of how we are chunking and perhaps planning to isolate.

[l-monninger]

@apenzk can likely additionally comment to some of the tradeoffs we've discussed in terms of replay-ability.

One conceptual problem we have is that we have not rigorously tracked node versions over time nor ensured any well-defined standards for backwards compatibility.

Likely, there isn't something lurking that would affect the ledger--it's not like we've changed native implementations or similar. But, we wouldn't actually know.

[apenzk]

From previous discussions it seems we cannot replay exactly due to genesis and version differences. What has been assumed is that the values in the state are semantically correct. There are a number of steps taken to ensure we minimize any risk here, but possibly we cannot guarantee to recreate the exact state transfers (e.g. block hashes etc)