From 7a4d9540abad407b9220f8be29f48817ce1a9943 Mon Sep 17 00:00:00 2001 From: Anton Puhach Date: Thu, 30 Jan 2025 21:24:18 +0100 Subject: [PATCH] refactor: remove slashing info and signature verification from epoch manager (#12844) * `get_validator_by_account_id` no longer returns slashing info and `get_fisherman_by_account` is removed * all functions for signature verification (except `verify_validator_signature`) are moved out of EpochManager as free functions * bunch of slashing tests are disabled --- chain/chain/src/chain.rs | 16 +- chain/chain/src/runtime/tests.rs | 52 +++--- .../stateless_validation/chunk_endorsement.rs | 6 +- chain/chain/src/test_utils/kv_runtime.rs | 65 +------- chain/chain/src/validate.rs | 37 +++-- chain/client/src/client.rs | 61 +++---- chain/client/src/client_actor.rs | 6 +- chain/client/src/debug.rs | 8 +- chain/client/src/info.rs | 9 +- .../src/stateless_validation/validate.rs | 60 +++++-- chain/client/src/view_client_actor.rs | 3 - chain/epoch-manager/src/adapter.rs | 153 ++---------------- chain/epoch-manager/src/lib.rs | 12 -- chain/epoch-manager/src/tests/mod.rs | 3 +- .../src/tests/client/challenges.rs | 5 - 15 files changed, 148 insertions(+), 348 deletions(-) diff --git a/chain/chain/src/chain.rs b/chain/chain/src/chain.rs index bc2dab915fa..b1c0d6a56f5 100644 --- a/chain/chain/src/chain.rs +++ b/chain/chain/src/chain.rs @@ -898,14 +898,8 @@ impl Chain { fn partial_verify_orphan_header_signature(&self, header: &BlockHeader) -> Result { let block_producer = self.epoch_manager.get_block_producer(header.epoch_id(), header.height())?; - // DEVNOTE: we pass head which is not necessarily on block's chain, but it's only used for - // slashing info which we will ignore - let head = self.head()?; - let (block_producer, _slashed) = self.epoch_manager.get_validator_by_account_id( - header.epoch_id(), - &head.last_block_hash, - &block_producer, - )?; + let block_producer = + self.epoch_manager.get_validator_by_account_id(header.epoch_id(), &block_producer)?; Ok(header.signature().verify(header.hash().as_ref(), block_producer.public_key())) } @@ -1203,7 +1197,6 @@ impl Chain { &self, challenges: &[Challenge], epoch_id: &EpochId, - prev_block_hash: &CryptoHash, ) -> Result<(ChallengesResult, Vec), Error> { let _span = tracing::debug_span!( target: "chain", @@ -1217,7 +1210,6 @@ impl Chain { self.epoch_manager.as_ref(), self.runtime_adapter.as_ref(), epoch_id, - prev_block_hash, challenge, ) { Ok((hash, account_ids)) => { @@ -1554,7 +1546,7 @@ impl Chain { /// soon as possible and allow next block producer to skip invalid blocks. pub fn process_challenge(&mut self, challenge: &Challenge) { let head = unwrap_or_return!(self.head()); - match self.verify_challenges(&[challenge.clone()], &head.epoch_id, &head.last_block_hash) { + match self.verify_challenges(&[challenge.clone()], &head.epoch_id) { Ok((_, challenged_blocks)) => { let mut chain_update = self.chain_update(); for block_hash in challenged_blocks { @@ -2331,7 +2323,7 @@ impl Chain { } let (challenges_result, challenged_blocks) = - self.verify_challenges(block.challenges(), header.epoch_id(), header.prev_hash())?; + self.verify_challenges(block.challenges(), header.epoch_id())?; let prev_block = self.get_block(&prev_hash)?; diff --git a/chain/chain/src/runtime/tests.rs b/chain/chain/src/runtime/tests.rs index 460a6f82ac6..ffc85ce7a93 100644 --- a/chain/chain/src/runtime/tests.rs +++ b/chain/chain/src/runtime/tests.rs @@ -702,13 +702,7 @@ fn test_verify_validator_signature() { let signature = signer.sign(&data); assert!(env .epoch_manager - .verify_validator_signature( - &env.head.epoch_id, - &env.head.last_block_hash, - &validators[0], - &data, - &signature - ) + .verify_validator_signature(&env.head.epoch_id, &validators[0], &data, &signature) .unwrap()); } @@ -1021,6 +1015,7 @@ fn test_get_validator_info() { assert_eq!(response.epoch_start_height, 3); } +#[ignore = "Ignoring challenge and slashing related tests"] #[test] fn test_challenges() { let mut env = @@ -1043,16 +1038,16 @@ fn test_challenges() { let msg = vec![0, 1, 2]; let signer = InMemorySigner::test_signer(&"test2".parse().unwrap()); let signature = signer.sign(&msg); - assert!(!env - .epoch_manager - .verify_validator_signature( - &env.head.epoch_id, - &env.head.last_block_hash, - &"test2".parse().unwrap(), - &msg, - &signature, - ) - .unwrap()); + assert!( + !env.epoch_manager + .verify_validator_signature( + &env.head.epoch_id, + &"test2".parse().unwrap(), + &msg, + &signature, + ) + .unwrap() + ); // Run for 3 epochs, to finalize the given block and make sure that slashed stake actually correctly propagates. for _ in 0..6 { env.step(vec![vec![]], vec![true], vec![]); @@ -1061,6 +1056,7 @@ fn test_challenges() { /// Test that in case of a double sign, not all stake is slashed if the double signed stake is /// less than 33% and all stake is slashed if the stake is more than 33% +#[ignore = "Ignoring challenge and slashing related tests"] #[test] fn test_double_sign_challenge_not_all_slashed() { init_test_logger(); @@ -1099,16 +1095,16 @@ fn test_double_sign_challenge_not_all_slashed() { let msg = vec![0, 1, 2]; let signer = InMemorySigner::test_signer(&"test2".parse().unwrap()); let signature = signer.sign(&msg); - assert!(!env - .epoch_manager - .verify_validator_signature( - &env.head.epoch_id, - &env.head.last_block_hash, - &"test2".parse().unwrap(), - &msg, - &signature, - ) - .unwrap()); + assert!( + !env.epoch_manager + .verify_validator_signature( + &env.head.epoch_id, + &"test2".parse().unwrap(), + &msg, + &signature, + ) + .unwrap() + ); for _ in 2..11 { env.step(vec![vec![]], vec![true], vec![]); @@ -1141,6 +1137,7 @@ fn test_double_sign_challenge_not_all_slashed() { } /// Test that double sign from multiple accounts may result in all of their stake slashed. +#[ignore = "Ignoring challenge and slashing related tests"] #[test] fn test_double_sign_challenge_all_slashed() { init_test_logger(); @@ -1159,7 +1156,6 @@ fn test_double_sign_challenge_all_slashed() { .epoch_manager .verify_validator_signature( &env.head.epoch_id, - &env.head.last_block_hash, &AccountId::try_from(format!("test{}", i + 1)).unwrap(), &msg, &signature, diff --git a/chain/chain/src/stateless_validation/chunk_endorsement.rs b/chain/chain/src/stateless_validation/chunk_endorsement.rs index a419665be31..7dfa63e5e00 100644 --- a/chain/chain/src/stateless_validation/chunk_endorsement.rs +++ b/chain/chain/src/stateless_validation/chunk_endorsement.rs @@ -88,11 +88,7 @@ pub fn validate_chunk_endorsements_in_block( let mut endorsed_chunk_validators = HashMap::new(); for (account_id, signature) in ordered_chunk_validators.iter().zip(signatures) { let Some(signature) = signature else { continue }; - let (validator, _) = epoch_manager.get_validator_by_account_id( - &epoch_id, - block.header().prev_hash(), - account_id, - )?; + let validator = epoch_manager.get_validator_by_account_id(&epoch_id, account_id)?; // Block should not be produced with an invalid signature. if !ChunkEndorsement::validate_signature( diff --git a/chain/chain/src/test_utils/kv_runtime.rs b/chain/chain/src/test_utils/kv_runtime.rs index c1f010632bf..41e02d29eed 100644 --- a/chain/chain/src/test_utils/kv_runtime.rs +++ b/chain/chain/src/test_utils/kv_runtime.rs @@ -10,7 +10,7 @@ use itertools::Itertools; use near_async::time::Duration; use near_chain_configs::{ProtocolConfig, DEFAULT_GC_NUM_EPOCHS_TO_KEEP}; use near_chain_primitives::Error; -use near_crypto::{KeyType, PublicKey, SecretKey, Signature}; +use near_crypto::{KeyType, PublicKey, SecretKey}; use near_epoch_manager::{EpochManagerAdapter, RngSeed}; use near_parameters::RuntimeConfig; use near_pool::types::TransactionGroupIterator; @@ -28,10 +28,6 @@ use near_primitives::hash::{hash, CryptoHash}; use near_primitives::receipt::{ActionReceipt, Receipt, ReceiptEnum, ReceiptV0}; use near_primitives::shard_layout::{ShardLayout, ShardUId}; use near_primitives::state_part::PartId; -use near_primitives::stateless_validation::chunk_endorsement::ChunkEndorsement; -use near_primitives::stateless_validation::contract_distribution::{ - ChunkContractAccesses, ContractCodeRequest, -}; use near_primitives::stateless_validation::validator_assignment::ChunkValidatorAssignments; use near_primitives::stateless_validation::ChunkProductionKey; use near_primitives::transaction::{ @@ -766,32 +762,22 @@ impl EpochManagerAdapter for MockEpochManager { fn get_validator_by_account_id( &self, epoch_id: &EpochId, - _last_known_block_hash: &CryptoHash, account_id: &AccountId, - ) -> Result<(ValidatorStake, bool), EpochError> { + ) -> Result { let validators = &self.validators_by_valset[self.get_valset_for_epoch(epoch_id)?]; for validator_stake in validators.block_producers.iter() { if validator_stake.account_id() == account_id { - return Ok((validator_stake.clone(), false)); + return Ok(validator_stake.clone()); } } for validator_stake in validators.chunk_producers.iter().flatten() { if validator_stake.account_id() == account_id { - return Ok((validator_stake.clone(), false)); + return Ok(validator_stake.clone()); } } Err(EpochError::NotAValidator(account_id.clone(), *epoch_id)) } - fn get_fisherman_by_account_id( - &self, - epoch_id: &EpochId, - _last_known_block_hash: &CryptoHash, - account_id: &AccountId, - ) -> Result<(ValidatorStake, bool), EpochError> { - Err(EpochError::NotAValidator(account_id.clone(), *epoch_id)) - } - fn get_validator_info( &self, _epoch_id: ValidatorInfoIdentifier, @@ -843,49 +829,6 @@ impl EpochManagerAdapter for MockEpochManager { false } - fn verify_validator_signature( - &self, - _epoch_id: &EpochId, - _last_known_block_hash: &CryptoHash, - _account_id: &AccountId, - _data: &[u8], - _signature: &Signature, - ) -> Result { - Ok(true) - } - - fn verify_validator_or_fisherman_signature( - &self, - _epoch_id: &EpochId, - _last_known_block_hash: &CryptoHash, - _account_id: &AccountId, - _data: &[u8], - _signature: &Signature, - ) -> Result { - Ok(true) - } - - fn verify_chunk_endorsement_signature( - &self, - _endorsement: &ChunkEndorsement, - ) -> Result { - Ok(true) - } - - fn verify_witness_contract_accesses_signature( - &self, - _accesses: &ChunkContractAccesses, - ) -> Result { - Ok(true) - } - - fn verify_witness_contract_code_request_signature( - &self, - _request: &ContractCodeRequest, - ) -> Result { - Ok(true) - } - fn cares_about_shard_in_epoch( &self, epoch_id: &EpochId, diff --git a/chain/chain/src/validate.rs b/chain/chain/src/validate.rs index a499c690f6b..28e6811f5e6 100644 --- a/chain/chain/src/validate.rs +++ b/chain/chain/src/validate.rs @@ -10,6 +10,7 @@ use near_primitives::challenge::{ BlockDoubleSign, Challenge, ChallengeBody, ChunkProofs, ChunkState, MaybeEncodedShardChunk, }; use near_primitives::congestion_info::CongestionInfo; +use near_primitives::errors::EpochError; use near_primitives::hash::CryptoHash; use near_primitives::merkle::merklize; use near_primitives::sharding::{ShardChunk, ShardChunkHeader}; @@ -259,14 +260,12 @@ fn validate_double_sign( && left_block_header.height() == right_block_header.height() && epoch_manager.verify_validator_signature( left_block_header.epoch_id(), - left_block_header.prev_hash(), &block_producer, left_block_header.hash().as_ref(), left_block_header.signature(), )? && epoch_manager.verify_validator_signature( right_block_header.epoch_id(), - right_block_header.prev_hash(), &block_producer, right_block_header.hash().as_ref(), right_block_header.signature(), @@ -455,19 +454,9 @@ pub fn validate_challenge( epoch_manager: &dyn EpochManagerAdapter, runtime: &dyn RuntimeAdapter, epoch_id: &EpochId, - last_block_hash: &CryptoHash, challenge: &Challenge, ) -> Result<(CryptoHash, Vec), Error> { - // Check signature is correct on the challenge. - if !epoch_manager.verify_validator_or_fisherman_signature( - epoch_id, - last_block_hash, - &challenge.account_id, - challenge.hash.as_ref(), - &challenge.signature, - )? { - return Err(Error::InvalidChallenge); - } + validate_challenge_signature(epoch_manager, epoch_id, challenge)?; match &challenge.body { ChallengeBody::BlockDoubleSign(block_double_sign) => { validate_double_sign(epoch_manager, block_double_sign) @@ -481,6 +470,28 @@ pub fn validate_challenge( } } +fn validate_challenge_signature( + epoch_manager: &dyn EpochManagerAdapter, + epoch_id: &EpochId, + challenge: &Challenge, +) -> Result<(), Error> { + if !epoch_manager.should_validate_signatures() { + return Ok(()); + } + let data = challenge.hash.as_ref(); + let account_id = &challenge.account_id; + let epoch_info = epoch_manager.get_epoch_info(epoch_id)?; + let validator = epoch_info + .get_validator_by_account(account_id) + .or_else(|| epoch_info.get_fisherman_by_account(account_id)) + .ok_or_else(|| EpochError::NotAValidator(account_id.clone(), *epoch_id))?; + if !challenge.signature.verify(data, validator.public_key()) { + return Err(Error::InvalidChallenge); + } + + Ok(()) +} + #[cfg(test)] mod tests { use near_crypto::{InMemorySigner, KeyType}; diff --git a/chain/client/src/client.rs b/chain/client/src/client.rs index 19a3aaad747..1195eff7bb1 100644 --- a/chain/client/src/client.rs +++ b/chain/client/src/client.rs @@ -573,11 +573,8 @@ impl Client { return Err(Error::BlockProducer("Should reschedule".to_string())); } - let (validator_stake, _) = self.epoch_manager.get_validator_by_account_id( - &epoch_id, - &prev_header.hash(), - &next_block_proposer, - )?; + let validator_stake = + self.epoch_manager.get_validator_by_account_id(&epoch_id, &next_block_proposer)?; let validator_pk = validator_stake.take_public_key(); if validator_pk != validator_signer.public_key() { @@ -2127,23 +2124,13 @@ impl Client { self.process_block_processing_artifact(blocks_processing_artifacts, signer); } - pub fn is_validator( - &self, - epoch_id: &EpochId, - block_hash: &CryptoHash, - signer: &Option>, - ) -> bool { + pub fn is_validator(&self, epoch_id: &EpochId, signer: &Option>) -> bool { match signer { None => false, Some(signer) => { let account_id = signer.validator_id(); - match self - .epoch_manager - .get_validator_by_account_id(epoch_id, block_hash, account_id) - { - Ok((validator_stake, is_slashed)) => { - !is_slashed && validator_stake.take_public_key() == signer.public_key() - } + match self.epoch_manager.get_validator_by_account_id(epoch_id, account_id) { + Ok(validator_stake) => validator_stake.take_public_key() == signer.public_key(), Err(_) => false, } } @@ -2157,27 +2144,19 @@ impl Client { check_validator: bool, error: near_chain::Error, ) { - let is_validator = - |epoch_id, block_hash, account_id, epoch_manager: &dyn EpochManagerAdapter| { - match epoch_manager.get_validator_by_account_id(epoch_id, block_hash, account_id) { - Ok((_, is_slashed)) => !is_slashed, - Err(_) => false, - } - }; + let is_validator = |epoch_id, account_id, epoch_manager: &dyn EpochManagerAdapter| { + epoch_manager.get_validator_by_account_id(epoch_id, account_id).is_ok() + }; if let near_chain::Error::DBNotFoundErr(_) = error { if check_validator { let head = unwrap_or_return!(self.chain.head()); - if !is_validator( - &head.epoch_id, - &head.last_block_hash, - &approval.account_id, - self.epoch_manager.as_ref(), - ) && !is_validator( - &head.next_epoch_id, - &head.last_block_hash, - &approval.account_id, - self.epoch_manager.as_ref(), - ) { + if !is_validator(&head.epoch_id, &approval.account_id, self.epoch_manager.as_ref()) + && !is_validator( + &head.next_epoch_id, + &approval.account_id, + self.epoch_manager.as_ref(), + ) + { return; } } @@ -2263,11 +2242,10 @@ impl Client { // exist in the epoch of the next block, we use the epoch after next to validate the // signature. We don't care here if the block is actually on the epochs boundary yet, // `Doomslug::on_approval_message` below will handle it. - let validator_epoch_id = match self.epoch_manager.get_validator_by_account_id( - &next_block_epoch_id, - &parent_hash, - account_id, - ) { + let validator_epoch_id = match self + .epoch_manager + .get_validator_by_account_id(&next_block_epoch_id, account_id) + { Ok(_) => next_block_epoch_id, Err(EpochError::NotAValidator(_, _)) => { match self.epoch_manager.get_next_epoch_id_from_prev_block(&parent_hash) { @@ -2279,7 +2257,6 @@ impl Client { }; match self.epoch_manager.verify_validator_signature( &validator_epoch_id, - &parent_hash, account_id, Approval::get_data_for_sig(inner, *target_height).as_ref(), signature, diff --git a/chain/client/src/client_actor.rs b/chain/client/src/client_actor.rs index 13a1d30981f..9c78b7ded12 100644 --- a/chain/client/src/client_actor.rs +++ b/chain/client/src/client_actor.rs @@ -963,7 +963,7 @@ impl ClientActorInner { .get_next_epoch_id_from_prev_block(&prev_block_hash)); // Check client is part of the futures validators - if self.client.is_validator(&next_epoch_id, &prev_block_hash, validator_signer) { + if self.client.is_validator(&next_epoch_id, validator_signer) { debug!(target: "client", "Sending announce account for {}", signer.validator_id()); self.last_validator_announce_time = Some(now); @@ -1327,8 +1327,8 @@ impl ClientActorInner { match chain_store_update.commit() { Ok(_) => { let head = unwrap_or_return!(self.client.chain.head()); - if self.client.is_validator(&head.epoch_id, &head.last_block_hash, &signer) - || self.client.is_validator(&head.next_epoch_id, &head.last_block_hash, &signer) + if self.client.is_validator(&head.epoch_id, &signer) + || self.client.is_validator(&head.next_epoch_id, &signer) { for approval in approvals { if let Err(e) = self.client.send_block_approval( diff --git a/chain/client/src/debug.rs b/chain/client/src/debug.rs index 7b5be135a5d..bbc19b3d2fc 100644 --- a/chain/client/src/debug.rs +++ b/chain/client/src/debug.rs @@ -746,11 +746,9 @@ impl ClientActorInner { let mut endorsed_chunk_validators = HashMap::new(); for (account_id, signature) in ordered_chunk_validators.iter().zip(signatures) { let Some(signature) = signature else { continue }; - let Ok((validator, _)) = self.client.epoch_manager.get_validator_by_account_id( - &epoch_id, - block.header().prev_hash(), - account_id, - ) else { + let Ok(validator) = + self.client.epoch_manager.get_validator_by_account_id(&epoch_id, account_id) + else { continue; }; if !ChunkEndorsement::validate_signature( diff --git a/chain/client/src/info.rs b/chain/client/src/info.rs index deca734dfbb..3d3e347412f 100644 --- a/chain/client/src/info.rs +++ b/chain/client/src/info.rs @@ -317,14 +317,7 @@ impl InfoHelper { self.get_num_validators(client.epoch_manager.as_ref(), &head.epoch_id); let account_id = signer.as_ref().map(|x| x.validator_id()); let is_validator = if let Some(account_id) = account_id { - match client.epoch_manager.get_validator_by_account_id( - &head.epoch_id, - &head.last_block_hash, - account_id, - ) { - Ok((_, is_slashed)) => !is_slashed, - Err(_) => false, - } + client.epoch_manager.get_validator_by_account_id(&head.epoch_id, account_id).is_ok() } else { false }; diff --git a/chain/client/src/stateless_validation/validate.rs b/chain/client/src/stateless_validation/validate.rs index c059aa2058a..4fc834994ce 100644 --- a/chain/client/src/stateless_validation/validate.rs +++ b/chain/client/src/stateless_validation/validate.rs @@ -103,10 +103,7 @@ pub fn validate_chunk_endorsement( )? { return Ok(false); } - - if !epoch_manager.verify_chunk_endorsement_signature(endorsement)? { - return Err(Error::InvalidChunkEndorsement); - } + validate_chunk_endorsement_signature(epoch_manager, endorsement)?; Ok(true) } @@ -122,9 +119,8 @@ pub fn validate_chunk_contract_accesses( if !validate_chunk_relevant_as_validator(epoch_manager, key, signer.validator_id(), store)? { return Ok(false); } - if !epoch_manager.verify_witness_contract_accesses_signature(accesses)? { - return Err(Error::Other("Invalid witness contract accesses signature".to_owned())); - } + validate_witness_contract_accesses_signature(epoch_manager, accesses)?; + Ok(true) } @@ -138,9 +134,7 @@ pub fn validate_contract_code_request( if !validate_chunk_relevant_as_validator(epoch_manager, key, request.requester(), store)? { return Ok(false); } - if !epoch_manager.verify_witness_contract_code_request_signature(request)? { - return Err(Error::Other("Invalid witness contract code request signature".to_owned())); - } + validate_witness_contract_code_request_signature(epoch_manager, request)?; Ok(true) } @@ -266,3 +260,49 @@ fn validate_exclude_witness_contracts_enabled( Err(Error::Other(format!("ProtocolFeature::ExcludeContractCodeFromStateWitness is disabled for protocol version {protocol_version}"))) } } + +fn validate_chunk_endorsement_signature( + epoch_manager: &dyn EpochManagerAdapter, + endorsement: &ChunkEndorsement, +) -> Result<(), Error> { + if epoch_manager.should_validate_signatures() { + let validator = epoch_manager.get_validator_by_account_id( + &endorsement.chunk_production_key().epoch_id, + &endorsement.account_id(), + )?; + if !endorsement.verify(validator.public_key()) { + return Err(Error::InvalidChunkEndorsement); + } + } + Ok(()) +} + +fn validate_witness_contract_code_request_signature( + epoch_manager: &dyn EpochManagerAdapter, + request: &ContractCodeRequest, +) -> Result<(), Error> { + if epoch_manager.should_validate_signatures() { + let validator = epoch_manager.get_validator_by_account_id( + &request.chunk_production_key().epoch_id, + &request.requester(), + )?; + if !request.verify_signature(validator.public_key()) { + return Err(Error::Other("Invalid witness contract code request signature".to_owned())); + } + } + Ok(()) +} + +fn validate_witness_contract_accesses_signature( + epoch_manager: &dyn EpochManagerAdapter, + accesses: &ChunkContractAccesses, +) -> Result<(), Error> { + if epoch_manager.should_validate_signatures() { + let chunk_producer = + epoch_manager.get_chunk_producer_info(accesses.chunk_production_key())?; + if !accesses.verify_signature(chunk_producer.public_key()) { + return Err(Error::Other("Invalid witness contract accesses signature".to_owned())); + } + } + Ok(()) +} diff --git a/chain/client/src/view_client_actor.rs b/chain/client/src/view_client_actor.rs index 43b8d80523d..b01df57799e 100644 --- a/chain/client/src/view_client_actor.rs +++ b/chain/client/src/view_client_actor.rs @@ -655,12 +655,9 @@ impl ViewClientActorInner { announce_account: &AnnounceAccount, ) -> Result { let announce_hash = announce_account.hash(); - let head = self.chain.head()?; - self.epoch_manager .verify_validator_signature( &announce_account.epoch_id, - &head.last_block_hash, &announce_account.account_id, announce_hash.as_ref(), &announce_account.signature, diff --git a/chain/epoch-manager/src/adapter.rs b/chain/epoch-manager/src/adapter.rs index c29f3e874ae..792dffe690e 100644 --- a/chain/epoch-manager/src/adapter.rs +++ b/chain/epoch-manager/src/adapter.rs @@ -8,10 +8,6 @@ use near_primitives::epoch_manager::{EpochConfig, ShardConfig}; use near_primitives::errors::EpochError; use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::ShardLayout; -use near_primitives::stateless_validation::chunk_endorsement::ChunkEndorsement; -use near_primitives::stateless_validation::contract_distribution::{ - ChunkContractAccesses, ContractCodeRequest, -}; use near_primitives::stateless_validation::validator_assignment::ChunkValidatorAssignments; use near_primitives::stateless_validation::ChunkProductionKey; use near_primitives::types::validator_stake::ValidatorStake; @@ -229,16 +225,13 @@ pub trait EpochManagerAdapter: Send + Sync { fn get_validator_by_account_id( &self, epoch_id: &EpochId, - last_known_block_hash: &CryptoHash, - account_id: &AccountId, - ) -> Result<(ValidatorStake, bool), EpochError>; - - fn get_fisherman_by_account_id( - &self, - epoch_id: &EpochId, - last_known_block_hash: &CryptoHash, account_id: &AccountId, - ) -> Result<(ValidatorStake, bool), EpochError>; + ) -> Result { + let epoch_info = self.get_epoch_info(epoch_id)?; + epoch_info + .get_validator_by_account(account_id) + .ok_or_else(|| EpochError::NotAValidator(account_id.clone(), *epoch_id)) + } /// WARNING: this call may be expensive. /// @@ -305,40 +298,20 @@ pub trait EpochManagerAdapter: Send + Sync { } /// Verify validator signature for the given epoch. - /// Note: doesn't account for slashed accounts within given epoch. USE WITH CAUTION. fn verify_validator_signature( &self, epoch_id: &EpochId, - last_known_block_hash: &CryptoHash, - account_id: &AccountId, - data: &[u8], - signature: &Signature, - ) -> Result; - - /// Verify signature for validator or fisherman. Used for validating challenges. - fn verify_validator_or_fisherman_signature( - &self, - epoch_id: &EpochId, - last_known_block_hash: &CryptoHash, account_id: &AccountId, data: &[u8], signature: &Signature, - ) -> Result; - - fn verify_chunk_endorsement_signature( - &self, - endorsement: &ChunkEndorsement, - ) -> Result; - - fn verify_witness_contract_accesses_signature( - &self, - accesses: &ChunkContractAccesses, - ) -> Result; - - fn verify_witness_contract_code_request_signature( - &self, - request: &ContractCodeRequest, - ) -> Result; + ) -> Result { + if self.should_validate_signatures() { + let validator = self.get_validator_by_account_id(epoch_id, account_id)?; + Ok(signature.verify(data, validator.public_key())) + } else { + Ok(true) + } + } fn cares_about_shard_in_epoch( &self, @@ -717,30 +690,6 @@ impl EpochManagerAdapter for EpochManagerHandle { epoch_manager.get_chunk_validator_assignments(epoch_id, shard_id, height) } - fn get_validator_by_account_id( - &self, - epoch_id: &EpochId, - last_known_block_hash: &CryptoHash, - account_id: &AccountId, - ) -> Result<(ValidatorStake, bool), EpochError> { - let epoch_manager = self.read(); - let validator = epoch_manager.get_validator_by_account_id(epoch_id, account_id)?; - let block_info = epoch_manager.get_block_info(last_known_block_hash)?; - Ok((validator, block_info.slashed().contains_key(account_id))) - } - - fn get_fisherman_by_account_id( - &self, - epoch_id: &EpochId, - last_known_block_hash: &CryptoHash, - account_id: &AccountId, - ) -> Result<(ValidatorStake, bool), EpochError> { - let epoch_manager = self.read(); - let fisherman = epoch_manager.get_fisherman_by_account_id(epoch_id, account_id)?; - let block_info = epoch_manager.get_block_info(last_known_block_hash)?; - Ok((fisherman, block_info.slashed().contains_key(account_id))) - } - /// WARNING: this function calls EpochManager::get_epoch_info_aggregator_upto_last /// underneath which can be very expensive. fn get_validator_info( @@ -796,80 +745,6 @@ impl EpochManagerAdapter for EpochManagerHandle { ) } - fn verify_validator_signature( - &self, - epoch_id: &EpochId, - last_known_block_hash: &CryptoHash, - account_id: &AccountId, - data: &[u8], - signature: &Signature, - ) -> Result { - let (validator, is_slashed) = - self.get_validator_by_account_id(epoch_id, last_known_block_hash, account_id)?; - if is_slashed { - return Ok(false); - } - Ok(signature.verify(data, validator.public_key())) - } - - fn verify_validator_or_fisherman_signature( - &self, - epoch_id: &EpochId, - last_known_block_hash: &CryptoHash, - account_id: &AccountId, - data: &[u8], - signature: &Signature, - ) -> Result { - match self.verify_validator_signature( - epoch_id, - last_known_block_hash, - account_id, - data, - signature, - ) { - Err(Error::NotAValidator(_)) => { - let (fisherman, is_slashed) = - self.get_fisherman_by_account_id(epoch_id, last_known_block_hash, account_id)?; - if is_slashed { - return Ok(false); - } - Ok(signature.verify(data, fisherman.public_key())) - } - other => other, - } - } - - fn verify_chunk_endorsement_signature( - &self, - endorsement: &ChunkEndorsement, - ) -> Result { - let epoch_manager = self.read(); - let epoch_id = endorsement.chunk_production_key().epoch_id; - let validator = - epoch_manager.get_validator_by_account_id(&epoch_id, &endorsement.account_id())?; - Ok(endorsement.verify(validator.public_key())) - } - - fn verify_witness_contract_accesses_signature( - &self, - accesses: &ChunkContractAccesses, - ) -> Result { - let chunk_producer = - self.read().get_chunk_producer_info(accesses.chunk_production_key())?; - Ok(accesses.verify_signature(chunk_producer.public_key())) - } - - fn verify_witness_contract_code_request_signature( - &self, - request: &ContractCodeRequest, - ) -> Result { - let validator = self.read().get_validator_by_account_id( - &request.chunk_production_key().epoch_id, - request.requester(), - )?; - Ok(request.verify_signature(validator.public_key())) - } - fn cares_about_shard_from_prev_block( &self, parent_hash: &CryptoHash, diff --git a/chain/epoch-manager/src/lib.rs b/chain/epoch-manager/src/lib.rs index 4d841612388..fcdfcf732d7 100644 --- a/chain/epoch-manager/src/lib.rs +++ b/chain/epoch-manager/src/lib.rs @@ -1213,18 +1213,6 @@ impl EpochManager { .ok_or_else(|| EpochError::NotAValidator(account_id.clone(), *epoch_id)) } - /// Returns fisherman for given account id for given epoch. - pub fn get_fisherman_by_account_id( - &self, - epoch_id: &EpochId, - account_id: &AccountId, - ) -> Result { - let epoch_info = self.get_epoch_info(epoch_id)?; - epoch_info - .get_fisherman_by_account(account_id) - .ok_or_else(|| EpochError::NotAValidator(account_id.clone(), *epoch_id)) - } - pub fn get_epoch_id(&self, block_hash: &CryptoHash) -> Result { Ok(*self.get_block_info(block_hash)?.epoch_id()) } diff --git a/chain/epoch-manager/src/tests/mod.rs b/chain/epoch-manager/src/tests/mod.rs index 556436b9373..62c63d1be59 100644 --- a/chain/epoch-manager/src/tests/mod.rs +++ b/chain/epoch-manager/src/tests/mod.rs @@ -3355,8 +3355,7 @@ fn test_verify_partial_witness_signature() { let epoch_id = epoch_manager.get_epoch_id(&h[1]).unwrap(); // Verify if the test signer has same public key as the chunk validator. - let (validator, _) = - epoch_manager.get_validator_by_account_id(&epoch_id, &h[0], &account_id).unwrap(); + let validator = epoch_manager.get_validator_by_account_id(&epoch_id, &account_id).unwrap(); let chunk_producer: AccountId = "test1".parse().unwrap(); let signer = Arc::new(create_test_signer(chunk_producer.as_str())); assert_eq!(signer.public_key(), validator.public_key().clone()); diff --git a/integration-tests/src/tests/client/challenges.rs b/integration-tests/src/tests/client/challenges.rs index 5e82fc69929..3e3534708ca 100644 --- a/integration-tests/src/tests/client/challenges.rs +++ b/integration-tests/src/tests/client/challenges.rs @@ -133,7 +133,6 @@ fn test_verify_block_double_sign_challenge() { env.clients[1].chain.epoch_manager.as_ref(), env.clients[1].chain.runtime_adapter.as_ref(), &epoch_id, - genesis.hash(), &valid_challenge ) .unwrap() @@ -151,7 +150,6 @@ fn test_verify_block_double_sign_challenge() { env.clients[1].chain.epoch_manager.as_ref(), env.clients[1].chain.runtime_adapter.as_ref(), &epoch_id, - genesis.hash(), &invalid_challenge, ) .is_err()); @@ -167,7 +165,6 @@ fn test_verify_block_double_sign_challenge() { env.clients[1].chain.epoch_manager.as_ref(), env.clients[1].chain.runtime_adapter.as_ref(), &epoch_id, - genesis.hash(), &invalid_challenge, ) .is_err()); @@ -330,7 +327,6 @@ fn challenge( env.clients[0].chain.epoch_manager.as_ref(), env.clients[0].chain.runtime_adapter.as_ref(), block.header().epoch_id(), - block.header().prev_hash(), &valid_challenge, ) } @@ -493,7 +489,6 @@ fn test_verify_chunk_invalid_state_challenge() { client.chain.epoch_manager.as_ref(), client.chain.runtime_adapter.as_ref(), block.header().epoch_id(), - block.header().prev_hash(), &challenge, ) .unwrap_err(),