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 New State Type in Prysm's Runtime #4614

Closed
wants to merge 78 commits into from
Closed

Conversation

rauljordan
Copy link
Contributor

@rauljordan rauljordan commented Jan 22, 2020

Resolves #2011 and is a follow-up to #4602


Description

Write why you are making the changes in this pull request

This PR utilizes the new state type defined in #4602 and integrates it into the Prysm runtime. This is a major PR that modifies much of our core code and needs to be executed carefully before merge.

Link anything that would be helpful or relevant to the reviewers

Here is what has been done so far:

  • Rewriting the DB interface to use the new *state.BeaconState type instead of the protobuf definition for all state related functions
  • Ensure all DB tests pass and that saving the new state to the DB is backwards compatible with old DB's
  • Utilize the new beacon state as the cached HeadState in the blockchain service and modify the head fetcher interface to return this type

In terms of modifying our core/ package, here's what has been done:

  • Modify most code to use the new state type in terms of inputs and outputs
  • Create helpful functions in beacon-chain/state/getters.go and setters.go such as state.AppendValidators or state.NumValidators() to prevent giant cloning of state.Validators() in a bunch of core functions
  • Create other useful setters such as state.UpdateCurrentEpochAttestationsAtIndex setters which modify a single element of a field in the new beacon state type
  • Prevent multiple calls to state.Validators() in the core state transition function and instead try to pass in the same value downstream to the helper functions within the state transition func
  • Modify the output of many helper/core functions to return error instead of returning state since we are no longer doing the pattern of state = ProcessAttestations(state, block) with this new type, but instead if err := ProcessAttestations(state, block); err != nil as an alternative
  • Fix the state transition function to fully utilize this new type
  • core/state/ package has yet to be touched and has yet to use this new type

In terms of whats left in the repo:

  • Fix tests (oh boy)
  • Integrate in other services such as RPC/Sync
  • Integrate into blockchain/forkchoice
  • Fix up spec tests

Runtime

  • Improve Calculation of Balance and Registry Roots. Currently it is a large bottleneck when updating the balance of each validator
  • Ensure runtime works, can finalize, and sync with network

beacon-chain/blockchain/forkchoice/log.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/forkchoice/metrics.go Show resolved Hide resolved
beacon-chain/blockchain/forkchoice/process_attestation.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/forkchoice/process_block.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/forkchoice/service.go Outdated Show resolved Hide resolved
beacon-chain/state/getters.go Outdated Show resolved Hide resolved
beacon-chain/state/getters.go Outdated Show resolved Hide resolved
// at a specific index to a new value.
func (b *BeaconState) UpdateBlockRootAtIndex(idx uint64, blockRoot [32]byte) error {
if len(b.state.BlockRoots) <= int(idx) {
return errors.Errorf("invalid index provided %d", idx)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use fmt.Errorf

beacon-chain/sync/subscriber_beacon_blocks.go Outdated Show resolved Hide resolved
beacon-chain/sync/validate_aggregate_proof.go Outdated Show resolved Hide resolved
"github.com/prysmaticlabs/prysm/shared/params"
"go.opencensus.io/trace"
)

// New gets called at the beginning of process epoch cycle to return
// pre computed instances of validators attesting records and total
// balances attested in an epoch.
func New(ctx context.Context, state *pb.BeaconState) ([]*Validator, *Balance) {
func New(ctx context.Context, state *stateTrie.BeaconState) ([]*Validator, *Balance) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make this take in validators as an argument

func ProcessRewardsAndPenaltiesPrecompute(
state *stateTrie.BeaconState,
bp *Balance,
vp []*Validator,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Take in validators and balances as arguments here

"github.com/prysmaticlabs/prysm/shared/mathutil"
"github.com/prysmaticlabs/prysm/shared/params"
)

// ProcessSlashingsPrecompute processes the slashed validators during epoch processing.
// This is an optimized version by passing in precomputed total epoch balances.
func ProcessSlashingsPrecompute(state *pb.BeaconState, p *Balance) *pb.BeaconState {
func ProcessSlashingsPrecompute(state *stateTrie.BeaconState, p *Balance) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Take in validators as an argument here

@@ -17,15 +17,16 @@ import (

// ValidateVoluntaryExit validates the voluntary exit.
// If it is invalid for some reason an error, if valid it will return no error.
func ValidateVoluntaryExit(state *pb.BeaconState, genesisTime time.Time, signed *ethpb.SignedVoluntaryExit) error {
func ValidateVoluntaryExit(state *stateTrie.BeaconState, genesisTime time.Time, signed *ethpb.SignedVoluntaryExit) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Take in validators as an argument

@@ -56,7 +57,7 @@ func IsSlashableValidator(validator *ethpb.Validator, epoch uint64) bool {
// Return the sequence of active validator indices at ``epoch``.
// """
// return [ValidatorIndex(i) for i, v in enumerate(state.validators) if is_active_validator(v, epoch)]
func ActiveValidatorIndices(state *pb.BeaconState, epoch uint64) ([]uint64, error) {
func ActiveValidatorIndices(state *stateTrie.BeaconState, epoch uint64) ([]uint64, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Take in list of validators as an index instead

rauljordan and others added 17 commits January 22, 2020 14:22
* compute on read

* fix up eth1 data votes

* looking into slashings bug introduced in core/

* able to advance more slots

* add logging

* can now sync with testnet yay

* remove the leaves algorithm and other merkle imports

* expose initialize unsafe funcs

* Update beacon-chain/db/kv/state.go

* lint

Co-authored-by: Raul Jordan <[email protected]>
* map optimization

* more optimizations

* use a custom hasher

* comment

* block operations optimizations

* Update beacon-chain/state/types.go

Co-Authored-By: Raul Jordan <[email protected]>

* fixed up various operations to use the validator index map access

Co-authored-by: Raul Jordan <[email protected]>
* parallelize shuffling

* clean up

* lint

* fix build

* use callback to read from registry

* fix array roots and size map

* new improvements

* reduce hash allocs

* improved shuffling

* terence's review

* use different method

* raul's comment

* new array roots

* remove clone in pre-compute

* Update beacon-chain/state/types.go

Co-Authored-By: Raul Jordan <[email protected]>

* raul's review

* lint

* fix build issues

* fix visibility

Co-authored-by: Raul Jordan <[email protected]>
@rauljordan
Copy link
Contributor Author

Closing in favor of #4646

@rauljordan rauljordan closed this Jan 29, 2020
@rauljordan rauljordan deleted the use-state-in-runtime branch February 5, 2020 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Priority: High High priority item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Perform State Caching With Merkle Structure
2 participants