Switch default slasher backend to LMDB (#4360)

## Issue Addressed

Closes #4354
Closes #3987

Replaces #4305, #4283

## Proposed Changes

This switches the default slasher backend _back_ to LMDB.

If an MDBX database exists and the MDBX backend is enabled then MDBX will continue to be used. Our release binaries and Docker images will continue to include MDBX for as long as it is practical, so users of these should not notice any difference.

The main benefit is to users compiling from source and devs running tests. These users no longer have to struggle to compile MDBX and deal with the compatibility issues that arises. Similarly, devs don't need to worry about toggling feature flags in tests or risk forgetting to run the slasher tests due to backend issues.

Makefile

@@ -145,8 +145,9 @@ test-op-pool-%:
 
 # Run the tests in the `slasher` crate for all supported database backends.
 test-slasher:
-	cargo test --release -p slasher --features mdbx
+	cargo test --release -p slasher --features lmdb
-	cargo test --release -p slasher --no-default-features --features lmdb
+	cargo test --release -p slasher --no-default-features --features mdbx
+	cargo test --release -p slasher --features lmdb,mdbx # both backends enabled
 
 # Runs only the tests/state_transition_vectors tests.
 run-state-transition-tests:

beacon_node/Cargo.toml

@@ -36,7 +36,7 @@ clap_utils = { path = "../common/clap_utils" }
 hyper = "0.14.4"
 lighthouse_version = { path = "../common/lighthouse_version" }
 hex = "0.4.2"
-slasher = { path = "../slasher", default-features = false }
+slasher = { path = "../slasher" }
 monitoring_api = { path = "../common/monitoring_api" }
 sensitive_url = { path = "../common/sensitive_url" }
 http_api = { path = "http_api" }

beacon_node/beacon_chain/Cargo.toml

@@ -54,7 +54,7 @@ fork_choice = { path = "../../consensus/fork_choice" }
 task_executor = { path = "../../common/task_executor" }
 derivative = "2.1.1"
 itertools = "0.10.0"
-slasher = { path = "../../slasher", default-features = false }
+slasher = { path = "../../slasher" }
 eth2 = { path = "../../common/eth2" }
 strum = { version = "0.24.0", features = ["derive"] }
 logging = { path = "../../common/logging" }

beacon_node/client/Cargo.toml

@@ -39,7 +39,7 @@ time = "0.3.5"
 directory = {path = "../../common/directory"}
 http_api = { path = "../http_api" }
 http_metrics = { path = "../http_metrics" }
-slasher = { path = "../../slasher", default-features = false }
+slasher = { path = "../../slasher" }
 slasher_service = { path = "../../slasher/service" }
 monitoring_api = {path = "../../common/monitoring_api"}
 execution_layer = { path = "../execution_layer" }

beacon_node/src/lib.rs

@@ -16,7 +16,7 @@ pub use client::{Client, ClientBuilder, ClientConfig, ClientGenesis};
 pub use config::{get_config, get_data_dir, get_slots_per_restore_point, set_network_config};
 use environment::RuntimeContext;
 pub use eth2_config::Eth2Config;
-use slasher::Slasher;
+use slasher::{DatabaseBackendOverride, Slasher};
 use slog::{info, warn};
 use std::ops::{Deref, DerefMut};
 use std::sync::Arc;
@@ -86,7 +86,27 @@ impl<E: EthSpec> ProductionBeaconNode<E> {
             .http_api_config(client_config.http_api.clone())
             .disk_store(&db_path, &freezer_db_path, store_config, log.clone())?;
 
-        let builder = if let Some(slasher_config) = client_config.slasher.clone() {
+        let builder = if let Some(mut slasher_config) = client_config.slasher.clone() {
+            match slasher_config.override_backend() {
+                DatabaseBackendOverride::Success(old_backend) => {
+                    info!(
+                        log,
+                        "Slasher backend overriden";
+                        "reason" => "database exists",
+                        "configured_backend" => %old_backend,
+                        "override_backend" => %slasher_config.backend,
+                    );
+                }
+                DatabaseBackendOverride::Failure(path) => {
+                    warn!(
+                        log,
+                        "Slasher backend override failed";
+                        "advice" => "delete old MDBX database or enable MDBX backend",
+                        "path" => path.display()
+                    );
+                }
+                _ => {}
+            }
             let slasher = Arc::new(
                 Slasher::open(slasher_config, log.new(slog::o!("service" => "slasher")))
                     .map_err(|e| format!("Slasher open error: {:?}", e))?,

beacon_node/store/Cargo.toml

@@ -10,7 +10,7 @@ beacon_chain = {path = "../beacon_chain"}
 
 [dependencies]
 db-key = "0.0.5"
-leveldb = { version = "0.8.6", default-features = false }
+leveldb = { version = "0.8.6" }
 parking_lot = "0.12.0"
 itertools = "0.10.0"
 ethereum_ssz = "0.5.0"

book/src/installation-source.md

@@ -159,13 +159,13 @@ Commonly used features include:
 * `gnosis`: support for the Gnosis Beacon Chain.
 * `portable`: support for legacy hardware.
 * `modern`: support for exclusively modern hardware.
-* `slasher-mdbx`: support for the MDBX slasher backend. Enabled by default.
+* `slasher-lmdb`: support for the LMDB slasher backend. Enabled by default.
-* `slasher-lmdb`: support for the LMDB slasher backend.
+* `slasher-mdbx`: support for the MDBX slasher backend.
 * `jemalloc`: use [`jemalloc`][jemalloc] to allocate memory. Enabled by default on Linux and macOS.
   Not supported on Windows.
 * `spec-minimal`: support for the minimal preset (useful for testing).
 
-Default features (e.g. `slasher-mdbx`) may be opted out of using the `--no-default-features`
+Default features (e.g. `slasher-lmdb`) may be opted out of using the `--no-default-features`
 argument for `cargo`, which can be plumbed in via the `CARGO_INSTALL_EXTRA_FLAGS` environment variable.
 E.g.
 

book/src/slasher.md

@@ -46,23 +46,49 @@ directory.
 
 * Flag: `--slasher-backend NAME`
 * Argument: one of `mdbx`, `lmdb` or `disabled`
-* Default: `mdbx`
+* Default: `lmdb` for new installs, `mdbx` if an MDBX database already exists
 
-Since Lighthouse v2.6.0 it is possible to use one of several database backends with the slasher:
+It is possible to use one of several database backends with the slasher:
 
-- MDBX (default)
+- LMDB (default)
-- LMDB
+- MDBX
 
 The advantage of MDBX is that it performs compaction, resulting in less disk usage over time. The
-disadvantage is that upstream MDBX has removed support for Windows and macOS, so Lighthouse is stuck
+disadvantage is that upstream MDBX is unstable, so Lighthouse is pinned to a specific version.
-on an older version. If bugs are found in our pinned version of MDBX it may be deprecated in future.
+If bugs are found in our pinned version of MDBX it may be deprecated in future.
 
-LMDB does not have compaction but is more stable upstream than MDBX. It is not currently recommended
+LMDB does not have compaction but is more stable upstream than MDBX. If running with the LMDB
-to use the LMDB backend on Windows.
+backend on Windows it is recommended to allow extra space due to this issue:
+[sigp/lighthouse#2342](https://github.com/sigp/lighthouse/issues/2342).
 
 More backends may be added in future.
 
-### Switching Backends
+#### Backend Override
+
+The default backend was changed from MDBX to LMDB in Lighthouse v4.3.0.
+
+If an MDBX database is already found on disk, then Lighthouse will try to use it. This will result
+in a log at start-up:
+
+```
+INFO Slasher backend overriden    reason: database exists, configured_backend: lmdb, overriden_backend: mdbx
+```
+
+If the running Lighthouse binary doesn't have the MDBX backend enabled but an existing database is
+found, then a warning will be logged and Lighthouse will use the LMDB backend and create a new database:
+
+```
+WARN Slasher backend override failed    advice: delete old MDBX database or enable MDBX backend, path: /home/user/.lighthouse/mainnet/beacon/slasher_db/mdbx.dat
+```
+
+In this case you should either obtain a Lighthouse binary with the MDBX backend enabled, or delete
+the files for the old backend. The pre-built Lighthouse binaries and Docker images have MDBX enabled,
+or if you're [building from source](./installation-source.md) you can enable the `slasher-mdbx` feature.
+
+To delete the files, use the `path` from the `WARN` log, and then delete the `mbdx.dat` and
+`mdbx.lck` files.
+
+#### Switching Backends
 
 If you change database backends and want to reclaim the space used by the old backend you can
 delete the following files from your `slasher_db` directory:

lighthouse/Cargo.toml

@@ -7,7 +7,7 @@ autotests = false
 rust-version = "1.68.2"
 
 [features]
-default = ["slasher-mdbx"]
+default = ["slasher-lmdb"]
 # Writes debugging .ssz files to /tmp during block processing.
 write_ssz_files = ["beacon_node/write_ssz_files"]
 # Compiles the BLS crypto code so that the binary is portable across machines.
@@ -55,7 +55,7 @@ malloc_utils = { path = "../common/malloc_utils" }
 directory = { path = "../common/directory" }
 unused_port = { path = "../common/unused_port" }
 database_manager = { path = "../database_manager" }
-slasher = { path = "../slasher", default-features = false }
+slasher = { path = "../slasher" }
 
 [dev-dependencies]
 tempfile = "3.1.0"

lighthouse/tests/beacon_node.rs

@@ -1767,10 +1767,12 @@ fn no_reconstruct_historic_states_flag() {
 }
 
 // Tests for Slasher flags.
+// Using `--slasher-max-db-size` to work around https://github.com/sigp/lighthouse/issues/2342
 #[test]
 fn slasher_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .run_with_zero_port()
         .with_config_and_dir(|config, dir| {
             if let Some(slasher_config) = &config.slasher {
@@ -1788,6 +1790,7 @@ fn slasher_dir_flag() {
     let dir = TempDir::new().expect("Unable to create temporary directory");
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-dir", dir.path().as_os_str().to_str())
         .run_with_zero_port()
         .with_config(|config| {
@@ -1802,6 +1805,7 @@ fn slasher_dir_flag() {
 fn slasher_update_period_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-update-period", Some("100"))
         .run_with_zero_port()
         .with_config(|config| {
@@ -1816,6 +1820,7 @@ fn slasher_update_period_flag() {
 fn slasher_slot_offset_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-slot-offset", Some("11.25"))
         .run_with_zero_port()
         .with_config(|config| {
@@ -1828,6 +1833,7 @@ fn slasher_slot_offset_flag() {
 fn slasher_slot_offset_nan_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-slot-offset", Some("NaN"))
         .run_with_zero_port();
 }
@@ -1835,6 +1841,7 @@ fn slasher_slot_offset_nan_flag() {
 fn slasher_history_length_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-history-length", Some("2048"))
         .run_with_zero_port()
         .with_config(|config| {
@@ -1849,20 +1856,21 @@ fn slasher_history_length_flag() {
 fn slasher_max_db_size_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
-        .flag("slasher-max-db-size", Some("10"))
+        .flag("slasher-max-db-size", Some("2"))
         .run_with_zero_port()
         .with_config(|config| {
             let slasher_config = config
                 .slasher
                 .as_ref()
                 .expect("Unable to parse Slasher config");
-            assert_eq!(slasher_config.max_db_size_mbs, 10240);
+            assert_eq!(slasher_config.max_db_size_mbs, 2048);
         });
 }
 #[test]
 fn slasher_attestation_cache_size_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-att-cache-size", Some("10000"))
         .run_with_zero_port()
         .with_config(|config| {
@@ -1877,6 +1885,7 @@ fn slasher_attestation_cache_size_flag() {
 fn slasher_chunk_size_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-chunk-size", Some("32"))
         .run_with_zero_port()
         .with_config(|config| {
@@ -1891,6 +1900,7 @@ fn slasher_chunk_size_flag() {
 fn slasher_validator_chunk_size_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-validator-chunk-size", Some("512"))
         .run_with_zero_port()
         .with_config(|config| {
@@ -1905,6 +1915,7 @@ fn slasher_validator_chunk_size_flag() {
 fn slasher_broadcast_flag() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .flag("slasher-broadcast", None)
         .run_with_zero_port()
         .with_config(|config| {
@@ -1920,10 +1931,11 @@ fn slasher_broadcast_flag() {
 fn slasher_backend_default() {
     CommandLineTest::new()
         .flag("slasher", None)
+        .flag("slasher-max-db-size", Some("1"))
         .run_with_zero_port()
         .with_config(|config| {
             let slasher_config = config.slasher.as_ref().unwrap();
-            assert_eq!(slasher_config.backend, slasher::DatabaseBackend::Mdbx);
+            assert_eq!(slasher_config.backend, slasher::DatabaseBackend::Lmdb);
         });
 }
 
@@ -1933,11 +1945,12 @@ fn slasher_backend_override_to_default() {
     // called "disabled" results in a panic.
     CommandLineTest::new()
         .flag("slasher", None)
-        .flag("slasher-backend", Some("mdbx"))
+        .flag("slasher-max-db-size", Some("1"))
+        .flag("slasher-backend", Some("lmdb"))
         .run_with_zero_port()
         .with_config(|config| {
             let slasher_config = config.slasher.as_ref().unwrap();
-            assert_eq!(slasher_config.backend, slasher::DatabaseBackend::Mdbx);
+            assert_eq!(slasher_config.backend, slasher::DatabaseBackend::Lmdb);
         });
 }
 

slasher/Cargo.toml

@@ -5,7 +5,7 @@ authors = ["Michael Sproul <michael@sigmaprime.io>"]
 edition = "2021"
 
 [features]
-default = ["mdbx"]
+default = ["lmdb"]
 mdbx = ["dep:mdbx"]
 lmdb = ["lmdb-rkv", "lmdb-rkv-sys"]
 

slasher/service/Cargo.toml

@@ -9,7 +9,7 @@ beacon_chain = { path = "../../beacon_node/beacon_chain" }
 directory = { path = "../../common/directory" }
 lighthouse_network = { path = "../../beacon_node/lighthouse_network" }
 network = { path = "../../beacon_node/network" }
-slasher = { path = "..", default-features = false }
+slasher = { path = ".." }
 slog = "2.5.2"
 slot_clock = { path = "../../common/slot_clock" }
 state_processing = { path = "../../consensus/state_processing" }

slasher/src/config.rs

@@ -13,15 +13,16 @@ pub const DEFAULT_MAX_DB_SIZE: usize = 256 * 1024; // 256 GiB
 pub const DEFAULT_ATTESTATION_ROOT_CACHE_SIZE: usize = 100_000;
 pub const DEFAULT_BROADCAST: bool = false;
 
-#[cfg(feature = "mdbx")]
+#[cfg(all(feature = "mdbx", not(feature = "lmdb")))]
 pub const DEFAULT_BACKEND: DatabaseBackend = DatabaseBackend::Mdbx;
-#[cfg(all(feature = "lmdb", not(feature = "mdbx")))]
+#[cfg(feature = "lmdb")]
 pub const DEFAULT_BACKEND: DatabaseBackend = DatabaseBackend::Lmdb;
 #[cfg(not(any(feature = "mdbx", feature = "lmdb")))]
 pub const DEFAULT_BACKEND: DatabaseBackend = DatabaseBackend::Disabled;
 
 pub const MAX_HISTORY_LENGTH: usize = 1 << 16;
 pub const MEGABYTE: usize = 1 << 20;
+pub const MDBX_DATA_FILENAME: &str = "mdbx.dat";
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Config {
@@ -64,6 +65,13 @@ pub enum DatabaseBackend {
     Disabled,
 }
 
+#[derive(Debug, PartialEq)]
+pub enum DatabaseBackendOverride {
+    Success(DatabaseBackend),
+    Failure(PathBuf),
+    Noop,
+}
+
 impl Config {
     pub fn new(database_path: PathBuf) -> Self {
         Self {
@@ -161,4 +169,28 @@ impl Config {
             .filter(move |v| self.validator_chunk_index(**v) == validator_chunk_index)
             .copied()
     }
+
+    pub fn override_backend(&mut self) -> DatabaseBackendOverride {
+        let mdbx_path = self.database_path.join(MDBX_DATA_FILENAME);
+
+        #[cfg(feature = "mdbx")]
+        let already_mdbx = self.backend == DatabaseBackend::Mdbx;
+        #[cfg(not(feature = "mdbx"))]
+        let already_mdbx = false;
+
+        if !already_mdbx && mdbx_path.exists() {
+            #[cfg(feature = "mdbx")]
+            {
+                let old_backend = self.backend;
+                self.backend = DatabaseBackend::Mdbx;
+                DatabaseBackendOverride::Success(old_backend)
+            }
+            #[cfg(not(feature = "mdbx"))]
+            {
+                DatabaseBackendOverride::Failure(mdbx_path)
+            }
+        } else {
+            DatabaseBackendOverride::Noop
+        }
+    }
 }

slasher/src/lib.rs

@@ -21,7 +21,7 @@ pub use crate::slasher::Slasher;
 pub use attestation_queue::{AttestationBatch, AttestationQueue, SimpleBatch};
 pub use attester_record::{AttesterRecord, CompactAttesterRecord, IndexedAttesterRecord};
 pub use block_queue::BlockQueue;
-pub use config::{Config, DatabaseBackend};
+pub use config::{Config, DatabaseBackend, DatabaseBackendOverride};
 pub use database::{
     interface::{Database, Environment, RwTransaction},
     IndexedAttestationId, SlasherDB,

slasher/tests/backend.rs

@@ -0,0 +1,57 @@
+#![cfg(all(feature = "lmdb"))]
+
+use slasher::{config::MDBX_DATA_FILENAME, Config, DatabaseBackend, DatabaseBackendOverride};
+use std::fs::File;
+use tempfile::tempdir;
+
+#[test]
+#[cfg(all(feature = "mdbx", feature = "lmdb"))]
+fn override_no_existing_db() {
+    let tempdir = tempdir().unwrap();
+    let mut config = Config::new(tempdir.path().into());
+    assert_eq!(config.override_backend(), DatabaseBackendOverride::Noop);
+}
+
+#[test]
+#[cfg(all(feature = "mdbx", feature = "lmdb"))]
+fn override_with_existing_mdbx_db() {
+    let tempdir = tempdir().unwrap();
+    let mut config = Config::new(tempdir.path().into());
+
+    File::create(config.database_path.join(MDBX_DATA_FILENAME)).unwrap();
+
+    assert_eq!(
+        config.override_backend(),
+        DatabaseBackendOverride::Success(DatabaseBackend::Lmdb)
+    );
+    assert_eq!(config.backend, DatabaseBackend::Mdbx);
+}
+
+#[test]
+#[cfg(all(feature = "mdbx", feature = "lmdb"))]
+fn no_override_with_existing_mdbx_db() {
+    let tempdir = tempdir().unwrap();
+    let mut config = Config::new(tempdir.path().into());
+    config.backend = DatabaseBackend::Mdbx;
+
+    File::create(config.database_path.join(MDBX_DATA_FILENAME)).unwrap();
+
+    assert_eq!(config.override_backend(), DatabaseBackendOverride::Noop);
+    assert_eq!(config.backend, DatabaseBackend::Mdbx);
+}
+
+#[test]
+#[cfg(all(not(feature = "mdbx"), feature = "lmdb"))]
+fn failed_override_with_existing_mdbx_db() {
+    let tempdir = tempdir().unwrap();
+    let mut config = Config::new(tempdir.path().into());
+
+    let filename = config.database_path.join(MDBX_DATA_FILENAME);
+    File::create(&filename).unwrap();
+
+    assert_eq!(
+        config.override_backend(),
+        DatabaseBackendOverride::Failure(filename)
+    );
+    assert_eq!(config.backend, DatabaseBackend::Lmdb);
+}