Skip to content

Commit

Permalink
Prevent fd leak in random slasher tests (#6254)
Browse files Browse the repository at this point in the history
* Prevent fd leak in random slasher tests

* Clippy
  • Loading branch information
michaelsproul authored Aug 19, 2024
1 parent 0429158 commit 6faa9c6
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 18 deletions.
60 changes: 58 additions & 2 deletions slasher/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ mod mdbx_impl;
mod redb_impl;

use crate::{
metrics, AttesterRecord, AttesterSlashingStatus, CompactAttesterRecord, Config, Error,
ProposerSlashingStatus,
metrics, AttesterRecord, AttesterSlashingStatus, CompactAttesterRecord, Config, Database,
Error, ProposerSlashingStatus,
};
use byteorder::{BigEndian, ByteOrder};
use interface::{Environment, OpenDatabases, RwTransaction};
Expand Down Expand Up @@ -350,6 +350,18 @@ impl<E: EthSpec> SlasherDB<E> {
Ok(())
}

pub fn get_config(&self) -> &Config {
&self.config
}

/// TESTING ONLY.
///
/// Replace the config for this database. This is only a sane thing to do if the database
/// is empty (has been `reset`).
pub fn update_config(&mut self, config: Arc<Config>) {
self.config = config;
}

/// Load a config from disk.
///
/// This is generic in order to allow loading of configs for different schema versions.
Expand Down Expand Up @@ -799,6 +811,50 @@ impl<E: EthSpec> SlasherDB<E> {

Ok(())
}

/// Delete all data from the database, essentially re-initialising it.
///
/// We use this reset pattern in tests instead of leaking tonnes of file descriptors and
/// exhausting our allocation by creating (and leaking) databases.
///
/// THIS FUNCTION SHOULD ONLY BE USED IN TESTS.
pub fn reset(&self) -> Result<(), Error> {
// Clear the cache(s) first.
self.attestation_root_cache.lock().clear();

// Pattern match to avoid missing any database.
let OpenDatabases {
indexed_attestation_db,
indexed_attestation_id_db,
attesters_db,
attesters_max_targets_db,
min_targets_db,
max_targets_db,
current_epochs_db,
proposers_db,
metadata_db,
} = &self.databases;
let mut txn = self.begin_rw_txn()?;
self.reset_db(&mut txn, indexed_attestation_db)?;
self.reset_db(&mut txn, indexed_attestation_id_db)?;
self.reset_db(&mut txn, attesters_db)?;
self.reset_db(&mut txn, attesters_max_targets_db)?;
self.reset_db(&mut txn, min_targets_db)?;
self.reset_db(&mut txn, max_targets_db)?;
self.reset_db(&mut txn, current_epochs_db)?;
self.reset_db(&mut txn, proposers_db)?;
self.reset_db(&mut txn, metadata_db)?;
txn.commit()
}

fn reset_db(&self, txn: &mut RwTransaction<'_>, db: &Database<'static>) -> Result<(), Error> {
let mut cursor = txn.cursor(db)?;
if cursor.first_key()?.is_none() {
return Ok(());
}
cursor.delete_while(|_| Ok(true))?;
Ok(())
}
}

#[cfg(test)]
Expand Down
18 changes: 18 additions & 0 deletions slasher/src/slasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,19 @@ impl<E: EthSpec> Slasher<E> {
config.validate()?;
let config = Arc::new(config);
let db = SlasherDB::open(config.clone(), spec, log.clone())?;
Self::from_config_and_db(config, db, log)
}

/// TESTING ONLY.
///
/// Initialise a slasher database from an existing `db`. The caller must ensure that the
/// database's config matches the one provided.
pub fn from_config_and_db(
config: Arc<Config>,
db: SlasherDB<E>,
log: Logger,
) -> Result<Self, Error> {
config.validate()?;
let attester_slashings = Mutex::new(HashSet::new());
let proposer_slashings = Mutex::new(HashSet::new());
let attestation_queue = AttestationQueue::default();
Expand All @@ -48,6 +61,11 @@ impl<E: EthSpec> Slasher<E> {
})
}

pub fn into_reset_db(self) -> Result<SlasherDB<E>, Error> {
self.db.reset()?;
Ok(self.db)
}

/// Harvest all attester slashings found, removing them from the slasher.
pub fn get_attester_slashings(&self) -> HashSet<AttesterSlashing<E>> {
std::mem::take(&mut self.attester_slashings.lock())
Expand Down
61 changes: 45 additions & 16 deletions slasher/tests/random.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ use slasher::{
block, chain_spec, indexed_att, slashed_validators_from_attestations,
slashed_validators_from_slashings, E,
},
Config, Slasher,
Config, Slasher, SlasherDB,
};
use std::cmp::max;
use tempfile::tempdir;
use std::sync::Arc;
use tempfile::{tempdir, TempDir};
use types::{Epoch, EthSpec};

#[derive(Debug)]
Expand All @@ -32,26 +33,34 @@ impl Default for TestConfig {
}
}

fn random_test(seed: u64, test_config: TestConfig) {
fn make_db() -> (TempDir, SlasherDB<E>) {
let tempdir = tempdir().unwrap();
let initial_config = Arc::new(Config::new(tempdir.path().into()));
let logger = test_logger();
let spec = chain_spec();
let db = SlasherDB::open(initial_config.clone(), spec, logger).unwrap();
(tempdir, db)
}

fn random_test(seed: u64, mut db: SlasherDB<E>, test_config: TestConfig) -> SlasherDB<E> {
let check_slashings = test_config.check_slashings;
let num_validators = test_config.num_validators;
let max_attestations = test_config.max_attestations;

println!("Running with seed {}", seed);
let mut rng = StdRng::seed_from_u64(seed);

let tempdir = tempdir().unwrap();

let mut config = Config::new(tempdir.path().into());
let mut config = Config::new(db.get_config().database_path.clone());
config.validator_chunk_size = 1 << rng.gen_range(1..4);

let chunk_size_exponent = rng.gen_range(1..4);
config.chunk_size = 1 << chunk_size_exponent;
config.history_length = 1 << rng.gen_range(chunk_size_exponent..chunk_size_exponent + 3);

let spec = chain_spec();
let config = Arc::new(config);
db.update_config(config.clone());

let slasher = Slasher::<E>::open(config.clone(), spec, test_logger()).unwrap();
let slasher = Slasher::<E>::from_config_and_db(config.clone(), db, test_logger()).unwrap();

let validators = (0..num_validators as u64).collect::<Vec<u64>>();

Expand Down Expand Up @@ -121,7 +130,7 @@ fn random_test(seed: u64, test_config: TestConfig) {
}

if !check_slashings {
return;
return slasher.into_reset_db().unwrap();
}

slasher.process_queued(current_epoch).unwrap();
Expand All @@ -131,15 +140,19 @@ fn random_test(seed: u64, test_config: TestConfig) {
let slashed_validators = slashed_validators_from_slashings(&slashings);
let expected_slashed_validators = slashed_validators_from_attestations(&attestations);
assert_eq!(slashed_validators, expected_slashed_validators);

// Return the database for reuse.
slasher.into_reset_db().unwrap()
}

// Fuzz-like test that runs forever on different seeds looking for crashes.
#[test]
#[ignore]
fn no_crash() {
let mut rng = thread_rng();
let (_tempdir, mut db) = make_db();
loop {
random_test(rng.gen(), TestConfig::default());
db = random_test(rng.gen(), db, TestConfig::default());
}
}

Expand All @@ -148,9 +161,11 @@ fn no_crash() {
#[ignore]
fn no_crash_with_blocks() {
let mut rng = thread_rng();
let (_tempdir, mut db) = make_db();
loop {
random_test(
db = random_test(
rng.gen(),
db,
TestConfig {
add_blocks: true,
..TestConfig::default()
Expand All @@ -164,9 +179,11 @@ fn no_crash_with_blocks() {
#[ignore]
fn check_slashings() {
let mut rng = thread_rng();
let (_tempdir, mut db) = make_db();
loop {
random_test(
db = random_test(
rng.gen(),
db,
TestConfig {
check_slashings: true,
..TestConfig::default()
Expand All @@ -177,8 +194,10 @@ fn check_slashings() {

#[test]
fn check_slashings_example1() {
let (_tempdir, db) = make_db();
random_test(
1,
db,
TestConfig {
check_slashings: true,
..TestConfig::default()
Expand All @@ -188,8 +207,10 @@ fn check_slashings_example1() {

#[test]
fn check_slashings_example2() {
let (_tempdir, db) = make_db();
random_test(
2,
db,
TestConfig {
check_slashings: true,
max_attestations: 3,
Expand All @@ -200,8 +221,10 @@ fn check_slashings_example2() {

#[test]
fn check_slashings_example3() {
let (_tempdir, db) = make_db();
random_test(
3,
db,
TestConfig {
check_slashings: true,
max_attestations: 100,
Expand All @@ -212,23 +235,28 @@ fn check_slashings_example3() {

#[test]
fn no_crash_example1() {
random_test(1, TestConfig::default());
let (_tempdir, db) = make_db();
random_test(1, db, TestConfig::default());
}

#[test]
fn no_crash_example2() {
random_test(2, TestConfig::default());
let (_tempdir, db) = make_db();
random_test(2, db, TestConfig::default());
}

#[test]
fn no_crash_example3() {
random_test(3, TestConfig::default());
let (_tempdir, db) = make_db();
random_test(3, db, TestConfig::default());
}

#[test]
fn no_crash_blocks_example1() {
let (_tempdir, db) = make_db();
random_test(
1,
db,
TestConfig {
add_blocks: true,
..TestConfig::default()
Expand All @@ -238,5 +266,6 @@ fn no_crash_blocks_example1() {

#[test]
fn no_crash_aug_24() {
random_test(13519442335106054152, TestConfig::default())
let (_tempdir, db) = make_db();
random_test(13519442335106054152, db, TestConfig::default());
}

0 comments on commit 6faa9c6

Please sign in to comment.