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.

Sign up for GitHub

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

Validate the ChainConfig struct at construction time #1845

Merged
merged 14 commits into from
Sep 2, 2022
Merged

Validate the ChainConfig struct at construction time #1845

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7a7959e
most changes
tyshko5 Aug 22, 2022
9e9360b
requested change
tyshko5 Aug 22, 2022
bab321e
fix fmt
tyshko5 Aug 23, 2022
9cf2af0
new ways: delete claus, version_schedule, sort
tyshko5 Aug 30, 2022
ec53f94
fix lint
tyshko5 Aug 31, 2022
a9bcf06
requested changes
tyshko5 Aug 31, 2022
e40f143
compile time check
tyshko5 Sep 1, 2022
45dd497
from implementation
tyshko5 Sep 1, 2022
00af883
requested changes
tyshko5 Sep 1, 2022
70f458e
requested changes
tyshko5 Sep 1, 2022
c2c0883
requested changes
tyshko5 Sep 1, 2022
fa26316
requested changes
tyshko5 Sep 2, 2022
5c259bb
requested changes
tyshko5 Sep 2, 2022
2de70b6
delete Claus from deleg cns config
tyshko5 Sep 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions types/networks/src/calibnet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub const DEFAULT_BOOTSTRAP: &[&str] = &[
];

