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

refactor: move duplicate utils to test utils crate #127

Merged
merged 1 commit into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions simulators/history/portal-interop/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2021"

[dependencies]
ethportal-api = { git = "https://github.com/ethereum/trin", rev = "2a32224e3c2b0b80bc37c1b692c33016371f197a" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is portal-hive tagged to a particular hash of ethportal-api? If possible I think it's much more preferable to tag to the version

Copy link
Member Author

Choose a reason for hiding this comment

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

Because ethportal-api has broken in the past and has broken our builds.

Sure that would be a fine change to make in an upcoming PR

Copy link
Member Author

Choose a reason for hiding this comment

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

I will make an issue for it

portal-spec-test-utils-rs = { git = "https://github.com/ethereum/portal-spec-tests", rev = "d1e996d0d4dc2136b3cd38d9e25cdc3a6b74dcd9" }
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious, but is storing these constants in a separate repository necessary? Seems to me like we'd like to store them in this repo, to make it easier to add/remove/edit constants, rather than juggling them between multiple repositories. Is there any reason why we couldn't just create a "utils" workspace inside portal-hive?

Copy link
Contributor

Choose a reason for hiding this comment

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

@KolbyML I'm still curious about this question

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we can store it in the simulator folder I will check

Copy link
Contributor

@njgheorghita njgheorghita Jan 19, 2024

Choose a reason for hiding this comment

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

Honestly, I'm using these constants right now in some scripting work I'm doing in trin, so maybe we just store them in trin-utils trin_validation::constants?

hivesim = { path = "../../../hivesim-rs" }
itertools = "0.10.5"
serde_json = "1.0.87"
Expand Down
10 changes: 0 additions & 10 deletions simulators/history/portal-interop/src/constants.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1 @@
// Execution Layer hard forks https://ethereum.org/en/history/
pub const SHANGHAI_BLOCK_NUMBER: u64 = 17034870;
pub const MERGE_BLOCK_NUMBER: u64 = 15537394;
pub const LONDON_BLOCK_NUMBER: u64 = 12965000;
pub const BERLIN_BLOCK_NUMBER: u64 = 12244000;
pub const ISTANBUL_BLOCK_NUMBER: u64 = 9069000;
pub const CONSTANTINOPLE_BLOCK_NUMBER: u64 = 7280000;
pub const BYZANTIUM_BLOCK_NUMBER: u64 = 4370000;
pub const HOMESTEAD_BLOCK_NUMBER: u64 = 1150000;

pub const TEST_DATA_FILE_PATH: &str = "./test-data/test_data_collection_of_forks_blocks.yaml";
29 changes: 2 additions & 27 deletions simulators/history/portal-interop/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
mod constants;

use crate::constants::{
BERLIN_BLOCK_NUMBER, BYZANTIUM_BLOCK_NUMBER, CONSTANTINOPLE_BLOCK_NUMBER,
HOMESTEAD_BLOCK_NUMBER, ISTANBUL_BLOCK_NUMBER, LONDON_BLOCK_NUMBER, MERGE_BLOCK_NUMBER,
SHANGHAI_BLOCK_NUMBER, TEST_DATA_FILE_PATH,
};
use crate::constants::TEST_DATA_FILE_PATH;
use ethportal_api::types::portal::ContentInfo;
use ethportal_api::utils::bytes::hex_encode;
use ethportal_api::{
Expand All @@ -15,6 +11,7 @@ use hivesim::{
dyn_async, Client, NClientTestSpec, Simulation, Suite, Test, TestSpec, TwoClientTestSpec,
};
use itertools::Itertools;
use portal_spec_test_utils_rs::get_flair;
use serde_json::json;
use serde_yaml::Value;
use tokio::time::Duration;
Expand Down Expand Up @@ -130,28 +127,6 @@ fn process_content(
result
}

fn get_flair(block_number: u64) -> String {
if block_number > SHANGHAI_BLOCK_NUMBER {
" (post-shanghai)".to_string()
} else if block_number > MERGE_BLOCK_NUMBER {
" (post-merge)".to_string()
} else if block_number > LONDON_BLOCK_NUMBER {
" (post-london)".to_string()
} else if block_number > BERLIN_BLOCK_NUMBER {
" (post-berlin)".to_string()
} else if block_number > ISTANBUL_BLOCK_NUMBER {
" (post-istanbul)".to_string()
} else if block_number > CONSTANTINOPLE_BLOCK_NUMBER {
" (post-constantinople)".to_string()
} else if block_number > BYZANTIUM_BLOCK_NUMBER {
" (post-byzantium)".to_string()
} else if block_number > HOMESTEAD_BLOCK_NUMBER {
" (post-homestead)".to_string()
} else {
"".to_string()
}
}

dyn_async! {
async fn test_portal_interop<'a> (test: &'a mut Test, _client: Option<Client>) {
// Get all available portal clients
Expand Down
1 change: 1 addition & 0 deletions simulators/history/trin-bridge/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2021"

[dependencies]
ethportal-api = { git = "https://github.com/ethereum/trin", rev = "b530dc3d40d51c21c800089eacb2852ebd8c4d45" }
portal-spec-test-utils-rs = { git = "https://github.com/ethereum/portal-spec-tests", rev = "d1e996d0d4dc2136b3cd38d9e25cdc3a6b74dcd9" }
hivesim = { path = "../../../hivesim-rs" }
itertools = "0.10.5"
serde_yaml = "0.9"
Expand Down
10 changes: 0 additions & 10 deletions simulators/history/trin-bridge/src/constants.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
// Execution Layer hard forks https://ethereum.org/en/history/
pub const SHANGHAI_BLOCK_NUMBER: u64 = 17034870;
pub const MERGE_BLOCK_NUMBER: u64 = 15537394;
pub const LONDON_BLOCK_NUMBER: u64 = 12965000;
pub const BERLIN_BLOCK_NUMBER: u64 = 12244000;
pub const ISTANBUL_BLOCK_NUMBER: u64 = 9069000;
pub const CONSTANTINOPLE_BLOCK_NUMBER: u64 = 7280000;
pub const BYZANTIUM_BLOCK_NUMBER: u64 = 4370000;
pub const HOMESTEAD_BLOCK_NUMBER: u64 = 1150000;

pub const TRIN_BRIDGE_CLIENT_TYPE: &str = "trin-bridge";
pub const BOOTNODES_ENVIRONMENT_VARIABLE: &str = "HIVE_BOOTNODES";
pub const HIVE_CHECK_LIVE_PORT: &str = "HIVE_CHECK_LIVE_PORT";
Expand Down
29 changes: 3 additions & 26 deletions simulators/history/trin-bridge/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
mod constants;

use crate::constants::{
BERLIN_BLOCK_NUMBER, BOOTNODES_ENVIRONMENT_VARIABLE, BYZANTIUM_BLOCK_NUMBER,
CONSTANTINOPLE_BLOCK_NUMBER, HIVE_CHECK_LIVE_PORT, HOMESTEAD_BLOCK_NUMBER,
ISTANBUL_BLOCK_NUMBER, LONDON_BLOCK_NUMBER, MERGE_BLOCK_NUMBER, SHANGHAI_BLOCK_NUMBER,
TEST_DATA_FILE_PATH, TRIN_BRIDGE_CLIENT_TYPE,
BOOTNODES_ENVIRONMENT_VARIABLE, HIVE_CHECK_LIVE_PORT, TEST_DATA_FILE_PATH,
TRIN_BRIDGE_CLIENT_TYPE,
};
use ethportal_api::HistoryContentKey;
use ethportal_api::HistoryContentValue;
Expand All @@ -13,32 +11,11 @@ use ethportal_api::{Discv5ApiClient, HistoryNetworkApiClient};
use hivesim::types::ClientDefinition;
use hivesim::{dyn_async, Client, Simulation, Suite, Test, TestSpec, TwoClientTestSpec};
use itertools::Itertools;
use portal_spec_test_utils_rs::get_flair;
use serde_yaml::Value;
use std::collections::HashMap;
use tokio::time::Duration;

fn get_flair(block_number: u64) -> String {
if block_number > SHANGHAI_BLOCK_NUMBER {
" (post-shanghai)".to_string()
} else if block_number > MERGE_BLOCK_NUMBER {
" (post-merge)".to_string()
} else if block_number > LONDON_BLOCK_NUMBER {
" (post-london)".to_string()
} else if block_number > BERLIN_BLOCK_NUMBER {
" (post-berlin)".to_string()
} else if block_number > ISTANBUL_BLOCK_NUMBER {
" (post-istanbul)".to_string()
} else if block_number > CONSTANTINOPLE_BLOCK_NUMBER {
" (post-constantinople)".to_string()
} else if block_number > BYZANTIUM_BLOCK_NUMBER {
" (post-byzantium)".to_string()
} else if block_number > HOMESTEAD_BLOCK_NUMBER {
" (post-homestead)".to_string()
} else {
"".to_string()
}
}

fn process_content(content: Vec<(HistoryContentKey, HistoryContentValue)>) -> Vec<String> {
let mut last_header = content.get(0).unwrap().clone();

Expand Down