Skip to content

Commit

Permalink
[stateless_validation] validate incoming partial witness heights (#11398
Browse files Browse the repository at this point in the history
)

This adds logic to
`PartialWitnessActor::validate_partial_encoded_state_witness()` that
checks that the height is greater than the current final head height (as
is done in `Client::process_chunk_state_witness()`), and not more than
five ahead of the current head height, similar to what's done in
`EncodedChunksCache::height_within_horizon()`

For now we read the head and final head heights directlly from the
database, but in the future this should be changed so that we make a
request to the client for this info

Related issue: #11301
  • Loading branch information
marcelo-gonzalez authored May 30, 2024
1 parent 4f556da commit dfa392b
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 21 deletions.
19 changes: 0 additions & 19 deletions chain/client/src/stateless_validation/chunk_validator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,25 +768,6 @@ impl Client {
self.chain.chain_store.save_latest_chunk_state_witness(&witness)?;
}

// Avoid processing state witness for old chunks.
// In particular it is impossible for a chunk created at a height
// that doesn't exceed the height of the current final block to be
// included in the chain. This addresses both network-delayed messages
// as well as malicious behavior of a chunk producer.
if let Ok(final_head) = self.chain.final_head() {
if witness.chunk_header.height_created() <= final_head.height {
tracing::warn!(
target: "client",
chunk_hash=?witness.chunk_header.chunk_hash(),
shard_id=witness.chunk_header.shard_id(),
witness_height=witness.chunk_header.height_created(),
final_height=final_head.height,
"Skipping state witness below the last final block",
);
return Ok(());
}
}

match self.chain.get_block(witness.chunk_header.prev_block_hash()) {
Ok(block) => self.process_chunk_state_witness_with_prev_block(
witness,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ use near_network::state_witness::{
};
use near_network::types::{NetworkRequests, PeerManagerAdapter, PeerManagerMessageRequest};
use near_performance_metrics_macros::perf;
use near_primitives::block::Tip;
use near_primitives::reed_solomon::reed_solomon_encode;
use near_primitives::sharding::ShardChunkHeader;
use near_primitives::stateless_validation::{
ChunkStateWitness, ChunkStateWitnessAck, EncodedChunkStateWitness, PartialEncodedStateWitness,
};
use near_primitives::types::{AccountId, EpochId};
use near_primitives::types::{AccountId, BlockHeightDelta, EpochId};
use near_primitives::validator_signer::ValidatorSigner;
use near_store::{DBCol, Store, FINAL_HEAD_KEY, HEAD_KEY};

use crate::client_actor::ClientSenderForPartialWitness;
use crate::metrics;
Expand All @@ -40,8 +42,15 @@ pub struct PartialWitnessActor {
/// Reed Solomon encoder for encoding state witness parts.
/// We keep one wrapper for each length of chunk_validators to avoid re-creating the encoder.
rs_map: RsMap,
/// Currently used to find the chain HEAD when validating partial witnesses,
/// but should be removed if we implement retrieving this info from the client
store: Store,
}

/// This is taken to be the same value as near_chunks::chunk_cache::MAX_HEIGHTS_AHEAD, and we
/// reject partial witnesses with height more than this value above the height of our current HEAD
const MAX_HEIGHTS_AHEAD: BlockHeightDelta = 5;

impl Actor for PartialWitnessActor {}

#[derive(actix::Message, Debug)]
Expand Down Expand Up @@ -96,6 +105,7 @@ impl PartialWitnessActor {
client_sender: ClientSenderForPartialWitness,
my_signer: Arc<dyn ValidatorSigner>,
epoch_manager: Arc<dyn EpochManagerAdapter>,
store: Store,
) -> Self {
let partial_witness_tracker =
PartialEncodedStateWitnessTracker::new(client_sender, epoch_manager.clone());
Expand All @@ -106,6 +116,7 @@ impl PartialWitnessActor {
partial_witness_tracker,
state_witness_tracker: ChunkStateWitnessTracker::new(clock),
rs_map: RsMap::new(),
store,
}
}

Expand Down Expand Up @@ -327,6 +338,35 @@ impl PartialWitnessActor {
)));
}

// TODO(https://github.com/near/nearcore/issues/11301): replace these direct DB accesses with messages
// sent to the client actor. for a draft, see https://github.com/near/nearcore/commit/e186dc7c0b467294034c60758fe555c78a31ef2d
let head = self.store.get_ser::<Tip>(DBCol::BlockMisc, HEAD_KEY)?;
let final_head = self.store.get_ser::<Tip>(DBCol::BlockMisc, FINAL_HEAD_KEY)?;

// Avoid processing state witness for old chunks.
// In particular it is impossible for a chunk created at a height
// that doesn't exceed the height of the current final block to be
// included in the chain. This addresses both network-delayed messages
// as well as malicious behavior of a chunk producer.
if let Some(final_head) = final_head {
if partial_witness.height_created() <= final_head.height {
return Err(Error::InvalidPartialChunkStateWitness(format!(
"Height created of {} in PartialEncodedStateWitness not greater than final head height {}",
partial_witness.height_created(),
final_head.height,
)));
}
}
if let Some(head) = head {
if partial_witness.height_created() > head.height + MAX_HEIGHTS_AHEAD {
return Err(Error::InvalidPartialChunkStateWitness(format!(
"Height created of {} in PartialEncodedStateWitness more than {} blocks ahead of head height {}",
partial_witness.height_created(),
MAX_HEIGHTS_AHEAD,
head.height,
)));
}
}
if !self.epoch_manager.verify_partial_witness_signature(&partial_witness)? {
return Err(Error::InvalidPartialChunkStateWitness("Invalid signature".to_string()));
}
Expand Down
1 change: 1 addition & 0 deletions chain/client/src/test_utils/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ pub fn setup(
noop().into_multi_sender(),
signer.clone(),
epoch_manager.clone(),
store.clone(),
));
let partial_witness_adapter = partial_witness_addr.with_auto_span_context();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ fn test_client_with_multi_test_loop() {
shard_tracker.clone(),
builder.sender().for_index(idx).into_sender(),
builder.sender().for_index(idx).into_sender(),
ReadOnlyChunksStore::new(store),
ReadOnlyChunksStore::new(store.clone()),
client.chain.head().unwrap(),
client.chain.header_head().unwrap(),
Duration::milliseconds(100),
Expand Down Expand Up @@ -362,6 +362,7 @@ fn test_client_with_multi_test_loop() {
.into_wrapped_multi_sender::<ClientSenderForPartialWitnessMessage, _>(),
validator_signer,
epoch_manager.clone(),
store,
);

let future_spawner = builder.sender().for_index(idx).into_future_spawner();
Expand Down
1 change: 1 addition & 0 deletions integration-tests/src/tests/network/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ fn setup_network_node(
client_actor.clone().with_auto_span_context().into_multi_sender(),
signer,
epoch_manager,
runtime.store().clone(),
));
shards_manager_adapter.bind(shards_manager_actor.with_auto_span_context());
let peer_manager = PeerManagerActor::spawn(
Expand Down
1 change: 1 addition & 0 deletions nearcore/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ pub fn start_with_config_and_synchronization(
client_adapter_for_partial_witness_actor.as_multi_sender(),
my_signer,
epoch_manager.clone(),
storage.get_hot_store(),
));
(Some(partial_witness_actor), Some(partial_witness_arbiter))
} else {
Expand Down

0 comments on commit dfa392b

Please sign in to comment.