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

Use Static Sizes for State Fields #10007

Merged
merged 11 commits into from
Dec 13, 2021
Merged

Use Static Sizes for State Fields #10007

merged 11 commits into from
Dec 13, 2021

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Dec 10, 2021

What type of PR is this?

Cleanup

What does this PR do? Why is it needed?

  • Our state fields are made up of many large lists which have fixed lengths. However when hashing those
    fields we use parameters which can be modified at runtime. If we are running specialized devnets, the state root
    computation can differ from what is expected if these parameters are different. This PR adds those fields as constants so that they are never able to change in value.

Which issues(s) does this PR fix?

N.A

Other notes for review

@nisdas nisdas added the Ready For Review A pull request ready for code review label Dec 10, 2021
@nisdas nisdas requested a review from a team as a code owner December 10, 2021 15:13
Comment on lines 1 to 16
// +build !minimal

package field_params

const (
BlockRootsLength = 8192 // SLOTS_PER_HISTORICAL_ROOT
StateRootsLength = 8192 // SLOTS_PER_HISTORICAL_ROOT
RandaoMixesLength = 65536 // EPOCHS_PER_HISTORICAL_VECTOR
HistoricalRootsLength = 16777216 // HISTORICAL_ROOTS_LIMIT
ValidatorRegistryLimit = 1099511627776 // VALIDATOR_REGISTRY_LIMIT
Eth1DataVotesLength = 2048 // SLOTS_PER_ETH1_VOTING_PERIOD
PreviousEpochAttestationsLength = 4096 // MAX_ATTESTATIONS * SLOTS_PER_EPOCH
CurrentEpochAttestationsLength = 4096 // MAX_ATTESTATIONS * SLOTS_PER_EPOCH
SlashingsLength = 8192 // EPOCHS_PER_SLASHINGS_VECTOR
SyncCommitteeLength = 512 // SYNC_COMMITTEE_SIZE
)
Copy link
Member

Choose a reason for hiding this comment

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

We can name this file mainnet.go. It's less verbose, and the folder name already implies field_parameters

Copy link
Member

Choose a reason for hiding this comment

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

Some more options:
config/params/state_fields/mainnet.go
config/fieldparams/mainnet.go

No strong opinion

Comment on lines 5 to 8
srcs = [
"mainnet.go",
"minimal.go", #keep
],
Copy link
Contributor

@rkapka rkapka Dec 10, 2021

Choose a reason for hiding this comment

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

One thing that will have to change is we can no longer include both files in the build with # keep because of SSZ generation for the native beacon state (I think it will pick the first value it finds). There is a "better" way to do this which selects one file based on the current config. You might want to incorporate it into your PR. If not, I will change it in my feature branch.
https://discord.com/channels/476244492043812875/550365651118850081/915341315661778946

Copy link
Contributor

@rkapka rkapka Dec 10, 2021

Choose a reason for hiding this comment

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

The Discord message has a bug, the second config should have name = "minimal"

@rkapka rkapka merged commit 5ab88da into develop Dec 13, 2021
@rkapka rkapka deleted the staticFieldSizes branch December 13, 2021 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants