Add lcli tool for checking deposit data (#940)

* Add check-deposit-data tool

* Update help text

* Update function name

Cargo.lock

@@ -879,7 +879,7 @@ name = "deposit_contract"
 version = "0.1.0"
 dependencies = [
  "eth2_ssz 0.1.2",
- "ethabi 9.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethabi 11.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "reqwest 0.9.24 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)",
  "tree_hash 0.1.1",
@@ -1240,16 +1240,15 @@ dependencies = [
 
 [[package]]
 name = "ethabi"
-version = "9.0.1"
+version = "11.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "error-chain 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethereum-types 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-hex 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)",
- "serde_derive 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)",
  "tiny-keccak 1.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "uint 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -1839,6 +1838,7 @@ name = "lcli"
 version = "0.1.0"
 dependencies = [
  "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "deposit_contract 0.1.0",
  "dirs 2.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "environment 0.1.0",
  "eth1_test_rig 0.1.0",
@@ -1853,6 +1853,7 @@ dependencies = [
  "serde_yaml 0.8.11 (registry+https://github.com/rust-lang/crates.io-index)",
  "simple_logger 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "state_processing 0.1.0",
+ "tree_hash 0.1.1",
  "types 0.1.0",
  "web3 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -5226,8 +5227,8 @@ dependencies = [
 "checksum env_logger 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "aafcde04e90a5226a6443b7aabdb016ba2f8307c847d524724bd9b346dd1a2d3"
 "checksum env_logger 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)" = "44533bbbb3bb3c1fa17d9f2e4e38bbbaf8396ba82193c4cb1b6445d711445d36"
 "checksum error-chain 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)" = "3ab49e9dcb602294bc42f9a7dfc9bc6e936fca4418ea300dbfb84fe16de0b7d9"
+"checksum ethabi 11.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "97652a7d1f2504d6c885c87e242a06ccef5bd3054093d3fb742d8fb64806231a"
 "checksum ethabi 8.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ebdeeea85a6d217b9fcc862906d7e283c047e04114165c433756baf5dce00a6c"
-"checksum ethabi 9.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "965126c64662832991f5a748893577630b558e47fa94e7f35aefcd20d737cef7"
 "checksum ethbloom 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)" = "3932e82d64d347a045208924002930dc105a138995ccdc1479d0f05f0359f17c"
 "checksum ethbloom 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "32cfe1c169414b709cf28aa30c74060bdb830a03a8ba473314d079ac79d80a5f"
 "checksum ethereum-types 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "62d1bc682337e2c5ec98930853674dd2b4bd5d0d246933a9e98e5280f7c76c5f"

eth2/utils/deposit_contract/Cargo.toml

@@ -14,4 +14,4 @@ serde_json = "1.0"
 types = { path = "../../types"}
 eth2_ssz = { path = "../ssz"}
 tree_hash = { path = "../tree_hash"}
-ethabi = "9.0"
+ethabi = "11.0"

eth2/utils/deposit_contract/src/lib.rs

@@ -1,14 +1,31 @@
 use ethabi::{Contract, Token};
-use ssz::Encode;
+use ssz::{Decode, DecodeError as SszDecodeError, Encode};
 use tree_hash::TreeHash;
-use types::DepositData;
+use types::{DepositData, Hash256, PublicKeyBytes, SignatureBytes};
 
 pub use ethabi::Error;
 
+#[derive(Debug)]
+pub enum DecodeError {
+    EthabiError(ethabi::Error),
+    SszDecodeError(SszDecodeError),
+    MissingField,
+    UnableToGetBytes,
+    MissingToken,
+    InadequateBytes,
+}
+
+impl From<ethabi::Error> for DecodeError {
+    fn from(e: ethabi::Error) -> DecodeError {
+        DecodeError::EthabiError(e)
+    }
+}
+
 pub const CONTRACT_DEPLOY_GAS: usize = 4_000_000;
 pub const DEPOSIT_GAS: usize = 4_000_000;
 pub const ABI: &[u8] = include_bytes!("../contracts/v0.10.1_validator_registration.json");
 pub const BYTECODE: &[u8] = include_bytes!("../contracts/v0.10.1_validator_registration.bytecode");
+pub const DEPOSIT_DATA_LEN: usize = 420; // lol
 
 pub mod testnet {
     pub const ABI: &[u8] =
@@ -17,7 +34,7 @@ pub mod testnet {
         include_bytes!("../contracts/v0.10.1_testnet_validator_registration.bytecode");
 }
 
-pub fn eth1_tx_data(deposit_data: &DepositData) -> Result<Vec<u8>, Error> {
+pub fn encode_eth1_tx_data(deposit_data: &DepositData) -> Result<Vec<u8>, Error> {
     let params = vec![
         Token::Bytes(deposit_data.pubkey.as_ssz_bytes()),
         Token::Bytes(deposit_data.withdrawal_credentials.as_ssz_bytes()),
@@ -32,6 +49,40 @@ pub fn eth1_tx_data(deposit_data: &DepositData) -> Result<Vec<u8>, Error> {
     function.encode_input(&params)
 }
 
+pub fn decode_eth1_tx_data(
+    bytes: &[u8],
+    amount: u64,
+) -> Result<(DepositData, Hash256), DecodeError> {
+    let abi = Contract::load(ABI)?;
+    let function = abi.function("deposit")?;
+    let mut tokens =
+        function.decode_input(bytes.get(4..).ok_or_else(|| DecodeError::InadequateBytes)?)?;
+
+    macro_rules! decode_token {
+        ($type: ty, $to_fn: ident) => {
+            <$type>::from_ssz_bytes(
+                &tokens
+                    .pop()
+                    .ok_or_else(|| DecodeError::MissingToken)?
+                    .$to_fn()
+                    .ok_or_else(|| DecodeError::UnableToGetBytes)?,
+            )
+            .map_err(DecodeError::SszDecodeError)?
+        };
+    };
+
+    let root = decode_token!(Hash256, to_fixed_bytes);
+
+    let deposit_data = DepositData {
+        amount,
+        signature: decode_token!(SignatureBytes, to_bytes),
+        withdrawal_credentials: decode_token!(Hash256, to_bytes),
+        pubkey: decode_token!(PublicKeyBytes, to_bytes),
+    };
+
+    Ok((deposit_data, root))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -54,14 +105,27 @@ mod tests {
     }
 
     #[test]
-    fn basic() {
+    fn round_trip() {
         let spec = &E::default_spec();
 
         let keypair = generate_deterministic_keypair(42);
-        let deposit = get_deposit(keypair, spec);
+        let original = get_deposit(keypair, spec);
 
-        let data = eth1_tx_data(&deposit).expect("should produce tx data");
+        let data = encode_eth1_tx_data(&original).expect("should produce tx data");
 
-        assert_eq!(data.len(), 420, "bytes should be correct length");
+        assert_eq!(
+            data.len(),
+            DEPOSIT_DATA_LEN,
+            "bytes should be correct length"
+        );
+
+        let (decoded, root) = decode_eth1_tx_data(&data, original.amount).expect("should decode");
+
+        assert_eq!(decoded, original, "decoded should match original");
+        assert_eq!(
+            root,
+            original.tree_hash_root(),
+            "decode root should match original root"
+        );
     }
 }

lcli/Cargo.toml

@@ -25,3 +25,5 @@ web3 = "0.8.0"
 eth2_testnet_config = { path = "../eth2/utils/eth2_testnet_config" }
 dirs = "2.0"
 genesis = { path = "../beacon_node/genesis" }
+deposit_contract = { path = "../eth2/utils/deposit_contract" }
+tree_hash = { path = "../eth2/utils/tree_hash" }

lcli/src/check_deposit_data.rs

@@ -0,0 +1,31 @@
+use crate::helpers::{parse_hex_bytes, parse_u64};
+use clap::ArgMatches;
+use deposit_contract::{decode_eth1_tx_data, DEPOSIT_DATA_LEN};
+use tree_hash::TreeHash;
+use types::EthSpec;
+
+pub fn run<T: EthSpec>(matches: &ArgMatches) -> Result<(), String> {
+    let rlp_bytes = parse_hex_bytes(matches, "deposit-data")?;
+    let amount = parse_u64(matches, "deposit-amount")?;
+
+    if rlp_bytes.len() != DEPOSIT_DATA_LEN {
+        return Err(format!(
+            "The given deposit-data is {} bytes, expected {}",
+            rlp_bytes.len(),
+            DEPOSIT_DATA_LEN
+        ));
+    }
+
+    let (deposit_data, root) = decode_eth1_tx_data(&rlp_bytes, amount)
+        .map_err(|e| format!("Invalid deposit data bytes: {:?}", e))?;
+
+    let expected_root = deposit_data.tree_hash_root();
+    if root != expected_root {
+        return Err(format!(
+            "Deposit data root is invalid. Expected {:?}, but got {:?}. Perhaps the amount is incorrect?",
+            expected_root, root
+        ));
+    }
+
+    Ok(())
+}

lcli/src/helpers.rs

@@ -83,3 +83,16 @@ pub fn parse_fork_opt(matches: &ArgMatches, name: &'static str) -> Result<Option
         })
         .transpose()
 }
+
+pub fn parse_hex_bytes(matches: &ArgMatches, name: &'static str) -> Result<Vec<u8>, String> {
+    matches
+        .value_of(name)
+        .ok_or_else(|| format!("{} not specified", name))
+        .and_then(|val| {
+            if val.starts_with("0x") {
+                hex::decode(&val[2..]).map_err(|e| format!("Unable to parse {}: {:?}", name, e))
+            } else {
+                Err(format!("Unable to parse {}, must have 0x prefix", name))
+            }
+        })
+}

lcli/src/main.rs

@@ -2,6 +2,7 @@
 extern crate log;
 
 mod change_genesis_time;
+mod check_deposit_data;
 mod deploy_deposit_contract;
 mod eth1_genesis;
 mod helpers;
@@ -359,6 +360,29 @@ fn main() {
                               optimization for nodes, please do it."),
                 )
         )
+        .subcommand(
+            SubCommand::with_name("check-deposit-data")
+                .about(
+                    "Checks the integrity of some deposit data.",
+                )
+                .arg(
+                    Arg::with_name("deposit-amount")
+                        .index(1)
+                        .value_name("GWEI")
+                        .takes_value(true)
+                        .required(true)
+                        .help("The amount (in Gwei) that was deposited"),
+                )
+                .arg(
+                    Arg::with_name("deposit-data")
+                        .index(2)
+                        .value_name("HEX")
+                        .takes_value(true)
+                        .required(true)
+                        .help("A 0x-prefixed hex string of the deposit data. Should include the
+                            function signature."),
+                )
+        )
         .get_matches();
 
     macro_rules! run_with_spec {
@@ -445,6 +469,8 @@ fn run<T: EthSpec>(env_builder: EnvironmentBuilder<T>, matches: &ArgMatches) {
             .unwrap_or_else(|e| error!("Failed to run change-genesis-time command: {}", e)),
         ("new-testnet", Some(matches)) => new_testnet::run::<T>(matches)
             .unwrap_or_else(|e| error!("Failed to run new_testnet command: {}", e)),
+        ("check-deposit-data", Some(matches)) => check_deposit_data::run::<T>(matches)
+            .unwrap_or_else(|e| error!("Failed to run check-deposit-data command: {}", e)),
         (other, _) => error!("Unknown subcommand {}. See --help.", other),
     }
 }

tests/eth1_test_rig/src/lib.rs

@@ -7,7 +7,9 @@
 //! some initial issues.
 mod ganache;
 
-use deposit_contract::{eth1_tx_data, testnet, ABI, BYTECODE, CONTRACT_DEPLOY_GAS, DEPOSIT_GAS};
+use deposit_contract::{
+    encode_eth1_tx_data, testnet, ABI, BYTECODE, CONTRACT_DEPLOY_GAS, DEPOSIT_GAS,
+};
 use futures::{future, stream, Future, IntoFuture, Stream};
 use ganache::GanacheInstance;
 use std::time::{Duration, Instant};
@@ -201,12 +203,12 @@ impl DepositContract {
                     gas_price: None,
                     value: Some(from_gwei(deposit_data.amount)),
                     // Note: the reason we use this `TransactionRequest` instead of just using the
-                    // function in `self.contract` is so that the `eth1_tx_data` function gets used
+                    // function in `self.contract` is so that the `encode_eth1_tx_data` function gets used
                     // during testing.
                     //
-                    // It's important that `eth1_tx_data` stays correct and does not suffer from
+                    // It's important that `encode_eth1_tx_data` stays correct and does not suffer from
                     // code-rot.
-                    data: eth1_tx_data(&deposit_data).map(Into::into).ok(),
+                    data: encode_eth1_tx_data(&deposit_data).map(Into::into).ok(),
                     nonce: None,
                     condition: None,
                 };

validator_client/src/validator_directory.rs

@@ -1,5 +1,5 @@
 use bls::get_withdrawal_credentials;
-use deposit_contract::eth1_tx_data;
+use deposit_contract::encode_eth1_tx_data;
 use hex;
 use ssz::{Decode, Encode};
 use ssz_derive::{Decode, Encode};
@@ -275,7 +275,7 @@ impl ValidatorDirectoryBuilder {
 
             deposit_data.signature = deposit_data.create_signature(&voting_keypair.sk, &spec);
 
-            eth1_tx_data(&deposit_data)
+            encode_eth1_tx_data(&deposit_data)
                 .map_err(|e| format!("Unable to encode eth1 deposit tx data: {:?}", e))?
         };