Skip to content

Commit

Permalink
refactor: remove slashing info and signature verification from epoch …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
pugachAG authored Jan 30, 2025
1 parent 9a4504c commit 7a4d954
Show file tree
Hide file tree
Showing 15 changed files with 148 additions and 348 deletions.
16 changes: 4 additions & 12 deletions chain/chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,14 +898,8 @@ impl Chain {
fn partial_verify_orphan_header_signature(&self, header: &BlockHeader) -> Result<bool, Error> {
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()))
}

Expand Down Expand Up @@ -1203,7 +1197,6 @@ impl Chain {
&self,
challenges: &[Challenge],
epoch_id: &EpochId,
prev_block_hash: &CryptoHash,
) -> Result<(ChallengesResult, Vec<CryptoHash>), Error> {
let _span = tracing::debug_span!(
target: "chain",
Expand All @@ -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)) => {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)?;

Expand Down
52 changes: 24 additions & 28 deletions chain/chain/src/runtime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down Expand Up @@ -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 =
Expand All @@ -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![]);
Expand All @@ -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();
Expand Down Expand Up @@ -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![]);
Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand Down
6 changes: 1 addition & 5 deletions chain/chain/src/stateless_validation/chunk_endorsement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
65 changes: 4 additions & 61 deletions chain/chain/src/test_utils/kv_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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::{
Expand Down Expand Up @@ -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<ValidatorStake, EpochError> {
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,
Expand Down Expand Up @@ -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<bool, Error> {
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<bool, Error> {
Ok(true)
}

fn verify_chunk_endorsement_signature(
&self,
_endorsement: &ChunkEndorsement,
) -> Result<bool, Error> {
Ok(true)
}

fn verify_witness_contract_accesses_signature(
&self,
_accesses: &ChunkContractAccesses,
) -> Result<bool, Error> {
Ok(true)
}

fn verify_witness_contract_code_request_signature(
&self,
_request: &ContractCodeRequest,
) -> Result<bool, Error> {
Ok(true)
}

fn cares_about_shard_in_epoch(
&self,
epoch_id: &EpochId,
Expand Down
37 changes: 24 additions & 13 deletions chain/chain/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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<AccountId>), 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)
Expand All @@ -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};
Expand Down
Loading

0 comments on commit 7a4d954

Please sign in to comment.