[Merged by Bors] - Minify slashing protection interchange data

account_manager/src/validator/slashing_protection.rs

@@ -1,7 +1,7 @@
 use clap::{App, Arg, ArgMatches};
 use environment::Environment;
 use slashing_protection::{
-    interchange::Interchange, InterchangeImportOutcome, SlashingDatabase,
+    interchange::Interchange, InterchangeError, InterchangeImportOutcome, SlashingDatabase,
     SLASHING_PROTECTION_FILENAME,
 };
 use std::fs::File;
@@ -15,6 +15,8 @@ pub const EXPORT_CMD: &str = "export";
 pub const IMPORT_FILE_ARG: &str = "IMPORT-FILE";
 pub const EXPORT_FILE_ARG: &str = "EXPORT-FILE";
 
+pub const MINIFY_FLAG: &str = "minify";
+
 pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
     App::new(CMD)
         .about("Import or export slashing protection data to or from another client")
@@ -26,6 +28,17 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                         .takes_value(true)
                         .value_name("FILE")
                         .help("The slashing protection interchange file to import (.json)"),
+                )
+                .arg(
+                    Arg::with_name(MINIFY_FLAG)
+                        .long(MINIFY_FLAG)
+                        .takes_value(true)
+                        .default_value("true")
+                        .possible_values(&["false", "true"])
+                        .help(
+                            "Minify the input file before processing. This is *much* faster, \
+                             but will not detect slashable data in the input.",
+                        ),
                 ),
         )
         .subcommand(
@@ -36,6 +49,17 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                         .takes_value(true)
                         .value_name("FILE")
                         .help("The filename to export the interchange file to"),
+                )
+                .arg(
+                    Arg::with_name(MINIFY_FLAG)
+                        .long(MINIFY_FLAG)
+                        .takes_value(true)
+                        .default_value("false")
+                        .possible_values(&["false", "true"])
+                        .help(
+                            "Minify the output file. This will make it smaller and faster to \
+                             import, but not faster to generate.",
+                        ),
                 ),
         )
 }