/// Height epochs.
pub const HEIGHT_INFOS: [HeightInfo; 18] = [
pub const HEIGHT_INFOS: [HeightInfo; 17] = [
HeightInfo {
height: Height::Breeze,
epoch: -1,
Expand Down Expand Up @@ -58,10 +58,6 @@ pub const HEIGHT_INFOS: [HeightInfo; 18] = [
height: Height::Orange,
epoch: 300,
},
HeightInfo {
height: Height::Claus,
epoch: 270,
},
HeightInfo {
height: Height::Trust,
epoch: 330,
Expand Down
143 changes: 59 additions & 84 deletions types/networks/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright 2019-2022 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

#[macro_use]
extern crate lazy_static;
lemmih marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -21,75 +20,8 @@ mod mainnet;
/// Newest network version for all networks
pub const NEWEST_NETWORK_VERSION: NetworkVersion = NetworkVersion::V16;

const UPGRADE_INFOS: [UpgradeInfo; 16] = [
UpgradeInfo {
height: Height::Breeze,
version: NetworkVersion::V1,
},
UpgradeInfo {
height: Height::Smoke,
version: NetworkVersion::V2,
},
UpgradeInfo {
height: Height::Ignition,
version: NetworkVersion::V3,
},
UpgradeInfo {
height: Height::ActorsV2,
version: NetworkVersion::V4,
},
UpgradeInfo {
height: Height::Tape,
version: NetworkVersion::V5,
},
UpgradeInfo {
height: Height::Kumquat,
version: NetworkVersion::V6,
},
UpgradeInfo {
height: Height::Calico,
version: NetworkVersion::V7,
},
UpgradeInfo {
height: Height::Persian,
version: NetworkVersion::V8,
},
UpgradeInfo {
height: Height::Orange,
version: NetworkVersion::V9,
},
UpgradeInfo {
height: Height::Trust,
version: NetworkVersion::V10,
},
UpgradeInfo {
height: Height::Norwegian,
version: NetworkVersion::V11,
},
UpgradeInfo {
height: Height::Turbo,
version: NetworkVersion::V12,
},
UpgradeInfo {
height: Height::Hyperdrive,
version: NetworkVersion::V13,
},
UpgradeInfo {
height: Height::Chocolate,
version: NetworkVersion::V14,
},
UpgradeInfo {
height: Height::OhSnap,
version: NetworkVersion::V15,
},
UpgradeInfo {
height: Height::Skyr,
version: NetworkVersion::V16,
},
];

/// Defines the meaningful heights of the protocol.
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)]
pub enum Height {
Breeze,
Smoke,
Expand All @@ -101,7 +33,6 @@ pub enum Height {
Calico,
Persian,
Orange,
Claus,
Trust,
Norwegian,
Turbo,
Expand All @@ -111,12 +42,56 @@ pub enum Height {
Skyr,
}

pub const HEIGHT_VEC: [Height; 17] = [
Height::Breeze,
Height::Smoke,
Height::Ignition,
Height::ActorsV2,
Height::Tape,
Height::Liftoff,
Height::Kumquat,
Height::Calico,
Height::Persian,
Height::Orange,
Height::Trust,
Height::Norwegian,
Height::Turbo,
Height::Hyperdrive,
Height::Chocolate,
Height::OhSnap,
Height::Skyr,
];

Copy link
Contributor

Choose a reason for hiding this comment

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

This is unused.

impl Default for Height {
fn default() -> Height {
Self::Breeze
}
}

impl From<Height> for NetworkVersion {
fn from(height: Height) -> NetworkVersion {
match height {
Height::Breeze => NetworkVersion::V0,
Height::Smoke => NetworkVersion::V1,
Height::Ignition => NetworkVersion::V2,
Height::ActorsV2 => NetworkVersion::V3,
Height::Tape => NetworkVersion::V4,
Copy link
Contributor

Choose a reason for hiding this comment

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

Bump everything such that Breeze is V1 and Tape is V5. See the deleted UPGRADE_INFOS for ground truth.

Height::Liftoff => NetworkVersion::V5,
lemmih marked this conversation as resolved.
Show resolved Hide resolved
Height::Kumquat => NetworkVersion::V6,
Height::Calico => NetworkVersion::V7,
Height::Persian => NetworkVersion::V8,
Height::Orange => NetworkVersion::V9,
Height::Trust => NetworkVersion::V10,
Height::Norwegian => NetworkVersion::V11,
Height::Turbo => NetworkVersion::V12,
Height::Hyperdrive => NetworkVersion::V13,
Height::Chocolate => NetworkVersion::V14,
Height::OhSnap => NetworkVersion::V15,
Height::Skyr => NetworkVersion::V16,
}
}
}

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
pub struct UpgradeInfo {
pub height: Height,
Expand All @@ -131,6 +106,11 @@ pub struct HeightInfo {
pub epoch: ChainEpoch,
}

pub fn sort_by_epoch(mut height_info_vec: Vec<HeightInfo>) -> Vec<HeightInfo> {
height_info_vec.sort_by(|a, b| a.epoch.cmp(&b.epoch));
height_info_vec
}

Copy link
Contributor

Choose a reason for hiding this comment

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

If this function takes a slice of HeightInfo (and turns it into a Vec) then the rest of the code will become simpler.

#[derive(Clone)]
struct DrandPoint<'a> {
pub height: ChainEpoch,
Expand All @@ -144,7 +124,6 @@ pub struct ChainConfig {
pub name: String,
pub bootstrap_peers: Vec<String>,
pub block_delay_secs: u64,
pub version_schedule: Vec<UpgradeInfo>,
pub height_infos: Vec<HeightInfo>,
#[serde(default = "default_policy")]
#[serde(with = "serde_policy")]
Expand All @@ -155,11 +134,12 @@ pub struct ChainConfig {
// https://github.com/filecoin-project/builtin-actors/pull/497
impl PartialEq for ChainConfig {
fn eq(&self, other: &Self) -> bool {
let height_infos = sort_by_epoch(self.height_infos.clone());
let other_height_infos = sort_by_epoch(other.height_infos.clone());
self.name == other.name
&& self.bootstrap_peers == other.bootstrap_peers
&& self.block_delay_secs == other.block_delay_secs
&& self.version_schedule == other.version_schedule
&& self.height_infos == other.height_infos
&& height_infos == other_height_infos
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
&& height_infos == other_height_infos
&& sort_by_epoch(&self.height_infos) == sort_by_epoch(&other.height_infos)

&& (self.policy.max_aggregated_sectors == other.policy.max_aggregated_sectors
&& self.policy.min_aggregated_sectors == other.policy.min_aggregated_sectors
&& self.policy.max_aggregated_proof_size == other.policy.max_aggregated_proof_size
Expand Down Expand Up @@ -224,7 +204,6 @@ impl ChainConfig {
name: "calibnet".to_string(),
bootstrap_peers: DEFAULT_BOOTSTRAP.iter().map(|x| x.to_string()).collect(),
block_delay_secs: EPOCH_DURATION_SECONDS as u64,
version_schedule: UPGRADE_INFOS.to_vec(),
height_infos: HEIGHT_INFOS.to_vec(),
policy: Policy {
valid_post_proof_type: HashSet::<RegisteredPoStProof>::from([
Expand All @@ -242,19 +221,15 @@ impl ChainConfig {
}

pub fn network_version(&self, epoch: ChainEpoch) -> NetworkVersion {
let height = self
.height_infos
let height_infos = sort_by_epoch(self.height_infos.clone());
let height = height_infos
.iter()
.rev()
.find(|info| epoch > info.epoch)
.map(|info| info.height)
.unwrap_or(Height::Breeze);

self.version_schedule
.iter()
.find(|info| height == info.height)
.map(|info| info.version)
.expect("A network version should exist even if not specified in the config (a default exists).")
From::from(height)
}

pub async fn get_beacon_schedule(
Expand All @@ -279,11 +254,12 @@ impl ChainConfig {
}

pub fn epoch(&self, height: Height) -> ChainEpoch {
lemmih marked this conversation as resolved.
Show resolved Hide resolved
self.height_infos
let height_infos = sort_by_epoch(self.height_infos.clone());
height_infos
.iter()
.find(|info| height == info.height)
.map(|info| info.epoch)
.expect("Internal error: Protocol height not found in map. Please report to https://github.com/ChainSafe/forest/issues")
.unwrap_or(0)
}

pub fn genesis_bytes(&self) -> Option<&[u8]> {
Expand All @@ -308,7 +284,6 @@ impl Default for ChainConfig {
name: "mainnet".to_string(),
bootstrap_peers: DEFAULT_BOOTSTRAP.iter().map(|x| x.to_string()).collect(),
block_delay_secs: EPOCH_DURATION_SECONDS as u64,
version_schedule: UPGRADE_INFOS.to_vec(),
height_infos: HEIGHT_INFOS.to_vec(),
policy: Policy {
valid_post_proof_type: HashSet::<RegisteredPoStProof>::from([
Expand Down
6 changes: 1 addition & 5 deletions types/networks/src/mainnet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub const DEFAULT_BOOTSTRAP: &[&str] = &[
];

/// Height epochs.
pub const HEIGHT_INFOS: [HeightInfo; 18] = [
pub const HEIGHT_INFOS: [HeightInfo; 17] = [
HeightInfo {
height: Height::Breeze,
epoch: 41_280,
Expand Down Expand Up @@ -73,10 +73,6 @@ pub const HEIGHT_INFOS: [HeightInfo; 18] = [
height: Height::Orange,
epoch: 336_458,
},
HeightInfo {
height: Height::Claus,
epoch: 343_200,
},
HeightInfo {
height: Height::Trust,
epoch: 550_321,
Expand Down
2 changes: 0 additions & 2 deletions vm/interpreter/src/vm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ pub trait LookbackStateGetter {
#[derive(Clone, Copy)]
pub struct Heights {
pub calico: ChainEpoch,
pub claus: ChainEpoch,
pub turbo: ChainEpoch,
pub hyperdrive: ChainEpoch,
pub chocolate: ChainEpoch,
Expand All @@ -80,7 +79,6 @@ impl Heights {
pub fn new(chain_config: &ChainConfig) -> Self {
Heights {
calico: chain_config.epoch(Height::Calico),
claus: chain_config.epoch(Height::Claus),
turbo: chain_config.epoch(Height::Turbo),
hyperdrive: chain_config.epoch(Height::Hyperdrive),
chocolate: chain_config.epoch(Height::Chocolate),
Expand Down