Pass test vectors

Cargo.lock

@@ -7370,6 +7370,8 @@ dependencies = [
  "eth2_network_config",
  "eth2_serde_utils",
  "eth2_wallet",
+ "hex",
+ "regex",
  "serde",
  "serde_json",
  "tempfile",

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" }

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().tree_hash_root();
-        let deposit_message_root = deposit_data.as_deposit_message().signing_root(domain);
         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<E>(self, v: &str) -> Result<Self::Value, E>
+                where
+                    E: serde::de::Error,
+                {
+                    <$type>::from_str(&format!("0x{}", v)).map_err(serde::de::Error::custom)
+                }
+            }
+
+            /// Serialize with quotes.
+            pub fn serialize<S>(value: &$type, serializer: S) -> Result<S::Ok, S::Error>
+            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<S>(bytes: &[u8; BYTES_LEN], serializer: S) -> Result<S::Ok, S::Error>
+    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)
+    }
+}

validator_manager/src/validators/create_validators.rs

@@ -517,12 +517,16 @@ fn write_to_json_file<P: AsRef<Path>, 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<F: Fn(&mut CreateConfig)>(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()
+                        .as_deposit_message();
-                        .signing_root(spec.get_deposit_domain());
+                        assert!(deposit.signature.decompress().unwrap().verify(
-                        assert!(deposit
+                            &validator_pubkey,
-                            .signature
+                            deposit_message.signing_root(spec.get_deposit_domain())
-                            .decompress()
+                        ));
-                            .unwrap()
-                            .verify(&validator_pubkey, deposit_message_root));
                         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,
+                            deposit.deposit_message_root,
-                            spec.config_name.as_ref().unwrap()
+                            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<P: AsRef<Path>>(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::<E>()
+            .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<StandardDepositDataJson> =
+                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<StandardDepositDataJson> = {
+            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.
+         */
+    }
 }

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):
+def sdc_generate(network, first_index, count, eth1_withdrawal_address=None):
-    test_name = "{}_first_{}_count_{}".format(network, first_index, count)
+    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)
+    command = [
-    process = Popen([
         '/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()