@@ -64,6 +88,7 @@ pub fn cli_run<T: EthSpec>(
     match matches.subcommand() {
         (IMPORT_CMD, Some(matches)) => {
             let import_filename: PathBuf = clap_utils::parse_required(&matches, IMPORT_FILE_ARG)?;
+            let minify: bool = clap_utils::parse_required(&matches, MINIFY_FLAG)?;
             let import_file = File::open(&import_filename).map_err(|e| {
                 format!(
                     "Unable to open import file at {}: {:?}",
@@ -72,8 +97,18 @@ pub fn cli_run<T: EthSpec>(
                 )
             })?;
 
-            let interchange = Interchange::from_json_reader(&import_file)
+            eprint!("Loading JSON file into memory & deserializing");
+            let mut interchange = Interchange::from_json_reader(&import_file)
                 .map_err(|e| format!("Error parsing file for import: {:?}", e))?;
+            eprintln!(" [done].");
+
+            if minify {
+                eprint!("Minifying input file for faster loading");
+                interchange = interchange
+                    .minify()
+                    .map_err(|e| format!("Minification failed: {:?}", e))?;
+                eprintln!(" [done].");
+            }
 
             let slashing_protection_database =
                 SlashingDatabase::open_or_create(&slashing_protection_db_path).map_err(|e| {
@@ -84,16 +119,6 @@ pub fn cli_run<T: EthSpec>(
                     )
                 })?;
 
-            let outcomes = slashing_protection_database
-                .import_interchange_info(interchange, genesis_validators_root)
-                .map_err(|e| {
-                    format!(
-                        "Error during import: {:?}\n\
-                         IT IS NOT SAFE TO START VALIDATING",
-                        e
-                    )
-                })?;
-
             let display_slot = |slot: Option<Slot>| {
                 slot.map_or("none".to_string(), |slot| format!("{}", slot.as_u64()))
             };
@@ -105,48 +130,77 @@ pub fn cli_run<T: EthSpec>(
                 (source, target) => format!("{}=>{}", display_epoch(source), display_epoch(target)),
             };
 
-            let mut num_failed = 0;
-
-            for outcome in &outcomes {
-                match outcome {
-                    InterchangeImportOutcome::Success { pubkey, summary } => {
-                        eprintln!("- {:?} SUCCESS min block: {}, max block: {}, min attestation: {}, max attestation: {}",
-                            pubkey,
-                            display_slot(summary.min_block_slot),
-                            display_slot(summary.max_block_slot),
-                            display_attestation(summary.min_attestation_source, summary.min_attestation_target),
-                            display_attestation(summary.max_attestation_source,
-                            summary.max_attestation_target),
-                        );
+            match slashing_protection_database
+                .import_interchange_info(interchange, genesis_validators_root)
+            {
+                Ok(outcomes) => {
+                    eprintln!("All records imported successfully:");
+                    for outcome in &outcomes {
+                        match outcome {
+                            InterchangeImportOutcome::Success { pubkey, summary } => {
+                                eprintln!("- {:?}", pubkey);
+                                eprintln!(
+                                    "    - min block: {}",
+                                    display_slot(summary.min_block_slot)
+                                );
+                                eprintln!(
+                                    "    - min attestation: {}",
+                                    display_attestation(
+                                        summary.min_attestation_source,
+                                        summary.min_attestation_target
+                                    )
+                                );
+                                eprintln!(
+                                    "    - max attestation: {}",
+                                    display_attestation(
+                                        summary.max_attestation_source,
+                                        summary.max_attestation_target
+                                    )
+                                );
+                            }
+                            InterchangeImportOutcome::Failure { pubkey, error } => {
+                                panic!(
+                                    "import should be atomic, but key {:?} was imported despite error: {:?}",
+                                    pubkey, error
+                                );
+                            }
+                        }
                     }
-                    InterchangeImportOutcome::Failure { pubkey, error } => {
-                        eprintln!("- {:?} ERROR: {:?}", pubkey, error);
-                        num_failed += 1;
+                }
+                Err(InterchangeError::AtomicBatchAborted(outcomes)) => {
+                    eprintln!("ERROR, slashable data in input:");
+                    for outcome in &outcomes {
+                        if let InterchangeImportOutcome::Failure { pubkey, error } = outcome {
+                            eprintln!("- {:?}", pubkey);
+                            eprintln!("    - error: {:?}", error);
+                        }
                     }
+                    return Err(
+                        "ERROR: import aborted due to slashable data, see above.\n\
+                         Please see https://lighthouse-book.sigmaprime.io/slashing-protection.html#slashable-data-in-import\n\
+                         IT IS NOT SAFE TO START VALIDATING".to_string()
+                    );
+                }
+                Err(e) => {
+                    return Err(format!(
+                        "Fatal error during import: {:?}\n\
+                         IT IS NOT SAFE TO START VALIDATING",
+                        e
+                    ));
                 }
             }
 
-            if num_failed == 0 {
-                eprintln!("Import completed successfully.");
-                eprintln!(
-                    "Please double-check that the minimum and maximum blocks and slots above \
-                     match your expectations."
-                );
-            } else {
-                eprintln!(
-                    "WARNING: history was NOT imported for {} of {} records",
-                    num_failed,
-                    outcomes.len()
-                );
-                eprintln!("IT IS NOT SAFE TO START VALIDATING");
-                eprintln!("Please see https://lighthouse-book.sigmaprime.io/slashing-protection.html#slashable-data-in-import");
-                return Err("Partial import".to_string());
-            }
+            eprintln!("Import completed successfully.");
+            eprintln!(
+                "Please double-check that the minimum and maximum blocks and attestations above \
+                 match your expectations."
+            );
 
             Ok(())
         }
         (EXPORT_CMD, Some(matches)) => {
             let export_filename: PathBuf = clap_utils::parse_required(&matches, EXPORT_FILE_ARG)?;
+            let minify: bool = clap_utils::parse_required(&matches, MINIFY_FLAG)?;
 
             if !slashing_protection_db_path.exists() {
                 return Err(format!(
@@ -164,10 +218,17 @@ pub fn cli_run<T: EthSpec>(
                     )
                 })?;
 
-            let interchange = slashing_protection_database
+            let mut interchange = slashing_protection_database
                 .export_interchange_info(genesis_validators_root)
                 .map_err(|e| format!("Error during export: {:?}", e))?;
 
+            if minify {
+                eprintln!("Minifying output file");
+                interchange = interchange
+                    .minify()
+                    .map_err(|e| format!("Unable to minify output: {:?}", e))?;
+            }
+
             let output_file = File::create(export_filename)
                 .map_err(|e| format!("Error creating output file: {:?}", e))?;
 

book/src/slashing-protection.md

@@ -91,6 +91,32 @@ up to date.
 
 [EIP-3076]: https://eips.ethereum.org/EIPS/eip-3076
 
+### Minification
+
+Since version 1.5.0 Lighthouse automatically _minifies_ slashing protection data upon import.
+Minification safely shrinks the input file, making it faster to import.
+
+If an import file contains slashable data, then its minification is still safe to import _even
+though_ the non-minified file would fail to be imported. This means that leaving minification
+enabled is recommended if the input could contain slashable data. Conversely, if you would like to
+double-check that the input file is not slashable with respect to itself, then you should disable
+minification.
+
+Minification can be disabled for imports by adding `--minify=false` to the command:
+
+```
+lighthouse account validator slashing-protection import --minify=false <my_interchange.json>
+```
+
+It can also be enabled for exports (disabled by default):
+
+```
+lighthouse account validator slashing-protection export --minify=true <lighthouse_interchange.json>
+```
+
+Minifying the export file should make it faster to import, and may allow it to be imported into an
+implementation that is rejecting the non-minified equivalent due to slashable data.
+
 ## Troubleshooting
 
 ### Misplaced Slashing Database
@@ -137,11 +163,11 @@ and _could_ indicate a serious error or misconfiguration (see [Avoiding Slashing
 
 ### Slashable Data in Import
 
-If you receive a warning when trying to import an [interchange file](#import-and-export) about
+During import of an [interchange file](#import-and-export) if you receive an error about
 the file containing slashable data, then you must carefully consider whether you want to continue.
 
-There are several potential causes for this warning, each of which require a different reaction. If
-you have seen the warning for multiple validator keys, the cause could be different for each of them.
+There are several potential causes for this error, each of which require a different reaction. If
+the error output lists multiple validator keys, the cause could be different for each of them.
 
 1. Your validator has actually signed slashable data. If this is the case, you should assess
    whether your validator has been slashed (or is likely to be slashed). It's up to you
@@ -156,6 +182,9 @@ you have seen the warning for multiple validator keys, the cause could be differ
    It might be safe to continue as-is, or you could consider a [Drop and
    Re-import](#drop-and-re-import).
 
+If you are running the import command with `--minify=false`, you should consider enabling
+[minification](#minification).
+
 #### Drop and Re-import
 
 If you'd like to prioritize an interchange file over any existing database stored by Lighthouse

validator_client/slashing_protection/Cargo.toml

@@ -19,4 +19,5 @@ serde_utils = { path = "../../consensus/serde_utils" }
 filesystem = { path = "../../common/filesystem" }
 
 [dev-dependencies]
+lazy_static = "1.4.0"
 rayon = "1.4.1"

validator_client/slashing_protection/Makefile

@@ -1,4 +1,4 @@
-TESTS_TAG := f495032df9c26c678536cd2b7854e836ea94c217
+TESTS_TAG := v5.1.0-alpha.1
 GENERATE_DIR := generated-tests
 OUTPUT_DIR := interchange-tests
 TARBALL := $(OUTPUT_DIR)-$(TESTS_TAG).tar.gz

validator_client/slashing_protection/src/bin/test_generator.rs

@@ -224,10 +224,37 @@ fn main() {
                     .with_blocks(vec![(0, 20, false)]),
             ],
         ),
+        MultiTestCase::new(
+            "multiple_interchanges_single_validator_fail_iff_imported",
+            vec![
+                TestCase::new(interchange(vec![(0, vec![40], vec![])])),
+                TestCase::new(interchange(vec![(0, vec![20, 50], vec![])]))
+                    .allow_partial_import()
+                    .with_blocks(vec![(0, 20, false), (0, 50, false)]),
+            ],
+        ),
         MultiTestCase::single(
             "single_validator_source_greater_than_target",
             TestCase::new(interchange(vec![(0, vec![], vec![(8, 7)])])).allow_partial_import(),
         ),
+        MultiTestCase::single(
+            "single_validator_source_greater_than_target_surrounding",
+            TestCase::new(interchange(vec![(0, vec![], vec![(5, 2)])]))
+                .allow_partial_import()
+                .with_attestations(vec![(0, 3, 4, false)]),
+        ),
+        MultiTestCase::single(
+            "single_validator_source_greater_than_target_surrounded",
+            TestCase::new(interchange(vec![(0, vec![], vec![(5, 2)])]))
+                .allow_partial_import()
+                .with_attestations(vec![(0, 6, 1, false)]),
+        ),
+        MultiTestCase::single(
+            "single_validator_source_greater_than_target_sensible_iff_minified",
+            TestCase::new(interchange(vec![(0, vec![], vec![(5, 2), (6, 7)])]))
+                .allow_partial_import()
+                .with_attestations(vec![(0, 5, 8, false), (0, 6, 8, true)]),
+        ),
         MultiTestCase::single(
             "single_validator_out_of_order_blocks",
             TestCase::new(interchange(vec![(0, vec![6, 5], vec![])])).with_blocks(vec![
@@ -338,14 +365,42 @@ fn main() {
             .with_blocks(vec![(0, 10, false), (0, 13, false), (0, 14, true)])
             .with_attestations(vec![(0, 0, 2, false), (0, 1, 3, false)]),
         ),
+        MultiTestCase::single(
+            "duplicate_pubkey_slashable_block",
+            TestCase::new(interchange(vec![
+                (0, vec![10], vec![(0, 2)]),
+                (0, vec![10], vec![(1, 3)]),
+            ]))
+            .allow_partial_import()
+            .with_blocks(vec![(0, 10, false), (0, 11, true)]),
+        ),
+        MultiTestCase::single(
+            "duplicate_pubkey_slashable_attestation",
+            TestCase::new(interchange_with_signing_roots(vec![
+                (0, vec![], vec![(0, 3, Some(3))]),
+                (0, vec![], vec![(1, 2, None)]),
+            ]))
+            .allow_partial_import()
+            .with_attestations(vec![
+                (0, 0, 1, false),
+                (0, 0, 2, false),
+                (0, 0, 4, false),
+                (0, 1, 4, true),
+            ]),
+        ),
     ];
 
     let args = std::env::args().collect::<Vec<_>>();
     let output_dir = Path::new(&args[1]);
     fs::create_dir_all(output_dir).unwrap();
 
     for test in tests {
-        test.run();
+        // Check that test case passes without minification
+        test.run(false);
+
+        // Check that test case passes with minification
+        test.run(true);
+
         let f = File::create(output_dir.join(format!("{}.json", test.name))).unwrap();
         serde_json::to_writer_pretty(&f, &test).unwrap();
         writeln!(&f).unwrap();