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.

Sign up for GitHub

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

[Merged by Bors] - Make slashing protection import more resilient #2598

Closed
wants to merge 4 commits into from
Closed

[Merged by Bors] - Make slashing protection import more resilient #2598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ae8d83
Make slashing protection import more resilient
michaelsproul Sep 15, 2021
b49ae44
Add more multi interchange test cases
michaelsproul Sep 17, 2021
089a860
Tweak import failure message
michaelsproul Sep 17, 2021
76b8548
Update interchange tests to v5.2.0
michaelsproul Sep 21, 2021
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
61 changes: 31 additions & 30 deletions account_manager/src/validator/slashing_protection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,10 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
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.",
"Deprecated: Lighthouse no longer requires minification on import \
because it always minifies",
),
),
)
Expand Down Expand Up @@ -88,7 +87,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 minify: Option<bool> = clap_utils::parse_optional(matches, MINIFY_FLAG)?;
let import_file = File::open(&import_filename).map_err(|e| {
format!(
"Unable to open import file at {}: {:?}",
Expand All @@ -102,12 +101,17 @@ pub fn cli_run<T: EthSpec>(
.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].");
if let Some(minify) = minify {
eprintln!(
"WARNING: --minify flag is deprecated and will be removed in a future release"
);
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 =
Expand All @@ -120,14 +124,16 @@ pub fn cli_run<T: EthSpec>(
})?;

let display_slot = |slot: Option<Slot>| {
slot.map_or("none".to_string(), |slot| format!("{}", slot.as_u64()))
slot.map_or("none".to_string(), |slot| format!("slot {}", slot.as_u64()))
};
let display_epoch = |epoch: Option<Epoch>| {
epoch.map_or("?".to_string(), |epoch| format!("{}", epoch.as_u64()))
epoch.map_or("?".to_string(), |epoch| format!("epoch {}", epoch.as_u64()))
};
let display_attestation = |source, target| match (source, target) {
(None, None) => "none".to_string(),
(source, target) => format!("{}=>{}", display_epoch(source), display_epoch(target)),
(source, target) => {
format!("{} => {}", display_epoch(source), display_epoch(target))
}
};

match slashing_protection_database
Expand All @@ -140,18 +146,11 @@ pub fn cli_run<T: EthSpec>(
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
)
" - latest block: {}",
display_slot(summary.max_block_slot)
);
eprintln!(
" - max attestation: {}",
" - latest attestation: {}",
display_attestation(
summary.max_attestation_source,
summary.max_attestation_target
Expand All @@ -168,18 +167,20 @@ pub fn cli_run<T: EthSpec>(
}
}
Err(InterchangeError::AtomicBatchAborted(outcomes)) => {
eprintln!("ERROR, slashable data in input:");
eprintln!("ERROR: import aborted due to one or more errors");
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()
);
return Err("ERROR: import aborted due to errors, see above.\n\
No data has been imported and the slashing protection \
database is in the same state it was in before the import.\n\
Due to the failed import it is NOT SAFE to start validating\n\
with any newly imported validator keys, as your database lacks\n\
slashing protection data for them."
michaelsproul marked this conversation as resolved.
Show resolved Hide resolved
.to_string());
}
Err(e) => {
return Err(format!(
Expand All @@ -192,7 +193,7 @@ pub fn cli_run<T: EthSpec>(

eprintln!("Import completed successfully.");
eprintln!(
"Please double-check that the minimum and maximum blocks and attestations above \
"Please double-check that the latest blocks and attestations above \
match your expectations."
);

Expand Down
69 changes: 11 additions & 58 deletions book/src/slashing-protection.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ Instructions for exporting your existing client's database are out of scope for
please check the other client's documentation for instructions.

When importing an interchange file, you still need to import the validator keystores themselves
separately, using the instructions about [importing keystores into
separately, using the instructions for [importing keystores into
Lighthouse](./validator-import-launchpad.md).

---
Expand All @@ -91,31 +91,24 @@ 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.
### How Import Works

Minification can be disabled for imports by adding `--minify=false` to the command:
Since version 1.6.0 Lighthouse will ignore any slashable data in the import data and will safely
update the low watermarks for blocks and attestations. It will store only the maximum-slot block
for each validator, and the maximum source/target attestation. This is faster than importing
all data while also being more resilient to repeated imports & stale data.

```
lighthouse account validator slashing-protection import --minify=false <my_interchange.json>
```
### Minification

It can also be enabled for exports (disabled by default):
The exporter can be configured to minify (shrink) the data it exports by keeping only the
maximum-slot and maximum-epoch messages. Provide the `--minify=true` flag:

```
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.
This may make the file faster to import into other clients, but is unnecessary for Lighthouse to
Lighthouse transfers since v1.5.0.

## Troubleshooting

Expand Down Expand Up @@ -161,46 +154,6 @@ Sep 29 15:15:05.303 CRIT Not signing slashable attestation error: InvalidA
This log is still marked as `CRIT` because in general it should occur only very rarely,
and _could_ indicate a serious error or misconfiguration (see [Avoiding Slashing](#avoiding-slashing)).

### Slashable Data in Import

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 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
whether you'd like to continue.
2. You have exported data from Lighthouse to another client, and then back to Lighthouse,
_in a way that didn't preserve the signing roots_. A message with no signing roots
is considered slashable with respect to _any_ other message at the same slot/epoch,
so even if it was signed by Lighthouse originally, Lighthouse has no way of knowing this.
If you're sure you haven't run Lighthouse and the other client simultaneously, you
can [drop Lighthouse's DB in favour of the interchange file](#drop-and-re-import).
3. You have imported the same interchange file (which lacks signing roots) twice, e.g. from Teku.
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
then you can _move_ (not delete) Lighthouse's database and replace it like so:

```bash
mv $datadir/validators/slashing_protection.sqlite ~/slashing_protection_backup.sqlite
```

```
lighthouse account validator slashing-protection import <my_interchange.json>
```

If your interchange file doesn't cover all of your validators, you shouldn't do this. Please reach
out on Discord if you need help.

## Limitation of Liability

The Lighthouse developers do not guarantee the perfect functioning of this software, or accept
Expand Down
2 changes: 1 addition & 1 deletion validator_client/slashing_protection/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
TESTS_TAG := v5.1.0
TESTS_TAG := v5.2.0
GENERATE_DIR := generated-tests
OUTPUT_DIR := interchange-tests
TARBALL := $(OUTPUT_DIR)-$(TESTS_TAG).tar.gz
Expand Down
127 changes: 126 additions & 1 deletion validator_client/slashing_protection/src/bin/test_generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ fn main() {
],
),
MultiTestCase::new(
"multiple_interchanges_single_validator_single_message_out_of_order",
"multiple_interchanges_single_validator_single_block_out_of_order",
vec![
TestCase::new(interchange(vec![(0, vec![40], vec![])])),
TestCase::new(interchange(vec![(0, vec![20], vec![])]))
Expand All @@ -233,6 +233,131 @@ fn main() {
.with_blocks(vec![(0, 20, false), (0, 50, false)]),
],
),
MultiTestCase::new(
"multiple_interchanges_single_validator_single_att_out_of_order",
vec![
TestCase::new(interchange(vec![(0, vec![], vec![(12, 13)])])),
TestCase::new(interchange(vec![(0, vec![], vec![(10, 11)])]))
.contains_slashable_data()
.with_attestations(vec![
(0, 10, 14, false),
(0, 12, 13, false),
(0, 12, 14, true),
(0, 13, 15, true),
]),
],
),
MultiTestCase::new(
"multiple_interchanges_single_validator_second_surrounds_first",
vec![
TestCase::new(interchange(vec![(0, vec![], vec![(10, 20)])])),
TestCase::new(interchange(vec![(0, vec![], vec![(9, 21)])]))
.contains_slashable_data()
.with_attestations(vec![
(0, 10, 20, false),
(0, 10, 21, false),
(0, 9, 21, false),
(0, 9, 22, false),
(0, 10, 22, true),
]),
],
),
MultiTestCase::new(
"multiple_interchanges_single_validator_first_surrounds_second",
vec![
TestCase::new(interchange(vec![(0, vec![], vec![(9, 21)])])),
TestCase::new(interchange(vec![(0, vec![], vec![(10, 20)])]))
.contains_slashable_data()
.with_attestations(vec![
(0, 10, 20, false),
(0, 10, 21, false),
(0, 9, 21, false),
(0, 9, 22, false),
(0, 10, 22, true),
]),
],
),
MultiTestCase::new(
"multiple_interchanges_multiple_validators_repeat_idem",
vec![
TestCase::new(interchange(vec![
(0, vec![2, 4, 6], vec![(0, 1), (1, 2)]),
(1, vec![8, 10, 12], vec![(0, 1), (0, 3)]),
])),
TestCase::new(interchange(vec![
(0, vec![2, 4, 6], vec![(0, 1), (1, 2)]),
(1, vec![8, 10, 12], vec![(0, 1), (0, 3)]),
]))
.contains_slashable_data()
.with_blocks(vec![
(0, 0, false),
(0, 3, true),
(0, 7, true),
(0, 3, true),
(1, 0, false),
])
.with_attestations(vec![(0, 0, 4, false), (1, 0, 4, true)]),
],
),
MultiTestCase::new(
"multiple_interchanges_overlapping_validators_repeat_idem",
vec![
TestCase::new(interchange(vec![
(0, vec![2, 4, 6], vec![(0, 1), (1, 2)]),
(1, vec![8, 10, 12], vec![(0, 1), (0, 3)]),
])),
TestCase::new(interchange(vec![
(0, vec![2, 4, 6], vec![(0, 1), (1, 2)]),
(2, vec![8, 10, 12], vec![(0, 1), (0, 3)]),
]))
.contains_slashable_data(),
TestCase::new(interchange(vec![
(1, vec![8, 10, 12], vec![(0, 1), (0, 3)]),
(2, vec![8, 10, 12], vec![(0, 1), (0, 3)]),
]))
.contains_slashable_data()
.with_attestations(vec![
(0, 0, 4, false),
(1, 1, 2, false),
(2, 1, 2, false),
]),
],
),
MultiTestCase::new(
"multiple_interchanges_overlapping_validators_merge_stale",
vec![
TestCase::new(interchange(vec![
(0, vec![100], vec![(12, 13)]),
(1, vec![101], vec![(12, 13)]),
(2, vec![4], vec![(4, 5)]),
])),
TestCase::new(interchange(vec![
(0, vec![2], vec![(4, 5)]),
(1, vec![3], vec![(3, 4)]),
(2, vec![102], vec![(12, 13)]),
]))
.contains_slashable_data()
.with_blocks(vec![
(0, 100, false),
(1, 101, false),
(2, 102, false),
(0, 103, true),
(1, 104, true),
(2, 105, true),
])
.with_attestations(vec![
(0, 12, 13, false),
(0, 11, 14, false),
(1, 12, 13, false),
(1, 11, 14, false),
(2, 12, 13, false),
(2, 11, 14, false),
(0, 12, 14, true),
(1, 13, 14, true),
(2, 13, 14, true),
]),
],
),
MultiTestCase::single(
"single_validator_source_greater_than_target",
TestCase::new(interchange(vec![(0, vec![], vec![(8, 7)])])).contains_slashable_data(),
Expand Down
2 changes: 1 addition & 1 deletion validator_client/slashing_protection/src/interchange.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl Interchange {
}
}
(None, None) => {}
_ => return Err(InterchangeError::MinAndMaxInconsistent),
_ => return Err(InterchangeError::MaxInconsistent),
};

// Find maximum block slot.
Expand Down
Loading