eip-7251: New compute_consolidation_epoch_and_update_churn #13981
Conversation
| // Consolidation doesn't fit in the current earliest epoch. | ||
| if consolidationBalance > consolidationBalanceToConsume { | ||
| balanceToProcess := consolidationBalance - consolidationBalanceToConsume | ||
| // additional_epochs = (balance_to_process - 1) // per_epoch_consolidation_churn + 1 |
There was a problem hiding this comment.
Do we intend to leave // additional_epochs = (balance_to_process - 1) // per_epoch_consolidation_churn + 1 here?
There was a problem hiding this comment.
no, I meant the python comment in general
There was a problem hiding this comment.
Yes, I left the python code here because this line is a bit confusing where the division and addition are several lines apart.
There was a problem hiding this comment.
My only comment is with regards to the spec, It smells to me that there's a function that explicitlyhas two separate concerns, there's the code logic to compute the consodation epoch and update the churn, I would have preferred two separate helpers for this. We could implement it this way internally.
| } | ||
| additionalEpochs++ | ||
| earliestConsolidationEpoch += primitives.Epoch(additionalEpochs) | ||
| consolidationBalanceToConsume += math.Gwei(additionalEpochs) * perEpochConsolidationChurn |
There was a problem hiding this comment.
Not from this PR but I find it funny that Gwei is defined in the math package.
tests for compute_consolidation_epoch_and_update_churn
prestonvanloon
force-pushed
the
eip-7251-compute_consolidation_epoch_and_update_churn
branch
from
e5d71cf
to
9026515
Compare
prestonvanloon
deleted the
eip-7251-compute_consolidation_epoch_and_update_churn
branch
…ysmaticlabs#13981) tests for compute_consolidation_epoch_and_update_churn Co-authored-by: james-prysm <[email protected]>
…3981) tests for compute_consolidation_epoch_and_update_churn Co-authored-by: james-prysm <[email protected]>
What type of PR is this?
Feature
What does this PR do? Why is it needed?
Extracted from #13903.
This PR implements new spec method compute_consolidation_epoch_and_update_churn in electra.
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#new-compute_consolidation_epoch_and_update_churn