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

Altair review #2374

Merged
merged 3 commits into from
May 4, 2021
Merged

Altair review #2374

merged 3 commits into from
May 4, 2021

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented May 4, 2021

  • add some missing beacon-chain function comments
  • Reorder attestation logic to be in order [source, target, head] throughout
  • define is_{attestation_field} as having to have the parent is_{attestation_field} correct, thus enforcing chain coherence any time those booleans are used
  • add sync committee test for empty participants

@djrtwo djrtwo requested a review from ralexstokes May 4, 2021 13:47
specs/altair/beacon-chain.md Show resolved Hide resolved
specs/altair/beacon-chain.md Show resolved Hide resolved
specs/altair/beacon-chain.md Show resolved Hide resolved
@hwwhww hwwhww added the Altair aka HF1 label May 4, 2021
@hwwhww hwwhww added this to the v1.1.0-alpha.4 milestone May 4, 2021
Copy link
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

lgtm!

i left a comment about "increment based" accounting bc i think there is a more appropriate term but doesn't really block this PR

@@ -311,10 +320,17 @@ def get_base_reward_per_increment(state: BeaconState) -> Gwei:

#### `get_base_reward`

*Note*: The function `get_base_reward` is modified with the removal of `BASE_REWARDS_PER_EPOCH`.
*Note*: The function `get_base_reward` is modified with the removal of `BASE_REWARDS_PER_EPOCH` and the use of increment based accounting.
Copy link
Member

Choose a reason for hiding this comment

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

tbh i'm not really sure what "increment based" means but i don't think it blocks this PR

@protolambda protolambda merged commit e9580dc into dev May 4, 2021
@djrtwo djrtwo deleted the altair-pass branch May 4, 2021 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Altair aka HF1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants