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

Improve testing/util BeaconState creation function options #10429

Open
kasey opened this issue Mar 25, 2022 · 1 comment
Open

Improve testing/util BeaconState creation function options #10429

kasey opened this issue Mar 25, 2022 · 1 comment
Assignees
Labels
Cleanup Code health! Good First Issue Good for newcomers

Comments

@kasey
Copy link
Contributor

kasey commented Mar 25, 2022

Background

In testing/util we have functions to help create BeaconState objects for testing purposes: NewBeaconState, NewBeaconStateAltair, NewBeaconStateBellatrix. (example)[https://github.com/prysmaticlabs/prysm/blob/develop/testing/util/state.go#L80]. These functions take options which are tied to the type of the state object they create.

Description

Modify NewBeaconState, NewBeaconStateAltair, NewBeaconStateBellatrix to accept the same functional option type (which would receive a value with the state.BeaconState interface). Then the same methods could be used across states creator functions. For example, populating the state object with a list of validators w/ a given balance for testing the weak subjectivity calculation.

@alexchenzl
Copy link

alexchenzl commented Jun 28, 2022

Hi @kasey , I'm trying to refactor these three functions to accept the same functional option type which would receive a value with the state.BeaconState interface, but I encountered an obstacle. I have two possible workarounds and want to hear your suggestion.

The current functional options are applied to corresponding ProtoBuf instances first, and then the expected state.BeaconState instances are generated from the ProtoBuf instances in the previous step. When a functional option receives a value with the state.BeaconState interface, for a normal BeaconState instance that has an internal ProtoBuf pointer, the functional option can be passed into the corresponding InitializeFromProtoUnsafe function and then applied to the internal ProtoBuf instance. But for a native BeaconState instance, this option has to be applied to the new created native BeaconState instance directly.

As a result, every functional option will need to be implemented twice. The first one is for the normal BeaconSate with an internal protobuf pointer, and the other one is for the corresponding native beacon state object.

A brute-force workaround is to use an empty interface as the functional option's parameter type, but type conversion needs to be performed in the implementation of every functional option.

Another workaround is to add a new generic creation function with functional options. Though it didn't make the existing functions to accept a same functional option type, the generic function could make it easier to create beacon states of different versions for various test cases.

// PbBeaconState is a type constraint with a union of all supported versions of protobuf beacon state types.
type PbBeaconState interface {
    ethpb.BeaconState | ethpb.BeaconStateAltair | ethpb.BeaconStateBellatrix
}

// NewBeaconStateGeneric is a generic method to create a state.BeaconState.
func NewBeaconStateGeneric[T PbBeaconState](initializer func(s *T), generator func(s *T) (state.BeaconState, error), options ...func(s *T) error) (state.BeaconState, error) {
    seed := new(T)
    // Initialize seed with default values
    initializer(seed)
    // Apply options to seed
    for _, opt := range options {
        err := opt(seed)
        if err != nil {
            return nil, err
        }
    }
    // Generate a state.BeaconState object from the seed protobuf object
    return generator(seed)
}

The above is the generic creation function. Here is the complete refactored version of state.go for your reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code health! Good First Issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants