Skip to content

Commit

Permalink
Panic in debug mode or non-prod chain when there is a chunk-witness v…
Browse files Browse the repository at this point in the history
…alidation error (#11412)

This is an attempt to create a submittable version of [this
commit](0c6c822).

After this change chunk-witness validation errors will lead to panic for
non-production chains (eg. mocknet and localnet). mainnet and testnet
will continue to log error and continue on validation errors.
We also add a method to enable/disable this for tests, especially for
adversarial behavior where the chunk validation errors are intended as
part of the test scenario.
  • Loading branch information
tayfunelmas authored May 29, 2024
1 parent 2e1df80 commit 4f556da
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 5 deletions.
4 changes: 4 additions & 0 deletions chain/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,9 @@ impl Client {
);
let chunk_endorsement_tracker =
Arc::new(ChunkEndorsementTracker::new(epoch_manager.clone()));
// Chunk validator should panic if there is a validator error in non-production chains (eg. mocket and localnet).
let panic_on_validation_error = config.chain_id != near_primitives::chains::MAINNET
&& config.chain_id != near_primitives::chains::TESTNET;
let chunk_validator = ChunkValidator::new(
validator_signer.clone(),
epoch_manager.clone(),
Expand All @@ -368,6 +371,7 @@ impl Client {
chunk_endorsement_tracker.clone(),
config.orphan_state_witness_pool_size,
async_computation_spawner,
panic_on_validation_error,
);
let chunk_distribution_network = ChunkDistributionNetwork::from_config(&config);
Ok(Self {
Expand Down
34 changes: 29 additions & 5 deletions chain/client/src/stateless_validation/chunk_validator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ pub struct ChunkValidator {
orphan_witness_pool: OrphanStateWitnessPool,
validation_spawner: Arc<dyn AsyncComputationSpawner>,
main_state_transition_result_cache: MainStateTransitionCache,
/// If true, a chunk-witness validation error will lead to a panic.
/// This is used for non-production environments, eg. mocknet and localnet,
/// to quickly detect issues in validation code, and must NOT be set to true
/// for mainnet and testnet.
panic_on_validation_error: bool,
}

impl ChunkValidator {
Expand All @@ -90,6 +95,7 @@ impl ChunkValidator {
chunk_endorsement_tracker: Arc<ChunkEndorsementTracker>,
orphan_witness_pool_size: usize,
validation_spawner: Arc<dyn AsyncComputationSpawner>,
panic_on_validation_error: bool,
) -> Self {
Self {
my_signer,
Expand All @@ -100,6 +106,7 @@ impl ChunkValidator {
orphan_witness_pool: OrphanStateWitnessPool::new(orphan_witness_pool_size),
validation_spawner,
main_state_transition_result_cache: MainStateTransitionCache::default(),
panic_on_validation_error,
}
}

Expand Down Expand Up @@ -146,6 +153,7 @@ impl ChunkValidator {
chunk_header.shard_id(),
)?;
let shard_uid = epoch_manager.shard_id_to_uid(last_header.shard_id(), &epoch_id)?;
let panic_on_validation_error = self.panic_on_validation_error;

if let Ok(prev_chunk_extra) = chain.get_chunk_extra(prev_block_hash, &shard_uid) {
match validate_chunk_with_chunk_extra(
Expand All @@ -167,10 +175,14 @@ impl ChunkValidator {
return Ok(());
}
Err(err) => {
tracing::error!(
"Failed to validate chunk using existing chunk extra: {:?}",
err
);
if panic_on_validation_error {
panic!("Failed to validate chunk using existing chunk extra: {:?}", err);
} else {
tracing::error!(
"Failed to validate chunk using existing chunk extra: {:?}",
err
);
}
return Err(err);
}
}
Expand Down Expand Up @@ -200,12 +212,24 @@ impl ChunkValidator {
);
}
Err(err) => {
tracing::error!("Failed to validate chunk: {:?}", err);
if panic_on_validation_error {
panic!("Failed to validate chunk: {:?}", err);
} else {
tracing::error!("Failed to validate chunk: {:?}", err);
}
}
}
});
Ok(())
}

/// TESTING ONLY: Used to override the value of panic_on_validation_error, for example,
/// when the chunks validation errors are expected when testing adversarial behavior and
/// the test should not panic for the invalid chunks witnesses.
#[cfg(feature = "test_features")]
pub fn set_should_panic_on_validation_error(&mut self, value: bool) {
self.panic_on_validation_error = value;
}
}

/// Checks that proposed `transactions` are valid for a chunk with `chunk_header`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,9 @@ fn test_banning_chunk_producer_when_seeing_invalid_chunk() {
init_test_logger();
let mut test = AdversarialBehaviorTestData::new();
test.env.clients[7].produce_invalid_chunks = true;
for client in test.env.clients.iter_mut() {
client.chunk_validator.set_should_panic_on_validation_error(false);
}
test_banning_chunk_producer_when_seeing_invalid_chunk_base(test);
}

Expand All @@ -371,5 +374,8 @@ fn test_banning_chunk_producer_when_seeing_invalid_tx_in_chunk() {
init_test_logger();
let mut test = AdversarialBehaviorTestData::new();
test.env.clients[7].produce_invalid_tx_in_chunks = true;
for client in test.env.clients.iter_mut() {
client.chunk_validator.set_should_panic_on_validation_error(false);
}
test_banning_chunk_producer_when_seeing_invalid_chunk_base(test);
}

0 comments on commit 4f556da

Please sign in to comment.