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

Initialize MVP model scope #4

Merged
merged 17 commits into from Mar 21, 2021
Merged

Initialize MVP model scope #4

merged 17 commits into from Mar 21, 2021

Conversation

BenSchZA
Copy link
Collaborator

@BenSchZA BenSchZA commented Mar 9, 2021

There are a number points to review, indicated by NOTEs and TODOs in the code.

To review:

  • Initialization of states
  • Naming conventions for policies (p_ vs policy_) and mechanisms (s_ vs update_)
  • Python model software architecture and naming

Some of these could be reviewed and refactored in subsequent tickets, in the interest of getting the MVP implementation completed for further iteration and development.

@BenSchZA BenSchZA changed the title Add initial states and parameters Initialize MVP model scope Mar 18, 2021
@BenSchZA BenSchZA added this to Done in Collab Workstreams Mar 18, 2021
@BenSchZA BenSchZA moved this from Done to In Review in Collab Workstreams Mar 18, 2021
@BenSchZA BenSchZA requested a review from danlessa March 18, 2021 11:19
@BenSchZA BenSchZA self-assigned this Mar 18, 2021
EFFECTIVE_BALANCE_INCREMENT = [1 * constants.gwei],
PROPOSER_REWARD_QUOTIENT = [8],
WHISTLEBLOWER_REWARD_QUOTIENT = [512],
MIN_SLASHING_PENALTY_QUOTIENT = [32],
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it was intended, but on the eth2 specs, the MIN_SLASHING_PENALTY_QUOTIENT is set to 128.
image
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@marthendalnunes great work picking that up! So this is something we could introduce phases for, or consider the steady state which would be 32. I believe the "blueprint" economic report model chose to not introduce phases, mainly because they couldn't introduce too many degrees of freedom in a spreadsheet model. In our case we could.


return {
'total_basefee': total_basefee,
'total_tips_to_validators': total_tips_to_validators,
Copy link
Member

Choose a reason for hiding this comment

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

Does this nomenclature total_tips_to_validators considers a post eth1/eth2 merge?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See comment in Slack. EIP1559 would be introduced in phase 2, so I believe you're correct. The economic report considers the steady state, we can possibly introduce phases if necessary specifically for EIP1559.

validating_penalties = previous_state['validating_penalties']
total_tips_to_validators = previous_state['total_tips_to_validators']

total_online_validator_rewards = validating_rewards + whistleblower_rewards - validating_penalties + total_tips_to_validators
Copy link
Member

Choose a reason for hiding this comment

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

Is this considering the model is for phase 2?

@BenSchZA
Copy link
Collaborator Author

See python/typeshed#3500 for issue re. typing:

import sys

if sys.version_info >= (3, 8):
    from typing import TypedDict, Literal, overload  # pylint: disable=no-name-in-module
else:
    from typing_extensions import TypedDict, Literal, overload

@BenSchZA BenSchZA merged commit d78d892 into main Mar 21, 2021
Collab Workstreams automation moved this from In Review to Done Mar 21, 2021
@BenSchZA BenSchZA deleted the init-states-params branch March 21, 2021 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants