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

Standardize epoch configs #11265

Open
Longarithm opened this issue May 8, 2024 · 0 comments
Open

Standardize epoch configs #11265

Longarithm opened this issue May 8, 2024 · 0 comments
Labels
A-genesis Area: issues that are related to genesis A-resharding Area: State resharding A-stateless-validation Area: stateless validation C-good-first-issue Category: issues that are self-contained and easy for newcomers to work on. C-housekeeping Category: Refactoring, cleanups, code quality

Comments

@Longarithm
Copy link
Member

Longarithm commented May 8, 2024

EpochConfigs has to be stored and created in the similar fashion as RuntimeConfigStore: folder with YAML configs, each corresponding to its own protocol version. Currently it's quite hard to understand what are the values of fields because they are overridden in many places. Somewhere we even just use constants.

Genesis shouldn't be source of truth because we keep adding new fields there.

More specifically - logic in AllEpochConfig is quite chaotic, e.g. AllEpochConfigTestOverrides applies changes on top of all versions of epoch configs. Not sure if min_stake_ratio must be used in all tests, etc.

Example of relevant work #11252

@Longarithm Longarithm added C-housekeeping Category: Refactoring, cleanups, code quality C-good-first-issue Category: issues that are self-contained and easy for newcomers to work on. A-stateless-validation Area: stateless validation labels May 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 7, 2024
…from JSON files per version (#11896)

Issue: #11265

We introduce a mechanism for storing `EpochConfigs` for
mainnet/testnet/mocknet (forknet) in JSON files for various protocol
versions and retrieving them from a new library called
`EpochConfigStore`. The library parses JSON files containing the epoch
configs and loads them in a map from (chain-id x version) to
`EpochConfig`. The library is accessed from `AllEpochConfig` when
`use_production_config` is set to true AND outside of tests (for this we
explicitly set test overrides to default even if there is no explicit
override to distinguish cases where a test uses a mainnet/testnet chanin
id with a custom `EpochConfig`).

We change the existing `for_protocol_version` method of `AllEpochConfig`
to `generate_epoch_config` to indicate that it generates a config in
code. The new version of `for_protocol_version` internall calls the
config store or `generate_epoch_config` based on whether the config
store is initialized.

We will need to follow-up with more changes to this PR since there are
tests that use mainnet/testnet chain id with custom `EpochConfig` which
conflicts with the config store (which is supposed to use production
configs for these chains). Our eventual goal is to remove
`generate_epoch_config` and only have (1) explicitly documented
production configs in JSON files or (2) custom-built configs in test but
not using the hard-coded changes.
github-merge-queue bot pushed a commit that referenced this issue Sep 2, 2024
Integration test to check that validator is excluded before
`FixMinStakeRatio` takes place and included after that.

Some hack is needed to trigger production epoch config logic; in #11265
we should remove them all.
staffik pushed a commit that referenced this issue Sep 5, 2024
Integration test to check that validator is excluded before
`FixMinStakeRatio` takes place and included after that.

Some hack is needed to trigger production epoch config logic; in #11265
we should remove them all.
@Longarithm Longarithm added A-genesis Area: issues that are related to genesis A-resharding Area: State resharding labels Oct 1, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 1, 2024
### Goal

Write stub for test for resharding v3 switch. For this, I want the chain
to switch between shard layouts.
And for that, I switch to `EpochConfigStore` as much as I can, which
implies skipping `use_production_config`, overrides like
`AllEpochConfig::config_max_kickout_stake`, `EpochConfig` generations
from `GenesisConfig`. This is a big step towards #11265.

The most visible changes are: 
Now TestLoop generates `genesis_and_epoch_config_store` instead of just
`genesis`. Later we should have a separate `EpochConfigStoreBuilder`
which may accept some data shared between genesis and epoch configs,
e.g. validators set. This is done to minimise changes.

`EpochManager::new_arc_handle` is the way how epoch manager is
constructed on production nodes. Its logic is changed as follows:
* if chain = mainnet/testnet, only `EpochConfigStore::for_chain_id` is
used for getting epoch configs.
* if `chain_id.starts_with("test-chain-")`, we use only
`EpochConfig::from(genesis_config)` (see below!)
* otherwise, we use only `Genesis::test_epoch_config`. **It doesn't use
any genesis data**, just stays in this crate for now for convenience.
This is for simple tests in single module.

### Achievements

* `test_fix_min_stake_ratio` tests exactly what we want - we take
`EpochConfigStore::for_chain_id("mainnet")` and see that it allows to
include small validator after protocol upgrade.
* In `test_resharding_v3` we define old and new shard layouts, and test
the switch explicitly without hidden overrides.
* Usage of hacky overrides is reduced. For example,
`EpochManager::new_from_genesis_config_with_test_overrides` is removed.
* If we want to launch forknet with custom epoch config, the behaviour
will be more straightforward. For example, one can copy latest epoch
config from mainnet to mocknet/ folder and add new condition to
`for_epoch_id` for custom mocknet chain name.

### Failures

Nayduck often configures epoch config through genesis, e.g. by setting
`block_producer_kickout_threshold` to 0. It is much more work to change
this configuration, so I add a hack: if chain_id starts with
`test-chain-` - name which nayduck uses - epoch config is derived from
genesis. Many old integration tests use this chain id as well.
However, the improvement here is that we generate only **one** epoch
config, without any overrides.

epoch_length is sometimes taken from `ChainGenesis`, not from
`EpochConfig`. To be safe, I set epoch length in both genesis and epoch
configs.

This still lacks testing on live node. Using this on canary or forknet
could be insightful.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-genesis Area: issues that are related to genesis A-resharding Area: State resharding A-stateless-validation Area: stateless validation C-good-first-issue Category: issues that are self-contained and easy for newcomers to work on. C-housekeeping Category: Refactoring, cleanups, code quality
Projects
None yet
Development

No branches or pull requests

1 participant