Skip to content

Commit

Permalink
[stateless_validaiton] Skip validation of state witness for chunk rig…
Browse files Browse the repository at this point in the history
…ht after genesis (#10487)

We are skipping state witness validation for the chunk right after
genesis chunk due to some complications of trying to run the genesis
chunk in runtime (it's not possible to do so).

Initially this edge case check was a part of
`send_chunk_state_witness_to_chunk_validators` and we didn't send state
witness at all.

This has an issue that we do not process and send chunk endorsements
either due to this. Instead pushed this check into
`process_chunk_state_witness` and we send chunk endorsements for this
special case.

---------

Co-authored-by: Aleksandr Logunov <[email protected]>
  • Loading branch information
Shreyan Gupta and Longarithm authored Jan 24, 2024
1 parent 3b3b54b commit 669df43
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 29 deletions.
75 changes: 51 additions & 24 deletions chain/client/src/chunk_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,8 @@ impl ChunkValidator {
self.epoch_manager.as_ref(),
)?;

// Send the chunk endorsement to the next NUM_NEXT_BLOCK_PRODUCERS_TO_SEND_CHUNK_ENDORSEMENT block producers.
// It's possible we may reach the end of the epoch, in which case, ignore the error from get_block_producer.
let block_height = chunk_header.height_created();
let block_producers = (0..NUM_NEXT_BLOCK_PRODUCERS_TO_SEND_CHUNK_ENDORSEMENT)
.map_while(|i| self.epoch_manager.get_block_producer(&epoch_id, block_height + i).ok())
.collect_vec();
assert!(!block_producers.is_empty());

let network_sender = self.network_sender.clone();
let signer = self.my_signer.clone().unwrap();
let signer = my_signer.clone();
let epoch_manager = self.epoch_manager.clone();
let runtime_adapter = self.runtime_adapter.clone();
rayon::spawn(move || {
Expand All @@ -128,18 +120,12 @@ impl ChunkValidator {
runtime_adapter.as_ref(),
) {
Ok(()) => {
tracing::debug!(
target: "chunk_validation",
chunk_hash=?chunk_header.chunk_hash(),
?block_producers,
"Chunk validated successfully, sending endorsement",
send_chunk_endorsement_to_block_producers(
&chunk_header,
epoch_manager.as_ref(),
signer.as_ref(),
&network_sender,
);
let endorsement = ChunkEndorsement::new(chunk_header.chunk_hash(), signer);
for block_producer in block_producers {
network_sender.send(PeerManagerMessageRequest::NetworkRequests(
NetworkRequests::ChunkEndorsement(block_producer, endorsement.clone()),
));
}
}
Err(err) => {
tracing::error!("Failed to validate chunk: {:?}", err);
Expand Down Expand Up @@ -478,10 +464,55 @@ fn apply_result_to_chunk_extra(
)
}

fn send_chunk_endorsement_to_block_producers(
chunk_header: &ShardChunkHeader,
epoch_manager: &dyn EpochManagerAdapter,
signer: &dyn ValidatorSigner,
network_sender: &Sender<PeerManagerMessageRequest>,
) {
let epoch_id =
epoch_manager.get_epoch_id_from_prev_block(chunk_header.prev_block_hash()).unwrap();

// Send the chunk endorsement to the next NUM_NEXT_BLOCK_PRODUCERS_TO_SEND_CHUNK_ENDORSEMENT block producers.
// It's possible we may reach the end of the epoch, in which case, ignore the error from get_block_producer.
let block_height = chunk_header.height_created();
let block_producers = (0..NUM_NEXT_BLOCK_PRODUCERS_TO_SEND_CHUNK_ENDORSEMENT)
.map_while(|i| epoch_manager.get_block_producer(&epoch_id, block_height + i).ok())
.collect_vec();
assert!(!block_producers.is_empty());

tracing::debug!(
target: "chunk_validation",
chunk_hash=?chunk_header.chunk_hash(),
?block_producers,
"Chunk validated successfully, sending endorsement",
);

let endorsement = ChunkEndorsement::new(chunk_header.chunk_hash(), signer);
for block_producer in block_producers {
network_sender.send(PeerManagerMessageRequest::NetworkRequests(
NetworkRequests::ChunkEndorsement(block_producer, endorsement.clone()),
));
}
}

impl Client {
/// Responds to a network request to verify a `ChunkStateWitness`, which is
/// sent by chunk producers after they produce a chunk.
pub fn process_chunk_state_witness(&mut self, witness: ChunkStateWitness) -> Result<(), Error> {
// First chunk after genesis doesn't have to be endorsed.
if witness.chunk_header.prev_block_hash() == self.chain.genesis().hash() {
let Some(signer) = self.validator_signer.as_ref() else {
return Err(Error::NotAChunkValidator);
};
send_chunk_endorsement_to_block_producers(
&witness.chunk_header,
self.epoch_manager.as_ref(),
signer.as_ref(),
&self.chunk_validator.network_sender,
);
return Ok(());
}
// TODO(#10265): If the previous block does not exist, we should
// queue this (similar to orphans) to retry later.
self.chunk_validator.start_validating_chunk(witness, self.chain.chain_store())
Expand Down Expand Up @@ -556,10 +587,6 @@ impl Client {
if !checked_feature!("stable", ChunkValidation, protocol_version) {
return Ok(());
}
// First chunk after genesis doesn't have to be endorsed.
if prev_chunk_header.prev_block_hash() == &CryptoHash::default() {
return Ok(());
}

let chunk_header = chunk.cloned_header();
let chunk_validators = self
Expand Down
6 changes: 3 additions & 3 deletions chain/epoch-manager/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2708,15 +2708,15 @@ fn test_verify_chunk_endorsements() {
));

// check chunk endorsement validity
let mut chunk_endorsement = ChunkEndorsement::new(chunk_header.chunk_hash(), signer.clone());
let mut chunk_endorsement = ChunkEndorsement::new(chunk_header.chunk_hash(), signer.as_ref());
assert!(epoch_manager.verify_chunk_endorsement(&chunk_header, &chunk_endorsement).unwrap());

// check invalid chunk endorsement signature
chunk_endorsement.signature = Signature::default();
assert!(!epoch_manager.verify_chunk_endorsement(&chunk_header, &chunk_endorsement).unwrap());

// check chunk endorsement invalidity when chunk header and chunk endorsement don't match
let chunk_endorsement = ChunkEndorsement::new(h[3].into(), signer);
let chunk_endorsement = ChunkEndorsement::new(h[3].into(), signer.as_ref());
let err =
epoch_manager.verify_chunk_endorsement(&chunk_header, &chunk_endorsement).unwrap_err();
match err {
Expand All @@ -2726,7 +2726,7 @@ fn test_verify_chunk_endorsements() {

// check chunk endorsement invalidity when signer is not chunk validator
let bad_signer = Arc::new(create_test_signer("test2"));
let chunk_endorsement = ChunkEndorsement::new(chunk_header.chunk_hash(), bad_signer);
let chunk_endorsement = ChunkEndorsement::new(chunk_header.chunk_hash(), bad_signer.as_ref());
let err =
epoch_manager.verify_chunk_endorsement(&chunk_header, &chunk_endorsement).unwrap_err();
match err {
Expand Down
3 changes: 1 addition & 2 deletions core/primitives/src/chunk_validation.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::collections::{HashMap, HashSet};
use std::sync::Arc;

use crate::challenge::PartialState;
use crate::sharding::{ChunkHash, ReceiptProof, ShardChunkHeader};
Expand Down Expand Up @@ -106,7 +105,7 @@ pub struct ChunkEndorsement {
}

impl ChunkEndorsement {
pub fn new(chunk_hash: ChunkHash, signer: Arc<dyn ValidatorSigner>) -> ChunkEndorsement {
pub fn new(chunk_hash: ChunkHash, signer: &dyn ValidatorSigner) -> ChunkEndorsement {
let inner = ChunkEndorsementInner::new(chunk_hash);
let account_id = signer.validator_id().clone();
let signature = signer.sign_chunk_endorsement(&inner);
Expand Down

0 comments on commit 669df43

Please sign in to comment.