diff --git a/eth2/operation_pool/src/lib.rs b/eth2/operation_pool/src/lib.rs index ac2c1ed31..a99822519 100644 --- a/eth2/operation_pool/src/lib.rs +++ b/eth2/operation_pool/src/lib.rs @@ -485,7 +485,7 @@ mod release_tests { state_builder.teleport_to_slot(slot); state_builder.build_caches(&spec).unwrap(); let (state, keypairs) = state_builder.build(); - (state, keypairs, MainnetEthSpec::default_spec()) + (state, keypairs, spec) } #[test] @@ -890,4 +890,44 @@ mod release_tests { seen_indices.extend(fresh_indices); } } + + /// Insert two slashings for the same proposer and ensure only one is returned. + #[test] + fn duplicate_proposer_slashing() { + let spec = MainnetEthSpec::default_spec(); + let num_validators = 32; + let mut state_builder = + TestingBeaconStateBuilder::::from_default_keypairs_file_if_exists( + num_validators, + &spec, + ); + state_builder.build_caches(&spec).unwrap(); + let (state, keypairs) = state_builder.build(); + let op_pool = OperationPool::new(); + + let proposer_index = 0; + let slashing1 = TestingProposerSlashingBuilder::double_vote::( + ProposerSlashingTestTask::Valid, + proposer_index, + &keypairs[proposer_index as usize].sk, + &state.fork, + state.genesis_validators_root, + &spec, + ); + let slashing2 = ProposerSlashing { + signed_header_1: slashing1.signed_header_2.clone(), + signed_header_2: slashing1.signed_header_1.clone(), + }; + + // Both slashings should be accepted by the pool. + op_pool + .insert_proposer_slashing(slashing1.clone(), &state, &spec) + .unwrap(); + op_pool + .insert_proposer_slashing(slashing2.clone(), &state, &spec) + .unwrap(); + + // Should only get the second slashing back. + assert_eq!(op_pool.get_slashings(&state, &spec).0, vec![slashing2]); + } } diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index 8634bb0e1..a4f2be87e 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/eth2/state_processing/src/per_block_processing.rs @@ -283,26 +283,25 @@ pub fn process_proposer_slashings( verify_signatures: VerifySignatures, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { - // Verify proposer slashings in parallel. + // Verify and apply proposer slashings in series. + // We have to verify in series because an invalid block may contain multiple slashings + // for the same validator, and we need to correctly detect and reject that. proposer_slashings - .par_iter() + .into_iter() .enumerate() .try_for_each(|(i, proposer_slashing)| { verify_proposer_slashing(proposer_slashing, &state, verify_signatures, spec) - .map_err(|e| e.into_with_index(i)) - })?; + .map_err(|e| e.into_with_index(i))?; - // Update the state. - for proposer_slashing in proposer_slashings { - slash_validator( - state, - proposer_slashing.signed_header_1.message.proposer_index as usize, - None, - spec, - )?; - } + slash_validator( + state, + proposer_slashing.signed_header_1.message.proposer_index as usize, + None, + spec, + )?; - Ok(()) + Ok(()) + }) } /// Validates each `AttesterSlashing` and updates the state, short-circuiting on an invalid object. diff --git a/eth2/state_processing/src/per_block_processing/tests.rs b/eth2/state_processing/src/per_block_processing/tests.rs index f942cc29f..e7254db9d 100644 --- a/eth2/state_processing/src/per_block_processing/tests.rs +++ b/eth2/state_processing/src/per_block_processing/tests.rs @@ -1029,6 +1029,37 @@ fn invalid_proposer_slashing_not_slashable() { ); } +#[test] +fn invalid_proposer_slashing_duplicate_slashing() { + let spec = MainnetEthSpec::default_spec(); + let builder = get_builder(&spec, SLOT_OFFSET, VALIDATOR_COUNT); + let test_task = ProposerSlashingTestTask::Valid; + let (mut block, mut state) = + builder.build_with_proposer_slashing(test_task, 1, None, None, &spec); + + let slashing = block.message.body.proposer_slashings[0].clone(); + let slashed_proposer = slashing.signed_header_1.message.proposer_index; + block.message.body.proposer_slashings.push(slashing); + + let result = per_block_processing( + &mut state, + &block, + None, + BlockSignatureStrategy::NoVerification, + &spec, + ); + + // Expecting ProposerNotSlashable for the 2nd slashing because the validator has been + // slashed by the 1st slashing. + assert_eq!( + result, + Err(BlockProcessingError::ProposerSlashingInvalid { + index: 1, + reason: ProposerSlashingInvalid::ProposerNotSlashable(slashed_proposer) + }) + ); +} + #[test] fn invalid_bad_proposal_1_signature() { let spec = MainnetEthSpec::default_spec();