diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bb50e5dc8..232c865ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,7 +63,7 @@ jobs: # lag leanVM, and `--all-targets` leaves such targets out. Name them so # they keep compiling until they run again. - name: Clippy (skipped spectests) - run: cargo clippy --locked --workspace --test forkchoice_spectests --test signature_spectests --test stf_spectests --test ssz_spectests -- -D warnings + run: cargo clippy --locked --workspace --test forkchoice_spectests --test signature_spectests --test stf_spectests --test ssz_spectests --test interchange_vectors -- -D warnings # Two features gate code that no step in `lint` reaches, so both would rot # silently between merges. diff --git a/.gitignore b/.gitignore index b2e487fa2..cfaaffa96 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,10 @@ target # BLS and KZG test vectors, downloaded by `make cryptography-specs` /cryptography-specs +# EIP-3076 interchange test vectors, downloaded by +# `make slashing-protection-interchange-tests` +/slashing-protection-interchange-tests + # Personal Claude Code overrides (not shared with team) CLAUDE.local.md diff --git a/Makefile b/Makefile index 10796aff3..5fead8535 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help fmt lint bench update cooldown-check docker-build shadow-build shadow-docker-build run-devnet test test-consensus test-node test-beacon test-beacon-mainnet test-beacon-minimal consensus-spec-tests cryptography-specs docs docs-deps docs-serve +.PHONY: help fmt lint bench update cooldown-check docker-build shadow-build shadow-docker-build run-devnet test test-consensus test-node test-beacon test-beacon-mainnet test-beacon-minimal consensus-spec-tests cryptography-specs docs docs-deps docs-serve slashing-protection-interchange-tests help: ## ๐Ÿ“š Show help for each of the Makefile recipes @grep -E '^[a-zA-Z0-9_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' @@ -10,7 +10,7 @@ lint: ## ๐Ÿ” Run clippy on all workspace crates cargo clippy --locked --workspace --all-targets -- -D warnings # The spectests are `test = false` (stale leanSpec fixtures), which # `--all-targets` skips: lint them by name so they keep compiling - cargo clippy --locked --workspace --test forkchoice_spectests --test signature_spectests --test stf_spectests --test ssz_spectests -- -D warnings + cargo clippy --locked --workspace --test forkchoice_spectests --test signature_spectests --test stf_spectests --test ssz_spectests --test interchange_vectors -- -D warnings # release-fast: release-grade opt-level to avoid stack overflows during # signature verification/aggregation, without paying for LTO on every rebuild @@ -38,8 +38,9 @@ test: test-consensus test-node ## ๐Ÿงช Run all tests test-consensus: leanSpec/fixtures ## ๐Ÿงช Run the consensus half of the workspace suite $(TEST) $(addprefix -p ,$(CONSENSUS_CRATES)) -test-node: leanSpec/fixtures ## ๐Ÿงช Run the node half of the workspace suite +test-node: leanSpec/fixtures slashing-protection-interchange-tests ## ๐Ÿงช Run the node half of the workspace suite $(TEST) --workspace $(addprefix --exclude ,$(CONSENSUS_CRATES)) + $(TEST) -p ethlambda-validator --test interchange_vectors # --lib as well as the spec target: the BLS and KZG modules keep their fixture # vectors as unit tests, which are `ignore`d unless this feature is on. @@ -211,6 +212,33 @@ $(CRYPTOGRAPHY_SPECS_STAMP): unzip -q "$$tmpdir/tests.zip" -d cryptography-specs || exit 1 @touch $@ +# EIP-3076 slashing-protection interchange test vectors, for the validator +# client's `slashing_protection` module (crates/validator/tests/ +# interchange_vectors.rs). Pinned, and stamped by version like the fixture trees +# above, so a bump re-downloads rather than passing against the old cases. The +# release ships no asset for this tag, so the tag's source archive is fetched +# and checked against a pinned SHA-256 instead. +SLASHING_PROTECTION_TESTS_VERSION ?= v5.3.0 +SLASHING_PROTECTION_TESTS_SHA256 ?= 516d551cfb3e50e4ac2f42db0992f4ceb573a7cb1616d727a725c8161485329f +SLASHING_PROTECTION_TESTS_URL ?= https://github.com/eth-clients/slashing-protection-interchange-tests/archive/refs/tags/$(SLASHING_PROTECTION_TESTS_VERSION).tar.gz +SLASHING_PROTECTION_TESTS_STAMP=slashing-protection-interchange-tests/.version-$(SLASHING_PROTECTION_TESTS_VERSION) + +slashing-protection-interchange-tests: $(SLASHING_PROTECTION_TESTS_STAMP) ## โฌ‡๏ธ Download the EIP-3076 interchange test vectors + +$(SLASHING_PROTECTION_TESTS_STAMP): + @rm -rf slashing-protection-interchange-tests + @mkdir -p slashing-protection-interchange-tests + @tmpdir=$$(mktemp -d); trap 'rm -rf "$$tmpdir"' EXIT; \ + echo "Downloading EIP-3076 interchange test vectors ($(SLASHING_PROTECTION_TESTS_VERSION))"; \ + curl -L -f -o "$$tmpdir/tests.tar.gz" "$(SLASHING_PROTECTION_TESTS_URL)" || exit 1; \ + actual=$$(sha256sum "$$tmpdir/tests.tar.gz" | awk '{print $$1}'); \ + if [ "$$actual" != "$(SLASHING_PROTECTION_TESTS_SHA256)" ]; then \ + echo "SHA256 mismatch: expected $(SLASHING_PROTECTION_TESTS_SHA256), got $$actual" >&2; \ + exit 1; \ + fi; \ + tar -xzf "$$tmpdir/tests.tar.gz" -C slashing-protection-interchange-tests --strip-components=1 || exit 1 + @touch $@ + # lambdaclass fork of lean-quickstart: genesis keys come from `ethlambda keygen`, and the # partner clients run their devnet-5 images. An existing lean-quickstart/ is never # re-cloned, so delete it to pick up a new pin. diff --git a/README.md b/README.md index 99bfcbd1f..bbe23f7cc 100644 --- a/README.md +++ b/README.md @@ -249,7 +249,7 @@ cargo build --release ``` [`docs/cli.md`](./docs/cli.md) is the full flag reference for every sub-command, -`benchmark` and `replay-era` included. The +`benchmark`, `replay-era` and `slashing-protection` included. The [quickstart](#beacon-chain-follower) runs a release binary with the same arguments. #### `ethlambda node` โ€” the Lean consensus chain @@ -348,8 +348,9 @@ the standard Beacon API, so it works against `ethlambda beacon` (run with the > and only for keys that have signed nothing anywhere for at least two epochs; > even then it rules out a double vote, not a surround of an older vote, which > only the key's own history can. -> Importing history from another client is not supported yet, so never run -> these keys in another client at the same time. See +> A key moving from another client brings its history with +> `ethlambda slashing-protection import`, or in the same keymanager API request +> that imports the key, and never runs in two clients at once. See > [Spec deviations](./docs/spec_deviations.md#the-validator-client-keeps-eip-3076s-minimal-watermarks-not-full-history). ```sh diff --git a/bin/ethlambda/src/command.rs b/bin/ethlambda/src/command.rs index 7d0b9556a..d5cfe4d54 100644 --- a/bin/ethlambda/src/command.rs +++ b/bin/ethlambda/src/command.rs @@ -30,6 +30,7 @@ const EXPLICIT: &[&str] = &[ BENCHMARK, REPLAY_ERA, VALIDATOR, + SLASHING_PROTECTION, KEYGEN, "help", "-h", @@ -43,6 +44,7 @@ const BEACON: &str = "beacon"; const BENCHMARK: &str = "benchmark"; const REPLAY_ERA: &str = "replay-era"; const VALIDATOR: &str = "validator"; +const SLASHING_PROTECTION: &str = "slashing-protection"; const KEYGEN: &str = "keygen"; #[derive(Debug, clap::Parser)] @@ -84,6 +86,9 @@ pub(crate) enum Command { ReplayEra(ReplayEraOptions), /// Run the beacon-chain validator client against a beacon node's HTTP API. Validator(crate::validator::ValidatorOptions), + /// Import or export the validator client's EIP-3076 slashing-protection + /// history, offline. + SlashingProtection(crate::slashing_protection::SlashingProtectionOptions), /// Generate validator XMSS keys for a genesis. Keygen(KeygenOptions), } @@ -415,7 +420,14 @@ mod tests { /// `ethlambda node keygen ...`, which fails on a stray positional. #[test] fn sub_command_tokens_do_not_get_the_default_inserted() { - for token in [NODE, BEACON, BENCHMARK, VALIDATOR, KEYGEN] { + for token in [ + NODE, + BEACON, + BENCHMARK, + VALIDATOR, + SLASHING_PROTECTION, + KEYGEN, + ] { let args = [OsString::from("ethlambda"), OsString::from(token)]; assert_eq!( default_subcommand(&args), @@ -425,6 +437,30 @@ mod tests { } } + #[test] + fn slashing_protection_parses_as_its_own_sub_command() { + let args = [ + "ethlambda", + SLASHING_PROTECTION, + "export", + "--validators-dir", + "/tmp/validators", + "--pubkeys", + "0xaa,0xbb", + "out.json", + ]; + match try_parse_from(args.iter().map(OsString::from)).expect("parses") { + Command::SlashingProtection(options) => match options.action { + crate::slashing_protection::Action::Export { pubkeys, file, .. } => { + assert_eq!(pubkeys, vec!["0xaa", "0xbb"]); + assert_eq!(file, PathBuf::from("out.json")); + } + other => panic!("expected an export, got {other:?}"), + }, + other => panic!("expected a slashing-protection invocation, got {other:?}"), + } + } + #[test] fn keygen_parses_as_its_own_sub_command() { let args = [ diff --git a/bin/ethlambda/src/main.rs b/bin/ethlambda/src/main.rs index 9110da5de..db3f62368 100644 --- a/bin/ethlambda/src/main.rs +++ b/bin/ethlambda/src/main.rs @@ -8,6 +8,7 @@ mod era; mod fd_limit; mod keygen; mod network; +mod slashing_protection; mod validator; mod version; @@ -109,6 +110,11 @@ fn main() -> eyre::Result<()> { init_node_logging()?; validator::run(options) } + // Offline file work: no runtime, and stdout left alone. + Command::SlashingProtection(options) => { + init_info_stderr_logging()?; + slashing_protection::run(options) + } // Key generation is synchronous, CPU-bound work whose product is files, // so it runs on this thread too and leaves stdout alone. Command::Keygen(options) => { diff --git a/bin/ethlambda/src/slashing_protection.rs b/bin/ethlambda/src/slashing_protection.rs new file mode 100644 index 000000000..044e51250 --- /dev/null +++ b/bin/ethlambda/src/slashing_protection.rs @@ -0,0 +1,137 @@ +//! `ethlambda slashing-protection`: move a validator client's EIP-3076 history +//! in and out of its `--validators-dir`. +//! +//! Offline on purpose. It takes the same lock the validator client holds while +//! it runs, so it refuses while one is running there: history imported under a +//! running client could arrive after the client had already signed past it. +//! No beacon node is needed either: the record names its own chain, and an +//! import into a directory with no record yet creates one for the chain the +//! interchange names, which the client checks against its beacon node at its +//! next start. + +use std::io::Write as _; +use std::path::PathBuf; + +use ethlambda_validator::beacon_node::dto::{encode_hex, parse_pubkey}; +use ethlambda_validator::slashing_protection::SlashingProtection; +use tracing::info; + +#[derive(Debug, clap::Args)] +pub(crate) struct SlashingProtectionOptions { + #[command(subcommand)] + pub(crate) action: Action, +} + +#[derive(Debug, clap::Subcommand)] +pub(crate) enum Action { + /// Merge an EIP-3076 interchange file (complete or minimal, version 5) into + /// the record, never lowering what is already there. + Import { + /// The validator client's `--validators-dir`. + #[arg(long)] + validators_dir: PathBuf, + /// The interchange file, as exported by this or another client. + file: PathBuf, + }, + /// Write the record as a minimal EIP-3076 interchange file (version 5). + Export { + /// The validator client's `--validators-dir`. + #[arg(long)] + validators_dir: PathBuf, + /// Only these validators, `0x`-prefixed, comma-separated or repeated. + /// Every validator with a record when omitted. + #[arg(long, value_delimiter = ',')] + pubkeys: Vec, + /// Where to write it. Refused if it exists, rather than overwritten. + file: PathBuf, + }, +} + +pub(crate) fn run(options: SlashingProtectionOptions) -> eyre::Result<()> { + match options.action { + Action::Import { + validators_dir, + file, + } => { + let interchange = std::fs::read_to_string(&file) + .map_err(|err| eyre::eyre!("{}: {err}", file.display()))?; + // Parsed once: the file can be tens of megabytes in the complete + // format, and its chain is needed before the record is opened. + let staged = SlashingProtection::stage(&interchange)?; + let mut protection = + SlashingProtection::open_offline(&validators_dir, Some(staged.root()))?; + let validators = protection.absorb(Some(&staged), &[])?.named; + info!( + validators, + file = %file.display(), + "Imported slashing-protection history" + ); + } + Action::Export { + validators_dir, + pubkeys, + file, + } => { + let protection = SlashingProtection::open_offline(&validators_dir, None)?; + let pubkeys = if pubkeys.is_empty() { + protection.pubkeys() + } else { + pubkeys + .iter() + .map(|text| { + parse_pubkey(text).map_err(|err| eyre::eyre!("--pubkeys {text}: {err}")) + }) + .collect::, _>>()? + }; + // A requested key with no record fails the export, as Lighthouse + // does, rather than being left out of a file that then reads as + // complete: a typo, or the wrong directory, would otherwise be + // found out at the other end, after the keys moved. + let missing: Vec = pubkeys + .iter() + .filter(|pubkey| !protection.contains(pubkey)) + .map(|pubkey| encode_hex(&pubkey.0)) + .collect(); + if !missing.is_empty() { + eyre::bail!( + "no slashing-protection record for {} requested validator(s) (first: {}); \ + nothing was written", + missing.len(), + missing[0] + ); + } + let exported = pubkeys.len(); + let interchange = protection.export(&pubkeys)?; + write_export(&file, &interchange) + .map_err(|err| eyre::eyre!("{}: {err}", file.display()))?; + info!( + validators = exported, + file = %file.display(), + "Exported slashing-protection history" + ); + } + } + Ok(()) +} + +/// Write `interchange` to a new file at `path`, refusing an existing one. +/// +/// Private to the owner, like the record it was taken from. A write that +/// fails partway removes what it left, so the retry is not refused for a file +/// this run created and never finished. +fn write_export(path: &std::path::Path, interchange: &str) -> std::io::Result<()> { + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.mode(0o600); + } + let mut output = options.open(path)?; + output + .write_all(interchange.as_bytes()) + .and_then(|()| output.sync_all()) + .inspect_err(|_| { + let _ = std::fs::remove_file(path); + }) +} diff --git a/crates/validator/Cargo.toml b/crates/validator/Cargo.toml index 7c498eed1..b34e72488 100644 --- a/crates/validator/Cargo.toml +++ b/crates/validator/Cargo.toml @@ -77,3 +77,11 @@ tempfile = "3" [features] # Exposes `SlashingProtection::in_memory` to other crates' tests. test-utils = [] + +# Skipped by a plain `cargo test`: it reads the EIP-3076 interchange vectors, +# which `make slashing-protection-interchange-tests` downloads, and fails +# without them. `make test-node` runs it by name. +[[test]] +name = "interchange_vectors" +path = "tests/interchange_vectors.rs" +test = false diff --git a/crates/validator/src/http_api/keystores.rs b/crates/validator/src/http_api/keystores.rs index 26f35aebb..c8356decb 100644 --- a/crates/validator/src/http_api/keystores.rs +++ b/crates/validator/src/http_api/keystores.rs @@ -1,24 +1,41 @@ //! `GET`, `POST` and `DELETE /eth/v1/keystores`. //! -//! # Deviation +//! # Slashing protection //! -//! This client does not yet import or export EIP-3076 history, so the -//! interchange data the specification requires is not produced. `DELETE` declares -//! `slashing_protection` a required response field, so an empty but well-formed -//! EIP-3076 interchange is returned; `POST` refuses a request that carries the -//! optional `slashing_protection` field, importing none of its keys, since a -//! caller that hands over history must not be told it was taken. A caller -//! migrating keys between clients must not rely on this client to carry that -//! history. +//! The specification lets these routes carry a validator's EIP-3076 history in +//! and out, and both do: //! -//! The empty interchange [`DELETE`] returns is deliberately unusable as one: -//! its `genesis_validators_root` is all-zero rather than the chain's real -//! value. A real root would make the file look like legitimate, if empty, -//! history, which is worse than what it actually is (none of the history this -//! client recorded, since it does not export it yet). The all-zero root is chosen so a -//! conformant importer rejects the file outright instead of trusting it, on -//! the theory that a loud, obvious failure at import time beats a tool -//! silently believing a validator has never signed. +//! - `POST` judges and writes the request's `slashing_protection` before +//! touching any key: one that does not parse, or is for another chain, +//! fails every key in the request and imports none of them. Written first, +//! rather than with the keys' registration, because a key that was deleted +//! here earlier keeps its record: were the keys saved first, a crash before +//! the history landed would load such a key at its next start on the +//! watermarks it had before it left, with whatever it signed elsewhere +//! unknown. The history only ever raises watermarks, so writing it for keys +//! that then fail to save leaves nothing dangerous behind, and it is +//! written whole, entries for keys of the request that failed included: a +//! later import of such a key merges with it. The keys are registered after +//! they are saved, in one write, before any is loaded, and a registration +//! that fails takes the keys off disk again. A key with no history at all is +//! registered as never having signed, with a warning. The history may also +//! name validators that are not in the request: they get records too, and +//! a later `DELETE` of one answers `not_active`. One of them may be a key +//! this client is running now; its watermarks are raised like any other, +//! since a history saying it signed elsewhere is exactly what the record +//! must know, and the raise is logged so the operator finds out that the +//! key ran in two places. +//! - `DELETE` returns the deleted keys' records as a minimal interchange, +//! taken after the keys are out of the store, so it covers every signature +//! this process made with them. A key that is gone but still has a record is +//! reported `not_active`, and its record is kept, so importing it back later +//! starts from what this client signed with it (not from what the key may +//! have signed elsewhere in between: see `docs/spec_deviations.md` on +//! doppelganger protection). +//! +//! The history is this client's EIP-3076 minimal watermarks (see +//! [`crate::slashing_protection`]), so an export lists, per validator, its +//! highest slot and its highest source and target rather than every message. use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -37,9 +54,6 @@ use crate::keys::keystore::Keystore; use crate::keys::{ValidatorDefinition, ValidatorDefinitions, ValidatorStore}; use crate::secure_fs; -/// The EIP-3076 interchange version this client reports when it has no history. -const INTERCHANGE_VERSION: &str = "5"; - /// The most keystores accepted in a single import request. /// /// Each one costs a deliberately slow EIP-2335 key derivation (scrypt at the @@ -74,7 +88,8 @@ pub struct ImportRequest { /// memory until decryption runs, and a plain `Vec` would leave /// those bytes sitting in the allocator's freed memory afterwards. pub passwords: Zeroizing>, - /// Accepted and ignored: importing EIP-3076 history is not supported yet. + /// An EIP-3076 interchange, written before any key is saved; see the + /// module documentation. #[serde(default)] pub slashing_protection: Option, } @@ -122,8 +137,8 @@ pub struct DeleteRequest { #[derive(Debug, Serialize)] pub struct DeleteResponse { pub data: Vec, - /// Required by the specification even though this client has no history to - /// report. See the module documentation. + /// The deleted keys' records, as a minimal EIP-3076 interchange. See the + /// module documentation. pub slashing_protection: String, } @@ -144,27 +159,8 @@ pub async fn list(State(context): State) -> Json, - Json(request): Json, + Json(mut request): Json, ) -> Json { - // Refused rather than ignored: the caller handed over history, and a - // `200` with `imported` would tell it the history was taken, which is the - // standard way to move an active key and the surest way to double-sign. - if request.slashing_protection.is_some() { - warn!( - "Refusing an import that carries slashing_protection; importing history is not supported yet" - ); - let message = "not imported: this client does not import EIP-3076 history yet, so a \ - request carrying slashing_protection imports nothing; import the keys \ - without it only if they have signed nothing anywhere for at least two \ - epochs"; - let data = request - .keystores - .iter() - .map(|_| KeyStatus::error(message)) - .collect(); - return Json(StatusResponse { data }); - } - if request.keystores.len() > MAX_KEYSTORES_PER_IMPORT { let message = format!( "request carries {} keystores, more than the limit of {MAX_KEYSTORES_PER_IMPORT}", @@ -182,6 +178,76 @@ pub async fn import( return Json(StatusResponse { data }); } + // The history is parsed once, off every lock, and written before any key + // is touched, so a request whose history is refused imports no key at + // all, and no key can be saved ahead of the history it came with (see the + // module documentation for the crash that ordering closes). Parsed on the + // blocking pool: a complete-format interchange runs to tens of megabytes + // (see `KEYMANAGER_BODY_LIMIT`), and the executor thread this handler + // runs on also drives the duty services. + let history = match request.slashing_protection.take() { + None => None, + Some(interchange) => { + let staged = tokio::task::spawn_blocking(move || { + crate::slashing_protection::SlashingProtection::stage(&interchange) + }) + .await + .unwrap_or_else(|err| { + Err(Error::SlashingProtection { + path: "interchange".to_string(), + reason: format!("could not be parsed: {err}"), + }) + }); + let written = match staged { + Ok(staged) => { + let staged = Arc::new(staged); + let absorbed = staged.clone(); + crate::slashing_protection::blocking(&context.protection, move |protection| { + protection.absorb(Some(&absorbed), &[]) + }) + .await + .map(|absorbed| (staged, absorbed)) + } + Err(err) => Err(err), + }; + match written { + Ok((staged, absorbed)) => { + info!( + validators = absorbed.named, + "Imported slashing-protection history" + ); + // A key running here whose watermarks this history raised + // signed elsewhere while it ran here: worth knowing. + let store = context.store.read().await; + for pubkey in absorbed.raised.iter().filter(|key| store.contains(key)) { + warn!( + pubkey = %encode_hex(&pubkey.0), + "Imported history raised the slashing-protection watermarks of a \ + validator this client is running; it has signed elsewhere too" + ); + } + drop(store); + Some(staged) + } + // `Error::Unflushed` is refused here too, unlike at the keys' + // registration below: the history is on disk and in memory, + // but were it lost to a power loss, a key saved on the + // strength of it would load at the next start on the record + // it had before, which is the case writing it first closes. + Err(err) => { + let message = format!("slashing_protection was not imported: {err}"); + warn!(%err, "Refusing an import whose slashing-protection history was refused"); + let data = request + .keystores + .iter() + .map(|_| KeyStatus::error(&message)) + .collect(); + return Json(StatusResponse { data }); + } + } + } + }; + // Held across this whole function: the entire open-mutate-save cycle // over the definitions file, including the slow decrypt-and-persist loop // below. This is what actually serializes concurrent import/delete @@ -241,22 +307,26 @@ pub async fn import( prepared.push(prepare_import(&context, &mut definitions, json, password)); } - // Every persisted key is registered in the slashing-protection record in - // one write, before any of them is loaded: the duty path must never meet a - // key with no record. After persisting rather than before, so a key whose - // persist failed leaves no record behind to let a hand-copied keystore - // past the startup check later. Still under the definitions guard, so a - // concurrent delete cannot interleave. + // Every persisted key is registered in one durable write, before any of + // them is loaded: the duty path must never meet a key with no record. + // After persisting rather than before, so a key whose persist failed + // leaves no record behind to let a hand-copied keystore past the startup + // check later. The history goes along again so that a key it covers is + // left on it rather than warned about as having brought none; it was + // written above, so this costs nothing unless a key is new. Still under + // the definitions guard, so a concurrent delete cannot interleave. let persisted: Vec = prepared .iter() .filter_map(|result| result.as_ref().ok().map(|(pubkey, _, _)| *pubkey)) .collect(); + // Nothing to register for a request that saved no key. let registration = if persisted.is_empty() { - Ok(0) + Ok(crate::slashing_protection::Absorbed::default()) } else { let registering = persisted.clone(); + let history = history.clone(); crate::slashing_protection::blocking(&context.protection, move |protection| { - protection.register(®istering) + protection.absorb(history.as_deref(), ®istering) }) .await }; @@ -272,17 +342,17 @@ pub async fn import( "Slashing-protection records written, but the directory could not be flushed; \ the imported keys are registered" ); - Ok(0) + Ok(crate::slashing_protection::Absorbed::default()) } other => other, }; - if let Ok(registered) = registration - && registered > 0 + if let Ok(absorbed) = ®istration + && absorbed.registered > 0 { warn!( - registered, - "Imported validator keys with no slashing-protection history; each starts as never \ - having signed" + registered = absorbed.registered, + "Imported validator keys with no slashing-protection history; each starts as \ + never having signed" ); } @@ -700,29 +770,93 @@ pub async fn delete( let _definitions_guard = context.definitions_lock.lock().await; let mut store = context.store.write().await; - let data = request + let results: Vec<(Option, KeyStatus)> = request .pubkeys .iter() .map(|text| match parse_pubkey(text) { - Ok(pubkey) => delete_one(&context, &mut store, &pubkey), - Err(err) => KeyStatus::error(err), + Ok(pubkey) => (Some(pubkey), delete_one(&context, &mut store, &pubkey)), + Err(err) => (None, KeyStatus::error(err)), }) .collect(); drop(store); - // This interchange cannot be used to migrate the key(s) just deleted to - // another client: see the module doc for why `genesis_validators_root` - // is deliberately wrong rather than merely absent. Logged on every call, - // not just a failure, since a successful response is exactly the one a - // caller might mistake for "safe to import elsewhere". - warn!( - "DELETE /eth/v1/keystores returns an empty EIP-3076 interchange with no real signing \ - history; it cannot be used to migrate these keys to another client without slashing risk" - ); + // Exported only now, with the keys out of the store: a signature with one + // of them either finished before the write lock above was granted, and is + // in the record, or can no longer happen. That holds because the duty + // services record before they take the store's read lock to sign (see + // `attestation.rs` and `proposal.rs`), so no signature exists without its + // record already on disk. Only for the keys that are `deleted` or may + // become `not_active`, as the specification requires: a key whose delete + // failed is still on disk and comes back on restart, so its history is + // not handed over, and neither is one whose removal reached the disk but + // not a flush (`Error::Unflushed`, an `error` too), since it may come + // back. Teku and Prysm filter the same way. A key listed twice is + // `deleted` once and `not_found` once; `export` lists it once. + let requested: Vec> = results + .iter() + .map(|(pubkey, status)| match status.status.as_str() { + "deleted" | "not_found" => *pubkey, + _ => None, + }) + .collect(); + let exported = crate::slashing_protection::blocking(&context.protection, move |protection| { + let pubkeys: Vec = requested.iter().flatten().copied().collect(); + let interchange = protection.export(&pubkeys)?; + let recorded: Vec = requested + .iter() + .map(|pubkey| pubkey.is_some_and(|pubkey| protection.contains(&pubkey))) + .collect(); + Ok((interchange, recorded)) + }) + .await; + let (slashing_protection, recorded) = match exported { + Ok(exported) => exported, + Err(err) => { + // With the record unreadable, nothing here can tell a key with no + // history from one whose history could not be read, so every key + // that parsed becomes an error, as Prysm answers: a `not_found` + // here would tell a migrator there is nothing to carry. A + // malformed pubkey keeps its own error. The deletions themselves + // already happened and are not undone: the keys' records are + // kept, so a retry answers `not_active` with their history. The + // field is required, so it carries an empty string rather than + // an interchange that would read as "nothing signed". + warn!(%err, "Deleted keys whose slashing-protection history could not be exported"); + let message = format!("slashing_protection could not be exported: {err}"); + let data = results + .into_iter() + .map(|(pubkey, status)| match pubkey { + Some(_) => KeyStatus::error(&message), + None => status, + }) + .collect(); + return Json(DeleteResponse { + data, + slashing_protection: String::new(), + }); + } + }; + + // `not_active`: not held here, but with a record, which the response + // carries. The specification's word for "gone, history attached". + let data = results + .into_iter() + .zip(recorded) + .map(|((_, status), recorded)| { + if status.status == "not_found" && recorded { + KeyStatus { + status: "not_active".to_string(), + message: None, + } + } else { + status + } + }) + .collect(); Json(DeleteResponse { data, - slashing_protection: empty_interchange(), + slashing_protection, }) } @@ -845,23 +979,6 @@ fn persist_delete(context: &KeymanagerContext, pubkey: &BlsPubkey) -> crate::Res save_definitions(context, &definitions) } -/// A well-formed EIP-3076 interchange holding no history. -/// -/// `genesis_validators_root` is deliberately all-zero rather than the -/// chain's real value; see the module doc for why. Do not "fix" this to -/// carry the real root: that would make an empty history look legitimate -/// instead of getting it rejected. -fn empty_interchange() -> String { - serde_json::json!({ - "metadata": { - "interchange_format_version": INTERCHANGE_VERSION, - "genesis_validators_root": "0x0000000000000000000000000000000000000000000000000000000000000000" - }, - "data": [] - }) - .to_string() -} - #[cfg(test)] mod tests { use super::*; @@ -1642,6 +1759,366 @@ mod tests { ); } + const KEYSTORE_PUBKEY: &str = "0x9612d7a727c9d0a22e185a1c768478dfe919cada9266988cb32359c11f2b7b27f4ae4040902382ae2910c15e2b420d07"; + + /// A context over `dir` whose slashing protection is a real record there, + /// for the chain `0x00..00`, and the handle to inspect it with. + fn context_with_record( + dir: &std::path::Path, + ) -> ( + KeymanagerContext, + crate::slashing_protection::SharedProtection, + ) { + let protection = crate::slashing_protection::SlashingProtection::open( + dir, + ethlambda_types::beacon::primitives::Root::ZERO, + &[], + false, + ) + .expect("opens with no keys"); + let mut context = context_with_dirs(Arc::new(RwLock::new(ValidatorStore::new())), dir); + context.protection = Arc::new(std::sync::Mutex::new(protection)); + let protection = context.protection.clone(); + (context, protection) + } + + /// A minimal interchange for `KEYSTORE_PUBKEY` on the chain `root`. + fn history(root: &str, slot: u64, source: u64, target: u64) -> String { + history_for(KEYSTORE_PUBKEY, root, slot, source, target) + } + + fn history_for(pubkey: &str, root: &str, slot: u64, source: u64, target: u64) -> String { + serde_json::json!({ + "metadata": { "interchange_format_version": "5", "genesis_validators_root": root }, + "data": [{ + "pubkey": pubkey, + "signed_blocks": [{ "slot": slot.to_string() }], + "signed_attestations": [{ + "source_epoch": source.to_string(), + "target_epoch": target.to_string() + }] + }] + }) + .to_string() + } + + const ZERO_ROOT: &str = "0x0000000000000000000000000000000000000000000000000000000000000000"; + + /// History that arrives with a key is in force before the key can sign. + #[tokio::test] + async fn an_import_brings_its_slashing_protection_history() { + let dir = tempfile::tempdir().expect("temp dir"); + let (context, protection) = context_with_record(dir.path()); + + let (_, body) = send( + router(context, TOKEN.to_string()), + authed( + "POST", + "/eth/v1/keystores", + serde_json::json!({ + "keystores": [keystore_json()], + "passwords": [PASSWORD], + "slashing_protection": history(ZERO_ROOT, 100, 5, 10), + }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "imported", "got {body}"); + + let pubkey = parse_pubkey(KEYSTORE_PUBKEY).expect("pubkey"); + let protection = protection.lock().expect("not poisoned"); + assert!( + protection.check_block(&pubkey, 100).is_err(), + "slot 100 was signed elsewhere" + ); + assert!(protection.check_block(&pubkey, 101).is_ok()); + } + + /// History for another chain is refused, and with it every key in the + /// request: none may start signing without the history it came with. + #[tokio::test] + async fn an_import_whose_history_is_for_another_chain_imports_nothing() { + let dir = tempfile::tempdir().expect("temp dir"); + let (context, _) = context_with_record(dir.path()); + let store = context.store.clone(); + let other_chain = "0x0000000000000000000000000000000000000000000000000000000000000001"; + + let (_, body) = send( + router(context, TOKEN.to_string()), + authed( + "POST", + "/eth/v1/keystores", + serde_json::json!({ + "keystores": [keystore_json()], + "passwords": [PASSWORD], + "slashing_protection": history(other_chain, 100, 5, 10), + }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "error", "got {body}"); + assert!(store.read().await.is_empty(), "the key must not be loaded"); + } + + /// A delete hands over the record the key was protected by, so it can be + /// imported elsewhere, and keeps it here. + #[tokio::test] + async fn a_delete_returns_the_keys_history() { + let dir = tempfile::tempdir().expect("temp dir"); + let (context, protection) = context_with_record(dir.path()); + let app = router(context, TOKEN.to_string()); + + let (_, body) = send( + app.clone(), + authed( + "POST", + "/eth/v1/keystores", + serde_json::json!({ "keystores": [keystore_json()], "passwords": [PASSWORD] }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "imported", "got {body}"); + // Imported with no history, so the key starts at the current slot; + // a proposal must come after it. + const SLOT: u64 = 1 << 40; + let pubkey = parse_pubkey(KEYSTORE_PUBKEY).expect("pubkey"); + protection + .lock() + .expect("not poisoned") + .check_and_record_block(&pubkey, SLOT) + .expect("written") + .expect("allowed"); + + let (_, body) = send( + app.clone(), + authed( + "DELETE", + "/eth/v1/keystores", + serde_json::json!({ "pubkeys": [KEYSTORE_PUBKEY] }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "deleted", "got {body}"); + let interchange: serde_json::Value = + serde_json::from_str(body["slashing_protection"].as_str().expect("a string")) + .expect("json"); + assert_eq!( + interchange["metadata"]["genesis_validators_root"], + ZERO_ROOT + ); + assert_eq!(interchange["data"][0]["pubkey"], KEYSTORE_PUBKEY); + assert_eq!( + interchange["data"][0]["signed_blocks"][0]["slot"], + SLOT.to_string() + ); + + // Gone from this client, but its record is still here and still + // returned: the specification's `not_active`. + let (_, body) = send( + app, + authed( + "DELETE", + "/eth/v1/keystores", + serde_json::json!({ "pubkeys": [KEYSTORE_PUBKEY] }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "not_active", "got {body}"); + assert!( + protection + .lock() + .expect("not poisoned") + .check_block(&pubkey, SLOT) + .is_err() + ); + } + + /// The history may name a validator that is not in the request: it gets a + /// record too, so a later delete of it answers `not_active` with that + /// history rather than `not_found`. + #[tokio::test] + async fn history_for_a_key_outside_the_request_gets_a_record() { + let dir = tempfile::tempdir().expect("temp dir"); + let (context, protection) = context_with_record(dir.path()); + let app = router(context, TOKEN.to_string()); + let other = format!("0x{}", "ab".repeat(48)); + + let (_, body) = send( + app.clone(), + authed( + "POST", + "/eth/v1/keystores", + serde_json::json!({ + "keystores": [keystore_json()], + "passwords": [PASSWORD], + "slashing_protection": history_for(&other, ZERO_ROOT, 100, 5, 10), + }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "imported", "got {body}"); + + let other_pubkey = parse_pubkey(&other).expect("pubkey"); + { + let protection = protection.lock().expect("not poisoned"); + assert!(protection.contains(&other_pubkey)); + assert!(protection.check_block(&other_pubkey, 100).is_err()); + assert!(protection.contains(&parse_pubkey(KEYSTORE_PUBKEY).expect("pubkey"))); + } + + let (_, body) = send( + app, + authed( + "DELETE", + "/eth/v1/keystores", + serde_json::json!({ "pubkeys": [other] }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "not_active", "got {body}"); + let interchange: serde_json::Value = + serde_json::from_str(body["slashing_protection"].as_str().expect("a string")) + .expect("json"); + assert_eq!(interchange["data"][0]["pubkey"], other); + assert_eq!(interchange["data"][0]["signed_blocks"][0]["slot"], "100"); + } + + /// A key listed twice in one delete is `deleted` once and `not_active` + /// once, and the returned interchange names it once: one that named a + /// validator twice would be refused on import. + #[tokio::test] + async fn a_delete_listing_a_key_twice_exports_it_once() { + let dir = tempfile::tempdir().expect("temp dir"); + let (context, _) = context_with_record(dir.path()); + let app = router(context, TOKEN.to_string()); + + let (_, body) = send( + app.clone(), + authed( + "POST", + "/eth/v1/keystores", + serde_json::json!({ "keystores": [keystore_json()], "passwords": [PASSWORD] }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "imported", "got {body}"); + + let (_, body) = send( + app, + authed( + "DELETE", + "/eth/v1/keystores", + serde_json::json!({ "pubkeys": [KEYSTORE_PUBKEY, KEYSTORE_PUBKEY] }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "deleted", "got {body}"); + assert_eq!(body["data"][1]["status"], "not_active", "got {body}"); + let interchange: serde_json::Value = + serde_json::from_str(body["slashing_protection"].as_str().expect("a string")) + .expect("json"); + assert_eq!(interchange["data"].as_array().expect("array").len(), 1); + assert_eq!(interchange["data"][0]["pubkey"], KEYSTORE_PUBKEY); + } + + /// The history is written before its key is saved, so it is held even + /// when the key then fails to save. The key here was deleted earlier and + /// kept an older record: had the key been saved first, a crash before the + /// history landed would have loaded it on that older record at the next + /// start, which is the case the ordering exists for. + #[cfg(unix)] + #[tokio::test] + async fn history_is_written_before_its_key_is_saved() { + let dir = tempfile::tempdir().expect("temp dir"); + // A symlink at the keystore's path makes `persist_import` refuse. + let target = dir.path().join("elsewhere"); + std::fs::write(&target, "untouched").expect("writes target"); + std::os::unix::fs::symlink(&target, dir.path().join(format!("{KEYSTORE_PUBKEY}.json"))) + .expect("symlinks"); + let (context, protection) = context_with_record(dir.path()); + let pubkey = parse_pubkey(KEYSTORE_PUBKEY).expect("pubkey"); + { + let mut protection = protection.lock().expect("not poisoned"); + protection.register(&[pubkey]).expect("registers"); + protection + .check_and_record_block(&pubkey, 10) + .expect("written") + .expect("allowed"); + } + let store = context.store.clone(); + + let (_, body) = send( + router(context, TOKEN.to_string()), + authed( + "POST", + "/eth/v1/keystores", + serde_json::json!({ + "keystores": [keystore_json()], + "passwords": [PASSWORD], + "slashing_protection": history(ZERO_ROOT, 100, 5, 10), + }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "error", "got {body}"); + assert!(store.read().await.is_empty(), "the key did not load"); + assert!( + !dir.path().join("validator_definitions.yml").exists(), + "nothing was saved" + ); + let protection = protection.lock().expect("not poisoned"); + assert!( + protection.check_block(&pubkey, 100).is_err(), + "the history raised the key's record" + ); + assert!(protection.check_block(&pubkey, 101).is_ok()); + } + + /// The history is written before any key is touched: when that write + /// fails, no key is saved or loaded, so nothing of the request is on disk + /// to load and sign after a restart as if the import had half succeeded. + #[tokio::test] + async fn a_failed_history_write_saves_no_key() { + let dir = tempfile::tempdir().expect("temp dir"); + let (context, protection) = context_with_record(dir.path()); + // A directory in the record's place makes the write's rename fail. + let record = protection + .lock() + .expect("not poisoned") + .path() + .expect("persisted") + .to_path_buf(); + std::fs::remove_file(&record).expect("removes the record"); + std::fs::create_dir(&record).expect("a directory in its place"); + + let (_, body) = send( + router(context, TOKEN.to_string()), + authed( + "POST", + "/eth/v1/keystores", + serde_json::json!({ + "keystores": [keystore_json()], + "passwords": [PASSWORD], + "slashing_protection": history(ZERO_ROOT, 100, 5, 10), + }), + ), + ) + .await; + assert_eq!(body["data"][0]["status"], "error", "got {body}"); + assert!( + body["data"][0]["message"] + .as_str() + .unwrap_or_default() + .starts_with("slashing_protection was not imported"), + "got {body}" + ); + assert!(protection.lock().expect("not poisoned").is_empty()); + assert!( + !dir.path().join(format!("{KEYSTORE_PUBKEY}.json")).exists(), + "no keystore was saved" + ); + } + #[tokio::test] async fn deleting_an_unknown_key_reports_not_found() { let (_, body) = send( @@ -1984,20 +2461,6 @@ mod tests { .to_string() } - /// Finding 6: the interchange's `genesis_validators_root` must stay - /// all-zero. See the module doc: a real root would make an empty history - /// look legitimate, which is worse than the deliberately unusable file - /// this client returns instead. - #[test] - fn the_empty_interchange_root_stays_all_zero() { - let interchange: serde_json::Value = - serde_json::from_str(&empty_interchange()).expect("valid json"); - assert_eq!( - interchange["metadata"]["genesis_validators_root"], - "0x0000000000000000000000000000000000000000000000000000000000000000", - ); - } - /// Finding 3: `require_bearer` must reject a wrong token of a different /// length too, not just one that fails a byte comparison partway /// through. Not a timing assertion (this crate has no timing harness), diff --git a/crates/validator/src/http_api/mod.rs b/crates/validator/src/http_api/mod.rs index f16de2850..189e5ac7a 100644 --- a/crates/validator/src/http_api/mod.rs +++ b/crates/validator/src/http_api/mod.rs @@ -163,8 +163,16 @@ pub fn router(context: KeymanagerContext, token: String) -> Router { .with_state(context) .layer(middleware::from_fn(require_bearer)) .layer(Extension(ApiToken(token))) + // `POST /eth/v1/keystores` carries a whole EIP-3076 interchange, which + // in the complete format for a batch of validators runs past axum's + // 2 MB default. Nimbus allows 16 MiB and Lodestar 20 MB; Lighthouse + // and Prysm set no limit. + .layer(axum::extract::DefaultBodyLimit::max(KEYMANAGER_BODY_LIMIT)) } +/// The largest request body the keymanager API reads. +const KEYMANAGER_BODY_LIMIT: usize = 64 * 1024 * 1024; + #[derive(Clone)] struct ApiToken(String); diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index faf9f35e7..24fc99dfd 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -19,12 +19,10 @@ //! //! [`docs/spec_deviations.md`]: https://github.com/lambdaclass/ethlambda/blob/main/docs/spec_deviations.md //! -//! The keymanager API registers a key it imports before loading it. It does -//! not yet carry history in or out: its specification makes -//! `slashing_protection` a required field of the `DELETE /eth/v1/keystores` -//! response, so this implementation returns a well-formed but empty EIP-3076 -//! interchange, and accepts and ignores the optional `slashing_protection` -//! field on import. +//! History moves in and out as EIP-3076 interchange files: through the +//! keymanager API's `POST` and `DELETE /eth/v1/keystores`, and offline through +//! `ethlambda slashing-protection`. A key imported with no history is +//! registered as never having signed before it is loaded. pub mod aggregation; pub mod aggregation_selection; diff --git a/crates/validator/src/slashing_protection/file.rs b/crates/validator/src/slashing_protection/file.rs index 84e2e9607..865a8237c 100644 --- a/crates/validator/src/slashing_protection/file.rs +++ b/crates/validator/src/slashing_protection/file.rs @@ -95,6 +95,40 @@ struct SignedAttestation { signing_root: Option, } +/// Parse an EIP-3076 version 5 interchange, complete or minimal, into its +/// chain and each entry's watermarks, in file order. +/// +/// A pubkey listed more than once is returned more than once: an import merges +/// them, while the record this client writes must never contain one twice. +/// The error is a reason, for the caller to attach to whatever it was reading. +pub(crate) fn parse(text: &str) -> std::result::Result<(Root, Vec<(BlsPubkey, Record)>), String> { + if text.trim().is_empty() { + return Err("empty; refusing to treat it as \"nothing signed\"".to_string()); + } + let interchange: Interchange = + serde_json::from_str(text).map_err(|err| format!("malformed: {err}"))?; + if interchange.metadata.interchange_format_version != INTERCHANGE_VERSION { + return Err(format!( + "interchange format version {:?}, expected {INTERCHANGE_VERSION:?}", + interchange.metadata.interchange_format_version + )); + } + let mut entries = Vec::with_capacity(interchange.data.len()); + for entry in &interchange.data { + let record = record_of(entry).map_err(|reason| { + format!( + "validator {}: {reason}", + crate::beacon_node::dto::encode_hex(&entry.pubkey) + ) + })?; + entries.push((BlsPubkey(entry.pubkey), record)); + } + Ok(( + Root::from(interchange.metadata.genesis_validators_root), + entries, + )) +} + /// Read and validate the record at `path`. pub(crate) fn read(path: &Path) -> Result<(Root, HashMap)> { let invalid = |reason: String| Error::SlashingProtection { @@ -106,29 +140,10 @@ pub(crate) fn read(path: &Path) -> Result<(Root, HashMap)> { path: path.display().to_string(), source, })?; - if text.trim().is_empty() { - return Err(invalid( - "the file is empty; refusing to treat it as \"nothing signed\"".to_string(), - )); - } - let interchange: Interchange = - serde_json::from_str(&text).map_err(|err| invalid(format!("malformed: {err}")))?; - if interchange.metadata.interchange_format_version != INTERCHANGE_VERSION { - return Err(invalid(format!( - "interchange format version {:?}, expected {INTERCHANGE_VERSION:?}", - interchange.metadata.interchange_format_version - ))); - } + let (root, entries) = parse(&text).map_err(invalid)?; - let mut records = HashMap::with_capacity(interchange.data.len()); - for entry in interchange.data { - let pubkey = BlsPubkey(entry.pubkey); - let record = record_of(&entry).map_err(|reason| { - invalid(format!( - "validator {}: {reason}", - crate::beacon_node::dto::encode_hex(&pubkey.0) - )) - })?; + let mut records = HashMap::with_capacity(entries.len()); + for (pubkey, record) in entries { if records.insert(pubkey, record).is_some() { return Err(invalid(format!( "validator {} appears more than once", @@ -136,15 +151,17 @@ pub(crate) fn read(path: &Path) -> Result<(Root, HashMap)> { ))); } } - Ok(( - Root::from(interchange.metadata.genesis_validators_root), - records, - )) + Ok((root, records)) } /// The watermarks one entry stands for: the highest slot, source and target it /// lists. More than one entry of a kind is reduced rather than refused, since /// taking the maximum is never less strict than any one of them. +/// +/// An attestation whose source is at or past its target is refused outside +/// genesis, the one shape the importer does not absorb: it is never valid, and +/// read into the record it would refuse every honest attestation after it +/// until the chain justified past that source. fn record_of(entry: &Entry) -> std::result::Result { let block_slot = entry.signed_blocks.iter().map(|block| block.slot).max(); @@ -197,17 +214,17 @@ pub(crate) fn exists(path: &Path) -> Result { } } -/// Durably replace the record at `path` with `records`. +/// `records` as a minimal EIP-3076 interchange for the chain named by +/// `genesis_validators_root`. /// -/// Compact rather than pretty: the file is rewritten once per slot and per +/// Compact rather than pretty: the record is rewritten once per slot and per /// block, and its size grows with the number of validators. Entries are /// written in pubkey order so the bytes depend only on the content, which /// keeps a diff of two files meaningful. -pub(crate) fn write<'a>( - path: &Path, +pub(crate) fn render<'a>( genesis_validators_root: Root, records: impl Iterator, -) -> Result<()> { +) -> serde_json::Result { let mut data: Vec = records .map(|(pubkey, record)| Entry { pubkey: pubkey.0, @@ -239,10 +256,20 @@ pub(crate) fn write<'a>( }, data, }; - let contents = serde_json::to_vec(&interchange).map_err(|err| Error::SlashingProtection { - path: path.display().to_string(), - reason: format!("could not encode the record: {err}"), - })?; + serde_json::to_string(&interchange) +} + +/// Durably replace the record at `path` with `records`. +pub(crate) fn write<'a>( + path: &Path, + genesis_validators_root: Root, + records: impl Iterator, +) -> Result<()> { + let contents = + render(genesis_validators_root, records).map_err(|err| Error::SlashingProtection { + path: path.display().to_string(), + reason: format!("could not encode the record: {err}"), + })?; crate::secure_fs::write_private_durably(path, contents).map_err(|err| { let path = path.display().to_string(); if err.after_rename { diff --git a/crates/validator/src/slashing_protection/mod.rs b/crates/validator/src/slashing_protection/mod.rs index 55ba67f2e..04f7c861d 100644 --- a/crates/validator/src/slashing_protection/mod.rs +++ b/crates/validator/src/slashing_protection/mod.rs @@ -59,7 +59,7 @@ pub mod file; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -185,6 +185,35 @@ impl Signed { } } +/// An EIP-3076 interchange parsed once, for [`SlashingProtection::absorb`]: +/// the chain it names and each validator's watermarks, reduced as +/// [`SlashingProtection::import`] describes. +#[derive(Debug, Clone)] +pub struct StagedHistory { + root: Root, + entries: Vec<(BlsPubkey, Record)>, +} + +impl StagedHistory { + /// The chain the interchange names: what a record created for it must + /// belong to. + pub fn root(&self) -> Root { + self.root + } +} + +/// What [`SlashingProtection::absorb`] did. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct Absorbed { + /// How many distinct validators the history named. + pub named: usize, + /// How many keys were newly registered as never having signed. + pub registered: usize, + /// The validators that already had a record and whose watermarks the + /// history raised: a caller running some of them can say so. + pub raised: Vec, +} + /// What one validator has signed: its watermarks. /// /// `None` is "never signed", kept distinct from zero so that slot 0 and epoch @@ -239,6 +268,18 @@ impl Record { } } + /// Both records' maxima: what an import leaves when it meets an existing + /// record. Never less strict than either. + fn merged(self, other: Record) -> Record { + Record { + block_slot: self.block_slot.max(other.block_slot), + attestation: match (self.attestation, other.attestation) { + (Some(ours), Some(theirs)) => Some(ours.max_with(theirs)), + (ours, theirs) => ours.or(theirs), + }, + } + } + fn with_attestation(self, data: &AttestationData) -> Record { let next = Signed { source: data.source.epoch, @@ -340,7 +381,8 @@ pub struct SlashingProtection { impl SlashingProtection { /// Open the record in `validators_dir` for the chain named by - /// `genesis_validators_root`, and hold its lock until this is dropped. + /// `genesis_validators_root`, and hold its lock until the returned record is + /// dropped. /// /// Every key in `keys` must already have a record. A key without one is /// refused unless `init` is set, in which case it is registered as never @@ -446,6 +488,37 @@ impl SlashingProtection { }) } + /// Open the record in `validators_dir` without a beacon node, for the + /// `slashing-protection` command, holding its lock until the returned record + /// is dropped. + /// + /// The chain is whatever the record names. With no record yet, one is + /// started for `create_for` if given, which is how an interchange is + /// imported into a directory before the client first runs there; with + /// neither, this is an error. The file itself is only written by the + /// first change, so an import that fails, or imports nothing, leaves the + /// directory as it found it. A record created for the wrong chain is not + /// dangerous either: [`Self::open`] refuses it at the client's next start. + pub fn open_offline(validators_dir: &Path, create_for: Option) -> Result { + let lock = Self::lock(validators_dir)?; + let path = validators_dir.join(file::FILE_NAME); + let (genesis_validators_root, records) = if file::exists(&path)? { + file::read(&path)? + } else if let Some(root) = create_for { + (root, HashMap::new()) + } else { + return Err(Error::SlashingProtection { + path: path.display().to_string(), + reason: "there is no slashing-protection record in this directory".to_string(), + }); + }; + Ok(Self { + genesis_validators_root, + records, + storage: Some(Storage { path, _lock: lock }), + }) + } + /// A record kept in memory only, with every key treated as registered on /// first use. /// @@ -562,16 +635,154 @@ impl SlashingProtection { /// only after this returns. A key that already has a record keeps it: /// registering never lowers a watermark. Returns how many were new. pub fn register(&mut self, pubkeys: &[BlsPubkey]) -> Result { - let new: HashMap = pubkeys - .iter() - .filter(|pubkey| !self.records.contains_key(*pubkey)) - .map(|pubkey| (*pubkey, Record::default())) + self.absorb(None, pubkeys) + .map(|absorbed| absorbed.registered) + } + + /// The chain this record belongs to. + pub fn genesis_validators_root(&self) -> Root { + self.genesis_validators_root + } + + /// Whether `pubkey` has a record. + pub fn contains(&self, pubkey: &BlsPubkey) -> bool { + self.records.contains_key(pubkey) + } + + /// Every validator with a record. + pub fn pubkeys(&self) -> Vec { + self.records.keys().copied().collect() + } + + /// Merge an EIP-3076 version 5 interchange, complete or minimal, into the + /// record, in one durable write. + /// + /// Each validator in it is reduced to its highest slot, source and target + /// and merged with any existing record by taking the maximum of each, so an + /// import never lowers a watermark, and slashable data inside it (two + /// blocks for one slot, a surround) is absorbed rather than refused: the + /// result is at least as strict as every message the file lists. The one + /// shape refused is an attestation whose source is at or past its target + /// outside genesis: never valid, and read into the record it would refuse + /// every honest attestation until the chain justified past that source. + /// A validator it names that has no record yet is registered with the + /// imported watermarks. + /// + /// All or nothing: an interchange for another chain or of another version, + /// or one that does not parse, changes nothing. Returns how many distinct + /// validators it named. + pub fn import(&mut self, interchange: &str) -> Result { + let staged = Self::stage(interchange)?; + self.absorb(Some(&staged), &[]) + .map(|absorbed| absorbed.named) + } + + /// Parse `interchange` into what [`Self::absorb`] takes, off any lock: + /// its version and shape are judged here, its chain by `absorb`. + pub fn stage(interchange: &str) -> Result { + let (root, entries) = + file::parse(interchange).map_err(|reason| Error::SlashingProtection { + path: "interchange".to_string(), + reason, + })?; + Ok(StagedHistory { root, entries }) + } + + /// Whether `staged` is for this record's chain, changing nothing. + fn check_staged(&self, staged: &StagedHistory) -> Result<()> { + if staged.root != self.genesis_validators_root { + return Err(Error::SlashingProtection { + path: "interchange".to_string(), + reason: format!( + "it is for genesis_validators_root {}, but this record is for {}", + encode_hex(&staged.root.0), + encode_hex(&self.genesis_validators_root.0) + ), + }); + } + Ok(()) + } + + /// Merge `history`, if any, as [`Self::import`] describes, and register + /// every key in `register` that then still has no record, in one durable + /// write: either all of it is on disk or none of it is. Reports how many + /// validators the history named, how many keys were newly registered, + /// and which existing records the history raised. + /// + /// A key in `register` that already has a record and is not in the + /// history keeps that record, with a warning: it is what this client last + /// signed with the key, which says nothing about what the key signed + /// elsewhere since. Nothing is written when no watermark would change, + /// so absorbing a history twice costs one write, not two. + pub fn absorb( + &mut self, + history: Option<&StagedHistory>, + register: &[BlsPubkey], + ) -> Result { + let mut staged: HashMap = HashMap::new(); + if let Some(history) = history { + self.check_staged(history)?; + for (pubkey, imported) in &history.entries { + let current = staged + .get(pubkey) + .or_else(|| self.records.get(pubkey)) + .copied() + .unwrap_or_default(); + staged.insert(*pubkey, current.merged(*imported)); + } + } + let named = staged.len(); + let mut registered = 0; + for pubkey in register { + if staged.contains_key(pubkey) { + continue; + } + if self.records.contains_key(pubkey) { + warn!( + pubkey = %encode_hex(&pubkey.0), + "Registering a validator that already has a slashing-protection record and \ + brought no history; it continues from what this client last signed with it" + ); + continue; + } + staged.insert(*pubkey, Record::default()); + registered += 1; + } + staged.retain(|pubkey, record| self.records.get(pubkey) != Some(record)); + let raised = staged + .keys() + .filter(|pubkey| self.records.contains_key(*pubkey)) + .copied() .collect(); - let count = new.len(); - if count > 0 { - self.commit(new)?; + if !staged.is_empty() { + self.commit(staged)?; } - Ok(count) + Ok(Absorbed { + named, + registered, + raised, + }) + } + + /// The records of `pubkeys` as a minimal EIP-3076 version 5 interchange. + /// + /// A key with no record is left out rather than listed as never having + /// signed: an empty entry would tell the importer something this client + /// does not know. A key listed twice is exported once: an interchange that + /// names a validator twice is refused by stricter importers, this client's + /// own record reader among them. + pub fn export(&self, pubkeys: &[BlsPubkey]) -> Result { + let mut seen = HashSet::with_capacity(pubkeys.len()); + let selected = pubkeys + .iter() + .filter(|pubkey| seen.insert(**pubkey)) + .filter_map(|pubkey| self.records.get_key_value(pubkey)); + file::render(self.genesis_validators_root, selected).map_err(|err| { + Error::SlashingProtection { + path: "interchange".to_string(), + reason: format!("could not encode the export: {err}"), + } + }) } /// Apply `changes` to the record: written to disk first, and taken into @@ -1081,6 +1292,221 @@ mod tests { ); } + /// A minimal interchange for `key` on `root`. + fn interchange(root_byte: u8, key: u8, slot: Slot, source: Epoch, target: Epoch) -> String { + format!( + r#"{{"metadata":{{"interchange_format_version":"5","genesis_validators_root":"{}"}},"data":[{{"pubkey":"{}","signed_blocks":[{{"slot":"{slot}"}}],"signed_attestations":[{{"source_epoch":"{source}","target_epoch":"{target}"}}]}}]}}"#, + encode_hex(&root(root_byte).0), + encode_hex(&pubkey(key).0) + ) + } + + #[test] + fn an_import_raises_the_watermarks_and_survives_a_restart() { + let dir = tempfile::tempdir().expect("temp dir"); + { + let mut protection = open(dir.path(), &[], false).expect("opens"); + assert_eq!( + protection + .import(&interchange(0x42, 1, 97, 2, 3)) + .expect("imports"), + 1 + ); + } + let mut protection = open(dir.path(), &[1], false).expect("the imported key has a record"); + assert!(propose(&mut protection, 1, 97).is_err()); + assert!(attest(&mut protection, 1, &data(96, 2, 3, 0xaa)).is_err()); + assert!(attest(&mut protection, 1, &data(128, 3, 4, 0xaa)).is_ok()); + } + + /// A newer history imported over an older record raises it, and the raised + /// watermarks survive a restart: the merge updates an existing entry, not + /// only inserts a missing one. + #[test] + fn an_import_raises_an_existing_watermark() { + let dir = tempfile::tempdir().expect("temp dir"); + { + let mut protection = open(dir.path(), &[1], true).expect("init"); + propose(&mut protection, 1, 10).expect("propose"); + attest(&mut protection, 1, &data(96, 2, 3, 0xaa)).expect("attest"); + + protection + .import(&interchange(0x42, 1, 50, 4, 6)) + .expect("imports"); + assert!(propose(&mut protection, 1, 40).is_err()); + assert!(attest(&mut protection, 1, &data(160, 3, 5, 0xaa)).is_err()); + } + let mut protection = open(dir.path(), &[1], false).expect("reopens"); + assert!(propose(&mut protection, 1, 50).is_err()); + assert!(propose(&mut protection, 1, 51).is_ok()); + assert!(attest(&mut protection, 1, &data(192, 4, 6, 0xaa)).is_err()); + assert!(attest(&mut protection, 1, &data(224, 4, 7, 0xaa)).is_ok()); + } + + /// `absorb` says which existing records it raised, and only those: a + /// record the history creates, or leaves as it was, is not one. + #[test] + fn absorb_reports_the_records_it_raised() { + let dir = tempfile::tempdir().expect("temp dir"); + let mut protection = open(dir.path(), &[1, 2], true).expect("init"); + propose(&mut protection, 1, 10).expect("propose"); + propose(&mut protection, 2, 200).expect("propose"); + + let staged = SlashingProtection::stage(&interchange(0x42, 1, 50, 4, 6)).expect("stages"); + let absorbed = protection.absorb(Some(&staged), &[]).expect("absorbs"); + assert_eq!(absorbed.raised, vec![pubkey(1)]); + assert_eq!(absorbed.named, 1); + + // Already at these watermarks: nothing raised, nothing registered. + let again = protection + .absorb(Some(&staged), &[pubkey(1)]) + .expect("absorbs"); + assert_eq!( + again, + Absorbed { + named: 1, + ..Absorbed::default() + } + ); + + // A record the history creates is not a raised one. + let fresh = SlashingProtection::stage(&interchange(0x42, 3, 50, 4, 6)).expect("stages"); + let created = protection.absorb(Some(&fresh), &[]).expect("absorbs"); + assert!(created.raised.is_empty()); + assert!(protection.contains(&pubkey(3))); + } + + /// A source at or past its target is refused whole outside genesis, where + /// `(0, 0)` is what every validator's first attestation looks like. + #[test] + fn an_import_with_a_source_past_its_target_is_refused_whole() { + let dir = tempfile::tempdir().expect("temp dir"); + let mut protection = open(dir.path(), &[], false).expect("opens"); + let err = protection + .import(&interchange(0x42, 1, 97, 5, 2)) + .expect_err("source past target"); + assert!(err.to_string().contains("at or after"), "{err}"); + protection + .import(&interchange(0x42, 1, 97, 5, 5)) + .expect_err("source at target"); + assert!(protection.is_empty(), "nothing of a refused import is held"); + + protection + .import(&interchange(0x42, 1, 0, 0, 0)) + .expect("the genesis attestation imports"); + assert!(attest(&mut protection, 1, &data(32, 0, 1, 0xaa)).is_ok()); + } + + /// An older history imported over a newer record must not lower it. + #[test] + fn an_import_never_lowers_a_watermark() { + let dir = tempfile::tempdir().expect("temp dir"); + let mut protection = open(dir.path(), &[1], true).expect("init"); + propose(&mut protection, 1, 200).expect("propose"); + attest(&mut protection, 1, &data(320, 9, 10, 0xaa)).expect("attest"); + + protection + .import(&interchange(0x42, 1, 97, 2, 3)) + .expect("imports"); + assert!(propose(&mut protection, 1, 150).is_err()); + assert!(attest(&mut protection, 1, &data(160, 4, 5, 0xaa)).is_err()); + } + + #[test] + fn an_import_for_another_chain_changes_nothing() { + let dir = tempfile::tempdir().expect("temp dir"); + let mut protection = open(dir.path(), &[], false).expect("opens"); + let err = protection + .import(&interchange(0x99, 1, 97, 2, 3)) + .expect_err("another chain"); + assert!(err.to_string().contains("genesis_validators_root"), "{err}"); + assert!(protection.is_empty()); + } + + #[test] + fn an_export_lists_only_keys_with_a_record() { + let dir = tempfile::tempdir().expect("temp dir"); + let mut protection = open(dir.path(), &[1], true).expect("init"); + propose(&mut protection, 1, 97).expect("propose"); + + let exported = protection.export(&[pubkey(1), pubkey(2)]).expect("exports"); + let json: serde_json::Value = serde_json::from_str(&exported).expect("json"); + assert_eq!(json["data"].as_array().expect("array").len(), 1); + assert_eq!(json["data"][0]["signed_blocks"][0]["slot"], "97"); + + // And it imports back into a fresh record as the same watermarks. + let other = tempfile::tempdir().expect("temp dir"); + let mut fresh = open(other.path(), &[], false).expect("opens"); + fresh.import(&exported).expect("round trip"); + assert!(propose(&mut fresh, 1, 97).is_err()); + } + + /// A key requested twice is exported once: this client's own record + /// reader refuses a file naming a validator twice, and so may others'. + #[test] + fn an_export_lists_a_key_requested_twice_once() { + let dir = tempfile::tempdir().expect("temp dir"); + let mut protection = open(dir.path(), &[1], true).expect("init"); + propose(&mut protection, 1, 97).expect("propose"); + + let exported = protection.export(&[pubkey(1), pubkey(1)]).expect("exports"); + let json: serde_json::Value = serde_json::from_str(&exported).expect("json"); + assert_eq!(json["data"].as_array().expect("array").len(), 1); + + let other = tempfile::tempdir().expect("temp dir"); + let mut fresh = open(other.path(), &[], false).expect("opens"); + fresh.import(&exported).expect("round trip"); + assert!(propose(&mut fresh, 1, 97).is_err()); + } + + #[test] + fn opening_offline_needs_a_record_or_a_chain_to_create_one_for() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join(file::FILE_NAME); + SlashingProtection::open_offline(dir.path(), None).expect_err("nothing to open"); + let mut created = + SlashingProtection::open_offline(dir.path(), Some(root(0x42))).expect("creates"); + assert_eq!(created.genesis_validators_root(), root(0x42)); + // Nothing is on disk until something is recorded, so an import that + // fails leaves no record behind for a chain nothing then runs on. + assert!(!path.exists(), "the record is not written at open"); + created + .import(&interchange(0x42, 1, 97, 2, 3)) + .expect("imports"); + assert!(path.exists(), "the first change writes the record"); + drop(created); + let reopened = SlashingProtection::open_offline(dir.path(), Some(root(0x99))) + .expect("an existing record keeps its own chain"); + assert_eq!(reopened.genesis_validators_root(), root(0x42)); + } + + /// Importing a history twice is the same as importing it once: the merge + /// takes maxima, so the second pass changes nothing. + #[test] + fn an_import_is_idempotent() { + let dir = tempfile::tempdir().expect("temp dir"); + let mut protection = open(dir.path(), &[], false).expect("opens"); + let history = interchange(0x42, 1, 97, 2, 3); + protection.import(&history).expect("imports"); + let once = protection.export(&[pubkey(1)]).expect("exports"); + assert_eq!(protection.import(&history).expect("imports again"), 1); + let twice = protection.export(&[pubkey(1)]).expect("exports"); + assert_eq!(once, twice); + assert!(propose(&mut protection, 1, 97).is_err()); + assert!(propose(&mut protection, 1, 98).is_ok()); + } + + #[test] + fn an_empty_interchange_is_refused() { + let dir = tempfile::tempdir().expect("temp dir"); + let mut protection = open(dir.path(), &[], false).expect("opens"); + for text in ["", "{}", "[]"] { + protection.import(text).expect_err("not an interchange"); + SlashingProtection::stage(text).expect_err("not an interchange"); + } + assert!(protection.is_empty()); + } + #[test] fn an_unregistered_key_is_refused_on_a_persisted_record() { let dir = tempfile::tempdir().expect("temp dir"); diff --git a/crates/validator/tests/interchange_vectors.rs b/crates/validator/tests/interchange_vectors.rs new file mode 100644 index 000000000..a338d8b73 --- /dev/null +++ b/crates/validator/tests/interchange_vectors.rs @@ -0,0 +1,208 @@ +//! EIP-3076's shared interchange test vectors +//! (`eth-clients/slashing-protection-interchange-tests`), run against +//! `SlashingProtection`. +//! +//! Each case starts from an empty record, then for every step imports the +//! step's interchange and attempts its blocks and attestations, each of which +//! must be allowed or refused as the case says. This client keeps EIP-3076's +//! minimal watermarks, so the expectation read is `should_succeed`, the +//! minimal strategy's, rather than `should_succeed_complete`. +//! +//! The vectors are a download: `make slashing-protection-interchange-tests`, +//! which `make test` runs first. + +use std::collections::HashSet; +use std::path::{Path, PathBuf}; + +use ethlambda_types::beacon::containers::shared::{AttestationData, Checkpoint}; +use ethlambda_types::beacon::primitives::{BlsPubkey, Root}; +use ethlambda_validator::beacon_node::dto::parse_pubkey; +use ethlambda_validator::slashing_protection::SlashingProtection; +use serde::Deserialize; + +#[derive(Deserialize)] +struct Case { + name: String, + genesis_validators_root: String, + steps: Vec, +} + +#[derive(Deserialize)] +struct Step { + should_succeed: bool, + contains_slashable_data: bool, + interchange: serde_json::Value, + blocks: Vec, + attestations: Vec, +} + +#[derive(Deserialize)] +struct Block { + pubkey: String, + slot: String, + should_succeed: bool, +} + +#[derive(Deserialize)] +struct Attestation { + pubkey: String, + source_epoch: String, + target_epoch: String, + should_succeed: bool, +} + +fn vectors_dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../slashing-protection-interchange-tests/tests/generated") +} + +#[test] +fn eip_3076_interchange_vectors() { + let dir = vectors_dir(); + assert!( + dir.is_dir(), + "{} is missing: run `make slashing-protection-interchange-tests`", + dir.display() + ); + + let mut paths: Vec = std::fs::read_dir(&dir) + .expect("reads the vectors directory") + .map(|entry| entry.expect("reads an entry").path()) + .filter(|path| { + path.extension() + .is_some_and(|extension| extension == "json") + }) + .collect(); + paths.sort(); + assert!(!paths.is_empty(), "no vectors in {}", dir.display()); + + let failures: Vec = paths + .iter() + .filter_map(|path| { + let text = std::fs::read_to_string(path).expect("reads a vector"); + let case: Case = serde_json::from_str(&text).expect("parses a vector"); + run(&case) + .err() + .map(|reason| format!("{}: {reason}", case.name)) + }) + .collect(); + assert!( + failures.is_empty(), + "{} of {} vectors failed:\n{}", + failures.len(), + paths.len(), + failures.join("\n") + ); +} + +fn run(case: &Case) -> Result<(), String> { + let root = parse_root(&case.genesis_validators_root)?; + + // The vectors start from an empty database in which any validator may + // sign, so every key a case signs for is registered as never having signed: + // with no current slot, as before genesis. + let mut keys: Vec = Vec::new(); + let mut seen = HashSet::new(); + for step in &case.steps { + let texts = step.blocks.iter().map(|block| &block.pubkey).chain( + step.attestations + .iter() + .map(|attestation| &attestation.pubkey), + ); + for text in texts { + let pubkey = parse_pubkey(text).map_err(|err| err.to_string())?; + if seen.insert(pubkey.0) { + keys.push(pubkey); + } + } + } + + let dir = tempfile::tempdir().map_err(|err| err.to_string())?; + let mut protection = + SlashingProtection::open(dir.path(), root, &keys, true).map_err(|err| err.to_string())?; + + for (index, step) in case.steps.iter().enumerate() { + let imported = protection.import(&step.interchange.to_string()); + match (imported.is_ok(), step.should_succeed) { + (true, true) | (false, false) => {} + // The README lets an importer refuse an interchange holding + // slashable data, in which case the rest of the case is skipped. + // This one absorbs all of it but an attestation whose source is at + // or past its target, so a refusal on such a step ends the case, + // as it does in Lighthouse; on any other step it is a failure. + (false, true) if step.contains_slashable_data => return Ok(()), + (false, true) => { + return Err(format!( + "step {index}: import failed but should succeed (slashable data: {}): {}", + step.contains_slashable_data, + imported + .err() + .map(|err| err.to_string()) + .unwrap_or_default() + )); + } + (true, false) => { + return Err(format!("step {index}: import succeeded but should fail")); + } + } + + for (position, block) in step.blocks.iter().enumerate() { + let pubkey = parse_pubkey(&block.pubkey).map_err(|err| err.to_string())?; + let slot = parse_u64(&block.slot)?; + let allowed = protection + .check_and_record_block(&pubkey, slot) + .map_err(|err| err.to_string())? + .is_ok(); + if allowed != block.should_succeed { + return Err(format!( + "step {index}, block {position} at slot {slot}: allowed {allowed}, \ + expected {}", + block.should_succeed + )); + } + } + + for (position, attestation) in step.attestations.iter().enumerate() { + let pubkey = parse_pubkey(&attestation.pubkey).map_err(|err| err.to_string())?; + let source = parse_u64(&attestation.source_epoch)?; + let target = parse_u64(&attestation.target_epoch)?; + let verdicts = protection + .check_and_record_attestations(&[pubkey], &data(source, target)) + .map_err(|err| err.to_string())?; + let allowed = verdicts.first().is_some_and(|verdict| verdict.is_ok()); + if allowed != attestation.should_succeed { + return Err(format!( + "step {index}, attestation {position} ({source} -> {target}): allowed \ + {allowed}, expected {}", + attestation.should_succeed + )); + } + } + } + Ok(()) +} + +/// Only the epochs are judged; the rest of the data is filler. +fn data(source: u64, target: u64) -> AttestationData { + AttestationData { + slot: target * 32, + index: 0, + beacon_block_root: Root::ZERO, + source: Checkpoint { + epoch: source, + root: Root::ZERO, + }, + target: Checkpoint { + epoch: target, + root: Root::ZERO, + }, + } +} + +fn parse_u64(text: &str) -> Result { + text.parse().map_err(|err| format!("{text:?}: {err}")) +} + +fn parse_root(text: &str) -> Result { + ethlambda_validator::beacon_node::dto::parse_root(text).map_err(|err| err.to_string()) +} diff --git a/docs/cli.md b/docs/cli.md index 4455072c4..e77aa426a 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -8,7 +8,7 @@ | `ethlambda beacon ` | The Ethereum Beacon Chain, as a gossip follower | | `ethlambda ` | `node`: the subcommand is injected | -Three subcommands follow no chain. `ethlambda replay-era` replays real chain +Four subcommands follow no chain. `ethlambda replay-era` replays real chain history from `.era` files and checks it against the states stored in them; see [`era_replay.md`](./era_replay.md) for its flags and checks. `ethlambda benchmark` is the offline benchmarking harness, covering block building (`benchmark synthetic`, run @@ -18,7 +18,9 @@ what each workload measures; `benchmark import`'s flags are in [their own section](#benchmark-import-flags) below. `ethlambda validator` is the validator client: a separate process that holds keys and performs duties against a beacon node over the standard REST API, documented in -[its own section](#validator-flags) below. +[its own section](#validator-flags) below. `ethlambda slashing-protection` +moves that client's EIP-3076 history in and out of its directory, offline; see +[its section](#slashing-protection) below. ## `node` is the default @@ -276,8 +278,9 @@ implementation's, so none of the [common flags](#common-flags) apply to it. > the directory should be a local disk, since every signature waits for the > record to be flushed to it, and the lock that keeps a second process off it > is an `flock`, which a network filesystem may not honour. Unix only. A key with no record is refused at startup unless -> `--init-slashing-protection` is passed. Importing another client's history is -> not supported yet. See +> `--init-slashing-protection` is passed. A key moving here from another client +> brings its history with [`slashing-protection import`](#slashing-protection), +> or in the same keymanager API request that imports the key. See > [Spec Deviations](./spec_deviations.md#the-validator-client-keeps-eip-3076s-minimal-watermarks-not-full-history). | Flag | Default | Meaning | @@ -433,6 +436,14 @@ specification requires; `POST graffiti` refuses text over 32 bytes rather than truncating it; and every route answers `404` for a key this client does not hold. +The keystore routes carry slashing-protection history. `POST` refuses every +key in the request if its `slashing_protection` does not parse or is for +another chain, and otherwise writes that history before any key is saved, so a +key never loads ahead of it; a validator the history names without a keystore +in the request gets a record too. `DELETE` +returns the deleted keys' records as a minimal EIP-3076 interchange, answering +`not_active` for a key that is gone but still has a record. + The specification requires TLS. This binds to `--http-address`, loopback by default; exposing it further means putting a TLS terminator in front. @@ -444,6 +455,27 @@ beacon chain. Every series is registered at startup so it reads zero rather than being absent before its first event, which is what lets an alert fire on "attestations stopped". See [Metrics](./metrics.md#validator-client). +## `slashing-protection` + +Moves the validator client's EIP-3076 history in and out of its +`--validators-dir`, offline. It takes the lock the client holds while running, +so it refuses while a client runs on that directory, and it needs no beacon +node: the record names its own chain. + +```sh +# Bring a key's history from another client before its first start here. +ethlambda slashing-protection import --validators-dir ./validators interchange.json + +# Hand the history over before running the keys elsewhere. +ethlambda slashing-protection export --validators-dir ./validators out.json +ethlambda slashing-protection export --validators-dir ./validators --pubkeys 0xab..,0xcd.. out.json +``` + +| Sub-command | Does | +|---|---| +| `import ` | Merges a version 5 interchange, complete or minimal, into the record. Each validator is reduced to its highest slot, source and target and merged by maximum, so nothing already recorded is lowered and slashable data in the file is absorbed rather than refused, except an attestation whose source is at or past its target outside genesis, which is refused. All or nothing. With no record yet, the import creates one for the interchange's chain, and a failed import creates nothing; the client refuses the record at its next start if that is not its beacon node's chain | +| `export [--pubkeys ...] ` | Writes the record, or the named validators' part of it, as a minimal version 5 interchange, readable by its owner only. A named validator with no record fails the export, so a typo or the wrong directory cannot produce a file that reads as complete. Takes the record's lock, so the directory must be writable. Refuses to overwrite an existing file | + ## `benchmark import` flags Neither sub-command follows a chain; see [`benchmarking.md`](./benchmarking.md#import-workload) diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 0e96cf6f4..49ee15c03 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -114,15 +114,17 @@ anything that does not advance past them. slot is equivocation that peers penalize in gossip, not a slashing. - **What is not covered:** the same keys running in another client on another machine. No local record can see that. -- **Keymanager consequences:** history is not carried in or out yet. - `DELETE /eth/v1/keystores` must return an EIP-3076 interchange; this client - returns a well-formed but empty one whose `genesis_validators_root` is - deliberately all-zero, so a conformant importer rejects it outright rather - than trusting an empty history. `POST` refuses a request that carries the - `slashing_protection` field, importing none of its keys, so a migration that - hands over history fails loudly instead of starting the key as never having - signed; a key imported without it starts that way, like one registered by - `--init-slashing-protection`. +- **Interchange:** an import (`ethlambda slashing-protection import`, or the + keymanager API's `POST /eth/v1/keystores`) reduces each validator to its + highest slot, source and target and merges them with the record by maximum, + so slashable data in the file is absorbed rather than refused, as Lighthouse + and Nimbus do. The one exception is an attestation whose source is at or + past its target outside genesis, which fails the whole import: it is never + valid, and read into the record it would refuse every honest attestation + until the chain justified past that source. An export (`ethlambda slashing-protection export`, or `DELETE + /eth/v1/keystores`) is the record itself, in the minimal format. Both pass + the shared `eth-clients/slashing-protection-interchange-tests` v5.3.0 vectors + under their `should_succeed` (minimal) expectations. ## Builder bids: gossip and API only (no builder API)