From bb7e88e160035d6059ce9aaa68633c1789e08bec Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 29 Sep 2022 10:02:09 +1000 Subject: [PATCH] Remove remaining proposer index recalculations --- .../src/common/slash_validator.rs | 15 ++++--- .../src/per_block_processing.rs | 2 +- .../process_operations.rs | 29 ++++++++---- .../src/per_block_processing/tests.rs | 44 ++++++++++++++++--- testing/ef_tests/src/cases/operations.rs | 31 ++++++++++--- 5 files changed, 94 insertions(+), 27 deletions(-) diff --git a/consensus/state_processing/src/common/slash_validator.rs b/consensus/state_processing/src/common/slash_validator.rs index e9d94a10625..ac2dba875e7 100644 --- a/consensus/state_processing/src/common/slash_validator.rs +++ b/consensus/state_processing/src/common/slash_validator.rs @@ -1,9 +1,13 @@ -use crate::common::{decrease_balance, increase_balance, initiate_validator_exit}; +use crate::{ + common::{decrease_balance, increase_balance, initiate_validator_exit}, + per_block_processing::errors::BlockProcessingError, + ConsensusContext, +}; use safe_arith::SafeArith; use std::cmp; use types::{ consts::altair::{PROPOSER_WEIGHT, WEIGHT_DENOMINATOR}, - BeaconStateError as Error, *, + *, }; /// Slash the validator with index `slashed_index`. @@ -11,8 +15,9 @@ pub fn slash_validator( state: &mut BeaconState, slashed_index: usize, opt_whistleblower_index: Option, + ctxt: &mut ConsensusContext, spec: &ChainSpec, -) -> Result<(), Error> { +) -> Result<(), BlockProcessingError> { let epoch = state.current_epoch(); initiate_validator_exit(state, slashed_index, spec)?; @@ -39,7 +44,7 @@ pub fn slash_validator( )?; // Apply proposer and whistleblower rewards - let proposer_index = state.get_beacon_proposer_index(state.slot(), spec)?; + let proposer_index = ctxt.get_proposer_index(state, spec)? as usize; let whistleblower_index = opt_whistleblower_index.unwrap_or(proposer_index); let whistleblower_reward = validator_effective_balance.safe_div(spec.whistleblower_reward_quotient)?; @@ -52,7 +57,7 @@ pub fn slash_validator( // Ensure the whistleblower index is in the validator registry. if state.validators().get(whistleblower_index).is_none() { - return Err(BeaconStateError::UnknownValidator(whistleblower_index)); + return Err(BeaconStateError::UnknownValidator(whistleblower_index).into()); } increase_balance(state, proposer_index, proposer_reward)?; diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 063c65e56e9..cccc8eacd9f 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -164,7 +164,7 @@ pub fn per_block_processing>( process_randao(state, block, verify_randao, ctxt, spec)?; process_eth1_data(state, block.body().eth1_data())?; - process_operations(state, block.body(), proposer_index, verify_signatures, spec)?; + process_operations(state, block.body(), verify_signatures, ctxt, spec)?; if let Ok(sync_aggregate) = block.body().sync_aggregate() { process_sync_aggregate( diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 31a4ac1fb42..1000586e660 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -12,23 +12,25 @@ use types::consts::altair::{PARTICIPATION_FLAG_WEIGHTS, PROPOSER_WEIGHT, WEIGHT_ pub fn process_operations<'a, T: EthSpec, Payload: ExecPayload>( state: &mut BeaconState, block_body: BeaconBlockBodyRef<'a, T, Payload>, - proposer_index: u64, verify_signatures: VerifySignatures, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { process_proposer_slashings( state, block_body.proposer_slashings(), verify_signatures, + ctxt, spec, )?; process_attester_slashings( state, block_body.attester_slashings(), verify_signatures, + ctxt, spec, )?; - process_attestations(state, block_body, proposer_index, verify_signatures, spec)?; + process_attestations(state, block_body, verify_signatures, ctxt, spec)?; process_deposits(state, block_body.deposits(), spec)?; process_exits(state, block_body.voluntary_exits(), verify_signatures, spec)?; Ok(()) @@ -45,12 +47,13 @@ pub mod base { state: &mut BeaconState, attestations: &[Attestation], verify_signatures: VerifySignatures, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { // Ensure the previous epoch cache exists. state.build_committee_cache(RelativeEpoch::Previous, spec)?; - let proposer_index = state.get_beacon_proposer_index(state.slot(), spec)? as u64; + let proposer_index = ctxt.get_proposer_index(state, spec)?; // Verify and apply each attestation. for (i, attestation) in attestations.iter().enumerate() { @@ -87,10 +90,11 @@ pub mod altair { pub fn process_attestations( state: &mut BeaconState, attestations: &[Attestation], - proposer_index: u64, verify_signatures: VerifySignatures, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { + let proposer_index = ctxt.get_proposer_index(state, spec)?; attestations .iter() .enumerate() @@ -170,6 +174,7 @@ pub fn process_proposer_slashings( state: &mut BeaconState, proposer_slashings: &[ProposerSlashing], verify_signatures: VerifySignatures, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { // Verify and apply proposer slashings in series. @@ -186,6 +191,7 @@ pub fn process_proposer_slashings( state, proposer_slashing.signed_header_1.message.proposer_index as usize, None, + ctxt, spec, )?; @@ -201,6 +207,7 @@ pub fn process_attester_slashings( state: &mut BeaconState, attester_slashings: &[AttesterSlashing], verify_signatures: VerifySignatures, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { for (i, attester_slashing) in attester_slashings.iter().enumerate() { @@ -211,7 +218,7 @@ pub fn process_attester_slashings( get_slashable_indices(state, attester_slashing).map_err(|e| e.into_with_index(i))?; for i in slashable_indices { - slash_validator(state, i as usize, None, spec)?; + slash_validator(state, i as usize, None, ctxt, spec)?; } } @@ -222,20 +229,26 @@ pub fn process_attester_slashings( pub fn process_attestations<'a, T: EthSpec, Payload: ExecPayload>( state: &mut BeaconState, block_body: BeaconBlockBodyRef<'a, T, Payload>, - proposer_index: u64, verify_signatures: VerifySignatures, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { match block_body { BeaconBlockBodyRef::Base(_) => { - base::process_attestations(state, block_body.attestations(), verify_signatures, spec)?; + base::process_attestations( + state, + block_body.attestations(), + verify_signatures, + ctxt, + spec, + )?; } BeaconBlockBodyRef::Altair(_) | BeaconBlockBodyRef::Merge(_) => { altair::process_attestations( state, block_body.attestations(), - proposer_index, verify_signatures, + ctxt, spec, )?; } diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index c1ceecb3903..b7d28832db0 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -391,11 +391,12 @@ async fn invalid_attestation_no_committee_for_index() { head_block.to_mut().body_mut().attestations_mut()[0] .data .index += 1; + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( &mut state, head_block.body(), - head_block.proposer_index(), VerifySignatures::True, + &mut ctxt, &spec, ); @@ -429,11 +430,12 @@ async fn invalid_attestation_wrong_justified_checkpoint() { .data .source = new_justified_checkpoint; + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( &mut state, head_block.body(), - head_block.proposer_index(), VerifySignatures::True, + &mut ctxt, &spec, ); @@ -468,11 +470,12 @@ async fn invalid_attestation_bad_aggregation_bitfield_len() { head_block.to_mut().body_mut().attestations_mut()[0].aggregation_bits = Bitfield::with_capacity(spec.target_committee_size).unwrap(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( &mut state, head_block.body(), - head_block.proposer_index(), VerifySignatures::True, + &mut ctxt, &spec, ); @@ -500,11 +503,12 @@ async fn invalid_attestation_bad_signature() { .0; head_block.to_mut().body_mut().attestations_mut()[0].signature = AggregateSignature::empty(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( &mut state, head_block.body(), - head_block.proposer_index(), VerifySignatures::True, + &mut ctxt, &spec, ); // Expecting BadSignature because we're signing with invalid secret_keys @@ -538,11 +542,12 @@ async fn invalid_attestation_included_too_early() { .data .slot = new_attesation_slot; + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( &mut state, head_block.body(), - head_block.proposer_index(), VerifySignatures::True, + &mut ctxt, &spec, ); @@ -580,11 +585,12 @@ async fn invalid_attestation_included_too_late() { .data .slot = new_attesation_slot; + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( &mut state, head_block.body(), - head_block.proposer_index(), VerifySignatures::True, + &mut ctxt, &spec, ); assert_eq!( @@ -618,11 +624,12 @@ async fn invalid_attestation_target_epoch_slot_mismatch() { .target .epoch += Epoch::new(1); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( &mut state, head_block.body(), - head_block.proposer_index(), VerifySignatures::True, + &mut ctxt, &spec, ); assert_eq!( @@ -645,10 +652,12 @@ async fn valid_insert_attester_slashing() { let attester_slashing = harness.make_attester_slashing(vec![1, 2]); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attester_slashings( &mut state, &[attester_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); @@ -665,10 +674,12 @@ async fn invalid_attester_slashing_not_slashable() { attester_slashing.attestation_1 = attester_slashing.attestation_2.clone(); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attester_slashings( &mut state, &[attester_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); @@ -691,10 +702,12 @@ async fn invalid_attester_slashing_1_invalid() { attester_slashing.attestation_1.attesting_indices = VariableList::from(vec![2, 1]); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attester_slashings( &mut state, &[attester_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); @@ -720,10 +733,12 @@ async fn invalid_attester_slashing_2_invalid() { attester_slashing.attestation_2.attesting_indices = VariableList::from(vec![2, 1]); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attester_slashings( &mut state, &[attester_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); @@ -746,10 +761,12 @@ async fn valid_insert_proposer_slashing() { let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; let proposer_slashing = harness.make_proposer_slashing(1); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_proposer_slashings( &mut state, &[proposer_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); // Expecting Ok(_) because we inserted a valid proposer slashing @@ -765,10 +782,12 @@ async fn invalid_proposer_slashing_proposals_identical() { proposer_slashing.signed_header_1.message = proposer_slashing.signed_header_2.message.clone(); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_proposer_slashings( &mut state, &[proposer_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); @@ -792,10 +811,12 @@ async fn invalid_proposer_slashing_proposer_unknown() { proposer_slashing.signed_header_2.message.proposer_index = 3_141_592; let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_proposer_slashings( &mut state, &[proposer_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); @@ -816,10 +837,12 @@ async fn invalid_proposer_slashing_duplicate_slashing() { let proposer_slashing = harness.make_proposer_slashing(1); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result_1 = process_operations::process_proposer_slashings( &mut state, &[proposer_slashing.clone()], VerifySignatures::False, + &mut ctxt, &spec, ); assert!(result_1.is_ok()); @@ -828,6 +851,7 @@ async fn invalid_proposer_slashing_duplicate_slashing() { &mut state, &[proposer_slashing], VerifySignatures::False, + &mut ctxt, &spec, ); // Expecting ProposerNotSlashable because we've already slashed the validator @@ -847,10 +871,12 @@ async fn invalid_bad_proposal_1_signature() { let mut proposer_slashing = harness.make_proposer_slashing(1); proposer_slashing.signed_header_1.signature = Signature::empty(); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_proposer_slashings( &mut state, &[proposer_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); @@ -871,10 +897,12 @@ async fn invalid_bad_proposal_2_signature() { let mut proposer_slashing = harness.make_proposer_slashing(1); proposer_slashing.signed_header_2.signature = Signature::empty(); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_proposer_slashings( &mut state, &[proposer_slashing], VerifySignatures::True, + &mut ctxt, &spec, ); @@ -896,10 +924,12 @@ async fn invalid_proposer_slashing_proposal_epoch_mismatch() { proposer_slashing.signed_header_1.message.slot = Slot::new(0); proposer_slashing.signed_header_2.message.slot = Slot::new(128); let mut state = harness.get_current_state(); + let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_proposer_slashings( &mut state, &[proposer_slashing], VerifySignatures::False, + &mut ctxt, &spec, ); diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index d8c94be0c37..42bc869a85f 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -79,11 +79,16 @@ impl Operation for Attestation { spec: &ChainSpec, _: &Operations, ) -> Result<(), BlockProcessingError> { - let proposer_index = state.get_beacon_proposer_index(state.slot(), spec)? as u64; + let mut ctxt = ConsensusContext::new(state.slot()); + let proposer_index = ctxt.get_proposer_index(state, spec)?; match state { - BeaconState::Base(_) => { - base::process_attestations(state, &[self.clone()], VerifySignatures::True, spec) - } + BeaconState::Base(_) => base::process_attestations( + state, + &[self.clone()], + VerifySignatures::True, + &mut ctxt, + spec, + ), BeaconState::Altair(_) | BeaconState::Merge(_) => altair::process_attestation( state, self, @@ -111,7 +116,14 @@ impl Operation for AttesterSlashing { spec: &ChainSpec, _: &Operations, ) -> Result<(), BlockProcessingError> { - process_attester_slashings(state, &[self.clone()], VerifySignatures::True, spec) + let mut ctxt = ConsensusContext::new(state.slot()); + process_attester_slashings( + state, + &[self.clone()], + VerifySignatures::True, + &mut ctxt, + spec, + ) } } @@ -145,7 +157,14 @@ impl Operation for ProposerSlashing { spec: &ChainSpec, _: &Operations, ) -> Result<(), BlockProcessingError> { - process_proposer_slashings(state, &[self.clone()], VerifySignatures::True, spec) + let mut ctxt = ConsensusContext::new(state.slot()); + process_proposer_slashings( + state, + &[self.clone()], + VerifySignatures::True, + &mut ctxt, + spec, + ) } }