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

eip-7251: process_effective_balance_updates #14003

Merged
merged 3 commits into from
May 25, 2024

Conversation

prestonvanloon
Copy link
Member

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Required for electra.

Extracted from #13903.

Which issues(s) does this PR fix?

Tracking @ #13849

Other notes for review

https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.2/specs/electra/beacon-chain.md#updated-process_effective_balance_updates

@prestonvanloon prestonvanloon force-pushed the eip-7251-process_effective_balance_updates branch from 65ae7fb to d349bb4 Compare May 15, 2024 11:29
@prestonvanloon prestonvanloon marked this pull request as ready for review May 15, 2024 11:30
@prestonvanloon prestonvanloon requested a review from a team as a code owner May 15, 2024 11:30
@prestonvanloon prestonvanloon added the Electra electra hardfork label May 15, 2024
@prestonvanloon prestonvanloon force-pushed the eip-7251-process_effective_balance_updates branch from d349bb4 to 3fe6947 Compare May 15, 2024 14:18
@prestonvanloon prestonvanloon force-pushed the eip-7251-process_effective_balance_updates branch from d6b9d0b to 9e37da4 Compare May 16, 2024 07:33
Spectests for process_effective_balance_updates

process_effective_balance_updates unit tests
@prestonvanloon prestonvanloon force-pushed the eip-7251-process_effective_balance_updates branch from 35652f5 to a0d5882 Compare May 24, 2024 19:03
}

if balance+downwardThreshold < val.EffectiveBalance || val.EffectiveBalance+upwardThreshold < balance {
effectiveBal := min(balance-balance%effBalanceInc, effectiveBalanceLimit)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this extra variable ? ( maybe reads more clearly?)

Copy link
Contributor

@james-prysm james-prysm left a comment

Choose a reason for hiding this comment

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

my comment is more of a nitpick i think this pr is fine

@prestonvanloon prestonvanloon added this pull request to the merge queue May 25, 2024
Merged via the queue into develop with commit 2542189 May 25, 2024
17 checks passed
@prestonvanloon prestonvanloon deleted the eip-7251-process_effective_balance_updates branch May 25, 2024 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Electra electra hardfork
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants