From 3bab10bb68eb1b86f734bf969adfc277395950c6 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 18 Aug 2022 16:14:22 +1000 Subject: [PATCH] Pass test vectors --- Cargo.lock | 2 + validator_manager/Cargo.toml | 3 + .../src/validators/common/mod.rs | 108 +++++++++++++- .../src/validators/create_validators.rs | 136 ++++++++++++++++-- validator_manager/test_vectors/generate.py | 25 +++- 5 files changed, 247 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2126306328..899dcb09d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7370,6 +7370,8 @@ dependencies = [ "eth2_network_config", "eth2_serde_utils", "eth2_wallet", + "hex", + "regex", "serde", "serde_json", "tempfile", diff --git a/validator_manager/Cargo.toml b/validator_manager/Cargo.toml index d5dca7d53e..38cbbae669 100644 --- a/validator_manager/Cargo.toml +++ b/validator_manager/Cargo.toml @@ -20,7 +20,10 @@ serde_json = "1.0.58" eth2_serde_utils = "0.1.1" tree_hash = "0.4.1" eth2 = { path = "../common/eth2", features = ["lighthouse"]} +hex = "0.4.2" [dev-dependencies] tempfile = "3.1.0" tokio = { version = "1.14.0", features = ["time", "rt-multi-thread", "macros"] } +regex = "1.6.0" +eth2_network_config = { path = "../common/eth2_network_config" } diff --git a/validator_manager/src/validators/common/mod.rs b/validator_manager/src/validators/common/mod.rs index a910d46e05..0b4743aaf5 100644 --- a/validator_manager/src/validators/common/mod.rs +++ b/validator_manager/src/validators/common/mod.rs @@ -6,6 +6,15 @@ use std::path::PathBuf; use tree_hash::TreeHash; use types::*; +/// When the `ethereum/staking-deposit-cli` tool generates deposit data JSON, it adds a +/// `deposit_cli_version` to protect the web-based "Launchpad" tool against a breaking change that +/// was introduced in `ethereum/staking-deposit-cli`. Lighthouse don't really have a version that it +/// can use here, so we choose a static string that is: +/// +/// 1. High enough that it's accepted by Launchpad. +/// 2. Weird enough to identify Lighthouse. +const LIGHTHOUSE_DEPOSIT_CLI_VERSION: &str = "20.18.20"; + #[derive(Serialize, Deserialize)] pub struct ValidatorSpecification { pub voting_keystore: KeystoreJsonStr, @@ -35,16 +44,22 @@ pub struct CreateSpec { /// https://github.com/ethereum/staking-deposit-cli/blob/76ed78224fdfe3daca788d12442b3d1a37978296/staking_deposit/credentials.py#L131-L144 #[derive(Debug, PartialEq, Serialize, Deserialize)] pub struct StandardDepositDataJson { + #[serde(with = "public_key_bytes_without_0x_prefix")] pub pubkey: PublicKeyBytes, + #[serde(with = "hash256_without_0x_prefix")] pub withdrawal_credentials: Hash256, #[serde(with = "eth2_serde_utils::quoted_u64")] pub amount: u64, + #[serde(with = "signature_bytes_without_0x_prefix")] pub signature: SignatureBytes, - #[serde(with = "eth2_serde_utils::bytes_4_hex")] + #[serde(with = "bytes_4_without_0x_prefix")] pub fork_version: [u8; 4], - pub eth2_network_name: String, + pub network_name: String, + #[serde(with = "hash256_without_0x_prefix")] pub deposit_message_root: Hash256, + #[serde(with = "hash256_without_0x_prefix")] pub deposit_data_root: Hash256, + pub deposit_cli_version: String, } impl StandardDepositDataJson { @@ -65,8 +80,7 @@ impl StandardDepositDataJson { deposit_data }; - let domain = spec.get_deposit_domain(); - let deposit_message_root = deposit_data.as_deposit_message().signing_root(domain); + let deposit_message_root = deposit_data.as_deposit_message().tree_hash_root(); let deposit_data_root = deposit_data.tree_hash_root(); let DepositData { @@ -82,12 +96,96 @@ impl StandardDepositDataJson { amount, signature, fork_version: spec.genesis_fork_version, - eth2_network_name: spec + network_name: spec .config_name .clone() .ok_or("The network specification does not have a CONFIG_NAME set")?, deposit_message_root, deposit_data_root, + deposit_cli_version: LIGHTHOUSE_DEPOSIT_CLI_VERSION.to_string(), }) } } + +macro_rules! without_0x_prefix { + ($mod_name: ident, $type: ty) => { + pub mod $mod_name { + use super::*; + use std::str::FromStr; + + struct Visitor; + + impl<'de> serde::de::Visitor<'de> for Visitor { + type Value = $type; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("ascii hex without a 0x prefix") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + <$type>::from_str(&format!("0x{}", v)).map_err(serde::de::Error::custom) + } + } + + /// Serialize with quotes. + pub fn serialize(value: &$type, serializer: S) -> Result + where + S: serde::Serializer, + { + let with_prefix = format!("{:?}", value); + let without_prefix = with_prefix + .strip_prefix("0x") + .ok_or_else(|| serde::ser::Error::custom("serialization is missing 0x"))?; + serializer.serialize_str(&without_prefix) + } + + /// Deserialize with quotes. + pub fn deserialize<'de, D>(deserializer: D) -> Result<$type, D::Error> + where + D: serde::Deserializer<'de>, + { + deserializer.deserialize_str(Visitor) + } + } + }; +} + +without_0x_prefix!(hash256_without_0x_prefix, Hash256); +without_0x_prefix!(signature_bytes_without_0x_prefix, SignatureBytes); +without_0x_prefix!(public_key_bytes_without_0x_prefix, PublicKeyBytes); + +mod bytes_4_without_0x_prefix { + use serde::de::Error; + + const BYTES_LEN: usize = 4; + + pub fn serialize(bytes: &[u8; BYTES_LEN], serializer: S) -> Result + where + S: serde::Serializer, + { + let hex_string = &hex::encode(&bytes); + serializer.serialize_str(&hex_string) + } + + pub fn deserialize<'de, D>(deserializer: D) -> Result<[u8; BYTES_LEN], D::Error> + where + D: serde::Deserializer<'de>, + { + let decoded = deserializer.deserialize_str(eth2_serde_utils::hex::HexVisitor)?; + + if decoded.len() != BYTES_LEN { + return Err(D::Error::custom(format!( + "expected {} bytes for array, got {}", + BYTES_LEN, + decoded.len() + ))); + } + + let mut array = [0; BYTES_LEN]; + array.copy_from_slice(&decoded); + Ok(array) + } +} diff --git a/validator_manager/src/validators/create_validators.rs b/validator_manager/src/validators/create_validators.rs index c4dbea7df5..1ef870ac5e 100644 --- a/validator_manager/src/validators/create_validators.rs +++ b/validator_manager/src/validators/create_validators.rs @@ -517,12 +517,16 @@ fn write_to_json_file, S: Serialize>(path: P, contents: &S) -> Re #[cfg(test)] mod tests { use super::*; + use eth2_network_config::Eth2NetworkConfig; + use regex::Regex; use std::str::FromStr; use tempfile::{tempdir, TempDir}; use tree_hash::TreeHash; type E = MainnetEthSpec; + const TEST_VECTOR_DEPOSIT_CLI_VERSION: &str = "2.3.0"; + struct TestBuilder { spec: ChainSpec, output_dir: TempDir, @@ -532,7 +536,12 @@ mod tests { impl Default for TestBuilder { fn default() -> Self { - let spec = E::default_spec(); + Self::new(E::default_spec()) + } + } + + impl TestBuilder { + fn new(spec: ChainSpec) -> Self { let output_dir = tempdir().unwrap(); let mnemonic_dir = tempdir().unwrap(); let mnemonic_path = mnemonic_dir.path().join("mnemonic"); @@ -565,9 +574,7 @@ mod tests { config, } } - } - impl TestBuilder { fn mutate_config(mut self, func: F) -> Self { func(&mut self.config); self @@ -637,25 +644,23 @@ mod tests { ); } assert_eq!(deposit.amount, config.deposit_gwei); - let deposit_message_root = DepositData { + let deposit_message = DepositData { pubkey: deposit.pubkey, withdrawal_credentials: deposit.withdrawal_credentials, amount: deposit.amount, signature: SignatureBytes::empty(), } - .as_deposit_message() - .signing_root(spec.get_deposit_domain()); - assert!(deposit - .signature - .decompress() - .unwrap() - .verify(&validator_pubkey, deposit_message_root)); + .as_deposit_message(); + assert!(deposit.signature.decompress().unwrap().verify( + &validator_pubkey, + deposit_message.signing_root(spec.get_deposit_domain()) + )); assert_eq!(deposit.fork_version, spec.genesis_fork_version); + assert_eq!(&deposit.network_name, spec.config_name.as_ref().unwrap()); assert_eq!( - &deposit.eth2_network_name, - spec.config_name.as_ref().unwrap() + deposit.deposit_message_root, + deposit_message.tree_hash_root() ); - assert_eq!(deposit.deposit_message_root, deposit_message_root); assert_eq!( deposit.deposit_data_root, DepositData { @@ -668,7 +673,6 @@ mod tests { ); } } - // } // The directory containing the mnemonic can now be removed. @@ -766,4 +770,106 @@ mod tests { .await .assert_err(); } + + #[tokio::test] + async fn staking_deposit_cli_vectors() { + let vectors_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("test_vectors") + .join("vectors"); + for entry in fs::read_dir(vectors_dir).unwrap() { + let entry = entry.unwrap(); + let file_name = entry.file_name(); + let vector_name = file_name.to_str().unwrap(); + let path = entry.path(); + // Leave this `println!` so we can tell which test fails. + println!("Running test {}", vector_name); + run_test_vector(vector_name, &path).await; + } + } + + async fn run_test_vector>(name: &str, vectors_path: P) { + /* + * Parse the test vector name into a set of test parameters. + */ + let re = Regex::new(r"(.*)_(.*)_(.*)_(.*)_(.*)_(.*)_(.*)").unwrap(); + let capture = re.captures_iter(name).next().unwrap(); + let network = capture.get(1).unwrap().as_str(); + let first = u32::from_str(capture.get(3).unwrap().as_str()).unwrap(); + let count = u32::from_str(capture.get(5).unwrap().as_str()).unwrap(); + let uses_eth1 = bool::from_str(capture.get(7).unwrap().as_str()).unwrap(); + + /* + * Use the test parameters to generate equivalent files "locally" (i.e., with our code). + */ + + let spec = Eth2NetworkConfig::constant(network) + .unwrap() + .unwrap() + .chain_spec::() + .unwrap(); + + let test_result = TestBuilder::new(spec) + .mutate_config(|config| { + config.first_index = first; + config.count = count; + if uses_eth1 { + config.eth1_withdrawal_address = Some( + Address::from_str("0x0f51bb10119727a7e5ea3538074fb341f56b09ad").unwrap(), + ); + } + }) + .run_test() + .await; + let TestResult { result, output_dir } = test_result; + result.expect("local generation should succeed"); + + /* + * Ensure the deposit data is identical when parsed as JSON. + */ + + let local_deposits = { + let path = output_dir.path().join(DEPOSITS_FILENAME); + let contents = fs::read_to_string(&path).unwrap(); + let mut deposits: Vec = + serde_json::from_str(&contents).unwrap(); + for deposit in &mut deposits { + // Ensures we can match test vectors. + deposit.deposit_cli_version = TEST_VECTOR_DEPOSIT_CLI_VERSION.to_string(); + + // We use "prater" and the vectors use "goerli" now. The two names refer to the same + // network so there should be no issue here. + if deposit.network_name == "prater" { + deposit.network_name = "goerli".to_string(); + } + } + deposits + }; + let vector_deposits: Vec = { + let path = fs::read_dir(vectors_path.as_ref().join("validator_keys")) + .unwrap() + .find_map(|entry| { + let entry = entry.unwrap(); + let file_name = entry.file_name(); + if file_name.to_str().unwrap().starts_with("deposit_data") { + Some(entry.path()) + } else { + None + } + }) + .unwrap(); + let contents = fs::read_to_string(&path).unwrap(); + serde_json::from_str(&contents).unwrap() + }; + + assert_eq!(local_deposits, vector_deposits); + + /* + * Note: we don't check the keystores generated by the deposit-cli since there is little + * value in this. + * + * If we check the deposits then we are verifying the signature across the deposit message. + * This implicitly verifies that the keypair generated by the deposit-cli is identical to + * the one created by Lighthouse. + */ + } } diff --git a/validator_manager/test_vectors/generate.py b/validator_manager/test_vectors/generate.py index f9dfa5ed84..45f73fb7b4 100644 --- a/validator_manager/test_vectors/generate.py +++ b/validator_manager/test_vectors/generate.py @@ -58,13 +58,20 @@ def setup_sdc(): ], cwd=sdc_git_dir) assert(result.returncode == 0) -def sdc_generate(network, first_index, count): - test_name = "{}_first_{}_count_{}".format(network, first_index, count) +def sdc_generate(network, first_index, count, eth1_withdrawal_address=None): + if eth1_withdrawal_address is not None: + eth1_flags = ['--eth1_withdrawal_address', eth1_withdrawal_address] + uses_eth1 = True + else: + eth1_flags = [] + uses_eth1 = False + + test_name = "{}_first_{}_count_{}_eth1_{}".format(network, first_index, count, + str(uses_eth1).lower()) output_dir = os.path.join(vectors_dir, test_name) os.mkdir(output_dir) - print("Running " + test_name) - process = Popen([ + command = [ '/bin/sh', 'deposit.sh', '--language', 'english', @@ -73,10 +80,13 @@ def sdc_generate(network, first_index, count): '--validator_start_index', str(first_index), '--num_validators', str(count), '--mnemonic', TEST_MNEMONIC, - '--chain', 'mainnet', + '--chain', network, '--keystore_password', 'MyPassword', '--folder', os.path.abspath(output_dir), - ], cwd=sdc_git_dir, text=True, stdin = PIPE) + ] + eth1_flags + + print("Running " + test_name) + process = Popen(command, cwd=sdc_git_dir, text=True, stdin = PIPE) process.wait() @@ -84,10 +94,11 @@ def sdc_generate(network, first_index, count): def test_network(network): sdc_generate(network, first_index=0, count=1) sdc_generate(network, first_index=0, count=2) - sdc_generate(network, first_index=0, count=3) sdc_generate(network, first_index=12, count=1) sdc_generate(network, first_index=99, count=2) sdc_generate(network, first_index=1024, count=3) + sdc_generate(network, first_index=0, count=2, + eth1_withdrawal_address="0x0f51bb10119727a7e5ea3538074fb341f56b09ad") setup()