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

handling altered constants at forks #1101

Closed
djrtwo opened this issue May 20, 2019 · 6 comments
Closed

handling altered constants at forks #1101

djrtwo opened this issue May 20, 2019 · 6 comments

Comments

@djrtwo
Copy link
Contributor

djrtwo commented May 20, 2019

We currently don't have a good mechanism for handling a changed constant at a fork boundary. We likely want to use the same BeaconState versioning technique as for signatures.

The first constant we are discussing changing in production is MAX_TRANSFERS (from 0 to non-zero). We need to ensure that blocks prior to the fork epoch have validity condition len(block.transfers) == 0 but after the fork epoch have len(block.transfers) == non_zero_value.

We don't necessarily need to build this in to the spec at this point, but it will certainly be a requirement that clients can handle different constants per fork version.

Solution might look something like get_constant(state, var_string_name, epoch)

@dankrad
Copy link
Contributor

dankrad commented May 20, 2019

I agree that we need this. I'm a bit concerned that the code for this would be very clumsy. Is there a more elegant solution?
The simple pythonic way would be to just overwrite the constants at the epoch boundaries, but this might of course not work for all constants. But it would not blow up the code like this proposal.

@djrtwo
Copy link
Contributor Author

djrtwo commented May 20, 2019

Overwriting loses the ability to have a granular control on the constants on the border of a fork. See my above transfer example.

Given the current structure of the pyspec, I don't see a path that would be elegant :/

py-evm has a notion of VirtualMachines and StateMachines that are applied at certain block or slot heights. Constants can change at these heights and these machines can easily handle constants on the border.


Just remembered proto and I discussed this weeks ago in the context of "fork timelines" for testing and defining production and testnet configurations (https://github.com/ethereum/eth2.0-specs/tree/dev/configs/fork_timelines). The easy solution is to just define these constant changes here and let clients handle the fork complexity locally instead of defining a mechanism in spec like signature domains. We can then hack a solution in the pytests/test generation instead of in spec.

@hwwhww
Copy link
Contributor

hwwhww commented May 21, 2019

The current phase 1 executable PR #1061 builds the phases into test_libs/pyspec/eth2spec/phase0/spec.py and test_libs/pyspec/eth2spec/phase1/spec.py.

If there’s a hard fork <FORK_NAME> between phase 0 and phase 1, I suppose we can add specs/core/0_beacon-chain_<FORK_NAME>.md with altered constants, and build it into test_libs/pyspec/eth2spec/phase0_<FORK_NAME>/spec.py.

For the cross-fork tests, we can take the spec modules into:

import eth2spec.phase0.spec as phase0_spec
import eth2spec.phase1.spec as phase0_<FORK_NAME>_spec
import eth2spec.phase1.spec as phase1_spec

def get_spec(epoch, fork_timeline):
    if fork_timeline['phase0'] <= epoch < fork_timeline['phase0_<FORK_NAME>']:
        return phase0_spec
    elif fork_timeline['phase0_<FORK_NAME>'] <= epoch < fork_timeline['phase1']:
        return phase0_<FORK_NAME>_spec
    elif epoch >= fork_timeline['phase1']:
        return phase1_spec
    else:
        raise Exception('Out of fork_timeline')

@JustinDrake
Copy link
Collaborator

The first constant we are discussing changing in production is MAX_TRANSFERS (from 0 to non-zero).

How often is a pure constant change applicable? I would invest in infrastructure facilitating more general changes to the state transition function. (See also item 8 in #1054 which suggests removing transfers from the phase 0 spec.)

@hwwhww
Copy link
Contributor

hwwhww commented May 21, 2019

In @protolambda 's test_format design, it seems there's one config serves for different forks:

title: <string, short, one line> -- Display name for the test suite
summary: <string, average, 1-3 lines> -- Summarizes the test suite
forks_timeline: <string, reference to a fork definition file, without extension> -- Used to determine the forking timeline
forks: <list of strings> -- Defines the coverage. Test-runner code may decide to re-run with the different forks "activated", when applicable.
config: <string, reference to a config file, without extension> -- Used to determine which set of constants to run (possibly compile time) with
runner: <string, no spaces, python-like naming format> *MUST be consistent with folder structure*
handler: <string, no spaces, python-like naming format> *MUST be consistent with folder structure*

test_cases: <list, values being maps defining a test case each>
   ...

I guess we need to update constant_presets to serve for multiple forks/phases like:

phase0: 
    SHARD_COUNT: 8
    TARGET_COMMITTEE_SIZE: 4
    ...
    MAX_TRANSFERS: 0
phase1:
    BYTES_PER_SHARD_BLOCK: 16384
    ...
    MAX_TRANSFERS: 16

only list the new or altered constants.

@djrtwo
Copy link
Contributor Author

djrtwo commented Mar 17, 2020

Current approach is to never modify config vars. Instead to just have new vars at new forks

@djrtwo djrtwo closed this as completed Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants