From ec8edfb89a8358dc05fb2a6abfe0d315e2bed302 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Tue, 5 Dec 2023 00:49:50 +0100 Subject: [PATCH] EIP-3076 tests - complete database (#4958) * EIP-3076 interchange tests: Add `should_succeed_complete` boolean. This new added `should_succeed_complete` boolean means the test should succeed using complete anti-slashing DB, while the untouched `should_succeed` boolean is still implicitely reserved for minimal anti-slashing DB. Note: This commit only adds the new `should_succeed_complete` boolean, and copy the value from `should_succeed` without any meaning regarding the value this boolean should take. * `TestEIP3076SpecTests`: Modify two tests Those two tests are modified in a way they both comply with minimal AND complete anti-slashing DB. * Disallow false positives and differentiate more minimal vs complete cases * Fix my own typos * Update to v5.3.0 tag --------- Co-authored-by: Michael Sproul --- validator_client/slashing_protection/Makefile | 2 +- .../src/bin/test_generator.rs | 272 +++++++++--------- .../src/interchange_test.rs | 74 ++--- 3 files changed, 183 insertions(+), 165 deletions(-) diff --git a/validator_client/slashing_protection/Makefile b/validator_client/slashing_protection/Makefile index 0663b3cba2b..1b9729634e5 100644 --- a/validator_client/slashing_protection/Makefile +++ b/validator_client/slashing_protection/Makefile @@ -1,4 +1,4 @@ -TESTS_TAG := v5.2.1 +TESTS_TAG := v5.3.0 GENERATE_DIR := generated-tests OUTPUT_DIR := interchange-tests TARBALL := $(OUTPUT_DIR)-$(TESTS_TAG).tar.gz diff --git a/validator_client/slashing_protection/src/bin/test_generator.rs b/validator_client/slashing_protection/src/bin/test_generator.rs index b96dd8eb796..c95cb6917c5 100644 --- a/validator_client/slashing_protection/src/bin/test_generator.rs +++ b/validator_client/slashing_protection/src/bin/test_generator.rs @@ -70,14 +70,18 @@ fn interchange_with_signing_roots( } fn main() { - let single_validator_blocks = - vec![(0, 32, false), (0, 33, true), (0, 31, false), (0, 1, false)]; + let single_validator_blocks = vec![ + (0, 32, false, false), + (0, 33, true, true), + (0, 31, false, false), + (0, 1, false, false), + ]; let single_validator_attestations = vec![ - (0, 3, 4, false), - (0, 14, 19, false), - (0, 15, 20, false), - (0, 16, 20, false), - (0, 15, 21, true), + (0, 3, 4, false, false), + (0, 14, 19, false, false), + (0, 15, 20, false, false), + (0, 16, 20, false, false), + (0, 15, 21, true, true), ]; let tests = vec![ @@ -104,7 +108,7 @@ fn main() { MultiTestCase::single( "single_validator_genesis_attestation", TestCase::new(interchange(vec![(0, vec![], vec![(0, 0)])])) - .with_attestations(vec![(0, 0, 0, false)]), + .with_attestations(vec![(0, 0, 0, false, false)]), ), MultiTestCase::single( "single_validator_multiple_blocks_and_attestations", @@ -114,23 +118,23 @@ fn main() { vec![(10, 11), (12, 13), (20, 24)], )])) .with_blocks(vec![ - (0, 1, false), - (0, 2, false), - (0, 3, false), - (0, 10, false), - (0, 1200, false), - (0, 4, true), - (0, 256, true), - (0, 1201, true), + (0, 1, false, false), + (0, 2, false, false), + (0, 3, false, false), + (0, 10, false, false), + (0, 1200, false, false), + (0, 4, false, true), + (0, 256, false, true), + (0, 1201, true, true), ]) .with_attestations(vec![ - (0, 9, 10, false), - (0, 12, 13, false), - (0, 11, 14, false), - (0, 21, 22, false), - (0, 10, 24, false), - (0, 11, 12, true), - (0, 20, 25, true), + (0, 9, 10, false, false), + (0, 12, 13, false, false), + (0, 11, 14, false, false), + (0, 21, 22, false, false), + (0, 10, 24, false, false), + (0, 11, 12, false, true), + (0, 20, 25, true, true), ]), ), MultiTestCase::single( @@ -157,30 +161,30 @@ fn main() { (2, vec![10, 15, 20], vec![(1, 2), (1, 3), (2, 4)]), ])) .with_blocks(vec![ - (0, 9, false), - (0, 10, false), - (0, 21, true), - (0, 11, true), - (1, 2, false), - (1, 3, false), - (1, 0, false), - (1, 101, true), - (2, 9, false), - (2, 10, false), - (2, 22, true), + (0, 9, false, false), + (0, 10, false, false), + (0, 21, true, true), + (0, 11, false, true), + (1, 2, false, false), + (1, 3, false, false), + (1, 0, false, false), + (1, 101, true, true), + (2, 9, false, false), + (2, 10, false, false), + (2, 22, true, true), ]) .with_attestations(vec![ - (0, 0, 5, false), - (0, 3, 6, false), - (0, 4, 6, true), - (0, 5, 7, true), - (0, 6, 8, true), - (1, 1, 7, false), - (1, 1, 4, true), - (1, 5, 7, true), - (2, 0, 0, false), - (2, 0, 1, false), - (2, 2, 5, true), + (0, 0, 5, false, false), + (0, 3, 6, false, false), + (0, 4, 6, true, true), + (0, 5, 7, true, true), + (0, 6, 8, true, true), + (1, 1, 7, false, false), + (1, 1, 4, false, true), + (1, 5, 7, true, true), + (2, 0, 0, false, false), + (2, 0, 1, false, false), + (2, 2, 5, true, true), ]), ), MultiTestCase::single( @@ -202,16 +206,16 @@ fn main() { TestCase::new(interchange(vec![(0, vec![40], vec![(2, 30)])])), TestCase::new(interchange(vec![(0, vec![50], vec![(10, 50)])])) .with_blocks(vec![ - (0, 41, false), - (0, 45, false), - (0, 49, false), - (0, 50, false), - (0, 51, true), + (0, 41, false, true), + (0, 45, false, true), + (0, 49, false, true), + (0, 50, false, false), + (0, 51, true, true), ]) .with_attestations(vec![ - (0, 3, 31, false), - (0, 9, 49, false), - (0, 10, 51, true), + (0, 3, 31, false, true), + (0, 9, 49, false, true), + (0, 10, 51, true, true), ]), ], ), @@ -221,20 +225,20 @@ fn main() { TestCase::new(interchange(vec![(0, vec![40], vec![])])), TestCase::new(interchange(vec![(0, vec![20], vec![])])) .contains_slashable_data() - .with_blocks(vec![(0, 20, false)]), + .with_blocks(vec![(0, 20, false, false)]), ], ), MultiTestCase::new( "multiple_interchanges_single_validator_multiple_blocks_out_of_order", vec![ TestCase::new(interchange(vec![(0, vec![0], vec![])])).with_blocks(vec![ - (0, 10, true), - (0, 20, true), - (0, 30, true), + (0, 10, true, true), + (0, 20, true, true), + (0, 30, true, true), ]), TestCase::new(interchange(vec![(0, vec![20], vec![])])) .contains_slashable_data() - .with_blocks(vec![(0, 29, false)]), + .with_blocks(vec![(0, 29, false, true)]), ], ), MultiTestCase::new( @@ -243,7 +247,7 @@ fn main() { TestCase::new(interchange(vec![(0, vec![40], vec![])])), TestCase::new(interchange(vec![(0, vec![20, 50], vec![])])) .contains_slashable_data() - .with_blocks(vec![(0, 20, false), (0, 50, false)]), + .with_blocks(vec![(0, 20, false, false), (0, 50, false, false)]), ], ), MultiTestCase::new( @@ -253,10 +257,10 @@ fn main() { TestCase::new(interchange(vec![(0, vec![], vec![(10, 11)])])) .contains_slashable_data() .with_attestations(vec![ - (0, 10, 14, false), - (0, 12, 13, false), - (0, 12, 14, true), - (0, 13, 15, true), + (0, 10, 14, false, false), + (0, 12, 13, false, false), + (0, 12, 14, true, true), + (0, 13, 15, true, true), ]), ], ), @@ -267,11 +271,11 @@ fn main() { TestCase::new(interchange(vec![(0, vec![], vec![(9, 21)])])) .contains_slashable_data() .with_attestations(vec![ - (0, 10, 20, false), - (0, 10, 21, false), - (0, 9, 21, false), - (0, 9, 22, false), - (0, 10, 22, true), + (0, 10, 20, false, false), + (0, 10, 21, false, false), + (0, 9, 21, false, false), + (0, 9, 22, false, false), + (0, 10, 22, true, true), ]), ], ), @@ -282,11 +286,11 @@ fn main() { TestCase::new(interchange(vec![(0, vec![], vec![(10, 20)])])) .contains_slashable_data() .with_attestations(vec![ - (0, 10, 20, false), - (0, 10, 21, false), - (0, 9, 21, false), - (0, 9, 22, false), - (0, 10, 22, true), + (0, 10, 20, false, false), + (0, 10, 21, false, false), + (0, 9, 21, false, false), + (0, 9, 22, false, false), + (0, 10, 22, true, true), ]), ], ), @@ -303,13 +307,13 @@ fn main() { ])) .contains_slashable_data() .with_blocks(vec![ - (0, 0, false), - (0, 3, true), - (0, 7, true), - (0, 3, true), - (1, 0, false), + (0, 0, false, false), + (0, 3, false, true), + (0, 7, true, true), + (0, 3, false, true), + (1, 0, false, false), ]) - .with_attestations(vec![(0, 0, 4, false), (1, 0, 4, true)]), + .with_attestations(vec![(0, 0, 4, false, false), (1, 0, 4, true, true)]), ], ), MultiTestCase::new( @@ -330,9 +334,9 @@ fn main() { ])) .contains_slashable_data() .with_attestations(vec![ - (0, 0, 4, false), - (1, 1, 2, false), - (2, 1, 2, false), + (0, 0, 4, false, false), + (1, 1, 2, false, false), + (2, 1, 2, false, false), ]), ], ), @@ -351,23 +355,23 @@ fn main() { ])) .contains_slashable_data() .with_blocks(vec![ - (0, 100, false), - (1, 101, false), - (2, 102, false), - (0, 103, true), - (1, 104, true), - (2, 105, true), + (0, 100, false, false), + (1, 101, false, false), + (2, 102, false, false), + (0, 103, true, true), + (1, 104, true, true), + (2, 105, true, true), ]) .with_attestations(vec![ - (0, 12, 13, false), - (0, 11, 14, false), - (1, 12, 13, false), - (1, 11, 14, false), - (2, 12, 13, false), - (2, 11, 14, false), - (0, 12, 14, true), - (1, 13, 14, true), - (2, 13, 14, true), + (0, 12, 13, false, false), + (0, 11, 14, false, false), + (1, 12, 13, false, false), + (1, 11, 14, false, false), + (2, 12, 13, false, false), + (2, 11, 14, false, false), + (0, 12, 14, true, true), + (1, 13, 14, true, true), + (2, 13, 14, true, true), ]), ], ), @@ -379,36 +383,36 @@ fn main() { "single_validator_source_greater_than_target_surrounding", TestCase::new(interchange(vec![(0, vec![], vec![(5, 2)])])) .contains_slashable_data() - .with_attestations(vec![(0, 3, 4, false)]), + .with_attestations(vec![(0, 3, 4, false, false)]), ), MultiTestCase::single( "single_validator_source_greater_than_target_surrounded", TestCase::new(interchange(vec![(0, vec![], vec![(5, 2)])])) .contains_slashable_data() - .with_attestations(vec![(0, 6, 1, false)]), + .with_attestations(vec![(0, 6, 1, false, false)]), ), MultiTestCase::single( "single_validator_source_greater_than_target_sensible_iff_minified", TestCase::new(interchange(vec![(0, vec![], vec![(5, 2), (6, 7)])])) .contains_slashable_data() - .with_attestations(vec![(0, 5, 8, false), (0, 6, 8, true)]), + .with_attestations(vec![(0, 5, 8, false, false), (0, 6, 8, true, true)]), ), MultiTestCase::single( "single_validator_out_of_order_blocks", TestCase::new(interchange(vec![(0, vec![6, 5], vec![])])).with_blocks(vec![ - (0, 5, false), - (0, 6, false), - (0, 7, true), + (0, 5, false, false), + (0, 6, false, false), + (0, 7, true, true), ]), ), MultiTestCase::single( "single_validator_out_of_order_attestations", TestCase::new(interchange(vec![(0, vec![], vec![(4, 5), (3, 4)])])).with_attestations( vec![ - (0, 3, 4, false), - (0, 4, 5, false), - (0, 1, 10, false), - (0, 3, 3, false), + (0, 3, 4, false, false), + (0, 4, 5, false, false), + (0, 1, 10, false, false), + (0, 3, 3, false, false), ], ), ), @@ -417,15 +421,15 @@ fn main() { MultiTestCase::single( "single_validator_two_blocks_no_signing_root", TestCase::new(interchange(vec![(0, vec![10, 20], vec![])])) - .with_blocks(vec![(0, 20, false)]), + .with_blocks(vec![(0, 20, false, false)]), ), MultiTestCase::single( "single_validator_multiple_block_attempts", TestCase::new(interchange(vec![(0, vec![15, 16, 17], vec![])])) .with_signing_root_blocks(vec![ - (0, 16, 0, false), - (0, 16, 1, false), - (0, 16, u64::MAX, false), + (0, 16, 0, false, false), + (0, 16, 1, false, false), + (0, 16, u64::MAX, false, false), ]), ), MultiTestCase::single( @@ -436,15 +440,15 @@ fn main() { vec![], )])) .with_signing_root_blocks(vec![ - (0, 15, 151, true), - (0, 16, 161, true), - (0, 17, 171, true), - (0, 15, 152, false), - (0, 15, 0, false), - (0, 16, 151, false), - (0, 17, 151, false), - (0, 18, 151, true), - (0, 14, 171, false), + (0, 15, 151, false, true), + (0, 16, 161, false, true), + (0, 17, 171, false, true), + (0, 15, 152, false, false), + (0, 15, 0, false, false), + (0, 16, 151, false, false), + (0, 17, 151, false, false), + (0, 18, 151, true, true), + (0, 14, 171, false, false), ]), ), MultiTestCase::single( @@ -455,11 +459,11 @@ fn main() { vec![(5, 15, Some(515))], )])) .with_signing_root_attestations(vec![ - (0, 5, 15, 0, false), - (0, 5, 15, 1, false), - (0, 5, 15, 515, true), - (0, 6, 15, 615, false), - (0, 5, 14, 515, false), + (0, 5, 15, 0, false, false), + (0, 5, 15, 1, false, false), + (0, 5, 15, 515, false, true), + (0, 6, 15, 615, false, false), + (0, 5, 14, 515, false, false), ]), ), MultiTestCase::single( @@ -500,8 +504,12 @@ fn main() { (0, vec![10, 11], vec![(0, 2)]), (0, vec![12, 13], vec![(1, 3)]), ])) - .with_blocks(vec![(0, 10, false), (0, 13, false), (0, 14, true)]) - .with_attestations(vec![(0, 0, 2, false), (0, 1, 3, false)]), + .with_blocks(vec![ + (0, 10, false, false), + (0, 13, false, false), + (0, 14, true, true), + ]) + .with_attestations(vec![(0, 0, 2, false, false), (0, 1, 3, false, false)]), ), MultiTestCase::single( "duplicate_pubkey_slashable_block", @@ -510,7 +518,7 @@ fn main() { (0, vec![10], vec![(1, 3)]), ])) .contains_slashable_data() - .with_blocks(vec![(0, 10, false), (0, 11, true)]), + .with_blocks(vec![(0, 10, false, false), (0, 11, true, true)]), ), MultiTestCase::single( "duplicate_pubkey_slashable_attestation", @@ -520,10 +528,10 @@ fn main() { ])) .contains_slashable_data() .with_attestations(vec![ - (0, 0, 1, false), - (0, 0, 2, false), - (0, 0, 4, false), - (0, 1, 4, true), + (0, 0, 1, false, false), + (0, 0, 2, false, false), + (0, 0, 4, false, false), + (0, 1, 4, true, true), ]), ), ]; diff --git a/validator_client/slashing_protection/src/interchange_test.rs b/validator_client/slashing_protection/src/interchange_test.rs index 1bb1fc550bf..d88bb93a0d5 100644 --- a/validator_client/slashing_protection/src/interchange_test.rs +++ b/validator_client/slashing_protection/src/interchange_test.rs @@ -33,6 +33,7 @@ pub struct TestBlock { pub slot: Slot, pub signing_root: Hash256, pub should_succeed: bool, + pub should_succeed_complete: bool, } #[derive(Debug, Clone, Deserialize, Serialize)] @@ -43,6 +44,7 @@ pub struct TestAttestation { pub target_epoch: Epoch, pub signing_root: Hash256, pub should_succeed: bool, + pub should_succeed_complete: bool, } impl MultiTestCase { @@ -68,10 +70,6 @@ impl MultiTestCase { let slashing_db_file = dir.path().join("slashing_protection.sqlite"); let slashing_db = SlashingDatabase::create(&slashing_db_file).unwrap(); - // Now that we are using implicit minification on import, we must always allow - // false positives. - let allow_false_positives = true; - for test_case in &self.steps { // If the test case is marked as containing slashable data, then the spec allows us to // fail to import the file. However, we minify on import and ignore slashable data, so @@ -124,7 +122,7 @@ impl MultiTestCase { i, self.name, safe ); } - Err(e) if block.should_succeed && !allow_false_positives => { + Err(e) if block.should_succeed => { panic!( "block {} from `{}` failed when it should have succeeded: {:?}", i, self.name, e @@ -147,7 +145,7 @@ impl MultiTestCase { i, self.name, safe ); } - Err(e) if att.should_succeed && !allow_false_positives => { + Err(e) if att.should_succeed => { panic!( "attestation {} from `{}` failed when it should have succeeded: {:?}", i, self.name, e @@ -181,53 +179,65 @@ impl TestCase { self } - pub fn with_blocks(self, blocks: impl IntoIterator) -> Self { - self.with_signing_root_blocks( - blocks - .into_iter() - .map(|(index, slot, should_succeed)| (index, slot, 0, should_succeed)), - ) + pub fn with_blocks(self, blocks: impl IntoIterator) -> Self { + self.with_signing_root_blocks(blocks.into_iter().map( + |(index, slot, should_succeed, should_succeed_complete)| { + (index, slot, 0, should_succeed, should_succeed_complete) + }, + )) } pub fn with_signing_root_blocks( mut self, - blocks: impl IntoIterator, + blocks: impl IntoIterator, ) -> Self { - self.blocks.extend( - blocks - .into_iter() - .map(|(pk, slot, signing_root, should_succeed)| TestBlock { + self.blocks.extend(blocks.into_iter().map( + |(pk, slot, signing_root, should_succeed, should_succeed_complete)| { + assert!( + !should_succeed || should_succeed_complete, + "if should_succeed is true then should_succeed_complete must also be true" + ); + TestBlock { pubkey: pubkey(pk), slot: Slot::new(slot), signing_root: Hash256::from_low_u64_be(signing_root), should_succeed, - }), - ); + should_succeed_complete, + } + }, + )); self } pub fn with_attestations( self, - attestations: impl IntoIterator, + attestations: impl IntoIterator, ) -> Self { - self.with_signing_root_attestations( - attestations - .into_iter() - .map(|(id, source, target, succeed)| (id, source, target, 0, succeed)), - ) + self.with_signing_root_attestations(attestations.into_iter().map( + |(id, source, target, succeed, succeed_complete)| { + (id, source, target, 0, succeed, succeed_complete) + }, + )) } pub fn with_signing_root_attestations( mut self, - attestations: impl IntoIterator, + attestations: impl IntoIterator, ) -> Self { self.attestations.extend(attestations.into_iter().map( - |(pk, source, target, signing_root, should_succeed)| TestAttestation { - pubkey: pubkey(pk), - source_epoch: Epoch::new(source), - target_epoch: Epoch::new(target), - signing_root: Hash256::from_low_u64_be(signing_root), - should_succeed, + |(pk, source, target, signing_root, should_succeed, should_succeed_complete)| { + assert!( + !should_succeed || should_succeed_complete, + "if should_succeed is true then should_succeed_complete must also be true" + ); + TestAttestation { + pubkey: pubkey(pk), + source_epoch: Epoch::new(source), + target_epoch: Epoch::new(target), + signing_root: Hash256::from_low_u64_be(signing_root), + should_succeed, + should_succeed_complete, + } }, )); self