gnosis/lighthouse
1
0
Fork 0
mirror of https://github.com/sigp/lighthouse.git synced 2026-04-20 22:38:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

v1.1.6 Fork Choice changes (#2822)

Browse Source 
## Issue Addressed

Resolves: https://github.com/sigp/lighthouse/issues/2741
Includes: https://github.com/sigp/lighthouse/pull/2853 so that we can get ssz static tests passing here on v1.1.6. If we want to merge that first, we can make this diff slightly smaller

## Proposed Changes

- Changes the `justified_epoch` and `finalized_epoch` in the `ProtoArrayNode` each to an `Option<Checkpoint>`. The `Option` is necessary only for the migration, so not ideal. But does allow us to add a default logic to `None` on these fields during the database migration.
- Adds a database migration from a legacy fork choice struct to the new one, search for all necessary block roots in fork choice by iterating through blocks in the db.
- updates related to https://github.com/ethereum/consensus-specs/pull/2727
  -  We will have to update the persisted forkchoice to make sure the justified checkpoint stored is correct according to the updated fork choice logic. This boils down to setting the forkchoice store's justified checkpoint to the justified checkpoint of the block that advanced the finalized checkpoint to the current one. 
  - AFAICT there's no migration steps necessary for the update to allow applying attestations from prior blocks, but would appreciate confirmation on that
- I updated the consensus spec tests to v1.1.6 here, but they will fail until we also implement the proposer score boost updates. I confirmed that the previously failing scenario `new_finalized_slot_is_justified_checkpoint_ancestor` will now pass after the boost updates, but haven't confirmed _all_ tests will pass because I just quickly stubbed out the proposer boost test scenario formatting.
- This PR now also includes proposer boosting https://github.com/ethereum/consensus-specs/pull/2730

## Additional Info
I realized checking justified and finalized roots in fork choice makes it more likely that we trigger this bug: https://github.com/ethereum/consensus-specs/pull/2727

It's possible the combination of justified checkpoint and finalized checkpoint in the forkchoice store is different from in any block in fork choice. So when trying to startup our store's justified checkpoint seems invalid to the rest of fork choice (but it should be valid). When this happens we get an `InvalidBestNode` error and fail to start up. So I'm including that bugfix in this branch.

Todo:

- [x] Fix fork choice tests
- [x] Self review
- [x] Add fix for https://github.com/ethereum/consensus-specs/pull/2727
- [x] Rebase onto Kintusgi 
- [x] Fix `num_active_validators` calculation as @michaelsproul pointed out
- [x] Clean up db migrations

Co-authored-by: realbigsean <seananderson33@gmail.com>
This commit is contained in:
realbigsean
2021-12-13 20:43:22 +00:00
parent e391b32858
commit b22ac95d7f
44 changed files with 1802 additions and 658 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
beacon_node/beacon_chain/src/schema_change.rs
View File

@@ -1,9 +1,14 @@
//! Utilities for managing database schema changes.
mod migration_schema_v6;
mod migration_schema_v7;
mod types;
use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY, OP_POOL_DB_KEY};
use crate::persisted_fork_choice::PersistedForkChoice;
use crate::persisted_fork_choice::{PersistedForkChoiceV1, PersistedForkChoiceV7};
use crate::store::{get_key_for_col, KeyValueStoreOp};
use crate::validator_pubkey_cache::ValidatorPubkeyCache;
use operation_pool::{PersistedOperationPool, PersistedOperationPoolBase};
use proto_array::ProtoArrayForkChoice;
use slog::{warn, Logger};
use ssz::{Decode, Encode};
use ssz_derive::{Decode, Encode};
use std::fs;
@@ -22,6 +27,7 @@ pub fn migrate_schema<T: BeaconChainTypes>(
datadir: &Path,
from: SchemaVersion,
to: SchemaVersion,
log: Logger,
) -> Result<(), StoreError> {
match (from, to) {
// Migrating from the current schema version to iself is always OK, a no-op.
@@ -29,8 +35,8 @@ pub fn migrate_schema<T: BeaconChainTypes>(
// Migrate across multiple versions by recursively migrating one step at a time.
(_, _) if from.as_u64() + 1 < to.as_u64() => {
let next = SchemaVersion(from.as_u64() + 1);
migrate_schema::<T>(db.clone(), datadir, from, next)?;
migrate_schema::<T>(db, datadir, next, to)
migrate_schema::<T>(db.clone(), datadir, from, next, log.clone())?;
migrate_schema::<T>(db, datadir, next, to, log)
}
// Migration from v0.3.0 to v0.3.x, adding the temporary states column.
// Nothing actually needs to be done, but once a DB uses v2 it shouldn't go back.
@@ -95,25 +101,77 @@ pub fn migrate_schema<T: BeaconChainTypes>(
Ok(())
}
// Migration for adding `is_merge_complete` field to the fork choice store.
// Migration for adding `execution_status` field to the fork choice store.
(SchemaVersion(5), SchemaVersion(6)) => {
let fork_choice_opt = db
.get_item::<PersistedForkChoice>(&FORK_CHOICE_DB_KEY)?
.map(|mut persisted_fork_choice| {
let fork_choice = ProtoArrayForkChoice::from_bytes_legacy(
&persisted_fork_choice.fork_choice.proto_array_bytes,
)?;
persisted_fork_choice.fork_choice.proto_array_bytes = fork_choice.as_bytes();
Ok::<_, String>(persisted_fork_choice)
})
.transpose()
.map_err(StoreError::SchemaMigrationError)?;
if let Some(fork_choice) = fork_choice_opt {
// Store the converted fork choice store under the same key.
db.put_item::<PersistedForkChoice>(&FORK_CHOICE_DB_KEY, &fork_choice)?;
// Database operations to be done atomically
let mut ops = vec![];
// The top-level `PersistedForkChoice` struct is still V1 but will have its internal
// bytes for the fork choice updated to V6.
let fork_choice_opt = db.get_item::<PersistedForkChoiceV1>(&FORK_CHOICE_DB_KEY)?;
if let Some(mut persisted_fork_choice) = fork_choice_opt {
migration_schema_v6::update_execution_statuses::<T>(&mut persisted_fork_choice)
.map_err(StoreError::SchemaMigrationError)?;
let column = PersistedForkChoiceV1::db_column().into();
let key = FORK_CHOICE_DB_KEY.as_bytes();
let db_key = get_key_for_col(column, key);
let op =
KeyValueStoreOp::PutKeyValue(db_key, persisted_fork_choice.as_store_bytes());
ops.push(op);
}
db.store_schema_version(to)?;
db.store_schema_version_atomically(to, ops)?;
Ok(())
}
// 1. Add `proposer_boost_root`.
// 2. Update `justified_epoch` to `justified_checkpoint` and `finalized_epoch` to
// `finalized_checkpoint`.
// 3. This migration also includes a potential update to the justified
// checkpoint in case the fork choice store's justified checkpoint and finalized checkpoint
// combination does not actually exist for any blocks in fork choice. This was possible in
// the consensus spec prior to v1.1.6.
//
// Relevant issues:
//
// https://github.com/sigp/lighthouse/issues/2741
// https://github.com/ethereum/consensus-specs/pull/2727
// https://github.com/ethereum/consensus-specs/pull/2730
(SchemaVersion(6), SchemaVersion(7)) => {
// Database operations to be done atomically
let mut ops = vec![];
let fork_choice_opt = db.get_item::<PersistedForkChoiceV1>(&FORK_CHOICE_DB_KEY)?;
if let Some(persisted_fork_choice_v1) = fork_choice_opt {
// This migrates the `PersistedForkChoiceStore`, adding the `proposer_boost_root` field.
let mut persisted_fork_choice_v7 = persisted_fork_choice_v1.into();
let result = migration_schema_v7::update_fork_choice::<T>(
&mut persisted_fork_choice_v7,
db.clone(),
);
// Fall back to re-initializing fork choice from an anchor state if necessary.
if let Err(e) = result {
warn!(log, "Unable to migrate to database schema 7, re-initializing fork choice"; "error" => ?e);
migration_schema_v7::update_with_reinitialized_fork_choice::<T>(
&mut persisted_fork_choice_v7,
db.clone(),
)
.map_err(StoreError::SchemaMigrationError)?;
}
// Store the converted fork choice store under the same key.
let column = PersistedForkChoiceV7::db_column().into();
let key = FORK_CHOICE_DB_KEY.as_bytes();
let db_key = get_key_for_col(column, key);
let op =
KeyValueStoreOp::PutKeyValue(db_key, persisted_fork_choice_v7.as_store_bytes());
ops.push(op);
}
db.store_schema_version_atomically(to, ops)?;
Ok(())
}