Skip to content

Align to consensus release v1.4.0-beta.2#12908

Closed
terencechain wants to merge 12 commits intodevelopfrom
update-max-per-epoch-churn-limit
Closed

Align to consensus release v1.4.0-beta.2#12908
terencechain wants to merge 12 commits intodevelopfrom
update-max-per-epoch-churn-limit

Conversation

)

// churnLimit is normally 4 unless the validator set is extremely large.
var churnLimit = 4
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am removing this to use the default config

@terencechain terencechain added the >1-approves-required Fragile changes that need sufficient review label Sep 19, 2023
@terencechain terencechain force-pushed the update-max-per-epoch-churn-limit branch from caae3c7 to 9b4d116 Compare September 20, 2023 14:16
// Subnet value
BlobsidecarSubnetCount: 6,

MaxPerEpochActivationChurnLimit: 8,
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 not need the min here too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there's no change here

e2eConfig.DenebForkVersion = []byte{4, 0, 0, 254}

// Deneb changes.
e2eConfig.MinPerEpochChurnLimit = 2
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 not need max here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should just use the default which is 8

limit = churnLimit
}
// Cap churn limit to max per epoch churn limit. New in EIP7514.
if limit > params.BeaconConfig().MaxPerEpochActivationChurnLimit {
Copy link
Contributor

Choose a reason for hiding this comment

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

where is the minimum set for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Minimal is used in other places. It's not part of the change besides the value

@terencechain terencechain force-pushed the update-max-per-epoch-churn-limit branch from 430daff to af124df Compare September 21, 2023 12:16

if deposits != churnLimit {
return fmt.Errorf("expected %d deposits to be processed in epoch %d, received %d", churnLimit, epoch, deposits)
if deposits != int(params.BeaconConfig().MinPerEpochChurnLimit) {
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer to cast deposits to uint

@terencechain terencechain force-pushed the update-max-per-epoch-churn-limit branch from ee84348 to 9948ed4 Compare September 21, 2023 15:28
rauljordan
rauljordan previously approved these changes Sep 21, 2023
limit = churnLimit
}

if slots.ToEpoch(state.Slot()) >= params.BeaconConfig().DenebForkEpoch {
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't the state version be better here ? we would only apply this update on deneb states

@terencechain terencechain force-pushed the update-max-per-epoch-churn-limit branch from ddd1397 to 341a1ac Compare September 22, 2023 18:50
@terencechain terencechain force-pushed the update-max-per-epoch-churn-limit branch 2 times, most recently from 63553bf to 41366d8 Compare September 25, 2023 13:28
@terencechain terencechain force-pushed the update-max-per-epoch-churn-limit branch from 41366d8 to a87113d Compare September 25, 2023 13:29
@terencechain terencechain changed the title Update and use max per epoch churn limit Align to consensus release v1.4.0-beta.2 Sep 25, 2023
@terencechain terencechain changed the title Align to consensus release v1.4.0-beta.2 Align to consensus release v1.4.0-beta.2 Sep 25, 2023
@terencechain
Copy link
Collaborator Author

superseded by #12959

@terencechain terencechain deleted the update-max-per-epoch-churn-limit branch June 5, 2024 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>1-approves-required Fragile changes that need sufficient review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants