Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stateless_validaiton] Skip validation of state witness for chunk right after genesis #10487

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 50 additions & 24 deletions chain/client/src/chunk_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,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 @@ -127,18 +119,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 @@ -421,10 +407,54 @@ fn apply_result_to_chunk_extra(
)
}

fn send_chunk_endorsement_to_block_producers(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could name it endorse_chunk for demonstrativeness. Not a strong opinion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the current name as it's more explicit

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,
);
}
shreyan-gupta marked this conversation as resolved.
Show resolved Hide resolved
// 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 @@ -499,10 +529,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
Loading