From 6c714118340edea2176fc56a5ec5bb99d772e628 Mon Sep 17 00:00:00 2001 From: Anton Puhach Date: Mon, 8 Apr 2024 16:25:09 +0200 Subject: [PATCH] refactor: state witness struct (#10934) This PR is a preparation for introducing state witness compression as part of #10780. It addresses the following issues: * State witness is implicitly deserialised when it is received via network and then serialised again in order to verify signature. While this is not a big issue now, we can't afford doing that once compression is introduced. In this PR `ChunkStateWitness` is renamed to `SignedChunkStateWitness` and now it holds raw bytes of `ChunkStateWitnessInner` (now renamed to just `ChunkStateWitness`). * Duplicate validation in orphan witness handling and normal validation process. Currently we perform bunch of validations multiple times: first in `handle_orphan_state_witness` and then in `start_validating_chunk`, it would be nice to avoid that. This is addressed by introducing redundant fields as part of `ChunkStateWitness`: `chunk_producer` account id and `epoch_id`. This way we don't need to have previous block available in order to verify signature and perform a bunch of sanity checks (such as validity of `shard_id`), so duplicated logic is moved out of orphan witness handling to `partially_validate_state_witness_in_epoch`. * Currently we don't send state witness ack in case of orphan state witness because we cannot determine chunk producer account id. This is fixed by using newly introduced `chunk_producer` field of `ChunkStateWitness`. --- chain/chain/src/test_utils/kv_runtime.rs | 12 +- .../chunk_validator/mod.rs | 157 +++++++++++------- .../orphan_witness_handling.rs | 76 +-------- .../chunk_validator/orphan_witness_pool.rs | 14 +- .../stateless_validation/shadow_validate.rs | 7 +- .../state_witness_producer.rs | 47 +++--- .../state_witness_tracker.rs | 2 +- chain/client/src/test_utils/test_env.rs | 13 +- chain/epoch-manager/src/adapter.rs | 35 +--- chain/epoch-manager/src/tests/mod.rs | 27 +-- chain/network/src/client.rs | 4 +- chain/network/src/network_protocol/mod.rs | 4 +- chain/network/src/types.rs | 4 +- core/primitives/src/stateless_validation.rs | 76 +++++++-- core/primitives/src/validator_signer.rs | 14 +- .../features/orphan_chunk_state_witness.rs | 119 +++++++------ .../client/features/stateless_validation.rs | 10 +- 17 files changed, 319 insertions(+), 302 deletions(-) diff --git a/chain/chain/src/test_utils/kv_runtime.rs b/chain/chain/src/test_utils/kv_runtime.rs index 300e7610966..561bce24571 100644 --- a/chain/chain/src/test_utils/kv_runtime.rs +++ b/chain/chain/src/test_utils/kv_runtime.rs @@ -28,7 +28,7 @@ use near_primitives::shard_layout::{ShardLayout, ShardUId}; use near_primitives::sharding::{ChunkHash, ShardChunkHeader}; use near_primitives::state_part::PartId; use near_primitives::stateless_validation::{ - ChunkEndorsement, ChunkStateWitness, ChunkValidatorAssignments, + ChunkEndorsement, ChunkValidatorAssignments, SignedEncodedChunkStateWitness, }; use near_primitives::transaction::{ Action, ExecutionMetadata, ExecutionOutcome, ExecutionOutcomeWithId, ExecutionStatus, @@ -950,14 +950,8 @@ impl EpochManagerAdapter for MockEpochManager { fn verify_chunk_state_witness_signature( &self, - _state_witness: &ChunkStateWitness, - ) -> Result { - Ok(true) - } - - fn verify_chunk_state_witness_signature_in_epoch( - &self, - _state_witness: &ChunkStateWitness, + _signed_witness: &SignedEncodedChunkStateWitness, + _chunk_producer: &AccountId, _epoch_id: &EpochId, ) -> Result { Ok(true) diff --git a/chain/client/src/stateless_validation/chunk_validator/mod.rs b/chain/client/src/stateless_validation/chunk_validator/mod.rs index 607fd09100a..f5516eaeff0 100644 --- a/chain/client/src/stateless_validation/chunk_validator/mod.rs +++ b/chain/client/src/stateless_validation/chunk_validator/mod.rs @@ -27,7 +27,7 @@ use near_primitives::merkle::merklize; use near_primitives::receipt::Receipt; use near_primitives::sharding::{ChunkHash, ReceiptProof, ShardChunkHeader}; use near_primitives::stateless_validation::{ - ChunkEndorsement, ChunkStateWitness, ChunkStateWitnessAck, ChunkStateWitnessInner, + ChunkEndorsement, ChunkStateWitness, ChunkStateWitnessAck, SignedEncodedChunkStateWitness, }; use near_primitives::transaction::SignedTransaction; use near_primitives::types::chunk_extra::ChunkExtra; @@ -93,38 +93,25 @@ impl ChunkValidator { chain: &Chain, processing_done_tracker: Option, ) -> Result<(), Error> { - if !self.epoch_manager.verify_chunk_state_witness_signature(&state_witness)? { - return Err(Error::InvalidChunkStateWitness("Invalid signature".to_string())); - } - - let state_witness_inner = state_witness.inner; - let chunk_header = state_witness_inner.chunk_header.clone(); - let Some(my_signer) = self.my_signer.as_ref() else { - return Err(Error::NotAValidator); - }; - let epoch_id = - self.epoch_manager.get_epoch_id_from_prev_block(chunk_header.prev_block_hash())?; - // We will only validate something if we are a chunk validator for this chunk. - // Note this also covers the case before the protocol upgrade for chunk validators, - // because the chunk validators will be empty. - let chunk_validator_assignments = self.epoch_manager.get_chunk_validator_assignments( - &epoch_id, - chunk_header.shard_id(), - chunk_header.height_created(), - )?; - if !chunk_validator_assignments.contains(my_signer.validator_id()) { - return Err(Error::NotAChunkValidator); + let prev_block_hash = state_witness.chunk_header.prev_block_hash(); + let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(prev_block_hash)?; + if epoch_id != state_witness.epoch_id { + return Err(Error::InvalidChunkStateWitness(format!( + "Invalid EpochId {:?} for previous block {}, expected {:?}", + state_witness.epoch_id, prev_block_hash, epoch_id + ))); } let pre_validation_result = pre_validate_chunk_state_witness( - &state_witness_inner, + &state_witness, chain, self.epoch_manager.as_ref(), self.runtime_adapter.as_ref(), )?; + let chunk_header = state_witness.chunk_header.clone(); let network_sender = self.network_sender.clone(); - let signer = my_signer.clone(); + let signer = self.my_signer.as_ref().ok_or(Error::NotAValidator)?.clone(); let epoch_manager = self.epoch_manager.clone(); let runtime_adapter = self.runtime_adapter.clone(); let chunk_endorsement_tracker = self.chunk_endorsement_tracker.clone(); @@ -133,7 +120,7 @@ impl ChunkValidator { let _processing_done_tracker_capture = processing_done_tracker; match validate_chunk_state_witness( - state_witness_inner, + state_witness, pre_validation_result, epoch_manager.as_ref(), runtime_adapter.as_ref(), @@ -182,7 +169,7 @@ pub(crate) fn validate_prepared_transactions( /// We do this before handing off the computationally intensive part to a /// validation thread. pub(crate) fn pre_validate_chunk_state_witness( - state_witness: &ChunkStateWitnessInner, + state_witness: &ChunkStateWitness, chain: &Chain, epoch_manager: &dyn EpochManagerAdapter, runtime_adapter: &dyn RuntimeAdapter, @@ -467,7 +454,7 @@ pub(crate) struct PreValidationOutput { } pub(crate) fn validate_chunk_state_witness( - state_witness: ChunkStateWitnessInner, + state_witness: ChunkStateWitness, pre_validation_output: PreValidationOutput, epoch_manager: &dyn EpochManagerAdapter, runtime_adapter: &dyn RuntimeAdapter, @@ -634,54 +621,41 @@ impl Client { /// you can use the `processing_done_tracker` argument (but it's optional, it's safe to pass None there). pub fn process_chunk_state_witness( &mut self, - witness: ChunkStateWitness, + signed_witness: SignedEncodedChunkStateWitness, processing_done_tracker: Option, ) -> Result<(), Error> { + let witness = self.partially_validate_state_witness(&signed_witness)?; + // Send the acknowledgement for the state witness back to the chunk producer. // This is currently used for network roundtrip time measurement, so we do not need to // wait for validation to finish. - if let Err(err) = self.send_state_witness_ack(&witness) { - tracing::warn!(target: "stateless_validation", error = &err as &dyn std::error::Error, - "Error sending chunk state witness acknowledgement"); - } + self.send_state_witness_ack(&witness); - let prev_block_hash = witness.inner.chunk_header.prev_block_hash(); - let prev_block = match self.chain.get_block(prev_block_hash) { - Ok(block) => block, + match self.chain.get_block(witness.chunk_header.prev_block_hash()) { + Ok(block) => self.process_chunk_state_witness_with_prev_block( + witness, + &block, + processing_done_tracker, + ), Err(Error::DBNotFoundErr(_)) => { // Previous block isn't available at the moment, add this witness to the orphan pool. - self.handle_orphan_state_witness(witness)?; - return Ok(()); + self.handle_orphan_state_witness( + witness, + signed_witness.witness_bytes.size_bytes(), + )?; + Ok(()) } - Err(err) => return Err(err), - }; - - self.process_chunk_state_witness_with_prev_block( - witness, - &prev_block, - processing_done_tracker, - ) + Err(err) => Err(err), + } } - fn send_state_witness_ack(&self, witness: &ChunkStateWitness) -> Result<(), Error> { - // First find the AccountId for the chunk producer and then send the ack to that account. - let chunk_header = &witness.inner.chunk_header; - let prev_block_hash = chunk_header.prev_block_hash(); - let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(prev_block_hash)?; - let chunk_producer = self.epoch_manager.get_chunk_producer( - &epoch_id, - chunk_header.height_created(), - chunk_header.shard_id(), - )?; - + fn send_state_witness_ack(&self, witness: &ChunkStateWitness) { self.network_adapter.send(PeerManagerMessageRequest::NetworkRequests( NetworkRequests::ChunkStateWitnessAck( - chunk_producer, - ChunkStateWitnessAck::new(&witness), + witness.chunk_producer.clone(), + ChunkStateWitnessAck::new(witness), ), )); - - Ok(()) } pub fn process_chunk_state_witness_with_prev_block( @@ -690,14 +664,73 @@ impl Client { prev_block: &Block, processing_done_tracker: Option, ) -> Result<(), Error> { - if witness.inner.chunk_header.prev_block_hash() != prev_block.hash() { + if witness.chunk_header.prev_block_hash() != prev_block.hash() { return Err(Error::Other(format!( "process_chunk_state_witness_with_prev_block - prev_block doesn't match ({} != {})", - witness.inner.chunk_header.prev_block_hash(), + witness.chunk_header.prev_block_hash(), prev_block.hash() ))); } self.chunk_validator.start_validating_chunk(witness, &self.chain, processing_done_tracker) } + + /// Performs state witness decoding and partial validation without requiring the previous block. + /// Here we rely on epoch_id provided as part of the state witness. Later we verify that this + /// epoch_id actually corresponds to the chunk's previous block. + fn partially_validate_state_witness( + &self, + signed_witness: &SignedEncodedChunkStateWitness, + ) -> Result { + let witness = signed_witness.witness_bytes.decode()?; + let chunk_header = &witness.chunk_header; + let witness_height = chunk_header.height_created(); + let witness_shard = chunk_header.shard_id(); + + if !self + .epoch_manager + .get_shard_layout(&witness.epoch_id)? + .shard_ids() + .contains(&witness_shard) + { + return Err(Error::InvalidChunkStateWitness(format!( + "Invalid shard_id in ChunkStateWitness: {}", + witness_shard + ))); + } + + let chunk_producer = self.epoch_manager.get_chunk_producer( + &witness.epoch_id, + witness_height, + witness_shard, + )?; + if witness.chunk_producer != chunk_producer { + return Err(Error::InvalidChunkStateWitness(format!( + "Incorrect chunk producer for epoch {:?} at height {}: expected {}, got {}", + witness.epoch_id, witness_height, chunk_producer, witness.chunk_producer, + ))); + } + + // Reject witnesses for chunks for which this node isn't a validator. + // It's an error, as chunk producer shouldn't send the witness to a non-validator node. + let my_signer = self.chunk_validator.my_signer.as_ref().ok_or(Error::NotAValidator)?; + let chunk_validator_assignments = self.epoch_manager.get_chunk_validator_assignments( + &witness.epoch_id, + witness_shard, + witness_height, + )?; + if !chunk_validator_assignments.contains(my_signer.validator_id()) { + return Err(Error::NotAChunkValidator); + } + + if !self.epoch_manager.verify_chunk_state_witness_signature( + &signed_witness, + &witness.chunk_producer, + &witness.epoch_id, + )? { + return Err(Error::InvalidChunkStateWitness("Invalid signature".to_string())); + } + + Ok(witness) + } } diff --git a/chain/client/src/stateless_validation/chunk_validator/orphan_witness_handling.rs b/chain/client/src/stateless_validation/chunk_validator/orphan_witness_handling.rs index 5e711b35fd3..aaa696b59c9 100644 --- a/chain/client/src/stateless_validation/chunk_validator/orphan_witness_handling.rs +++ b/chain/client/src/stateless_validation/chunk_validator/orphan_witness_handling.rs @@ -6,7 +6,6 @@ //! arrives, all witnesses that were waiting for it can be processed. use crate::Client; -use itertools::Itertools; use near_chain::Block; use near_chain_primitives::Error; use near_primitives::stateless_validation::ChunkStateWitness; @@ -24,8 +23,9 @@ impl Client { pub fn handle_orphan_state_witness( &mut self, witness: ChunkStateWitness, + witness_size: usize, ) -> Result { - let chunk_header = &witness.inner.chunk_header; + let chunk_header = &witness.chunk_header; let witness_height = chunk_header.height_created(); let witness_shard = chunk_header.shard_id(); @@ -53,7 +53,6 @@ impl Client { } // Don't save orphaned state witnesses which are bigger than the allowed limit. - let witness_size = borsh::to_vec(&witness)?.len(); let witness_size_u64: u64 = witness_size.try_into().map_err(|_| { Error::Other(format!("Cannot convert witness size to u64: {}", witness_size)) })?; @@ -77,31 +76,8 @@ impl Client { let possible_epochs = self.epoch_manager.possible_epochs_of_height_around_tip(&chain_head, witness_height)?; - // Try to validate the witness assuming that it resides in one of the possible epochs. - // The witness must pass validation in one of these epochs before it can be admitted to the pool. - let mut epoch_validation_result: Option> = None; - for epoch_id in possible_epochs { - match self.partially_validate_orphan_witness_in_epoch(&witness, &epoch_id) { - Ok(()) => { - epoch_validation_result = Some(Ok(())); - break; - } - Err(err) => epoch_validation_result = Some(Err(err)), - } - } - match epoch_validation_result { - Some(Ok(())) => {} // Validation passed in one of the possible epochs, witness can be added to the pool. - Some(Err(err)) => { - // Validation failed in all possible epochs, reject the witness - return Err(err); - } - None => { - // possible_epochs was empty. This shouldn't happen as all epochs around the chain head are known. - return Err(Error::Other(format!( - "Couldn't find any matching EpochId for orphan chunk state witness with height {}", - witness_height - ))); - } + if !possible_epochs.contains(&witness.epoch_id) { + return Ok(HandleOrphanWitnessOutcome::UnsupportedEpochId(witness.epoch_id)); } // Orphan witness is OK, save it to the pool @@ -110,45 +86,6 @@ impl Client { Ok(HandleOrphanWitnessOutcome::SavedToPool) } - fn partially_validate_orphan_witness_in_epoch( - &self, - witness: &ChunkStateWitness, - epoch_id: &EpochId, - ) -> Result<(), Error> { - let chunk_header = &witness.inner.chunk_header; - let witness_height = chunk_header.height_created(); - let witness_shard = chunk_header.shard_id(); - - // Validate shard_id - if !self.epoch_manager.get_shard_layout(&epoch_id)?.shard_ids().contains(&witness_shard) { - return Err(Error::InvalidChunkStateWitness(format!( - "Invalid shard_id in ChunkStateWitness: {}", - witness_shard - ))); - } - - // Reject witnesses for chunks for which which this node isn't a validator. - // It's an error, as the sender shouldn't send the witness to a non-validator node. - let Some(my_signer) = self.chunk_validator.my_signer.as_ref() else { - return Err(Error::NotAValidator); - }; - let chunk_validator_assignments = self.epoch_manager.get_chunk_validator_assignments( - &epoch_id, - witness_shard, - witness_height, - )?; - if !chunk_validator_assignments.contains(my_signer.validator_id()) { - return Err(Error::NotAChunkValidator); - } - - // Verify signature - if !self.epoch_manager.verify_chunk_state_witness_signature_in_epoch(&witness, &epoch_id)? { - return Err(Error::InvalidChunkStateWitness("Invalid signature".to_string())); - } - - Ok(()) - } - /// Once a new block arrives, we can process the orphaned chunk state witnesses that were waiting /// for this block. This function takes the ready witnesses out of the orhan pool and process them. /// It also removes old witnesses (below final height) from the orphan pool to save memory. @@ -158,7 +95,7 @@ impl Client { .orphan_witness_pool .take_state_witnesses_waiting_for_block(new_block.hash()); for witness in ready_witnesses { - let header = &witness.inner.chunk_header; + let header = &witness.chunk_header; tracing::debug!( target: "client", witness_height = header.height_created(), @@ -201,9 +138,10 @@ impl Client { /// of other reasons. In such cases the handler function returns Ok(outcome) to let the caller /// know what happened with the witness. /// It's useful in tests. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum HandleOrphanWitnessOutcome { SavedToPool, TooBig(usize), TooFarFromHead { head_height: BlockHeight, witness_height: BlockHeight }, + UnsupportedEpochId(EpochId), } diff --git a/chain/client/src/stateless_validation/chunk_validator/orphan_witness_pool.rs b/chain/client/src/stateless_validation/chunk_validator/orphan_witness_pool.rs index dbcd34073ac..cbd9b48356d 100644 --- a/chain/client/src/stateless_validation/chunk_validator/orphan_witness_pool.rs +++ b/chain/client/src/stateless_validation/chunk_validator/orphan_witness_pool.rs @@ -42,13 +42,13 @@ impl OrphanStateWitnessPool { /// `witness_size` is only used for metrics, it's okay to pass 0 if you don't care about the metrics. pub fn add_orphan_state_witness(&mut self, witness: ChunkStateWitness, witness_size: usize) { // Insert the new ChunkStateWitness into the cache - let chunk_header = &witness.inner.chunk_header; + let chunk_header = &witness.chunk_header; let cache_key = (chunk_header.shard_id(), chunk_header.height_created()); let metrics_tracker = OrphanWitnessMetricsTracker::new(&witness, witness_size); let cache_entry = CacheEntry { witness, _metrics_tracker: metrics_tracker }; if let Some((_, ejected_entry)) = self.witness_cache.push(cache_key, cache_entry) { // Another witness has been ejected from the cache due to capacity limit - let header = &ejected_entry.witness.inner.chunk_header; + let header = &ejected_entry.witness.chunk_header; tracing::debug!( target: "client", ejected_witness_height = header.height_created(), @@ -68,7 +68,7 @@ impl OrphanStateWitnessPool { ) -> Vec { let mut to_remove: Vec<(ShardId, BlockHeight)> = Vec::new(); for (cache_key, cache_entry) in self.witness_cache.iter() { - if cache_entry.witness.inner.chunk_header.prev_block_hash() == prev_block { + if cache_entry.witness.chunk_header.prev_block_hash() == prev_block { to_remove.push(*cache_key); } } @@ -91,7 +91,7 @@ impl OrphanStateWitnessPool { for ((witness_shard, witness_height), cache_entry) in self.witness_cache.iter() { if *witness_height < final_height { to_remove.push((*witness_shard, *witness_height)); - let header = &cache_entry.witness.inner.chunk_header; + let header = &cache_entry.witness.chunk_header; tracing::debug!( target: "client", final_height, @@ -136,7 +136,7 @@ mod metrics_tracker { witness: &ChunkStateWitness, witness_size: usize, ) -> OrphanWitnessMetricsTracker { - let shard_id = witness.inner.chunk_header.shard_id().to_string(); + let shard_id = witness.chunk_header.shard_id().to_string(); metrics::ORPHAN_CHUNK_STATE_WITNESSES_TOTAL_COUNT .with_label_values(&[shard_id.as_str()]) .inc(); @@ -188,7 +188,7 @@ mod tests { encoded_length: u64, ) -> ChunkStateWitness { let mut witness = ChunkStateWitness::new_dummy(height, shard_id, prev_block_hash); - match &mut witness.inner.chunk_header { + match &mut witness.chunk_header { ShardChunkHeader::V3(header) => match &mut header.inner { ShardChunkHeaderInner::V2(inner) => inner.encoded_length = encoded_length, _ => unimplemented!(), @@ -214,7 +214,7 @@ mod tests { expected.sort_by(sort_comparator); if observed != expected { let print_witness_info = |witness: &ChunkStateWitness| { - let header = &witness.inner.chunk_header; + let header = &witness.chunk_header; eprintln!( "- height = {}, shard_id = {}, encoded_length: {} prev_block: {}", header.height_created(), diff --git a/chain/client/src/stateless_validation/shadow_validate.rs b/chain/client/src/stateless_validation/shadow_validate.rs index 8a923c7cc69..8b8bc31fffc 100644 --- a/chain/client/src/stateless_validation/shadow_validate.rs +++ b/chain/client/src/stateless_validation/shadow_validate.rs @@ -4,6 +4,7 @@ use near_chain::types::{RuntimeStorageConfig, StorageDataSource}; use near_chain::{Block, BlockHeader}; use near_chain_primitives::Error; use near_primitives::sharding::{ShardChunk, ShardChunkHeader}; +use near_primitives::stateless_validation::EncodedChunkStateWitness; use crate::stateless_validation::chunk_validator::{ pre_validate_chunk_state_witness, validate_chunk_state_witness, validate_prepared_transactions, @@ -73,13 +74,15 @@ impl Client { )); }; - let witness = self.create_state_witness_inner( + let witness = self.create_state_witness( + // Setting arbitrary chunk producer is OK for shadow validation + "alice.near".parse().unwrap(), prev_block_header, prev_chunk_header, chunk, validated_transactions.storage_proof, )?; - let witness_size = borsh::to_vec(&witness)?.len(); + let witness_size = EncodedChunkStateWitness::encode(&witness).size_bytes(); metrics::CHUNK_STATE_WITNESS_TOTAL_SIZE .with_label_values(&[&shard_id.to_string()]) .observe(witness_size as f64); diff --git a/chain/client/src/stateless_validation/state_witness_producer.rs b/chain/client/src/stateless_validation/state_witness_producer.rs index 87cc3785c5d..7d9cc171f6f 100644 --- a/chain/client/src/stateless_validation/state_witness_producer.rs +++ b/chain/client/src/stateless_validation/state_witness_producer.rs @@ -10,10 +10,10 @@ use near_primitives::hash::{hash, CryptoHash}; use near_primitives::receipt::Receipt; use near_primitives::sharding::{ChunkHash, ReceiptProof, ShardChunk, ShardChunkHeader}; use near_primitives::stateless_validation::{ - ChunkStateTransition, ChunkStateWitness, ChunkStateWitnessAck, ChunkStateWitnessInner, + ChunkStateTransition, ChunkStateWitness, ChunkStateWitnessAck, SignedEncodedChunkStateWitness, StoredChunkStateTransitionData, }; -use near_primitives::types::EpochId; +use near_primitives::types::{AccountId, EpochId}; use std::collections::HashMap; use crate::stateless_validation::chunk_validator::send_chunk_endorsement_to_block_producers; @@ -46,19 +46,17 @@ impl Client { .ordered_chunk_validators(); let my_signer = self.validator_signer.as_ref().ok_or(Error::NotAValidator)?.clone(); - let (witness, witness_size) = { - let witness_inner = self.create_state_witness_inner( - prev_block_header, - prev_chunk_header, - chunk, - transactions_storage_proof, - )?; - let (signature, witness_size) = my_signer.sign_chunk_state_witness(&witness_inner); - metrics::CHUNK_STATE_WITNESS_TOTAL_SIZE - .with_label_values(&[&chunk_header.shard_id().to_string()]) - .observe(witness_size as f64); - (ChunkStateWitness { inner: witness_inner, signature }, witness_size) - }; + let witness = self.create_state_witness( + my_signer.validator_id().clone(), + prev_block_header, + prev_chunk_header, + chunk, + transactions_storage_proof, + )?; + let signed_witness = SignedEncodedChunkStateWitness::new(&witness, my_signer.as_ref()); + metrics::CHUNK_STATE_WITNESS_TOTAL_SIZE + .with_label_values(&[&chunk_header.shard_id().to_string()]) + .observe(signed_witness.witness_bytes.size_bytes() as f64); if chunk_validators.contains(my_signer.validator_id()) { // Bypass state witness validation if we created state witness. Endorse the chunk immediately. @@ -84,12 +82,12 @@ impl Client { // See process_chunk_state_witness_ack for the handling of the ack messages. self.state_witness_tracker.record_witness_sent( &witness, - witness_size, + signed_witness.witness_bytes.size_bytes(), chunk_validators.len(), ); self.network_adapter.send(PeerManagerMessageRequest::NetworkRequests( - NetworkRequests::ChunkStateWitness(chunk_validators, witness), + NetworkRequests::ChunkStateWitness(chunk_validators, signed_witness), )); Ok(()) } @@ -103,14 +101,17 @@ impl Client { self.state_witness_tracker.on_witness_ack_received(witness_ack); } - pub(crate) fn create_state_witness_inner( + pub(crate) fn create_state_witness( &mut self, + chunk_producer: AccountId, prev_block_header: &BlockHeader, prev_chunk_header: &ShardChunkHeader, chunk: &ShardChunk, transactions_storage_proof: Option, - ) -> Result { + ) -> Result { let chunk_header = chunk.cloned_header(); + let epoch_id = + self.epoch_manager.get_epoch_id_from_prev_block(chunk_header.prev_block_hash())?; let prev_chunk = self.chain.get_chunk(&prev_chunk_header.chunk_hash())?; let (main_state_transition, implicit_transitions, applied_receipts_hash) = self.collect_state_transition_data(&chunk_header, prev_chunk_header)?; @@ -131,8 +132,10 @@ impl Client { let source_receipt_proofs = self.collect_source_receipt_proofs(prev_block_header, prev_chunk_header)?; - let witness_inner = ChunkStateWitnessInner::new( - chunk_header.clone(), + let witness = ChunkStateWitness::new( + chunk_producer, + epoch_id, + chunk_header, main_state_transition, source_receipt_proofs, // (Could also be derived from iterating through the receipts, but @@ -144,7 +147,7 @@ impl Client { new_transactions, new_transactions_validation_state, ); - Ok(witness_inner) + Ok(witness) } /// Collect state transition data necessary to produce state witness for diff --git a/chain/client/src/stateless_validation/state_witness_tracker.rs b/chain/client/src/stateless_validation/state_witness_tracker.rs index 5be1af88505..3c4a45cbdb9 100644 --- a/chain/client/src/stateless_validation/state_witness_tracker.rs +++ b/chain/client/src/stateless_validation/state_witness_tracker.rs @@ -24,7 +24,7 @@ struct ChunkStateWitnessKey { impl ChunkStateWitnessKey { pub fn new(witness: &ChunkStateWitness) -> Self { - Self { chunk_hash: witness.inner.chunk_header.chunk_hash() } + Self { chunk_hash: witness.chunk_header.chunk_hash() } } } diff --git a/chain/client/src/test_utils/test_env.rs b/chain/client/src/test_utils/test_env.rs index 68a5d79a659..e85cc1e3cb3 100644 --- a/chain/client/src/test_utils/test_env.rs +++ b/chain/client/src/test_utils/test_env.rs @@ -27,7 +27,7 @@ use near_primitives::epoch_manager::RngSeed; use near_primitives::errors::InvalidTxError; use near_primitives::hash::CryptoHash; use near_primitives::sharding::{ChunkHash, PartialEncodedChunk}; -use near_primitives::stateless_validation::{ChunkEndorsement, ChunkStateWitness}; +use near_primitives::stateless_validation::{ChunkEndorsement, SignedEncodedChunkStateWitness}; use near_primitives::test_utils::create_test_signer; use near_primitives::transaction::{Action, FunctionCallAction, SignedTransaction}; use near_primitives::types::{AccountId, Balance, BlockHeight, EpochId, NumSeats, ShardId}; @@ -293,14 +293,11 @@ impl TestEnv { } fn found_differing_post_state_root_due_to_state_transitions( - chunk_state_witness: &ChunkStateWitness, + signed_witness: &SignedEncodedChunkStateWitness, ) -> bool { - let chunk_state_witness_inner = &chunk_state_witness.inner; - let mut post_state_roots = - HashSet::from([chunk_state_witness_inner.main_state_transition.post_state_root]); - post_state_roots.extend( - chunk_state_witness_inner.implicit_transitions.iter().map(|t| t.post_state_root), - ); + let witness = signed_witness.witness_bytes.decode().unwrap(); + let mut post_state_roots = HashSet::from([witness.main_state_transition.post_state_root]); + post_state_roots.extend(witness.implicit_transitions.iter().map(|t| t.post_state_root)); post_state_roots.len() >= 2 } diff --git a/chain/epoch-manager/src/adapter.rs b/chain/epoch-manager/src/adapter.rs index 40dfd7d7e93..c7168874f24 100644 --- a/chain/epoch-manager/src/adapter.rs +++ b/chain/epoch-manager/src/adapter.rs @@ -15,7 +15,7 @@ use near_primitives::hash::CryptoHash; use near_primitives::shard_layout::{account_id_to_shard_id, ShardLayout, ShardLayoutError}; use near_primitives::sharding::{ChunkHash, ShardChunkHeader}; use near_primitives::stateless_validation::{ - ChunkEndorsement, ChunkStateWitness, ChunkValidatorAssignments, + ChunkEndorsement, ChunkValidatorAssignments, SignedEncodedChunkStateWitness, }; use near_primitives::types::validator_stake::ValidatorStake; use near_primitives::types::{ @@ -413,12 +413,8 @@ pub trait EpochManagerAdapter: Send + Sync { fn verify_chunk_state_witness_signature( &self, - state_witness: &ChunkStateWitness, - ) -> Result; - - fn verify_chunk_state_witness_signature_in_epoch( - &self, - state_witness: &ChunkStateWitness, + signed_witness: &SignedEncodedChunkStateWitness, + chunk_producer: &AccountId, epoch_id: &EpochId, ) -> Result; @@ -1063,30 +1059,15 @@ impl EpochManagerAdapter for EpochManagerHandle { fn verify_chunk_state_witness_signature( &self, - state_witness: &ChunkStateWitness, - ) -> Result { - let epoch_manager = self.read(); - let chunk_header = &state_witness.inner.chunk_header; - let epoch_id = - epoch_manager.get_epoch_id_from_prev_block(chunk_header.prev_block_hash())?; - self.verify_chunk_state_witness_signature_in_epoch(state_witness, &epoch_id) - } - - fn verify_chunk_state_witness_signature_in_epoch( - &self, - state_witness: &ChunkStateWitness, + signed_witness: &SignedEncodedChunkStateWitness, + chunk_producer: &AccountId, epoch_id: &EpochId, ) -> Result { let epoch_manager = self.read(); - let chunk_header = &state_witness.inner.chunk_header; - let chunk_producer = epoch_manager.get_chunk_producer_info( - &epoch_id, - chunk_header.height_created(), - chunk_header.shard_id(), - )?; - Ok(state_witness + let validator = epoch_manager.get_validator_by_account_id(epoch_id, chunk_producer)?; + Ok(signed_witness .signature - .verify(&borsh::to_vec(&state_witness.inner)?, chunk_producer.public_key())) + .verify(signed_witness.witness_bytes.as_slice(), validator.public_key())) } fn cares_about_shard_from_prev_block( diff --git a/chain/epoch-manager/src/tests/mod.rs b/chain/epoch-manager/src/tests/mod.rs index c090c6af9d7..50c22371d44 100644 --- a/chain/epoch-manager/src/tests/mod.rs +++ b/chain/epoch-manager/src/tests/mod.rs @@ -18,7 +18,7 @@ use near_primitives::hash::hash; use near_primitives::shard_layout::ShardLayout; use near_primitives::sharding::{ShardChunkHeader, ShardChunkHeaderV3}; use near_primitives::stateless_validation::{ - ChunkStateTransition, ChunkStateWitness, ChunkStateWitnessInner, + ChunkStateTransition, ChunkStateWitness, SignedEncodedChunkStateWitness, }; use near_primitives::types::ValidatorKickoutReason::{NotEnoughBlocks, NotEnoughChunks}; use near_primitives::validator_signer::ValidatorSigner; @@ -2904,12 +2904,15 @@ fn test_verify_chunk_state_witness() { // 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 signer = Arc::new(create_test_signer("test1")); + 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()); // Build a chunk state witness with arbitrary data. let chunk_header = test_chunk_header(&h, signer.as_ref()); - let witness_inner = ChunkStateWitnessInner::new( + let witness = ChunkStateWitness::new( + chunk_producer.clone(), + epoch_id.clone(), chunk_header, ChunkStateTransition { block_hash: h[0], @@ -2923,21 +2926,25 @@ fn test_verify_chunk_state_witness() { vec![], Default::default(), ); - let signature = signer.sign_chunk_state_witness(&witness_inner).0; - // Check chunk state witness validity. - let mut chunk_state_witness = ChunkStateWitness { inner: witness_inner, signature }; - assert!(epoch_manager.verify_chunk_state_witness_signature(&chunk_state_witness).unwrap()); + let mut chunk_state_witness = SignedEncodedChunkStateWitness::new(&witness, signer.as_ref()); + assert!(epoch_manager + .verify_chunk_state_witness_signature(&chunk_state_witness, &chunk_producer, &epoch_id) + .unwrap()); // Check invalid chunk state witness signature. chunk_state_witness.signature = Signature::default(); - assert!(!epoch_manager.verify_chunk_state_witness_signature(&chunk_state_witness).unwrap()); + assert!(!epoch_manager + .verify_chunk_state_witness_signature(&chunk_state_witness, &chunk_producer, &epoch_id) + .unwrap()); // Check chunk state witness invalidity when signer is not a chunk validator. let bad_signer = Arc::new(create_test_signer("test2")); chunk_state_witness.signature = - bad_signer.sign_chunk_state_witness(&chunk_state_witness.inner).0; - assert!(!epoch_manager.verify_chunk_state_witness_signature(&chunk_state_witness).unwrap()); + bad_signer.sign_chunk_state_witness(&chunk_state_witness.witness_bytes); + assert!(!epoch_manager + .verify_chunk_state_witness_signature(&chunk_state_witness, &chunk_producer, &epoch_id) + .unwrap()); } /// Simulate the blockchain over a few epochs and verify that possible_epochs_of_height_around_tip() diff --git a/chain/network/src/client.rs b/chain/network/src/client.rs index 064a31b89d8..77b14871345 100644 --- a/chain/network/src/client.rs +++ b/chain/network/src/client.rs @@ -7,7 +7,7 @@ use near_primitives::errors::InvalidTxError; use near_primitives::hash::CryptoHash; use near_primitives::network::{AnnounceAccount, PeerId}; use near_primitives::stateless_validation::{ - ChunkEndorsement, ChunkStateWitness, ChunkStateWitnessAck, + ChunkEndorsement, ChunkStateWitnessAck, SignedEncodedChunkStateWitness, }; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{AccountId, EpochId, ShardId}; @@ -116,7 +116,7 @@ pub struct AnnounceAccountRequest(pub Vec<(AnnounceAccount, Option)>); #[derive(actix::Message, Debug, Clone, PartialEq, Eq)] #[rtype(result = "()")] -pub struct ChunkStateWitnessMessage(pub ChunkStateWitness); +pub struct ChunkStateWitnessMessage(pub SignedEncodedChunkStateWitness); #[derive(actix::Message, Debug, Clone, PartialEq, Eq)] #[rtype(result = "()")] diff --git a/chain/network/src/network_protocol/mod.rs b/chain/network/src/network_protocol/mod.rs index 26c88c7951d..df2a967ec01 100644 --- a/chain/network/src/network_protocol/mod.rs +++ b/chain/network/src/network_protocol/mod.rs @@ -8,8 +8,8 @@ mod proto_conv; mod state_sync; pub use edge::*; use near_primitives::stateless_validation::ChunkEndorsement; -use near_primitives::stateless_validation::ChunkStateWitness; use near_primitives::stateless_validation::ChunkStateWitnessAck; +use near_primitives::stateless_validation::SignedEncodedChunkStateWitness; pub use peer::*; pub use state_sync::*; @@ -530,7 +530,7 @@ pub enum RoutedMessageBody { VersionedPartialEncodedChunk(PartialEncodedChunk), _UnusedVersionedStateResponse, PartialEncodedChunkForward(PartialEncodedChunkForwardMsg), - ChunkStateWitness(ChunkStateWitness), + ChunkStateWitness(SignedEncodedChunkStateWitness), ChunkEndorsement(ChunkEndorsement), ChunkStateWitnessAck(ChunkStateWitnessAck), } diff --git a/chain/network/src/types.rs b/chain/network/src/types.rs index 6341cca3918..db69861001f 100644 --- a/chain/network/src/types.rs +++ b/chain/network/src/types.rs @@ -20,7 +20,7 @@ use near_primitives::hash::CryptoHash; use near_primitives::network::{AnnounceAccount, PeerId}; use near_primitives::sharding::PartialEncodedChunkWithArcReceipts; use near_primitives::stateless_validation::{ - ChunkEndorsement, ChunkStateWitness, ChunkStateWitnessAck, + ChunkEndorsement, ChunkStateWitnessAck, SignedEncodedChunkStateWitness, }; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{AccountId, BlockHeight, EpochHeight, ShardId}; @@ -259,7 +259,7 @@ pub enum NetworkRequests { /// A challenge to invalidate a block. Challenge(Challenge), /// A chunk's state witness. - ChunkStateWitness(Vec, ChunkStateWitness), + ChunkStateWitness(Vec, SignedEncodedChunkStateWitness), /// Acknowledgement to a chunk's state witness, sent back to the originating chunk producer. ChunkStateWitnessAck(AccountId, ChunkStateWitnessAck), /// Message for a chunk endorsement, sent by a chunk validator to the block producer. diff --git a/core/primitives/src/stateless_validation.rs b/core/primitives/src/stateless_validation.rs index 3dfc45cd944..912dbb71187 100644 --- a/core/primitives/src/stateless_validation.rs +++ b/core/primitives/src/stateless_validation.rs @@ -3,6 +3,7 @@ use std::collections::{HashMap, HashSet}; use crate::challenge::PartialState; use crate::sharding::{ChunkHash, ReceiptProof, ShardChunkHeader, ShardChunkHeaderV3}; use crate::transaction::SignedTransaction; +use crate::types::EpochId; use crate::validator_signer::{EmptyValidatorSigner, ValidatorSigner}; use borsh::{BorshDeserialize, BorshSerialize}; use near_crypto::{PublicKey, Signature}; @@ -17,13 +18,47 @@ use near_primitives_core::types::{AccountId, Balance, BlockHeight, ShardId}; /// This is a messy workaround until we know what to do with NEP 483. type SignatureDifferentiator = String; -/// Signable +/// Represents bytes of encoded ChunkStateWitness. +/// For now encoding is raw borsh serialization, later we plan +/// adding compression on top of that. #[derive(Debug, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)] -pub struct ChunkStateWitness { - pub inner: ChunkStateWitnessInner, +pub struct EncodedChunkStateWitness(Box<[u8]>); + +impl EncodedChunkStateWitness { + pub fn encode(witness: &ChunkStateWitness) -> Self { + Self(borsh::to_vec(witness).expect("borsh serialization should not fail").into()) + } + + pub fn decode(&self) -> Result { + ChunkStateWitness::try_from_slice(&self.0) + } + + pub fn size_bytes(&self) -> usize { + self.0.len() + } + + pub fn as_slice(&self) -> &[u8] { + &self.0 + } +} + +#[derive(Debug, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)] +pub struct SignedEncodedChunkStateWitness { + /// The content of the witness. It is convenient have it as bytes in order + /// to perform signature verification along with decoding. + pub witness_bytes: EncodedChunkStateWitness, + /// Signature corresponds to `witness_bytes.as_slice()` signed by the chunk producer pub signature: Signature, } +impl SignedEncodedChunkStateWitness { + pub fn new(witness: &ChunkStateWitness, signer: &dyn ValidatorSigner) -> Self { + let witness_bytes = EncodedChunkStateWitness::encode(witness); + let signature = signer.sign_chunk_state_witness(&witness_bytes); + Self { witness_bytes, signature } + } +} + /// An acknowledgement sent from the chunk producer upon receiving the state witness to /// the originator of the witness (chunk producer). /// @@ -39,15 +74,22 @@ pub struct ChunkStateWitnessAck { } impl ChunkStateWitnessAck { - pub fn new(witness_to_ack: &ChunkStateWitness) -> Self { - Self { chunk_hash: witness_to_ack.inner.chunk_header.chunk_hash() } + pub fn new(witness: &ChunkStateWitness) -> Self { + Self { chunk_hash: witness.chunk_header.chunk_hash() } } } /// The state witness for a chunk; proves the state transition that the /// chunk attests to. #[derive(Debug, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)] -pub struct ChunkStateWitnessInner { +pub struct ChunkStateWitness { + pub chunk_producer: AccountId, + /// EpochId corresponds to the next block after chunk's previous block. + /// This is effectively the output of EpochManager::get_epoch_id_from_prev_block + /// with chunk_header.prev_block_hash(). + /// This is needed to validate signature when the previous block is not yet + /// available on the validator side (aka orphan state witness). + pub epoch_id: EpochId, /// The chunk header that this witness is for. While this is not needed /// to apply the state transition, it is needed for a chunk validator to /// produce a chunk endorsement while knowing what they are endorsing. @@ -113,8 +155,10 @@ pub struct ChunkStateWitnessInner { signature_differentiator: SignatureDifferentiator, } -impl ChunkStateWitnessInner { +impl ChunkStateWitness { pub fn new( + chunk_producer: AccountId, + epoch_id: EpochId, chunk_header: ShardChunkHeader, main_state_transition: ChunkStateTransition, source_receipt_proofs: HashMap, @@ -125,6 +169,8 @@ impl ChunkStateWitnessInner { new_transactions_validation_state: PartialState, ) -> Self { Self { + chunk_producer, + epoch_id, chunk_header, main_state_transition, source_receipt_proofs, @@ -136,15 +182,8 @@ impl ChunkStateWitnessInner { signature_differentiator: "ChunkStateWitness".to_owned(), } } -} -impl ChunkStateWitness { - // Make a new dummy ChunkStateWitness for testing. - pub fn new_dummy( - height: BlockHeight, - shard_id: ShardId, - prev_block_hash: CryptoHash, - ) -> ChunkStateWitness { + pub fn new_dummy(height: BlockHeight, shard_id: ShardId, prev_block_hash: CryptoHash) -> Self { let header = ShardChunkHeader::V3(ShardChunkHeaderV3::new( prev_block_hash, Default::default(), @@ -161,7 +200,9 @@ impl ChunkStateWitness { Default::default(), &EmptyValidatorSigner::default(), )); - let inner = ChunkStateWitnessInner::new( + Self::new( + "alice.near".parse().unwrap(), + EpochId::default(), header, Default::default(), Default::default(), @@ -170,8 +211,7 @@ impl ChunkStateWitness { Default::default(), Default::default(), Default::default(), - ); - ChunkStateWitness { inner, signature: Signature::default() } + ) } } diff --git a/core/primitives/src/validator_signer.rs b/core/primitives/src/validator_signer.rs index 2f3a5647df1..d876bbe84cd 100644 --- a/core/primitives/src/validator_signer.rs +++ b/core/primitives/src/validator_signer.rs @@ -8,7 +8,7 @@ use crate::challenge::ChallengeBody; use crate::hash::CryptoHash; use crate::network::{AnnounceAccount, PeerId}; use crate::sharding::ChunkHash; -use crate::stateless_validation::{ChunkEndorsementInner, ChunkStateWitnessInner}; +use crate::stateless_validation::{ChunkEndorsementInner, EncodedChunkStateWitness}; use crate::telemetry::TelemetryInfo; use crate::types::{AccountId, BlockHeight, EpochId}; @@ -41,8 +41,7 @@ pub trait ValidatorSigner: Sync + Send { fn sign_chunk_endorsement(&self, inner: &ChunkEndorsementInner) -> Signature; /// Signs approval of the given chunk. - /// Returns signature and a signed payload size in bytes - fn sign_chunk_state_witness(&self, inner: &ChunkStateWitnessInner) -> (Signature, usize); + fn sign_chunk_state_witness(&self, witness_bytes: &EncodedChunkStateWitness) -> Signature; /// Signs challenge body. fn sign_challenge(&self, challenge_body: &ChallengeBody) -> (CryptoHash, Signature); @@ -120,8 +119,8 @@ impl ValidatorSigner for EmptyValidatorSigner { Signature::default() } - fn sign_chunk_state_witness(&self, _inner: &ChunkStateWitnessInner) -> (Signature, usize) { - (Signature::default(), 0) + fn sign_chunk_state_witness(&self, _witness_bytes: &EncodedChunkStateWitness) -> Signature { + Signature::default() } fn sign_challenge(&self, challenge_body: &ChallengeBody) -> (CryptoHash, Signature) { @@ -219,9 +218,8 @@ impl ValidatorSigner for InMemoryValidatorSigner { self.signer.sign(&borsh::to_vec(inner).unwrap()) } - fn sign_chunk_state_witness(&self, inner: &ChunkStateWitnessInner) -> (Signature, usize) { - let data = borsh::to_vec(inner).unwrap(); - (self.signer.sign(&data), data.len()) + fn sign_chunk_state_witness(&self, witness_bytes: &EncodedChunkStateWitness) -> Signature { + self.signer.sign(witness_bytes.as_slice()) } fn sign_challenge(&self, challenge_body: &ChallengeBody) -> (CryptoHash, Signature) { diff --git a/integration-tests/src/tests/client/features/orphan_chunk_state_witness.rs b/integration-tests/src/tests/client/features/orphan_chunk_state_witness.rs index aae878ad51b..154c23f1a92 100644 --- a/integration-tests/src/tests/client/features/orphan_chunk_state_witness.rs +++ b/integration-tests/src/tests/client/features/orphan_chunk_state_witness.rs @@ -9,15 +9,14 @@ use near_client::{Client, ProcessingDoneTracker, ProcessingDoneWaiter}; use near_crypto::Signature; use near_network::types::{NetworkRequests, PeerManagerMessageRequest}; use near_o11y::testonly::init_integration_logger; -use near_primitives::merkle::{Direction, MerklePathItem}; use near_primitives::sharding::{ ChunkHash, ReceiptProof, ShardChunkHeader, ShardChunkHeaderInner, ShardChunkHeaderInnerV2, ShardProof, }; -use near_primitives::stateless_validation::ChunkStateWitness; +use near_primitives::stateless_validation::EncodedChunkStateWitness; +use near_primitives::stateless_validation::SignedEncodedChunkStateWitness; +use near_primitives::types::AccountId; use near_primitives_core::checked_feature; -use near_primitives_core::hash::CryptoHash; -use near_primitives_core::types::AccountId; use near_primitives_core::version::PROTOCOL_VERSION; use nearcore::test_utils::TestEnvNightshadeSetupExt; @@ -25,7 +24,7 @@ struct OrphanWitnessTestEnv { env: TestEnv, block1: Block, block2: Block, - witness: ChunkStateWitness, + signed_witness: SignedEncodedChunkStateWitness, excluded_validator: AccountId, excluded_validator_idx: usize, chunk_producer: AccountId, @@ -158,7 +157,8 @@ fn setup_orphan_witness_test() -> OrphanWitnessTestEnv { } _ => Some(request), }); - let witness = witness_opt.unwrap(); + let signed_witness = witness_opt.unwrap(); + let witness = signed_witness.witness_bytes.decode().unwrap(); env.propagate_chunk_endorsements(false); @@ -169,7 +169,7 @@ fn setup_orphan_witness_test() -> OrphanWitnessTestEnv { block2.header().height(), "There should be no missing chunks." ); - assert_eq!(witness.inner.chunk_header.chunk_hash(), block2.chunks()[0].chunk_hash()); + assert_eq!(witness.chunk_header.chunk_hash(), block2.chunks()[0].chunk_hash()); for client_idx in clients_without_excluded { let blocks_processed = env.clients[client_idx] @@ -187,7 +187,7 @@ fn setup_orphan_witness_test() -> OrphanWitnessTestEnv { env, block1, block2, - witness, + signed_witness, excluded_validator, excluded_validator_idx, chunk_producer: block2_chunk_producer, @@ -208,7 +208,7 @@ fn test_orphan_witness_valid() { mut env, block1, block2, - witness, + signed_witness, excluded_validator, excluded_validator_idx, .. @@ -216,7 +216,7 @@ fn test_orphan_witness_valid() { // `excluded_validator` receives witness for chunk belonging to `block2`, but it doesn't have `block1`. // The witness should become an orphaned witness and it should be saved to the orphan pool. - env.client(&excluded_validator).process_chunk_state_witness(witness, None).unwrap(); + env.client(&excluded_validator).process_chunk_state_witness(signed_witness, None).unwrap(); let block_processed = env .client(&excluded_validator) @@ -239,14 +239,16 @@ fn test_orphan_witness_bad_signature() { return; } - let OrphanWitnessTestEnv { mut env, mut witness, excluded_validator, .. } = + let OrphanWitnessTestEnv { mut env, mut signed_witness, excluded_validator, .. } = setup_orphan_witness_test(); // Modify the witness to contain an invalid signature - witness.signature = Signature::default(); + signed_witness.signature = Signature::default(); - let error = - env.client(&excluded_validator).process_chunk_state_witness(witness, None).unwrap_err(); + let error = env + .client(&excluded_validator) + .process_chunk_state_witness(signed_witness, None) + .unwrap_err(); let error_message = format!("{error}").to_lowercase(); tracing::info!(target:"test", "Error message: {}", error_message); assert!(error_message.contains("invalid signature")); @@ -261,15 +263,17 @@ fn test_orphan_witness_signature_from_wrong_peer() { return; } - let OrphanWitnessTestEnv { mut env, mut witness, excluded_validator, .. } = + let OrphanWitnessTestEnv { mut env, mut signed_witness, excluded_validator, .. } = setup_orphan_witness_test(); // Sign the witness using another validator's key. // Only witnesses from the chunk producer that produced this witness should be accepted. - resign_witness(&mut witness, env.client(&excluded_validator)); + resign_witness(&mut signed_witness, env.client(&excluded_validator)); - let error = - env.client(&excluded_validator).process_chunk_state_witness(witness, None).unwrap_err(); + let error = env + .client(&excluded_validator) + .process_chunk_state_witness(signed_witness, None) + .unwrap_err(); let error_message = format!("{error}").to_lowercase(); tracing::info!(target:"test", "Error message: {}", error_message); assert!(error_message.contains("invalid signature")); @@ -284,16 +288,23 @@ fn test_orphan_witness_invalid_shard_id() { return; } - let OrphanWitnessTestEnv { mut env, mut witness, excluded_validator, chunk_producer, .. } = - setup_orphan_witness_test(); + let OrphanWitnessTestEnv { + mut env, + mut signed_witness, + excluded_validator, + chunk_producer, + .. + } = setup_orphan_witness_test(); // Set invalid shard_id in the witness header - modify_witness_header_inner(&mut witness, |header| header.shard_id = 10000000); - resign_witness(&mut witness, env.client(&chunk_producer)); + modify_witness_header_inner(&mut signed_witness, |header| header.shard_id = 10000000); + resign_witness(&mut signed_witness, env.client(&chunk_producer)); // The witness should be rejected - let error = - env.client(&excluded_validator).process_chunk_state_witness(witness, None).unwrap_err(); + let error = env + .client(&excluded_validator) + .process_chunk_state_witness(signed_witness, None) + .unwrap_err(); let error_message = format!("{error}").to_lowercase(); tracing::info!(target:"test", "Error message: {}", error_message); assert!(error_message.contains("shard")); @@ -308,23 +319,18 @@ fn test_orphan_witness_too_large() { return; } - let OrphanWitnessTestEnv { mut env, mut witness, excluded_validator, chunk_producer, .. } = + let OrphanWitnessTestEnv { mut env, signed_witness, excluded_validator, .. } = setup_orphan_witness_test(); - // Modify the witness to be larger than the allowed limit - let dummy_merkle_path_item = - MerklePathItem { hash: CryptoHash::default(), direction: Direction::Left }; - let max_size_usize: usize = - default_orphan_state_witness_max_size().as_u64().try_into().unwrap(); - let items_count = max_size_usize / std::mem::size_of::() + 1; - let big_path = vec![dummy_merkle_path_item; items_count]; - let big_receipt_proof = - ReceiptProof(Vec::new(), ShardProof { from_shard_id: 0, to_shard_id: 0, proof: big_path }); - witness.inner.source_receipt_proofs.insert(ChunkHash::default(), big_receipt_proof); - resign_witness(&mut witness, env.client(&chunk_producer)); - + let witness = signed_witness.witness_bytes.decode().unwrap(); // The witness should not be saved too the pool, as it's too big - let outcome = env.client(&excluded_validator).handle_orphan_state_witness(witness).unwrap(); + let outcome = env + .client(&excluded_validator) + .handle_orphan_state_witness( + witness, + default_orphan_state_witness_max_size().as_u64() as usize + 1, + ) + .unwrap(); assert!(matches!(outcome, HandleOrphanWitnessOutcome::TooBig(_))) } @@ -340,7 +346,7 @@ fn test_orphan_witness_far_from_head() { let OrphanWitnessTestEnv { mut env, - mut witness, + mut signed_witness, chunk_producer, block1, excluded_validator, @@ -348,10 +354,12 @@ fn test_orphan_witness_far_from_head() { } = setup_orphan_witness_test(); let bad_height = 10000; - modify_witness_header_inner(&mut witness, |header| header.height_created = bad_height); - resign_witness(&mut witness, env.client(&chunk_producer)); + modify_witness_header_inner(&mut signed_witness, |header| header.height_created = bad_height); + resign_witness(&mut signed_witness, env.client(&chunk_producer)); - let outcome = env.client(&excluded_validator).handle_orphan_state_witness(witness).unwrap(); + let witness = signed_witness.witness_bytes.decode().unwrap(); + let outcome = + env.client(&excluded_validator).handle_orphan_state_witness(witness, 2000).unwrap(); assert_eq!( outcome, HandleOrphanWitnessOutcome::TooFarFromHead { @@ -373,39 +381,48 @@ fn test_orphan_witness_not_fully_validated() { return; } - let OrphanWitnessTestEnv { mut env, mut witness, chunk_producer, excluded_validator, .. } = - setup_orphan_witness_test(); + let OrphanWitnessTestEnv { + mut env, + mut signed_witness, + chunk_producer, + excluded_validator, + .. + } = setup_orphan_witness_test(); + let mut witness = signed_witness.witness_bytes.decode().unwrap(); // Make the witness invalid in a way that won't be detected during orphan witness validation - witness.inner.source_receipt_proofs.insert( + witness.source_receipt_proofs.insert( ChunkHash::default(), ReceiptProof( vec![], ShardProof { from_shard_id: 100230230, to_shard_id: 383939, proof: vec![] }, ), ); - resign_witness(&mut witness, env.client(&chunk_producer)); + signed_witness.witness_bytes = EncodedChunkStateWitness::encode(&witness); + resign_witness(&mut signed_witness, env.client(&chunk_producer)); // The witness should be accepted and saved into the pool, even though it's invalid. // There is no way to fully validate an orphan witness, so this is the correct behavior. // The witness will later be fully validated when the required block arrives. - env.client(&excluded_validator).process_chunk_state_witness(witness, None).unwrap(); + env.client(&excluded_validator).process_chunk_state_witness(signed_witness, None).unwrap(); } fn modify_witness_header_inner( - witness: &mut ChunkStateWitness, + signed_witness: &mut SignedEncodedChunkStateWitness, f: impl FnOnce(&mut ShardChunkHeaderInnerV2), ) { - match &mut witness.inner.chunk_header { + let mut witness = signed_witness.witness_bytes.decode().unwrap(); + match &mut witness.chunk_header { ShardChunkHeader::V3(header) => match &mut header.inner { ShardChunkHeaderInner::V2(header_inner) => f(header_inner), _ => panic!(), }, _ => panic!(), }; + signed_witness.witness_bytes = EncodedChunkStateWitness::encode(&witness); } -fn resign_witness(witness: &mut ChunkStateWitness, signer: &Client) { +fn resign_witness(witness: &mut SignedEncodedChunkStateWitness, signer: &Client) { witness.signature = - signer.validator_signer.as_ref().unwrap().sign_chunk_state_witness(&witness.inner).0; + signer.validator_signer.as_ref().unwrap().sign_chunk_state_witness(&witness.witness_bytes); } diff --git a/integration-tests/src/tests/client/features/stateless_validation.rs b/integration-tests/src/tests/client/features/stateless_validation.rs index 0491e706354..33112106291 100644 --- a/integration-tests/src/tests/client/features/stateless_validation.rs +++ b/integration-tests/src/tests/client/features/stateless_validation.rs @@ -1,5 +1,7 @@ use near_epoch_manager::{EpochManager, EpochManagerAdapter}; -use near_primitives::stateless_validation::ChunkStateWitness; +use near_primitives::stateless_validation::{ + ChunkStateWitness, EncodedChunkStateWitness, SignedEncodedChunkStateWitness, +}; use near_store::test_utils::create_test_store; use rand::rngs::StdRng; use rand::{Rng, SeedableRng}; @@ -331,10 +333,14 @@ fn test_chunk_state_witness_bad_shard_id() { let previous_block = env.clients[0].chain.head().unwrap().prev_block_hash; let invalid_shard_id = 1000000000; let witness = ChunkStateWitness::new_dummy(upper_height, invalid_shard_id, previous_block); + let signed_witness = SignedEncodedChunkStateWitness { + witness_bytes: EncodedChunkStateWitness::encode(&witness), + signature: Default::default(), + }; // Client should reject this ChunkStateWitness and the error message should mention "shard" tracing::info!(target: "test", "Processing invalid ChunkStateWitness"); - let res = env.clients[0].process_chunk_state_witness(witness, None); + let res = env.clients[0].process_chunk_state_witness(signed_witness, None); let error = res.unwrap_err(); let error_message = format!("{}", error).to_lowercase(); tracing::info!(target: "test", "error message: {}", error_message);