Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Governance] Proof of Fee #1234

Closed

Conversation

0o-de-lally
Copy link
Collaborator

@0o-de-lally 0o-de-lally commented Feb 10, 2023

Implements the first version of Proof of Fee as per the paper https://0l.network/2022/10/15/proof-of-fee-part-1/.
Work started after the community poll: https://0l.network/2022/11/08/october-2022-governance-recap/

  • Initialize bids, and expiry.
  • Functions for auction: Fill the seats and determine the clearing price
  • Near complete refactor of epoch boundary
    • Elect validators based on auction
    • Separate Jail Mechanics
  • Remove fee calcluator, for hardcoded Baseline Fee.
  • Functions for charging arbitrary Fees to the epoch's transaction fee account.
  • Charge Fee on Admission, goes to transaction fee account
  • Pay validators Baseline fee from chain fee account
  • Burn remainder in transaction fee account
  • limits on bids: % above baseline, unlocked in account
  • Change the weight of consensus
  • Baseline fee ratcheting based on bids
  • Future proof for case where we want to limit bid retractions
  • Stretch Goal: Priority seating based on Jail reputation

Remove Stuff

  • Remove Cases, towers don't apply for validator compliance
  • Keep TowerState, but remove validator privileges from Proof of Height
  • NodeWeight

Transactional Tests

  • Make unrelated tests pass
  • Bid setting and initialization
  • Epoch Boundary makes progress
  • Validator election tests
  • Correct Fee payments based on compliance
  • Vouching tests
  • Thermostat adjustment tests
  • Median calc tests
  • Burn excess fees test
  • Testnet Validator creation sanity tests

…session from fake data to be able to apply migrations from Move system contracts.
implement functional tests for:
- exporting db backup to json
- creating genesis blob from json
- creating genesis blob in one shot from db backup

TODO: launch test node from from genesis.blob
TODO: e2e tests from a fixture file
@0o-de-lally
Copy link
Collaborator Author

@hemulin Reviewers should focus on the changed files in: diem-move/diem-framework/DPN/sources/0L/

Perhaps starting with:
diem-move/diem-framework/DPN/sources/0L/ProofOfFee.move
Then
diem-move/diem-framework/DPN/sources/0L/EpochBoundary.move and walk from there.

@hemulin
Copy link
Collaborator

hemulin commented Apr 11, 2023

Generally speaking, looking great 💪 Super impressive @0o-de-lally

The only comments I may suggest are:

  1. commented-out-code cleaning. The comments are very helpful to understand the idea behind the methods, however there are plenty of commented out lines that can and should be removed before merge (all the print(&whatever))

  2. Error handling - Not likely to happen but this is what error handling is for. For example, in fill_seats_and_get_price what will happen if i >= Vector::length(sorted_vals_by_bid)? The while loop will break, we fail to seat anyone and though the comment point to let EpochBoundary to deal with it, the EpochBoundary is again calling fill_seats_and_get_price which may fail again and we are in an endless cycle.

@0o-de-lally
Copy link
Collaborator Author

0o-de-lally commented Apr 11, 2023

@hemulin

Error handling - Not likely to happen but this is what error handling is for. For example, in fill_seats_and_get_price what will happen if i >= Vector::length(sorted_vals_by_bid)? The while loop will break, we fail to seat anyone and though the comment point to let EpochBoundary to deal with it, the EpochBoundary is again calling fill_seats_and_get_price which may fail again and we are in an endless cycle.

Thanks for this. I'm sure you are right that there are more places that could have error handling. A note about Move modules. You don't have Result types like Rust. So you need to do basic control flow with if/else. You can use assert! which can trigger aborts to prevent getting into a bad situation. However for code that only the VM can call, you never want to abort, since the chain will stop in an edge case.

In this case specifically the job of fill seats is to tell us if it could fill seats. An empty result is ok. Whether or not we start an epoch with that list is handled elsewhere. Take a look at EpochBoundary propose_new_set where there is a section //////// Failover Rules //////// which is a catch all on issues with validator selection.

So yes, let's do more on this topic. Keep the suggestions coming!

@0xzoz
Copy link
Collaborator

0xzoz commented Jul 5, 2023

Closing due to changes included #1261

@0xzoz 0xzoz closed this Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants