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

[Merged by Bors] - Static testnet configs #1603

Closed
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
53 changes: 47 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 18 additions & 5 deletions common/eth2_config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,30 @@ impl Eth2Config {
/// Used by the `eth2_testnet_config` crate to initialize testnet directories during build and
/// access them at runtime.
#[derive(Copy, Clone, Debug, PartialEq)]
pub struct Eth2NetDirectory<'a> {
pub struct Eth2NetArchiveAndDirectory<'a> {
pub name: &'a str,
pub unique_id: &'a str,
pub archive_name: &'a str,
pub commit: &'a str,
pub url_template: &'a str,
Copy link
Member

Choose a reason for hiding this comment

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

I dont think we need the url_template anymore right?

pub genesis_is_known: bool,
}

impl<'a> Eth2NetDirectory<'a> {
impl<'a> Eth2NetArchiveAndDirectory<'a> {
/// The directory that should be used to store files downloaded for this net.
pub fn dir(&self) -> PathBuf {
fn pwd(&self) -> PathBuf {
env::var("CARGO_MANIFEST_DIR")
.expect("should know manifest dir")
.parse::<PathBuf>()
.expect("should parse manifest dir as path")
.join(self.unique_id)
}

pub fn dir(&self) -> PathBuf {
self.pwd().join(self.unique_id)
}

pub fn archive_fullpath(&self) -> PathBuf {
self.pwd().join(self.archive_name)
}
}

Expand All @@ -72,6 +80,10 @@ macro_rules! unique_id {
($name: tt, $commit: tt, $genesis_is_known: tt) => {
concat!("testnet_", $name, "_", $commit, "_", $genesis_is_known);
};

($name: tt, $commit: tt) => {
concat!("testnet_", $name, "_", $commit, ".zip");
};
}

macro_rules! define_net {
Expand All @@ -80,9 +92,10 @@ macro_rules! define_net {
pub mod $title {
use super::*;

pub const ETH2_NET_DIR: Eth2NetDirectory = Eth2NetDirectory {
pub const ETH2_NET_DIR: Eth2NetArchiveAndDirectory = Eth2NetArchiveAndDirectory {
name: $name,
unique_id: unique_id!($name, $commit, $genesis_is_known),
archive_name: unique_id!($name, $commit),
commit: $commit,
url_template: $url_template,
genesis_is_known: $genesis_is_known,
Expand Down
2 changes: 1 addition & 1 deletion common/eth2_testnet_config/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
testnet*
testnet*/*
schlesi-*
witti-*
altona*
3 changes: 2 additions & 1 deletion common/eth2_testnet_config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ reqwest = { version = "0.10.4", features = ["blocking", "native-tls-vendored"] }
eth2_config = { path = "../eth2_config"}
handlebars = "3.3.0"
serde_json = "1.0.56"
zip = "0.5"

[dev-dependencies]
tempdir = "0.3.7"
Expand All @@ -21,4 +22,4 @@ serde_yaml = "0.8.11"
types = { path = "../../consensus/types"}
enr = { version = "0.1.0", features = ["libsecp256k1", "ed25519"] }
eth2_ssz = "0.1.2"
eth2_config = { path = "../eth2_config"}
eth2_config = { path = "../eth2_config"}
Copy link
Member

Choose a reason for hiding this comment

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

Add a newline here

50 changes: 33 additions & 17 deletions common/eth2_testnet_config/build.rs
Original file line number Diff line number Diff line change
@@ -1,37 +1,53 @@
//! Downloads a testnet configuration from Github.

use eth2_config::{altona, medalla, Eth2NetDirectory};
use eth2_config::{altona, medalla, Eth2NetArchiveAndDirectory};
use handlebars::Handlebars;
use serde_json::json;
use std::fs;
use std::fs::File;
use std::io;
use std::io::Write;
use zip::ZipArchive;

const ETH2_NET_DIRS: &[Eth2NetDirectory<'static>] = &[altona::ETH2_NET_DIR, medalla::ETH2_NET_DIR];
const ETH2_NET_DIRS: &[Eth2NetArchiveAndDirectory<'static>] =
&[altona::ETH2_NET_DIR, medalla::ETH2_NET_DIR];

fn main() {
for testnet in ETH2_NET_DIRS {
let testnet_dir = testnet.dir();
let archive_fullpath = testnet.archive_fullpath();
println!("archive fullpath: {:?}", archive_fullpath);
Copy link
Member

Choose a reason for hiding this comment

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

I dont think we should print things during a build script


if !testnet_dir.exists() {
std::fs::create_dir_all(&testnet_dir)
.unwrap_or_else(|_| panic!("Unable to create {:?}", testnet_dir));

match get_all_files(testnet) {
Ok(()) => (),
Err(e) => {
std::fs::remove_dir_all(&testnet_dir).unwrap_or_else(|_| panic!(
"{}. Failed to remove {:?}, please remove the directory manually because it may contains incomplete testnet data.",
e,
testnet_dir,
));
panic!(e);
if !testnet_dir.exists() && archive_fullpath.exists() {
//uncompress archive and continue
let archive_file = File::open(&archive_fullpath).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use the unwrap_or_else() pattern here to give a descriptive error as to why the build failed.

uncompress(archive_file);
}
}
}

fn uncompress(archive_file: File) {
Copy link
Member

Choose a reason for hiding this comment

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

A general rule of thumb is to not use unwrap() in any working code in lighthouse (except for tests). We should always handle the errors and only panic as an absolute last resort. If we do have to panic, we should give a helpful description as to why.

For this function, you can handle the errors more ergonomically by letting it return a result such as Result<(), io::Error>. Depending on the error types being produces, maybe you want to return a String here and print it in an explicit panic if it fails.
The errors in the function can then either be mapped to a String via map_err() or potentially already use io::Error. Then you can replace the unwraps with ?.

let mut archive = ZipArchive::new(archive_file).unwrap();
for i in 0..archive.len() {
let mut file = archive.by_index(i).unwrap();
let outpath = file.sanitized_name();

if (file.name().ends_with('/')) {
fs::create_dir_all(&outpath).unwrap();
} else {
if let Some(p) = outpath.parent() {
if !p.exists() {
fs::create_dir_all(&p).unwrap();
}
}

let mut outfile = File::create(&outpath).unwrap();
io::copy(&mut file, &mut outfile).unwrap();
}
}
}

fn get_all_files(testnet: &Eth2NetDirectory<'static>) -> Result<(), String> {
fn get_all_files(testnet: &Eth2NetArchiveAndDirectory<'static>) -> Result<(), String> {
get_file(testnet, "boot_enr.yaml")?;
get_file(testnet, "config.yaml")?;
get_file(testnet, "deploy_block.txt")?;
Expand All @@ -46,7 +62,7 @@ fn get_all_files(testnet: &Eth2NetDirectory<'static>) -> Result<(), String> {
Ok(())
}

fn get_file(testnet: &Eth2NetDirectory, filename: &str) -> Result<(), String> {
fn get_file(testnet: &Eth2NetArchiveAndDirectory, filename: &str) -> Result<(), String> {
let url = Handlebars::new()
.render_template(
testnet.url_template,
Expand Down
Binary file not shown.
Binary file not shown.