diff --git a/Cargo.lock b/Cargo.lock index f2476053e96..4364da5fa69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2856,6 +2856,7 @@ dependencies = [ "near-actix-test-utils", "near-chain", "near-chain-configs", + "near-chunks", "near-client", "near-crypto", "near-epoch-manager", @@ -3053,6 +3054,7 @@ dependencies = [ "near-crypto", "near-network", "near-o11y", + "near-performance-metrics", "near-pool", "near-primitives", "near-store", diff --git a/chain/chunks/Cargo.toml b/chain/chunks/Cargo.toml index b512d9767fc..9df29c32909 100644 --- a/chain/chunks/Cargo.toml +++ b/chain/chunks/Cargo.toml @@ -24,6 +24,7 @@ near-network = { path = "../network" } near-o11y = { path = "../../core/o11y" } near-chain = { path = "../chain" } near-pool = { path = "../pool" } +near-performance-metrics = { path = "../../utils/near-performance-metrics" } [dev-dependencies] assert_matches.workspace = true diff --git a/chain/chunks/src/adapter.rs b/chain/chunks/src/adapter.rs new file mode 100644 index 00000000000..79c7d93321a --- /dev/null +++ b/chain/chunks/src/adapter.rs @@ -0,0 +1,133 @@ +use actix::Message; +use near_chain::types::Tip; +use near_network::types::MsgRecipient; +use near_primitives::{ + hash::CryptoHash, + merkle::MerklePath, + receipt::Receipt, + sharding::{EncodedShardChunk, PartialEncodedChunk, ShardChunkHeader}, + types::EpochId, +}; + +/// The interface of ShardsManager that faces the Client. +/// It is thread safe (messages are posted to a dedicated thread that runs the +/// ShardsManager). +/// See also ShardsManagerAdapterForNetwork, which is the interface given to +/// the networking component. +/// See also ClientAdapterForShardsManager, which is the other direction - the +/// interface of the Client given to the ShardsManager. +pub trait ShardsManagerAdapterForClient: Send + Sync + 'static { + /// Processes the header seen from a block we received, if we have not already received the + /// header earlier from the chunk producer (via PartialEncodedChunk). + /// This can happen if we are not a validator, or if we are a validator but somehow missed + /// the chunk producer's message. + fn process_chunk_header_from_block(&self, chunk_header: &ShardChunkHeader); + /// Lets the ShardsManager know that the chain heads have been updated. + /// For a discussion of head vs header_head, see #8154. + fn update_chain_heads(&self, head: Tip, header_head: Tip); + /// As a chunk producer, distributes the given chunk to the other validators (by sending + /// PartialEncodedChunk messages to them). + /// The partial_chunk and encoded_chunk represent the same data, just in different formats. + /// + /// TODO(#8422): the arguments contain redundant information. + fn distribute_encoded_chunk( + &self, + partial_chunk: PartialEncodedChunk, + encoded_chunk: EncodedShardChunk, + merkle_paths: Vec, + outgoing_receipts: Vec, + ); + /// Requests the given chunks to be fetched from other nodes. + /// Only the parts and receipt proofs that this node cares about will be fetched; when + /// the fetching is complete, a response of ClientAdapterForShardsManager::did_complete_chunk + /// will be sent back to the client. + fn request_chunks(&self, chunks_to_request: Vec, prev_hash: CryptoHash); + /// Similar to request_chunks, but for orphan chunks. Since the chunk belongs to an orphan + /// block, the previous block is not known and thus we cannot derive epoch information from + /// that block. Therefore, an ancestor_hash must be provided which must correspond to a + /// block that is an ancestor of these chunks' blocks, and which must be in the same epoch. + fn request_chunks_for_orphan( + &self, + chunks_to_request: Vec, + epoch_id: EpochId, + ancestor_hash: CryptoHash, + ); + /// In response to processing a block, checks if there are any chunks that should have been + /// complete but are just waiting on the previous block to become available (e.g. a chunk + /// requested by request_chunks_for_orphan, which then received all needed parts and receipt + /// proofs, but cannot be marked as complete because the previous block isn't available), + /// and completes them if so. + fn check_incomplete_chunks(&self, prev_block_hash: CryptoHash); +} + +#[derive(Message)] +#[rtype(result = "()")] +pub enum ShardsManagerRequestFromClient { + ProcessChunkHeaderFromBlock(ShardChunkHeader), + UpdateChainHeads { + head: Tip, + header_head: Tip, + }, + DistributeEncodedChunk { + partial_chunk: PartialEncodedChunk, + encoded_chunk: EncodedShardChunk, + merkle_paths: Vec, + outgoing_receipts: Vec, + }, + RequestChunks { + chunks_to_request: Vec, + prev_hash: CryptoHash, + }, + RequestChunksForOrphan { + chunks_to_request: Vec, + epoch_id: EpochId, + ancestor_hash: CryptoHash, + }, + CheckIncompleteChunks(CryptoHash), +} + +impl> ShardsManagerAdapterForClient for A { + fn process_chunk_header_from_block(&self, chunk_header: &ShardChunkHeader) { + self.do_send(ShardsManagerRequestFromClient::ProcessChunkHeaderFromBlock( + chunk_header.clone(), + )); + } + fn update_chain_heads(&self, head: Tip, header_head: Tip) { + self.do_send(ShardsManagerRequestFromClient::UpdateChainHeads { head, header_head }); + } + fn distribute_encoded_chunk( + &self, + partial_chunk: PartialEncodedChunk, + encoded_chunk: EncodedShardChunk, + merkle_paths: Vec, + outgoing_receipts: Vec, + ) { + self.do_send(ShardsManagerRequestFromClient::DistributeEncodedChunk { + partial_chunk, + encoded_chunk, + merkle_paths, + outgoing_receipts, + }); + } + fn request_chunks(&self, chunks_to_request: Vec, prev_hash: CryptoHash) { + self.do_send(ShardsManagerRequestFromClient::RequestChunks { + chunks_to_request, + prev_hash, + }); + } + fn request_chunks_for_orphan( + &self, + chunks_to_request: Vec, + epoch_id: EpochId, + ancestor_hash: CryptoHash, + ) { + self.do_send(ShardsManagerRequestFromClient::RequestChunksForOrphan { + chunks_to_request, + epoch_id, + ancestor_hash, + }); + } + fn check_incomplete_chunks(&self, prev_block_hash: CryptoHash) { + self.do_send(ShardsManagerRequestFromClient::CheckIncompleteChunks(prev_block_hash)); + } +} diff --git a/chain/chunks/src/client.rs b/chain/chunks/src/client.rs index 72824875d29..45397cc0325 100644 --- a/chain/chunks/src/client.rs +++ b/chain/chunks/src/client.rs @@ -11,13 +11,25 @@ use near_primitives::{ types::{AccountId, ShardId}, }; -pub trait ClientAdapterForShardsManager { +pub trait ClientAdapterForShardsManager: Send + Sync + 'static { + /// Notifies the client that the ShardsManager has collected a complete chunk. + /// Note that this does NOT mean that the chunk is fully constructed. If we are + /// not tracking the shard this chunk is in, then being complete only means that + /// we have received the parts we own, and the receipt proofs corresponding to + /// shards that we do track. On the other hand if we are tracking the shard this + /// chunk is in, then being complete does mean having the full chunk, in which + /// case the shard_chunk is also provided. fn did_complete_chunk( &self, partial_chunk: PartialEncodedChunk, shard_chunk: Option, ); + /// Notifies the client that we have collected a full chunk but the chunk cannot + /// be properly decoded. fn saw_invalid_chunk(&self, chunk: EncodedShardChunk); + /// Notifies the client that the chunk header is ready for inclusion into a new + /// block, so that if we are a block producer, we may create a block that contains + /// this chunk now. The producer of this chunk is also provided. fn chunk_header_ready_for_inclusion( &self, chunk_header: ShardChunkHeader, diff --git a/chain/chunks/src/lib.rs b/chain/chunks/src/lib.rs index f6c291987d6..9bc4fc38b96 100644 --- a/chain/chunks/src/lib.rs +++ b/chain/chunks/src/lib.rs @@ -84,13 +84,18 @@ use std::collections::{btree_map, hash_map, BTreeMap, HashMap, HashSet}; use std::sync::Arc; use std::time::{Duration, Instant}; +use adapter::ShardsManagerRequestFromClient; use chrono::DateTime; use client::ClientAdapterForShardsManager; use logic::{ decode_encoded_chunk, make_outgoing_receipts_proofs, make_partial_encoded_chunk_from_owned_parts_and_needed_receipts, need_part, need_receipt, }; +use metrics::{ + PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER, PARTIAL_ENCODED_CHUNK_RESPONSE_DELAY, +}; use near_chain::chunks_store::ReadOnlyChunksStore; +use near_network::shards_manager::ShardsManagerRequestFromNetwork; use near_primitives::time::Utc; use rand::seq::{IteratorRandom, SliceRandom}; use tracing::{debug, error, warn}; @@ -128,10 +133,12 @@ use near_network::types::{ use near_o11y::WithSpanContextExt; use rand::Rng; +pub mod adapter; mod chunk_cache; pub mod client; pub mod logic; -mod metrics; +pub mod metrics; +pub mod shards_manager_actor; pub mod test_utils; const CHUNK_PRODUCER_BLACKLIST_SIZE: usize = 100; @@ -488,7 +495,13 @@ pub struct ShardsManager { // This is a best-effort cache of the chain's head, not the source of truth. The source // of truth is in the chain store and written to by the Client. - chain_head: Option, + chain_head: Tip, + // Similarly, this is the best-effort cache of the chain's header_head. This is used to + // determine how old a chunk is. We don't use the chain_head for that, because if we just + // ran header sync and are downloading the chunks for older blocks, head is behind while + // header_head is new, but we would only know that the older chunks are old because + // header_head is much newer. + chain_header_head: Tip, seals_mgr: SealsManager, } @@ -500,7 +513,8 @@ impl ShardsManager { network_adapter: Arc, client_adapter: Arc, store: ReadOnlyChunksStore, - initial_chain_head: Option, + initial_chain_head: Tip, + initial_chain_header_head: Tip, ) -> Self { Self { me: me.clone(), @@ -521,16 +535,18 @@ impl ShardsManager { ), chunk_forwards_cache: lru::LruCache::new(CHUNK_FORWARD_CACHE_SIZE), chain_head: initial_chain_head, + chain_header_head: initial_chain_header_head, seals_mgr: SealsManager::new(me, runtime_adapter), } } - pub fn update_chain_head(&mut self, tip: Tip) { + pub fn update_chain_heads(&mut self, head: Tip, header_head: Tip) { self.encoded_chunks.update_largest_seen_height( - tip.height, + head.height, &self.requested_partial_encoded_chunks.requests, ); - self.chain_head = Some(tip); + self.chain_head = head; + self.chain_header_head = header_head; } fn request_partial_encoded_chunk( @@ -776,7 +792,7 @@ impl ShardsManager { /// Only marks this chunk as being requested /// Note no requests are actually sent at this point. fn request_chunk_single_mark_only(&mut self, chunk_header: &ShardChunkHeader) { - self.request_chunk_single(chunk_header, chunk_header.prev_block_hash().clone(), None) + self.request_chunk_single(chunk_header, chunk_header.prev_block_hash().clone(), true) } /// send partial chunk requests for one chunk @@ -786,13 +802,12 @@ impl ShardsManager { /// 1) it is from the same epoch than `epoch_id` /// 2) it is processed /// If the above conditions are not met, the request will be dropped - /// `header_head`: header head of the current chain. If it is None, the request will be only - /// added to he request pool, but not sent. + /// `mark_only`: if true, only add the request to the pool, but do not send it. fn request_chunk_single( &mut self, chunk_header: &ShardChunkHeader, ancestor_hash: CryptoHash, - header_head: Option<&Tip>, + mark_only: bool, ) { let height = chunk_header.height_created(); let shard_id = chunk_header.shard_id(); @@ -829,14 +844,14 @@ impl ShardsManager { }, ); - if let Some(header_head) = header_head { + if !mark_only { let fetch_from_archival = self.runtime_adapter - .chunk_needs_to_be_fetched_from_archival(&ancestor_hash, &header_head.last_block_hash).unwrap_or_else(|err| { + .chunk_needs_to_be_fetched_from_archival(&ancestor_hash, &self.chain_header_head.last_block_hash).unwrap_or_else(|err| { error!(target: "chunks", "Error during requesting partial encoded chunk. Cannot determine whether to request from an archival node, defaulting to not: {}", err); false }); - let old_block = header_head.last_block_hash != prev_block_hash - && header_head.prev_block_hash != prev_block_hash; + let old_block = self.chain_header_head.last_block_hash != prev_block_hash + && self.chain_header_head.prev_block_hash != prev_block_hash; let should_wait_for_chunk_forwarding = self.should_wait_for_chunk_forwarding(&ancestor_hash, chunk_header.shard_id(), chunk_header.height_created()+1).unwrap_or_else(|_| { @@ -875,17 +890,13 @@ impl ShardsManager { /// `chunks_to_request`: chunks to request /// `prev_hash`: hash of prev block of the block we are requesting missing chunks for /// The function assumes the prev block is accepted - /// `header_head`: current head of the header chain - pub fn request_chunks( + pub fn request_chunks( &mut self, - chunks_to_request: T, + chunks_to_request: Vec, prev_hash: CryptoHash, - header_head: &Tip, - ) where - T: IntoIterator, - { + ) { for chunk_header in chunks_to_request { - self.request_chunk_single(&chunk_header, prev_hash, Some(header_head)); + self.request_chunk_single(&chunk_header, prev_hash, false); } } @@ -897,15 +908,12 @@ impl ShardsManager { /// 1) it is from the same epoch than `epoch_id` /// 2) it is processed /// If the above conditions are not met, the request will be dropped - pub fn request_chunks_for_orphan( + pub fn request_chunks_for_orphan( &mut self, - chunks_to_request: T, + chunks_to_request: Vec, epoch_id: &EpochId, ancestor_hash: CryptoHash, - header_head: &Tip, - ) where - T: IntoIterator, - { + ) { let ancestor_epoch_id = unwrap_or_return!(self.runtime_adapter.get_epoch_id_from_prev_block(&ancestor_hash)); if epoch_id != &ancestor_epoch_id { @@ -913,29 +921,29 @@ impl ShardsManager { } for chunk_header in chunks_to_request { - self.request_chunk_single(&chunk_header, ancestor_hash, Some(header_head)) + self.request_chunk_single(&chunk_header, ancestor_hash, false) } } /// Resends chunk requests if haven't received it within expected time. - pub fn resend_chunk_requests(&mut self, header_head: &Tip) { + pub fn resend_chunk_requests(&mut self) { let _span = tracing::debug_span!( target: "client", "resend_chunk_requests", - header_head_height = header_head.height, + header_head_height = self.chain_header_head.height, pool_size = self.requested_partial_encoded_chunks.len()) .entered(); // Process chunk one part requests. let requests = self.requested_partial_encoded_chunks.fetch(); for (chunk_hash, chunk_request) in requests { let fetch_from_archival = self.runtime_adapter - .chunk_needs_to_be_fetched_from_archival(&chunk_request.ancestor_hash, &header_head.last_block_hash).unwrap_or_else(|err| { + .chunk_needs_to_be_fetched_from_archival(&chunk_request.ancestor_hash, &self.chain_header_head.last_block_hash).unwrap_or_else(|err| { debug_assert!(false); error!(target: "chunks", "Error during re-requesting partial encoded chunk. Cannot determine whether to request from an archival node, defaulting to not: {}", err); false }); - let old_block = header_head.last_block_hash != chunk_request.prev_block_hash - && header_head.prev_block_hash != chunk_request.prev_block_hash; + let old_block = self.chain_header_head.last_block_hash != chunk_request.prev_block_hash + && self.chain_header_head.prev_block_hash != chunk_request.prev_block_hash; match self.request_partial_encoded_chunk( chunk_request.height, @@ -1395,6 +1403,7 @@ impl ShardsManager { // We don't know this chunk yet; cache the forwarded part // to be used after we get the header. self.insert_forwarded_chunk(forward); + PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER.inc(); return Err(Error::UnknownChunk); } Err(Error::ChainError(chain_error)) => { @@ -1405,6 +1414,7 @@ impl ShardsManager { // forwarded parts are later processed as partial encoded chunks, so we // can mark it as unknown for now. self.insert_forwarded_chunk(forward); + PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER.inc(); return Err(Error::UnknownChunk); } // Some other error occurred, we don't know how to handle it @@ -1437,11 +1447,7 @@ impl ShardsManager { // To achieve full validation, this function is called twice for each chunk entry // first when the chunk entry is inserted in `encoded_chunks` // then in `process_partial_encoded_chunk` after checking the previous block is ready - fn validate_chunk_header( - &self, - chain_head: Option<&Tip>, - header: &ShardChunkHeader, - ) -> Result<(), Error> { + fn validate_chunk_header(&self, header: &ShardChunkHeader) -> Result<(), Error> { let chunk_hash = header.chunk_hash(); // 1. check signature // Ideally, validating the chunk header needs the previous block to be accepted already. @@ -1468,15 +1474,13 @@ impl ShardsManager { let ancestor_hash = request_info.ancestor_hash; let epoch_id = self.runtime_adapter.get_epoch_id_from_prev_block(&ancestor_hash)?; (ancestor_hash, epoch_id, true) - } else if let Some(chain_head) = chain_head { + } else { // we can safely unwrap here because chain head must already be accepted let epoch_id = self .runtime_adapter - .get_epoch_id_from_prev_block(&chain_head.last_block_hash) + .get_epoch_id_from_prev_block(&self.chain_head.last_block_hash) .unwrap(); - (chain_head.last_block_hash, epoch_id, false) - } else { - return Err(epoch_id.err().unwrap().into()); + (self.chain_head.last_block_hash, epoch_id, false) } }; @@ -1608,9 +1612,9 @@ impl ShardsManager { } // 1.c checking header validity - match partial_encoded_chunk.validate_with(|pec| { - self.validate_chunk_header(self.chain_head.as_ref(), &pec.header).map(|()| true) - }) { + match partial_encoded_chunk + .validate_with(|pec| self.validate_chunk_header(&pec.header).map(|()| true)) + { Err(Error::ChainError(chain_error)) => match chain_error { // validate_chunk_header returns DBNotFoundError if the previous block is not ready // in this case, we return NeedBlock instead of error @@ -1671,14 +1675,15 @@ impl ShardsManager { &epoch_id, &partial_encoded_chunk.header.prev_block_hash(), )?; - } else if let Some(chain_head) = &self.chain_head { - let epoch_id = - self.runtime_adapter.get_epoch_id_from_prev_block(&chain_head.last_block_hash)?; + } else { + let epoch_id = self + .runtime_adapter + .get_epoch_id_from_prev_block(&self.chain_head.last_block_hash)?; self.send_partial_encoded_chunk_to_chunk_trackers( partial_encoded_chunk, new_part_ords, &epoch_id, - &chain_head.last_block_hash.clone(), + &self.chain_head.last_block_hash.clone(), )?; }; @@ -1746,7 +1751,7 @@ impl ShardsManager { // now that prev_block is processed if let Some(chunk_entry) = self.encoded_chunks.get(&chunk_hash) { if !chunk_entry.header_fully_validated { - let res = self.validate_chunk_header(None, header); + let res = self.validate_chunk_header(header); match res { Ok(()) => { self.encoded_chunks.mark_entry_validated(&chunk_hash); @@ -2159,6 +2164,84 @@ impl ShardsManager { Ok(()) } + + pub fn handle_client_request(&mut self, request: ShardsManagerRequestFromClient) { + match request { + ShardsManagerRequestFromClient::ProcessChunkHeaderFromBlock(chunk_header) => { + if let Err(e) = self.process_chunk_header_from_block(&chunk_header) { + warn!(target: "chunks", "Error processing chunk header from block: {:?}", e); + } + } + ShardsManagerRequestFromClient::UpdateChainHeads { head, header_head } => { + self.update_chain_heads(head, header_head) + } + ShardsManagerRequestFromClient::DistributeEncodedChunk { + partial_chunk, + encoded_chunk, + merkle_paths, + outgoing_receipts, + } => { + if let Err(e) = self.distribute_encoded_chunk( + partial_chunk, + encoded_chunk, + &merkle_paths, + outgoing_receipts, + ) { + warn!(target: "chunks", "Error distributing encoded chunk: {:?}", e); + } + } + ShardsManagerRequestFromClient::RequestChunks { chunks_to_request, prev_hash } => { + self.request_chunks(chunks_to_request, prev_hash) + } + ShardsManagerRequestFromClient::RequestChunksForOrphan { + chunks_to_request, + epoch_id, + ancestor_hash, + } => self.request_chunks_for_orphan(chunks_to_request, &epoch_id, ancestor_hash), + ShardsManagerRequestFromClient::CheckIncompleteChunks(prev_block_hash) => { + self.check_incomplete_chunks(&prev_block_hash) + } + } + } + + pub fn handle_network_request(&mut self, request: ShardsManagerRequestFromNetwork) { + match request { + ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunk(partial_encoded_chunk) => { + if let Err(e) = self.process_partial_encoded_chunk(partial_encoded_chunk.into()) { + warn!(target: "chunks", "Error processing partial encoded chunk: {:?}", e); + } + } + ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunkForward( + partial_encoded_chunk_forward, + ) => { + if let Err(e) = + self.process_partial_encoded_chunk_forward(partial_encoded_chunk_forward) + { + warn!(target: "chunks", "Error processing partial encoded chunk forward: {:?}", e); + } + } + ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunkResponse { + partial_encoded_chunk_response, + received_time, + } => { + PARTIAL_ENCODED_CHUNK_RESPONSE_DELAY.observe(received_time.elapsed().as_secs_f64()); + if let Err(e) = + self.process_partial_encoded_chunk_response(partial_encoded_chunk_response) + { + warn!(target: "chunks", "Error processing partial encoded chunk response: {:?}", e); + } + } + ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunkRequest { + partial_encoded_chunk_request, + route_back, + } => { + self.process_partial_encoded_chunk_request( + partial_encoded_chunk_request, + route_back, + ); + } + } + } } /// Indicates where we fetched the response to a PartialEncodedChunkRequest. @@ -2217,6 +2300,13 @@ mod test { #[test] fn test_request_partial_encoded_chunk_from_self() { let store = create_test_store(); + let mock_tip = Tip { + height: 0, + last_block_hash: CryptoHash::default(), + prev_block_hash: CryptoHash::default(), + epoch_id: EpochId::default(), + next_epoch_id: EpochId::default(), + }; let runtime_adapter = Arc::new(KeyValueRuntime::new(store.clone(), 5)); let network_adapter = Arc::new(MockPeerManagerAdapter::default()); let client_adapter = Arc::new(MockClientAdapterForShardsManager::default()); @@ -2226,7 +2316,8 @@ mod test { network_adapter.clone(), client_adapter, ReadOnlyChunksStore::new(store), - None, + mock_tip.clone(), + mock_tip, ); let added = Clock::instant(); shards_manager.requested_partial_encoded_chunks.insert( @@ -2241,13 +2332,7 @@ mod test { }, ); std::thread::sleep(Duration::from_millis(2 * CHUNK_REQUEST_RETRY_MS)); - shards_manager.resend_chunk_requests(&Tip { - height: 0, - last_block_hash: CryptoHash::default(), - prev_block_hash: CryptoHash::default(), - epoch_id: EpochId::default(), - next_epoch_id: EpochId::default(), - }); + shards_manager.resend_chunk_requests(); // For the chunks that would otherwise be requested from self we expect a request to be // sent to any peer tracking shard @@ -2276,13 +2361,21 @@ mod test { let network_adapter = Arc::new(MockPeerManagerAdapter::default()); let client_adapter = Arc::new(MockClientAdapterForShardsManager::default()); let chain_store = ChainStore::new(create_test_store(), 0, true); + let mock_tip = Tip { + height: 0, + last_block_hash: CryptoHash::default(), + prev_block_hash: CryptoHash::default(), + epoch_id: EpochId::default(), + next_epoch_id: EpochId::default(), + }; let mut shards_manager = ShardsManager::new( Some("test".parse().unwrap()), runtime_adapter.clone(), network_adapter, client_adapter, chain_store.new_read_only_chunks_store(), - None, + mock_tip.clone(), + mock_tip, ); let signer = create_test_signer("test"); let mut rs = ReedSolomonWrapper::new(4, 10); @@ -2444,7 +2537,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - Some(fixture.mock_chain_head.clone()), + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); // process chunk part 0 let partial_encoded_chunk = fixture.make_partial_encoded_chunk(&[0]); @@ -2457,7 +2551,7 @@ mod test { shards_manager.request_chunk_single( &fixture.mock_chunk_header, CryptoHash::default(), - Some(&fixture.mock_chain_head), + false, ); let collect_request_parts = |fixture: &mut ChunkTestFixture| -> HashSet { let mut parts = HashSet::new(); @@ -2485,14 +2579,14 @@ mod test { // resend request and check chunk part 0 and 1 are not requested again std::thread::sleep(Duration::from_millis(2 * CHUNK_REQUEST_RETRY_MS)); - shards_manager.resend_chunk_requests(&fixture.mock_chain_head); + shards_manager.resend_chunk_requests(); let requested_parts = collect_request_parts(&mut fixture); assert_eq!(requested_parts, (2..fixture.mock_chunk_parts.len() as u64).collect()); // immediately resend chunk requests // this should not send any new requests because it doesn't pass the time check - shards_manager.resend_chunk_requests(&fixture.mock_chain_head); + shards_manager.resend_chunk_requests(); let requested_parts = collect_request_parts(&mut fixture); assert_eq!(requested_parts, HashSet::new()); } @@ -2507,7 +2601,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); // part id > num parts @@ -2533,7 +2628,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let partial_encoded_chunk = fixture.make_partial_encoded_chunk(&fixture.mock_part_ords); let result = shards_manager @@ -2546,7 +2642,7 @@ mod test { shards_manager.request_chunk_single( &fixture.mock_chunk_header, *fixture.mock_chunk_header.prev_block_hash(), - Some(&fixture.mock_chain_head), + false, ); let count_forwards_and_requests = |fixture: &ChunkTestFixture| -> (usize, usize) { let mut forwards_count = 0; @@ -2568,14 +2664,7 @@ mod test { // After some time, we should send requests if we have not been forwarded the parts // we need. std::thread::sleep(Duration::from_millis(2 * CHUNK_REQUEST_RETRY_MS)); - let head = Tip { - height: 0, - last_block_hash: Default::default(), - prev_block_hash: Default::default(), - epoch_id: Default::default(), - next_epoch_id: Default::default(), - }; - shards_manager.resend_chunk_requests(&head); + shards_manager.resend_chunk_requests(); let (_, requests_count) = count_forwards_and_requests(&fixture); assert!(requests_count > 0); } @@ -2590,7 +2679,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let count_num_forward_msgs = |fixture: &ChunkTestFixture| { fixture @@ -2648,20 +2738,14 @@ mod test { fixture: &mut ChunkTestFixture, account_id: Option, ) -> RequestChunksResult { - let header_head = Tip { - height: 0, - last_block_hash: CryptoHash::default(), - prev_block_hash: CryptoHash::default(), - epoch_id: EpochId::default(), - next_epoch_id: EpochId::default(), - }; let mut shards_manager = ShardsManager::new( account_id, fixture.mock_runtime.clone(), fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); shards_manager.insert_header_if_not_exists_and_process_cached_chunk_forwards( &fixture.mock_chunk_header, @@ -2669,7 +2753,6 @@ mod test { shards_manager.request_chunks( vec![fixture.mock_chunk_header.clone()], fixture.mock_chunk_header.prev_block_hash().clone(), - &header_head, ); let marked_as_requested = shards_manager .requested_partial_encoded_chunks @@ -2680,7 +2763,7 @@ mod test { sent_request_message_immediately = true; } std::thread::sleep(Duration::from_millis(2 * CHUNK_REQUEST_RETRY_MS)); - shards_manager.resend_chunk_requests(&header_head); + shards_manager.resend_chunk_requests(); let mut sent_request_message_after_timeout = false; while let Some(_) = fixture.mock_network.pop() { sent_request_message_after_timeout = true; @@ -2745,7 +2828,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let partial_encoded_chunk = fixture.make_partial_encoded_chunk(&fixture.mock_part_ords); let _ = shards_manager @@ -2827,7 +2911,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let (most_parts, other_parts) = { let mut most_parts = fixture.mock_chunk_parts.clone(); @@ -2865,10 +2950,9 @@ mod test { vec![fixture.mock_chunk_header.clone()], &EpochId::default(), CryptoHash::default(), - &fixture.mock_chain_head, ); std::thread::sleep(Duration::from_millis(2 * CHUNK_REQUEST_RETRY_MS)); - shards_manager.resend_chunk_requests(&fixture.mock_chain_head); + shards_manager.resend_chunk_requests(); assert!(fixture .mock_network .requests @@ -2896,7 +2980,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let forward = PartialEncodedChunkForwardMsg::from_header_and_parts( &fixture.mock_chunk_header, @@ -2928,11 +3013,11 @@ mod test { shards_manager.request_chunk_single( &fixture.mock_chunk_header, *fixture.mock_chunk_header.prev_block_hash(), - Some(&fixture.mock_chain_head), + false, ); std::thread::sleep(Duration::from_millis(2 * CHUNK_REQUEST_RETRY_MS)); - shards_manager.resend_chunk_requests(&fixture.mock_chain_head); + shards_manager.resend_chunk_requests(); assert!(fixture .mock_network .requests @@ -2957,7 +3042,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); shards_manager @@ -2988,7 +3074,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); shards_manager @@ -3016,7 +3103,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); persist_chunk( @@ -3045,7 +3133,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let mut update = fixture.chain_store.store_update(); @@ -3073,7 +3162,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); // Split the part ords into two groups. assert!(fixture.all_part_ords.len() >= 2); @@ -3109,7 +3199,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); // Only add half of the parts to the cache. assert!(fixture.all_part_ords.len() >= 2); @@ -3149,7 +3240,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); // Split the part ords into three groups; put one in cache, the second in partial // and the third is missing. We should return the first two groups. @@ -3187,7 +3279,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let (source, response) = shards_manager.prepare_partial_encoded_chunk_response(PartialEncodedChunkRequestMsg { @@ -3208,7 +3301,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let (source, response) = shards_manager.prepare_partial_encoded_chunk_response(PartialEncodedChunkRequestMsg { @@ -3229,7 +3323,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let mut update = fixture.chain_store.store_update(); let shard_chunk = @@ -3258,7 +3353,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let mut update = fixture.chain_store.store_update(); let shard_chunk = @@ -3285,7 +3381,8 @@ mod test { fixture.mock_network.clone(), fixture.mock_client_adapter.clone(), fixture.chain_store.new_read_only_chunks_store(), - None, + fixture.mock_chain_head.clone(), + fixture.mock_chain_head.clone(), ); let part = fixture.make_partial_encoded_chunk(&fixture.mock_part_ords); shards_manager.process_partial_encoded_chunk(part.clone().into()).unwrap(); diff --git a/chain/chunks/src/metrics.rs b/chain/chunks/src/metrics.rs index 757bda52161..de2d3063dcb 100644 --- a/chain/chunks/src/metrics.rs +++ b/chain/chunks/src/metrics.rs @@ -1,4 +1,4 @@ -use near_o11y::metrics::exponential_buckets; +use near_o11y::metrics::{exponential_buckets, try_create_histogram, Counter, Histogram}; use once_cell::sync::Lazy; pub static PARTIAL_ENCODED_CHUNK_REQUEST_PROCESSING_TIME: Lazy = @@ -33,3 +33,21 @@ pub static DISTRIBUTE_ENCODED_CHUNK_TIME: Lazy ) .unwrap() }); + +pub(crate) static PARTIAL_ENCODED_CHUNK_RESPONSE_DELAY: Lazy = Lazy::new(|| { + try_create_histogram( + "near_partial_encoded_chunk_response_delay", + "Delay between when a partial encoded chunk response is sent from PeerActor and when it is received by ShardsManagerActor", + ) + .unwrap() +}); + +pub static PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER: Lazy = Lazy::new(|| { + near_o11y::metrics::try_create_counter( + "near_partial_encoded_chunk_forward_cached_without_header", + concat!( + "Number of times we received a valid partial encoded chunk forward without having the corresponding chunk header so we cached it" + ), + ) + .unwrap() +}); diff --git a/chain/chunks/src/shards_manager_actor.rs b/chain/chunks/src/shards_manager_actor.rs new file mode 100644 index 00000000000..248cd070e47 --- /dev/null +++ b/chain/chunks/src/shards_manager_actor.rs @@ -0,0 +1,94 @@ +use std::{sync::Arc, time::Duration}; + +use actix::{Actor, Addr, Arbiter, ArbiterHandle, Context, Handler}; +use near_chain::{chunks_store::ReadOnlyChunksStore, types::Tip, RuntimeWithEpochManagerAdapter}; +use near_network::{shards_manager::ShardsManagerRequestFromNetwork, types::PeerManagerAdapter}; +use near_primitives::types::AccountId; +use near_store::{DBCol, Store, HEADER_HEAD_KEY, HEAD_KEY}; + +use crate::{ + adapter::ShardsManagerRequestFromClient, client::ClientAdapterForShardsManager, ShardsManager, +}; + +pub struct ShardsManagerActor { + shards_mgr: ShardsManager, + chunk_request_retry_period: Duration, +} + +impl ShardsManagerActor { + fn new(shards_mgr: ShardsManager, chunk_request_retry_period: Duration) -> Self { + Self { shards_mgr, chunk_request_retry_period } + } + + fn periodically_resend_chunk_requests(&mut self, ctx: &mut Context) { + self.shards_mgr.resend_chunk_requests(); + + near_performance_metrics::actix::run_later( + ctx, + self.chunk_request_retry_period, + move |act, ctx| { + act.periodically_resend_chunk_requests(ctx); + }, + ); + } +} + +impl Actor for ShardsManagerActor { + type Context = Context; + + fn started(&mut self, ctx: &mut Self::Context) { + self.periodically_resend_chunk_requests(ctx); + } +} + +impl Handler for ShardsManagerActor { + type Result = (); + + fn handle(&mut self, msg: ShardsManagerRequestFromClient, _ctx: &mut Context) { + self.shards_mgr.handle_client_request(msg); + } +} + +impl Handler for ShardsManagerActor { + type Result = (); + + fn handle(&mut self, msg: ShardsManagerRequestFromNetwork, _ctx: &mut Context) { + self.shards_mgr.handle_network_request(msg); + } +} + +pub fn start_shards_manager( + runtime_adapter: Arc, + network_adapter: Arc, + client_adapter_for_shards_manager: Arc, + me: Option, + store: Store, + chunk_request_retry_period: Duration, +) -> (Addr, ArbiterHandle) { + let shards_manager_arbiter = Arbiter::new(); + let shards_manager_arbiter_handle = shards_manager_arbiter.handle(); + // TODO: make some better API for accessing chain properties like head. + let chain_head = store + .get_ser::(DBCol::BlockMisc, HEAD_KEY) + .unwrap() + .expect("ShardsManager must be initialized after the chain is initialized"); + let chain_header_head = store + .get_ser::(DBCol::BlockMisc, HEADER_HEAD_KEY) + .unwrap() + .expect("ShardsManager must be initialized after the chain is initialized"); + let chunks_store = ReadOnlyChunksStore::new(store); + let shards_manager = ShardsManager::new( + me, + runtime_adapter, + network_adapter, + client_adapter_for_shards_manager, + chunks_store, + chain_head, + chain_header_head, + ); + let shards_manager_addr = + ShardsManagerActor::start_in_arbiter(&shards_manager_arbiter_handle, move |_| { + ShardsManagerActor::new(shards_manager, chunk_request_retry_period) + }); + (shards_manager_addr, shards_manager_arbiter_handle) +} diff --git a/chain/chunks/src/test_utils.rs b/chain/chunks/src/test_utils.rs index 27ded3ec032..3c53f6be5dc 100644 --- a/chain/chunks/src/test_utils.rs +++ b/chain/chunks/src/test_utils.rs @@ -1,9 +1,10 @@ use std::collections::VecDeque; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex, RwLock}; use actix::MailboxError; use futures::future::BoxFuture; use futures::FutureExt; +use near_network::shards_manager::ShardsManagerRequestFromNetwork; use near_network::types::MsgRecipient; use near_primitives::receipt::Receipt; use near_primitives::test_utils::create_test_signer; @@ -27,6 +28,7 @@ use near_primitives::types::{BlockHeight, MerkleHash}; use near_primitives::version::PROTOCOL_VERSION; use near_store::Store; +use crate::adapter::ShardsManagerRequestFromClient; use crate::client::ShardsManagerResponse; use crate::{ Seal, SealsManager, ShardsManager, ACCEPTING_SEAL_PERIOD_MS, PAST_SEAL_HEIGHT_HORIZON, @@ -394,3 +396,63 @@ impl MockClientAdapterForShardsManager { self.requests.write().unwrap().pop_back() } } + +// Allows ShardsManagerActor-like behavior, except without having to spawn an actor, +// and without having to manually route ShardsManagerRequest messages. This only works +// for single-threaded (synchronous) tests. The ShardsManager is immediately called +// upon receiving a ShardsManagerRequest message. +#[derive(Clone)] +pub struct SynchronousShardsManagerAdapter { + // Need a mutex here even though we only support single-threaded tests, because + // MsgRecipient requires Sync. + pub shards_manager: Arc>, +} + +impl MsgRecipient for SynchronousShardsManagerAdapter { + fn send( + &self, + msg: ShardsManagerRequestFromClient, + ) -> BoxFuture<'static, Result<(), MailboxError>> { + self.do_send(msg); + futures::future::ok(()).boxed() + } + + fn do_send(&self, msg: ShardsManagerRequestFromClient) { + let mut shards_manager = self.shards_manager.lock().unwrap(); + shards_manager.handle_client_request(msg); + } +} + +impl MsgRecipient for SynchronousShardsManagerAdapter { + fn send( + &self, + msg: ShardsManagerRequestFromNetwork, + ) -> BoxFuture<'static, Result<(), MailboxError>> { + self.do_send(msg); + futures::future::ok(()).boxed() + } + + fn do_send(&self, msg: ShardsManagerRequestFromNetwork) { + let mut shards_manager = self.shards_manager.lock().unwrap(); + shards_manager.handle_network_request(msg); + } +} + +impl SynchronousShardsManagerAdapter { + pub fn new(shards_manager: ShardsManager) -> Self { + Self { shards_manager: Arc::new(Mutex::new(shards_manager)) } + } +} + +pub struct NoopShardsManagerAdapterForClient {} + +impl MsgRecipient for NoopShardsManagerAdapterForClient { + fn send( + &self, + _msg: ShardsManagerRequestFromClient, + ) -> BoxFuture<'static, Result<(), MailboxError>> { + futures::future::ok(()).boxed() + } + + fn do_send(&self, _msg: ShardsManagerRequestFromClient) {} +} diff --git a/chain/client/src/adapter.rs b/chain/client/src/adapter.rs index 5a5836725b9..7d01e46922c 100644 --- a/chain/client/src/adapter.rs +++ b/chain/client/src/adapter.rs @@ -1,6 +1,5 @@ use crate::client_actor::ClientActor; use crate::view_client::ViewClientActor; -use near_network::time; use near_network::types::{ NetworkInfo, PartialEncodedChunkForwardMsg, PartialEncodedChunkRequestMsg, PartialEncodedChunkResponseMsg, ReasonForBan, StateResponseInfo, @@ -266,50 +265,6 @@ impl near_network::client::Client for Adapter { } } - async fn partial_encoded_chunk_request( - &self, - req: PartialEncodedChunkRequestMsg, - msg_hash: CryptoHash, - ) { - match self - .client_addr - .send(RecvPartialEncodedChunkRequest(req, msg_hash).with_span_context()) - .await - { - Ok(()) => {} - Err(err) => tracing::error!("mailbox error: {err}"), - } - } - - async fn partial_encoded_chunk_response( - &self, - resp: PartialEncodedChunkResponseMsg, - timestamp: time::Instant, - ) { - match self - .client_addr - .send(RecvPartialEncodedChunkResponse(resp, timestamp.into()).with_span_context()) - .await - { - Ok(()) => {} - Err(err) => tracing::error!("mailbox error: {err}"), - } - } - - async fn partial_encoded_chunk(&self, chunk: PartialEncodedChunk) { - match self.client_addr.send(RecvPartialEncodedChunk(chunk).with_span_context()).await { - Ok(()) => {} - Err(err) => tracing::error!("mailbox error: {err}"), - } - } - - async fn partial_encoded_chunk_forward(&self, msg: PartialEncodedChunkForwardMsg) { - match self.client_addr.send(RecvPartialEncodedChunkForward(msg).with_span_context()).await { - Ok(()) => {} - Err(err) => tracing::error!("mailbox error: {err}"), - } - } - async fn block_request(&self, hash: CryptoHash) -> Option> { match self.view_client_addr.send(BlockRequest(hash).with_span_context()).await { Ok(res) => res, diff --git a/chain/client/src/client.rs b/chain/client/src/client.rs index 6335595eda5..faf44b6b11b 100644 --- a/chain/client/src/client.rs +++ b/chain/client/src/client.rs @@ -7,7 +7,8 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use lru::LruCache; -use near_chunks::client::{ClientAdapterForShardsManager, ShardedTransactionPool}; +use near_chunks::adapter::ShardsManagerAdapterForClient; +use near_chunks::client::ShardedTransactionPool; use near_chunks::logic::{ cares_about_shard_this_or_next_epoch, decode_encoded_chunk, persist_chunk, }; @@ -102,7 +103,7 @@ pub struct Client { pub chain: Chain, pub doomslug: Doomslug, pub runtime_adapter: Arc, - pub shards_mgr: ShardsManager, + pub shards_manager_adapter: Arc, pub sharded_tx_pool: ShardedTransactionPool, prev_block_to_chunk_headers_ready_for_inclusion: LruCache< CryptoHash, @@ -187,7 +188,7 @@ impl Client { chain_genesis: ChainGenesis, runtime_adapter: Arc, network_adapter: Arc, - client_adapter: Arc, + shards_manager_adapter: Arc, validator_signer: Option>, enable_doomslug: bool, rng_seed: RngSeed, @@ -215,14 +216,6 @@ impl Client { chain.store(), chain_config.background_migration_threads, )?; - let shards_mgr = ShardsManager::new( - me.clone(), - runtime_adapter.clone(), - network_adapter.clone(), - client_adapter.clone(), - chain.store().new_read_only_chunks_store(), - chain.head().ok(), - ); let sharded_tx_pool = ShardedTransactionPool::new(rng_seed); let sync_status = SyncStatus::AwaitingPeers; let genesis_block = chain.genesis_block(); @@ -280,7 +273,7 @@ impl Client { chain, doomslug, runtime_adapter, - shards_mgr, + shards_manager_adapter, sharded_tx_pool, prev_block_to_chunk_headers_ready_for_inclusion: LruCache::new( CHUNK_HEADERS_FOR_INCLUSION_CACHE_SIZE, @@ -1178,6 +1171,10 @@ impl Client { &mut block_processing_artifacts, apply_chunks_done_callback, ); + if accepted_blocks.iter().any(|accepted_block| accepted_block.status.is_new_head()) { + self.shards_manager_adapter + .update_chain_heads(self.chain.head().unwrap(), self.chain.header_head().unwrap()); + } self.process_block_processing_artifact(block_processing_artifacts); let accepted_blocks_hashes = accepted_blocks.iter().map(|accepted_block| accepted_block.hash.clone()).collect(); @@ -1210,21 +1207,17 @@ impl Client { } = block_processing_artifacts; // Send out challenges that accumulated via on_challenge. self.send_challenges(challenges); - // For any missing chunk, call the ShardsManager with it so that it may apply forwarded parts. - // This may end up completing the chunk. + // For any missing chunk, let the ShardsManager know of the chunk header so that it may + // apply forwarded parts. This may end up completing the chunk. let missing_chunks = blocks_missing_chunks .iter() .flat_map(|block| block.missing_chunks.iter()) .chain(orphans_missing_chunks.iter().flat_map(|block| block.missing_chunks.iter())); for chunk in missing_chunks { - match self.shards_mgr.process_chunk_header_from_block(chunk) { - Ok(_) => {} - Err(err) => { - warn!(target: "client", "Failed to process missing chunk from block: {:?}", err) - } - } + self.shards_manager_adapter.process_chunk_header_from_block(chunk); } - // Request any (still) missing chunks. + // Request any missing chunks (which may be completed by the + // process_chunk_header_from_block call, but that is OK as it would be noop). self.request_missing_chunks(blocks_missing_chunks, orphans_missing_chunks); for chunk_header in invalid_chunks { @@ -1276,9 +1269,11 @@ impl Client { apply_chunks_done_callback: DoneApplyChunkCallback, ) { let chunk_header = partial_chunk.cloned_header(); + self.chain.blocks_delay_tracker.mark_chunk_completed(&chunk_header, Clock::utc()); + self.block_production_info + .record_chunk_collected(partial_chunk.height_created(), partial_chunk.shard_id()); persist_chunk(partial_chunk, shard_chunk, self.chain.mut_store()) .expect("Could not persist chunk"); - self.chain.blocks_delay_tracker.mark_chunk_completed(&chunk_header, Clock::utc()); // We're marking chunk as accepted. self.chain.blocks_with_missing_chunks.accept_chunk(&chunk_header.chunk_hash()); // If this was the last chunk that was missing for a block, it will be processed now. @@ -1316,6 +1311,8 @@ impl Client { let mut challenges = vec![]; self.chain.sync_block_headers(headers, &mut challenges)?; self.send_challenges(challenges); + self.shards_manager_adapter + .update_chain_heads(self.chain.head().unwrap(), self.chain.header_head().unwrap()); Ok(()) } @@ -1439,7 +1436,6 @@ impl Client { } if status.is_new_head() { - self.shards_mgr.update_chain_head(Tip::from_header(&block.header())); let last_final_block = block.header().last_final_block(); let last_finalized_height = if last_final_block == &CryptoHash::default() { self.chain.genesis().height() @@ -1580,7 +1576,7 @@ impl Client { } } } - self.shards_mgr.check_incomplete_chunks(block.hash()); + self.shards_manager_adapter.check_incomplete_chunks(*block.hash()); } pub fn persist_and_distribute_encoded_chunk( @@ -1601,12 +1597,12 @@ impl Client { encoded_chunk.cloned_header(), validator_id.clone(), ); - self.shards_mgr.distribute_encoded_chunk( + self.shards_manager_adapter.distribute_encoded_chunk( partial_chunk, encoded_chunk, - &merkle_paths, + merkle_paths, receipts, - )?; + ); Ok(()) } @@ -1620,14 +1616,7 @@ impl Client { for chunk in &missing_chunks { self.chain.blocks_delay_tracker.mark_chunk_requested(chunk, now); } - self.shards_mgr.request_chunks( - missing_chunks, - prev_hash, - &self - .chain - .header_head() - .expect("header_head must be available when processing a block"), - ); + self.shards_manager_adapter.request_chunks(missing_chunks, prev_hash); } for OrphanMissingChunks { missing_chunks, epoch_id, ancestor_hash } in @@ -1636,14 +1625,10 @@ impl Client { for chunk in &missing_chunks { self.chain.blocks_delay_tracker.mark_chunk_requested(chunk, now); } - self.shards_mgr.request_chunks_for_orphan( + self.shards_manager_adapter.request_chunks_for_orphan( missing_chunks, - &epoch_id, + epoch_id, ancestor_hash, - &self - .chain - .header_head() - .expect("header_head must be available when processing a block"), ); } } diff --git a/chain/client/src/client_actor.rs b/chain/client/src/client_actor.rs index 32fcbb19438..05dcdb35fab 100644 --- a/chain/client/src/client_actor.rs +++ b/chain/client/src/client_actor.rs @@ -7,14 +7,12 @@ use crate::adapter::{ BlockApproval, BlockHeadersResponse, BlockResponse, ProcessTxRequest, ProcessTxResponse, - RecvChallenge, RecvPartialEncodedChunk, RecvPartialEncodedChunkForward, - RecvPartialEncodedChunkRequest, RecvPartialEncodedChunkResponse, SetNetworkInfo, StateResponse, + RecvChallenge, SetNetworkInfo, StateResponse, }; use crate::client::{Client, EPOCH_START_INFO_BLOCKS}; use crate::config_updater::ConfigUpdater; use crate::debug::new_network_info_view; use crate::info::{display_sync_status, InfoHelper}; -use crate::metrics::PARTIAL_ENCODED_CHUNK_RESPONSE_DELAY; use crate::sync::state::{StateSync, StateSyncResult}; use crate::{metrics, StatusResponse}; use actix::dev::SendError; @@ -34,6 +32,7 @@ use near_chain::{ ChainGenesis, DoneApplyChunkCallback, Provenance, RuntimeWithEpochManagerAdapter, }; use near_chain_configs::ClientConfig; +use near_chunks::adapter::ShardsManagerAdapterForClient; use near_chunks::client::ShardsManagerResponse; use near_chunks::logic::cares_about_shard_this_or_next_epoch; use near_client_primitives::types::{ @@ -105,7 +104,6 @@ pub struct ClientActor { block_production_started: bool, doomslug_timer_next_attempt: DateTime, sync_timer_next_attempt: DateTime, - chunk_request_retry_next_attempt: DateTime, sync_started: bool, state_parts_task_scheduler: Box, block_catch_up_scheduler: Box, @@ -145,16 +143,13 @@ fn wait_until_genesis(genesis_time: &DateTime) { impl ClientActor { pub fn new( + client: Client, address: Addr, config: ClientConfig, - chain_genesis: ChainGenesis, - runtime_adapter: Arc, node_id: PeerId, network_adapter: Arc, validator_signer: Option>, telemetry_actor: Addr, - enable_doomslug: bool, - rng_seed: RngSeed, ctx: &Context, shutdown_signal: Option>, adv: crate::adversarial::Controls, @@ -170,21 +165,10 @@ impl ClientActor { SyncJobsActor { client_addr: self_addr_clone } }, ); - wait_until_genesis(&chain_genesis.time); if let Some(vs) = &validator_signer { info!(target: "client", "Starting validator node: {}", vs.validator_id()); } let info_helper = InfoHelper::new(Some(telemetry_actor), &config, validator_signer.clone()); - let client = Client::new( - config, - chain_genesis, - runtime_adapter, - network_adapter.clone(), - Arc::new(self_addr.clone()), - validator_signer, - enable_doomslug, - rng_seed, - )?; let now = Utc::now(); Ok(ClientActor { @@ -212,7 +196,6 @@ impl ClientActor { block_production_started: false, doomslug_timer_next_attempt: now, sync_timer_next_attempt: now, - chunk_request_retry_next_attempt: now, sync_started: false, state_parts_task_scheduler: create_sync_job_scheduler::( sync_jobs_actor_addr.clone(), @@ -557,75 +540,6 @@ impl Handler> for ClientActor { } } -impl Handler> for ClientActor { - type Result = (); - - fn handle( - &mut self, - msg: WithSpanContext, - ctx: &mut Context, - ) { - self.wrap(msg, ctx, "RecvPartialEncodedChunkRequest", |this, msg| { - let RecvPartialEncodedChunkRequest(part_request_msg, route_back) = msg; - this.client - .shards_mgr - .process_partial_encoded_chunk_request(part_request_msg, route_back); - }) - } -} - -impl Handler> for ClientActor { - type Result = (); - - fn handle( - &mut self, - msg: WithSpanContext, - ctx: &mut Context, - ) { - self.wrap(msg, ctx, "RecvPartialEncodedChunkResponse", |this, msg| { - let RecvPartialEncodedChunkResponse(response, time) = msg; - PARTIAL_ENCODED_CHUNK_RESPONSE_DELAY.observe(time.elapsed().as_secs_f64()); - let _ = this.client.shards_mgr.process_partial_encoded_chunk_response(response); - }); - } -} - -impl Handler> for ClientActor { - type Result = (); - - fn handle(&mut self, msg: WithSpanContext, ctx: &mut Context) { - self.wrap(msg, ctx, "RecvPartialEncodedChunk", |this, msg| { - let RecvPartialEncodedChunk(partial_encoded_chunk) = msg; - this.client.block_production_info.record_chunk_collected( - partial_encoded_chunk.height_created(), - partial_encoded_chunk.shard_id(), - ); - let _ = - this.client.shards_mgr.process_partial_encoded_chunk(partial_encoded_chunk.into()); - }) - } -} - -impl Handler> for ClientActor { - type Result = (); - - fn handle( - &mut self, - msg: WithSpanContext, - ctx: &mut Context, - ) { - self.wrap(msg, ctx, "RectPartialEncodedChunkForward", |this, msg| { - let RecvPartialEncodedChunkForward(forward) = msg; - match this.client.shards_mgr.process_partial_encoded_chunk_forward(forward) { - Ok(_) => {} - // Unknown chunk is normal if we get parts before the header - Err(near_chunks::Error::UnknownChunk) => (), - Err(err) => error!(target: "client", "Error processing forwarded chunk: {}", err), - } - }) - } -} - impl Handler> for ClientActor { type Result = (); @@ -1225,27 +1139,8 @@ impl ClientActor { .to_std() .unwrap_or(delay), ); - - self.chunk_request_retry_next_attempt = self.run_timer( - self.client.config.chunk_request_retry_period, - self.chunk_request_retry_next_attempt, - ctx, - |act, _ctx| { - if let Ok(header_head) = act.client.chain.header_head() { - act.client.shards_mgr.resend_chunk_requests(&header_head) - } - }, - "resend_chunk_requests", - ); - timer.observe_duration(); - core::cmp::min( - delay, - self.chunk_request_retry_next_attempt - .signed_duration_since(now) - .to_std() - .unwrap_or(delay), - ) + delay } /// "Unfinished" blocks means that blocks that client has started the processing and haven't @@ -1999,6 +1894,7 @@ pub fn start_client( runtime_adapter: Arc, node_id: PeerId, network_adapter: Arc, + shards_manager_adapter: Arc, validator_signer: Option>, telemetry_actor: Addr, sender: Option>, @@ -2007,18 +1903,27 @@ pub fn start_client( ) -> (Addr, ArbiterHandle) { let client_arbiter = Arbiter::new(); let client_arbiter_handle = client_arbiter.handle(); + wait_until_genesis(&chain_genesis.time); + let client = Client::new( + client_config.clone(), + chain_genesis, + runtime_adapter, + network_adapter.clone(), + shards_manager_adapter, + validator_signer.clone(), + true, + random_seed_from_thread(), + ) + .unwrap(); let client_addr = ClientActor::start_in_arbiter(&client_arbiter_handle, move |ctx| { ClientActor::new( + client, ctx.address(), client_config, - chain_genesis, - runtime_adapter, node_id, network_adapter, validator_signer, telemetry_actor, - true, - random_seed_from_thread(), ctx, sender, adv, diff --git a/chain/client/src/metrics.rs b/chain/client/src/metrics.rs index 3feea635bb5..b127bf55dfe 100644 --- a/chain/client/src/metrics.rs +++ b/chain/client/src/metrics.rs @@ -174,14 +174,6 @@ pub(crate) static CHUNK_DROPPED_BECAUSE_OF_BANNED_CHUNK_PRODUCER: Lazy = Lazy::new(|| { - try_create_histogram( - "near_partial_encoded_chunk_response_delay", - "Delay between when a partial encoded chunk response is sent from PeerActor and when it is received by ClientActor", - ) - .unwrap() -}); - pub(crate) static CLIENT_MESSAGES_COUNT: Lazy = Lazy::new(|| { try_create_int_counter_vec( "near_client_messages_count", diff --git a/chain/client/src/test_utils.rs b/chain/client/src/test_utils.rs index 6de5cde215e..a2337c29adb 100644 --- a/chain/client/src/test_utils.rs +++ b/chain/client/src/test_utils.rs @@ -8,6 +8,11 @@ use std::time::Duration; use actix::{Actor, Addr, AsyncContext, Context}; use chrono::DateTime; use futures::{future, FutureExt}; +use near_chunks::shards_manager_actor::start_shards_manager; +use near_chunks::ShardsManager; +use near_network::shards_manager::{ + ShardsManagerAdapterForNetwork, ShardsManagerRequestFromNetwork, +}; use near_primitives::test_utils::create_test_signer; use num_rational::Ratio; use once_cell::sync::OnceCell; @@ -26,13 +31,14 @@ use near_chain::{ RuntimeWithEpochManagerAdapter, }; use near_chain_configs::ClientConfig; +use near_chunks::adapter::{ShardsManagerAdapterForClient, ShardsManagerRequestFromClient}; use near_chunks::client::{ClientAdapterForShardsManager, ShardsManagerResponse}; -use near_chunks::test_utils::MockClientAdapterForShardsManager; +use near_chunks::test_utils::{MockClientAdapterForShardsManager, SynchronousShardsManagerAdapter}; use near_client_primitives::types::Error; use near_crypto::{InMemorySigner, KeyType, PublicKey}; use near_network::test_utils::MockPeerManagerAdapter; use near_network::types::{ - AccountOrPeerIdOrHash, HighestHeightPeerInfo, PartialEncodedChunkRequestMsg, + AccountOrPeerIdOrHash, HighestHeightPeerInfo, MsgRecipient, PartialEncodedChunkRequestMsg, PartialEncodedChunkResponseMsg, PeerInfo, PeerType, }; use near_network::types::{BlockInfo, PeerChainInfo}; @@ -72,9 +78,8 @@ use near_telemetry::TelemetryActor; use crate::adapter::{ AnnounceAccountRequest, BlockApproval, BlockHeadersRequest, BlockHeadersResponse, BlockRequest, - BlockResponse, ProcessTxResponse, RecvPartialEncodedChunk, RecvPartialEncodedChunkForward, - RecvPartialEncodedChunkRequest, RecvPartialEncodedChunkResponse, SetNetworkInfo, - StateRequestHeader, StateRequestPart, StateResponse, + BlockResponse, ProcessTxResponse, SetNetworkInfo, StateRequestHeader, StateRequestPart, + StateResponse, }; pub struct PeerManagerMock { @@ -198,11 +203,15 @@ pub fn setup( transaction_validity_period: NumBlocks, genesis_time: DateTime, ctx: &Context, -) -> (Block, ClientActor, Addr) { +) -> (Block, ClientActor, Addr, Arc) { let store = create_test_store(); let num_validator_seats = vs.all_block_producers().count() as NumSeats; - let runtime = - Arc::new(KeyValueRuntime::new_with_validators_and_no_gc(store, vs, epoch_length, archive)); + let runtime = Arc::new(KeyValueRuntime::new_with_validators_and_no_gc( + store.clone(), + vs, + epoch_length, + archive, + )); let chain_genesis = ChainGenesis { time: genesis_time, height: 0, @@ -252,24 +261,42 @@ pub fn setup( adv.clone(), ); - let client = ClientActor::new( + let (shards_manager_addr, _) = start_shards_manager( + runtime.clone(), + network_adapter.clone(), + Arc::new(ctx.address()), + Some(account_id.clone()), + store.clone(), + config.chunk_request_retry_period, + ); + let shards_manager_adapter = Arc::new(shards_manager_addr); + + let client = Client::new( + config.clone(), + chain_genesis, + runtime.clone(), + network_adapter.clone(), + shards_manager_adapter.clone(), + Some(signer.clone()), + enable_doomslug, + TEST_SEED, + ) + .unwrap(); + let client_actor = ClientActor::new( + client, ctx.address(), config, - chain_genesis, - runtime, PeerId::new(PublicKey::empty(KeyType::ED25519)), network_adapter, Some(signer), telemetry, - enable_doomslug, - TEST_SEED, ctx, None, adv, None, ) .unwrap(); - (genesis_block, client, view_client_addr) + (genesis_block, client_actor, view_client_addr, shards_manager_adapter.clone()) } pub fn setup_only_view( @@ -353,7 +380,7 @@ pub fn setup_mock( Addr, ) -> PeerManagerMessageResponse, >, -) -> (Addr, Addr) { +) -> ActorHandlesForTesting { setup_mock_with_validity_period_and_no_epoch_sync( validators, account_id, @@ -377,12 +404,13 @@ pub fn setup_mock_with_validity_period_and_no_epoch_sync( ) -> PeerManagerMessageResponse, >, transaction_validity_period: NumBlocks, -) -> (Addr, Addr) { +) -> ActorHandlesForTesting { let network_adapter = Arc::new(NetworkRecipient::default()); let mut vca: Option> = None; + let mut sma: Option> = None; let client_addr = ClientActor::create(|ctx: &mut Context| { let vs = ValidatorSchedule::new().block_producers_per_epoch(vec![validators]); - let (_, client, view_client_addr) = setup( + let (_, client, view_client_addr, shards_manager_adapter) = setup( vs, 5, account_id, @@ -398,6 +426,7 @@ pub fn setup_mock_with_validity_period_and_no_epoch_sync( ctx, ); vca = Some(view_client_addr); + sma = Some(shards_manager_adapter); client }); let client_addr1 = client_addr.clone(); @@ -408,7 +437,11 @@ pub fn setup_mock_with_validity_period_and_no_epoch_sync( network_adapter.set_recipient(network_actor); - (client_addr, vca.unwrap()) + ActorHandlesForTesting { + client_actor: client_addr, + view_client_actor: vca.unwrap(), + shards_manager_adapter: sma.unwrap(), + } } pub struct BlockStats { @@ -510,8 +543,15 @@ impl BlockStats { } } +#[derive(Clone)] +pub struct ActorHandlesForTesting { + pub client_actor: Addr, + pub view_client_actor: Addr, + pub shards_manager_adapter: Arc, +} + fn send_chunks( - connectors: &[(Addr, Addr)], + connectors: &[ActorHandlesForTesting], recipients: I, target: T, drop_chunks: bool, @@ -519,12 +559,12 @@ fn send_chunks( ) where T: Eq, I: Iterator, - F: Fn(&Addr), + F: Fn(&Arc), { for (i, name) in recipients { if name == target { if !drop_chunks || !thread_rng().gen_ratio(1, 5) { - send_to(&connectors[i].0); + send_to(&connectors[i].shards_manager_adapter); } } } @@ -585,14 +625,14 @@ pub fn setup_mock_all_validators( peer_manager_mock: Box< dyn FnMut( // Peer validators - &[(Addr, Addr)], + &[ActorHandlesForTesting], // Validator that sends the message AccountId, // The message itself &PeerManagerMessageRequest, ) -> (PeerManagerMessageResponse, /* perform default */ bool), >, -) -> (Block, Vec<(Addr, Addr)>, Arc>) { +) -> (Block, Vec, Arc>) { let peer_manager_mock = Arc::new(RwLock::new(peer_manager_mock)); let validators = vs.all_validators().cloned().collect::>(); let key_pairs = key_pairs; @@ -601,8 +641,7 @@ pub fn setup_mock_all_validators( let genesis_time = Clock::utc(); let mut ret = vec![]; - let connectors: Arc, Addr)>>> = - Default::default(); + let connectors: Arc>> = Default::default(); let announced_accounts = Arc::new(RwLock::new(HashSet::new())); let genesis_block = Arc::new(RwLock::new(None)); @@ -617,6 +656,7 @@ pub fn setup_mock_all_validators( let vs = vs.clone(); let block_stats1 = block_stats.clone(); let mut view_client_addr_slot = None; + let mut shards_manager_adapter_slot = None; let validators_clone2 = validators.clone(); let genesis_block1 = genesis_block.clone(); let key_pairs = key_pairs.clone(); @@ -708,8 +748,8 @@ pub fn setup_mock_all_validators( block_stats2.check_stats(false); } - for (client, _) in connectors1 { - client.do_send( + for actor_handles in connectors1 { + actor_handles.client_actor.do_send( BlockResponse { block: block.clone(), peer_id: PeerInfo::random().id, @@ -731,67 +771,66 @@ pub fn setup_mock_all_validators( .insert(*block.header().hash(), block.header().height()); } NetworkRequests::PartialEncodedChunkRequest { target, request, .. } => { - let create_msg = || { - RecvPartialEncodedChunkRequest(request.clone(), my_address) - .with_span_context() - }; send_chunks( connectors1, validators_clone2.iter().map(|s| Some(s.clone())).enumerate(), target.account_id.as_ref().map(|s| s.clone()), drop_chunks, - |c| c.do_send(create_msg()), + |c| { + c.process_partial_encoded_chunk_request( + request.clone(), + my_address, + ) + }, ); } NetworkRequests::PartialEncodedChunkResponse { route_back, response } => { - let create_msg = || { - RecvPartialEncodedChunkResponse(response.clone(), Clock::instant()) - .with_span_context() - }; send_chunks( connectors1, addresses.iter().enumerate(), route_back, drop_chunks, - |c| c.do_send(create_msg()), + |c| { + c.process_partial_encoded_chunk_response( + response.clone(), + Instant::now(), + ) + }, ); } NetworkRequests::PartialEncodedChunkMessage { account_id, partial_encoded_chunk, } => { - let create_msg = || { - RecvPartialEncodedChunk(partial_encoded_chunk.clone().into()) - .with_span_context() - }; send_chunks( connectors1, validators_clone2.iter().cloned().enumerate(), account_id.clone(), drop_chunks, - |c| c.do_send(create_msg()), + |c| { + c.process_partial_encoded_chunk( + partial_encoded_chunk.clone().into(), + ) + }, ); } NetworkRequests::PartialEncodedChunkForward { account_id, forward } => { - let create_msg = || { - RecvPartialEncodedChunkForward(forward.clone()).with_span_context() - }; send_chunks( connectors1, validators_clone2.iter().cloned().enumerate(), account_id.clone(), drop_chunks, - |c| c.do_send(create_msg()), + |c| c.process_partial_encoded_chunk_forward(forward.clone()), ); } NetworkRequests::BlockRequest { hash, peer_id } => { for (i, peer_info) in key_pairs.iter().enumerate() { let peer_id = peer_id.clone(); if peer_info.id == peer_id { - let me = connectors1[my_ord].0.clone(); + let me = connectors1[my_ord].client_actor.clone(); actix::spawn( connectors1[i] - .1 + .view_client_actor .send(BlockRequest(*hash).with_span_context()) .then(move |response| { let response = response.unwrap(); @@ -818,10 +857,10 @@ pub fn setup_mock_all_validators( for (i, peer_info) in key_pairs.iter().enumerate() { let peer_id = peer_id.clone(); if peer_info.id == peer_id { - let me = connectors1[my_ord].0.clone(); + let me = connectors1[my_ord].client_actor.clone(); actix::spawn( connectors1[i] - .1 + .view_client_actor .send( BlockHeadersRequest(hashes.clone()) .with_span_context(), @@ -854,10 +893,10 @@ pub fn setup_mock_all_validators( }; for (i, name) in validators_clone2.iter().enumerate() { if name == target_account_id { - let me = connectors1[my_ord].0.clone(); + let me = connectors1[my_ord].client_actor.clone(); actix::spawn( connectors1[i] - .1 + .view_client_actor .send( StateRequestHeader { shard_id: *shard_id, @@ -891,10 +930,10 @@ pub fn setup_mock_all_validators( }; for (i, name) in validators_clone2.iter().enumerate() { if name == target_account_id { - let me = connectors1[my_ord].0.clone(); + let me = connectors1[my_ord].client_actor.clone(); actix::spawn( connectors1[i] - .1 + .view_client_actor .send( StateRequestPart { shard_id: *shard_id, @@ -920,7 +959,7 @@ pub fn setup_mock_all_validators( NetworkRequests::StateResponse { route_back, response } => { for (i, address) in addresses.iter().enumerate() { if route_back == address { - connectors1[i].0.do_send( + connectors1[i].client_actor.do_send( StateResponse(Box::new(response.clone())) .with_span_context(), ); @@ -935,8 +974,8 @@ pub fn setup_mock_all_validators( ); if aa.get(&key).is_none() { aa.insert(key); - for (_, view_client) in connectors1 { - view_client.do_send( + for actor_handles in connectors1 { + actor_handles.view_client_actor.do_send( AnnounceAccountRequest(vec![( announce_account.clone(), None, @@ -967,7 +1006,7 @@ pub fn setup_mock_all_validators( if do_propagate { for (i, name) in validators_clone2.iter().enumerate() { if name == &approval_message.target { - connectors1[i].0.do_send( + connectors1[i].client_actor.do_send( BlockApproval(approval.clone(), my_key_pair.id.clone()) .with_span_context(), ); @@ -1019,7 +1058,7 @@ pub fn setup_mock_all_validators( .start(); let network_adapter = NetworkRecipient::default(); network_adapter.set_recipient(pm); - let (block, client, view_client_addr) = setup( + let (block, client, view_client_addr, shards_manager_adapter) = setup( vs, epoch_length, _account_id, @@ -1035,17 +1074,22 @@ pub fn setup_mock_all_validators( ctx, ); view_client_addr_slot = Some(view_client_addr); + shards_manager_adapter_slot = Some(shards_manager_adapter); *genesis_block1.write().unwrap() = Some(block); client }); - ret.push((client_addr, view_client_addr_slot.unwrap())); + ret.push(ActorHandlesForTesting { + client_actor: client_addr, + view_client_actor: view_client_addr_slot.unwrap(), + shards_manager_adapter: shards_manager_adapter_slot.unwrap(), + }); } hash_to_height.write().unwrap().insert(CryptoHash::default(), 0); hash_to_height .write() .unwrap() .insert(*genesis_block.read().unwrap().as_ref().unwrap().header().clone().hash(), 0); - connectors.set(ret.clone()).unwrap(); + connectors.set(ret.clone()).ok().unwrap(); let value = genesis_block.read().unwrap(); (value.clone().unwrap(), ret, block_stats) } @@ -1056,7 +1100,7 @@ pub fn setup_no_network( account_id: AccountId, skip_sync_wait: bool, enable_doomslug: bool, -) -> (Addr, Addr) { +) -> ActorHandlesForTesting { setup_no_network_with_validity_period_and_no_epoch_sync( validators, account_id, @@ -1072,7 +1116,7 @@ pub fn setup_no_network_with_validity_period_and_no_epoch_sync( skip_sync_wait: bool, transaction_validity_period: NumBlocks, enable_doomslug: bool, -) -> (Addr, Addr) { +) -> ActorHandlesForTesting { setup_mock_with_validity_period_and_no_epoch_sync( validators, account_id, @@ -1090,7 +1134,7 @@ pub fn setup_client_with_runtime( account_id: Option, enable_doomslug: bool, network_adapter: Arc, - client_adapter: Arc, + shards_manager_adapter: Arc, chain_genesis: ChainGenesis, runtime_adapter: Arc, rng_seed: RngSeed, @@ -1107,7 +1151,7 @@ pub fn setup_client_with_runtime( chain_genesis, runtime_adapter, network_adapter, - client_adapter, + shards_manager_adapter.for_client(), validator_signer, enable_doomslug, rng_seed, @@ -1123,7 +1167,7 @@ pub fn setup_client( account_id: Option, enable_doomslug: bool, network_adapter: Arc, - client_adapter: Arc, + shards_manager_adapter: Arc, chain_genesis: ChainGenesis, rng_seed: RngSeed, archive: bool, @@ -1137,7 +1181,76 @@ pub fn setup_client( account_id, enable_doomslug, network_adapter, + shards_manager_adapter, + chain_genesis, + runtime_adapter, + rng_seed, + archive, + save_trie_changes, + ) +} + +pub fn setup_synchronous_shards_manager( + account_id: Option, + client_adapter: Arc, + network_adapter: Arc, + runtime_adapter: Arc, + chain_genesis: &ChainGenesis, +) -> Arc { + // Initialize the chain, to make sure that if the store is empty, we write the genesis + // into the store, and as a short cut to get the parameters needed to instantiate + // ShardsManager. This way we don't have to wait to construct the Client first. + // TODO(#8324): This should just be refactored so that we can construct Chain first + // before anything else. + let chain = Chain::new( + runtime_adapter.clone(), + chain_genesis, + DoomslugThresholdMode::TwoThirds, // irrelevant + ChainConfig { save_trie_changes: true, background_migration_threads: 1 }, // irrelevant + ) + .unwrap(); + let chain_head = chain.head().unwrap(); + let chain_header_head = chain.header_head().unwrap(); + let shards_manager = ShardsManager::new( + account_id, + runtime_adapter, + network_adapter, client_adapter, + chain.store().new_read_only_chunks_store(), + chain_head, + chain_header_head, + ); + Arc::new(SynchronousShardsManagerAdapter::new(shards_manager)) +} + +pub fn setup_client_with_synchronous_shards_manager( + store: Store, + vs: ValidatorSchedule, + account_id: Option, + enable_doomslug: bool, + network_adapter: Arc, + client_adapter: Arc, + chain_genesis: ChainGenesis, + rng_seed: RngSeed, + archive: bool, + save_trie_changes: bool, +) -> Client { + let num_validator_seats = vs.all_block_producers().count() as NumSeats; + let runtime_adapter = + Arc::new(KeyValueRuntime::new_with_validators(store, vs, chain_genesis.epoch_length)); + let shards_manager_adapter = setup_synchronous_shards_manager( + account_id.clone(), + client_adapter, + network_adapter.clone(), + runtime_adapter.clone(), + &chain_genesis, + ); + setup_client_with_runtime( + num_validator_seats, + account_id, + enable_doomslug, + network_adapter, + shards_manager_adapter, chain_genesis, runtime_adapter, rng_seed, @@ -1146,6 +1259,28 @@ pub fn setup_client( ) } +/// A combined trait bound for both the client side and network side of the ShardsManager API. +pub trait ShardsManagerAdapterForTest: + ShardsManagerAdapterForClient + ShardsManagerAdapterForNetwork +{ + fn for_client(&self) -> Arc; + fn for_network(&self) -> Arc; +} + +impl< + A: MsgRecipient + + MsgRecipient + + Clone, + > ShardsManagerAdapterForTest for A +{ + fn for_client(&self) -> Arc { + Arc::new(self.clone()) + } + fn for_network(&self) -> Arc { + Arc::new(self.clone()) + } +} + /// An environment for writing integration tests with multiple clients. /// This environment can simulate near nodes without network and it can be configured to use different runtimes. pub struct TestEnv { @@ -1153,6 +1288,7 @@ pub struct TestEnv { pub validators: Vec, pub network_adapters: Vec>, pub client_adapters: Vec>, + pub shards_manager_adapters: Vec>, pub clients: Vec, account_to_client_index: HashMap, paused_blocks: Arc>>>>, @@ -1284,73 +1420,78 @@ impl TestEnvBuilder { let validators = self.validators; let num_validators = validators.len(); let seeds = self.seeds; - let network_adapters = self - .network_adapters - .unwrap_or_else(|| (0..num_clients).map(|_| Arc::new(Default::default())).collect()); - let client_adapters = (0..num_clients) - .map(|_| Arc::new(MockClientAdapterForShardsManager::default())) - .collect::>(); - assert_eq!(clients.len(), network_adapters.len()); - let clients = match self.runtime_adapters { - None => clients - .into_iter() - .zip(network_adapters.iter()) - .zip(client_adapters.iter()) - .map(|((account_id, network_adapter), client_adapter)| { - let rng_seed = match seeds.get(&account_id) { - Some(seed) => *seed, - None => TEST_SEED, - }; + let runtime_adapters = match self.runtime_adapters { + Some(runtime_adapters) => { + assert_eq!(runtime_adapters.len(), num_clients); + runtime_adapters + } + None => (0..num_clients) + .map(|_| { let vs = ValidatorSchedule::new() .block_producers_per_epoch(vec![validators.clone()]); - setup_client( + Arc::new(KeyValueRuntime::new_with_validators( create_test_store(), vs, - Some(account_id), - false, - network_adapter.clone(), - client_adapter.clone(), - chain_genesis.clone(), - rng_seed, - self.archive, - self.save_trie_changes, - ) + chain_genesis.epoch_length, + )) as Arc }) .collect(), - Some(runtime_adapters) => { - assert!(clients.len() == runtime_adapters.len()); - - clients - .into_iter() - .zip((&network_adapters).iter()) - .zip(runtime_adapters.into_iter().zip(client_adapters.iter())) - .map(|((account_id, network_adapter), (runtime_adapter, client_adapter))| { - let rng_seed = match seeds.get(&account_id) { - Some(seed) => *seed, - None => TEST_SEED, - }; - setup_client_with_runtime( - u64::try_from(num_validators).unwrap(), - Some(account_id), - false, - network_adapter.clone(), - client_adapter.clone(), - chain_genesis.clone(), - runtime_adapter, - rng_seed, - self.archive, - self.save_trie_changes, - ) - }) - .collect() + }; + let network_adapters = match self.network_adapters { + Some(network_adapters) => { + assert_eq!(network_adapters.len(), num_clients); + network_adapters } + None => (0..num_clients).map(|_| Arc::new(Default::default())).collect(), }; + let client_adapters = (0..num_clients) + .map(|_| Arc::new(MockClientAdapterForShardsManager::default())) + .collect::>(); + let shards_manager_adapters = (0..num_clients) + .map(|i| { + let runtime_adapter = runtime_adapters[i].clone(); + let network_adapter = network_adapters[i].clone(); + let client_adapter = client_adapters[i].clone(); + setup_synchronous_shards_manager( + Some(clients[i].clone()), + client_adapter, + network_adapter, + runtime_adapter, + &chain_genesis, + ) + }) + .collect::>(); + let clients = (0..num_clients) + .map(|i| { + let account_id = clients[i].clone(); + let network_adapter = network_adapters[i].clone(); + let shards_manager_adapter = shards_manager_adapters[i].clone(); + let runtime_adapter = runtime_adapters[i].clone(); + let rng_seed = match seeds.get(&account_id) { + Some(seed) => *seed, + None => TEST_SEED, + }; + setup_client_with_runtime( + u64::try_from(num_validators).unwrap(), + Some(account_id), + false, + network_adapter, + shards_manager_adapter, + chain_genesis.clone(), + runtime_adapter, + rng_seed, + self.archive, + self.save_trie_changes, + ) + }) + .collect(); TestEnv { chain_genesis, validators, network_adapters, client_adapters, + shards_manager_adapters, clients, account_to_client_index: self .clients @@ -1428,6 +1569,10 @@ impl TestEnv { &mut self.clients[self.account_to_client_index[account_id]] } + pub fn shards_manager(&self, account: &AccountId) -> &Arc { + &self.shards_manager_adapters[self.account_to_client_index[account]] + } + pub fn process_partial_encoded_chunks(&mut self) { let network_adapters = self.network_adapters.clone(); for network_adapter in network_adapters { @@ -1440,12 +1585,9 @@ impl TestEnv { }, ) = request { - self.client(&account_id) - .shards_mgr - .process_partial_encoded_chunk( - PartialEncodedChunk::from(partial_encoded_chunk).into(), - ) - .unwrap(); + self.shards_manager(&account_id).process_partial_encoded_chunk( + PartialEncodedChunk::from(partial_encoded_chunk), + ); } } } @@ -1472,10 +1614,8 @@ impl TestEnv { let target_id = self.account_to_client_index[&target.account_id.unwrap()]; let response = self.get_partial_encoded_chunk_response(target_id, request); if let Some(response) = response { - self.clients[id] - .shards_mgr - .process_partial_encoded_chunk_response(response) - .unwrap(); + self.shards_manager_adapters[id] + .process_partial_encoded_chunk_response(response, Instant::now()); } } else { panic!("The request is not a PartialEncodedChunk request {:?}", request); @@ -1487,23 +1627,23 @@ impl TestEnv { id: usize, request: PartialEncodedChunkRequestMsg, ) -> Option { - let client = &mut self.clients[id]; - client - .shards_mgr + self.shards_manager_adapters[id] .process_partial_encoded_chunk_request(request.clone(), CryptoHash::default()); - - let response = self.network_adapters[id].pop_most_recent().unwrap(); - if let PeerManagerMessageRequest::NetworkRequests( - NetworkRequests::PartialEncodedChunkResponse { route_back: _, response }, - ) = response - { - Some(response) - } else { - panic!( - "did not find PartialEncodedChunkResponse from the network queue {:?}", - response - ); + let response = self.network_adapters[id].pop_most_recent(); + match response { + Some(PeerManagerMessageRequest::NetworkRequests( + NetworkRequests::PartialEncodedChunkResponse { route_back: _, response }, + )) => return Some(response), + Some(response) => { + self.network_adapters[id].put_back_most_recent(response); + } + None => {} } + + panic!( + "Failed to process PartialEncodedChunkRequest from shards manager {}: {:?}", + id, request + ); } pub fn process_shards_manager_responses(&mut self, id: usize) -> bool { @@ -1655,7 +1795,7 @@ impl TestEnv { Some(self.get_client_id(idx).clone()), false, self.network_adapters[idx].clone(), - self.client_adapters[idx].clone(), + self.shards_manager_adapters[idx].clone(), self.chain_genesis.clone(), runtime_adapter, rng_seed, diff --git a/chain/client/src/tests/bug_repros.rs b/chain/client/src/tests/bug_repros.rs index 4d12fca60f1..739894a5207 100644 --- a/chain/client/src/tests/bug_repros.rs +++ b/chain/client/src/tests/bug_repros.rs @@ -5,13 +5,13 @@ use std::cmp::max; use std::collections::HashMap; use std::sync::{Arc, RwLock}; -use actix::{Addr, System}; +use actix::System; use futures::FutureExt; use rand::{thread_rng, Rng}; -use crate::adapter::{BlockApproval, BlockResponse, ProcessTxRequest, RecvPartialEncodedChunk}; -use crate::test_utils::setup_mock_all_validators; -use crate::{ClientActor, GetBlock, ViewClientActor}; +use crate::adapter::{BlockApproval, BlockResponse, ProcessTxRequest}; +use crate::test_utils::{setup_mock_all_validators, ActorHandlesForTesting}; +use crate::GetBlock; use near_actix_test_utils::run_actix; use near_chain::test_utils::{account_id_to_shard_id, ValidatorSchedule}; use near_crypto::{InMemorySigner, KeyType}; @@ -29,8 +29,7 @@ use near_primitives::transaction::SignedTransaction; fn repro_1183() { init_test_logger(); run_actix(async { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let vs = ValidatorSchedule::new() .num_shards(4) @@ -70,8 +69,8 @@ fn repro_1183() { let mut delayed_one_parts = delayed_one_parts.write().unwrap(); if let Some(last_block) = last_block.clone() { - for (client, _) in connectors1.write().unwrap().iter() { - client.do_send( + for actor_handles in connectors1.write().unwrap().iter() { + actor_handles.client_actor.do_send( BlockResponse { block: last_block.clone(), peer_id: PeerInfo::random().id, @@ -90,12 +89,11 @@ fn repro_1183() { { for (i, name) in validators.iter().enumerate() { if name == account_id { - connectors1.write().unwrap()[i].0.do_send( - RecvPartialEncodedChunk( + connectors1.write().unwrap()[i] + .shards_manager_adapter + .process_partial_encoded_chunk( partial_encoded_chunk.clone().into(), - ) - .with_span_context(), - ); + ); } } } else { @@ -108,7 +106,7 @@ fn repro_1183() { for to in ["test1", "test2", "test3", "test4"].iter() { let (from, to) = (from.parse().unwrap(), to.parse().unwrap()); connectors1.write().unwrap()[account_id_to_shard_id(&from, 4) as usize] - .0 + .client_actor .do_send( ProcessTxRequest { transaction: SignedTransaction::send_money( @@ -208,9 +206,9 @@ fn test_sync_from_archival_node() { if *largest_height.read().unwrap() <= 30 { match msg { NetworkRequests::Block { block } => { - for (i, (client, _)) in conns.iter().enumerate() { + for (i, actor_handles) in conns.iter().enumerate() { if i != 3 { - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block.clone(), peer_id: PeerInfo::random().id, @@ -226,9 +224,9 @@ fn test_sync_from_archival_node() { (NetworkResponses::NoResponse.into(), false) } NetworkRequests::Approval { approval_message } => { - for (i, (client, _)) in conns.clone().into_iter().enumerate() { + for (i, actor_handles) in conns.clone().into_iter().enumerate() { if i != 3 { - client.do_send( + actor_handles.client_actor.do_send( BlockApproval( approval_message.approval.clone(), PeerInfo::random().id, @@ -246,7 +244,7 @@ fn test_sync_from_archival_node() { panic!("incorrect rebroadcasting of blocks"); } for (_, block) in blocks.write().unwrap().drain() { - conns[3].0.do_send( + conns[3].client_actor.do_send( BlockResponse { block, peer_id: PeerInfo::random().id, @@ -302,7 +300,9 @@ fn test_long_gap_between_blocks() { -> (PeerManagerMessageResponse, bool) { match msg.as_network_requests_ref() { NetworkRequests::Approval { approval_message } => { - let actor = conns[1].1.send(GetBlock::latest().with_span_context()); + let actor = conns[1] + .view_client_actor + .send(GetBlock::latest().with_span_context()); let actor = actor.then(move |res| { let res = res.unwrap().unwrap(); if res.header.height > target_height { diff --git a/chain/client/src/tests/catching_up.rs b/chain/client/src/tests/catching_up.rs index 0b5dd56720b..886914f90a3 100644 --- a/chain/client/src/tests/catching_up.rs +++ b/chain/client/src/tests/catching_up.rs @@ -7,8 +7,8 @@ use borsh::{BorshDeserialize, BorshSerialize}; use futures::{future, FutureExt}; use crate::adapter::ProcessTxRequest; -use crate::test_utils::setup_mock_all_validators; -use crate::{ClientActor, Query, ViewClientActor}; +use crate::test_utils::{setup_mock_all_validators, ActorHandlesForTesting}; +use crate::{ClientActor, Query}; use near_actix_test_utils::run_actix; use near_chain::test_utils::{account_id_to_shard_id, ValidatorSchedule}; use near_chain_configs::TEST_STATE_SYNC_TIMEOUT; @@ -133,8 +133,7 @@ fn test_catchup_receipts_sync_distant_epoch() { fn test_catchup_receipts_sync_common(wait_till: u64, send: u64, sync_hold: bool) { init_integration_logger(); run_actix(async move { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let (vs, key_pairs) = get_validators_and_key_pairs(); let archive = vec![true; vs.all_block_producers().count()]; @@ -191,7 +190,7 @@ fn test_catchup_receipts_sync_common(wait_till: u64, send: u64, sync_hold: bool) ); for i in 0..16 { send_tx( - &connectors1.write().unwrap()[i].0, + &connectors1.write().unwrap()[i].client_actor, account_from.clone(), account_to.clone(), 111, @@ -320,15 +319,16 @@ fn test_catchup_receipts_sync_common(wait_till: u64, send: u64, sync_hold: bool) } if block.header().height() == wait_till + 10 { for i in 0..16 { - let actor = connectors1.write().unwrap()[i].1.send( - Query::new( - BlockReference::latest(), - QueryRequest::ViewAccount { - account_id: account_to.clone(), - }, - ) - .with_span_context(), - ); + let actor = + connectors1.write().unwrap()[i].view_client_actor.send( + Query::new( + BlockReference::latest(), + QueryRequest::ViewAccount { + account_id: account_to.clone(), + }, + ) + .with_span_context(), + ); let actor = actor.then(move |res| { let res_inner = res.unwrap(); if let Ok(query_response) = res_inner { @@ -408,8 +408,7 @@ fn test_catchup_random_single_part_sync_height_6() { fn test_catchup_random_single_part_sync_common(skip_15: bool, non_zero: bool, height: u64) { init_integration_logger(); run_actix(async move { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let (vs, key_pairs) = get_validators_and_key_pairs(); let vs = vs.validator_groups(2); @@ -489,7 +488,7 @@ fn test_catchup_random_single_part_sync_common(skip_15: bool, non_zero: bool, he ); for conn in 0..validators.len() { send_tx( - &connectors1.write().unwrap()[conn].0, + &connectors1.write().unwrap()[conn].client_actor, validator1.clone(), validator2.clone(), amount, @@ -516,15 +515,16 @@ fn test_catchup_random_single_part_sync_common(skip_15: bool, non_zero: bool, he for j in 0..16 { let amounts1 = amounts.clone(); let validator = validators[j].clone(); - let actor = connectors1.write().unwrap()[i].1.send( - Query::new( - BlockReference::latest(), - QueryRequest::ViewAccount { - account_id: validators[j].clone(), - }, - ) - .with_span_context(), - ); + let actor = + connectors1.write().unwrap()[i].view_client_actor.send( + Query::new( + BlockReference::latest(), + QueryRequest::ViewAccount { + account_id: validators[j].clone(), + }, + ) + .with_span_context(), + ); let actor = actor.then(move |res| { let res_inner = res.unwrap(); if let Ok(query_response) = res_inner { @@ -606,8 +606,7 @@ fn test_catchup_random_single_part_sync_common(skip_15: bool, non_zero: bool, he fn test_catchup_sanity_blocks_produced() { init_integration_logger(); run_actix(async move { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let heights = Arc::new(RwLock::new(HashMap::new())); let heights1 = heights; @@ -680,8 +679,7 @@ enum ChunkGrievingPhases { fn test_chunk_grieving() { init_integration_logger(); run_actix(async move { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let (vs, key_pairs) = get_validators_and_key_pairs(); let archive = vec![false; vs.all_block_producers().count()]; @@ -846,8 +844,7 @@ fn test_all_chunks_accepted_common( ) { init_integration_logger(); run_actix(async move { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let (vs, key_pairs) = get_validators_and_key_pairs(); let archive = vec![false; vs.all_block_producers().count()]; diff --git a/chain/client/src/tests/chunks_management.rs b/chain/client/src/tests/chunks_management.rs index cab55e0acad..d2e2a92203d 100644 --- a/chain/client/src/tests/chunks_management.rs +++ b/chain/client/src/tests/chunks_management.rs @@ -21,13 +21,13 @@ fn test_request_chunk_restart() { part_ords: vec![0], tracking_shards: HashSet::default(), }; - let client = &mut env.clients[0]; - client.shards_mgr.process_partial_encoded_chunk_request(request.clone(), CryptoHash::default()); + env.shards_manager_adapters[0] + .process_partial_encoded_chunk_request(request.clone(), CryptoHash::default()); assert!(env.network_adapters[0].pop().is_some()); env.restart(0); - let client = &mut env.clients[0]; - client.shards_mgr.process_partial_encoded_chunk_request(request, CryptoHash::default()); + env.shards_manager_adapters[0] + .process_partial_encoded_chunk_request(request, CryptoHash::default()); let response = env.network_adapters[0].pop().unwrap().as_network_requests(); if let NetworkRequests::PartialEncodedChunkResponse { response: response_body, .. } = response { diff --git a/chain/client/src/tests/consensus.rs b/chain/client/src/tests/consensus.rs index c45c8ce341c..a5d2c490226 100644 --- a/chain/client/src/tests/consensus.rs +++ b/chain/client/src/tests/consensus.rs @@ -1,13 +1,12 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::{Arc, RwLock, RwLockWriteGuard}; -use actix::{Addr, System}; +use actix::System; use near_chain::test_utils::ValidatorSchedule; use rand::{thread_rng, Rng}; use crate::adapter::{BlockApproval, BlockResponse}; -use crate::test_utils::setup_mock_all_validators; -use crate::{ClientActor, ViewClientActor}; +use crate::test_utils::{setup_mock_all_validators, ActorHandlesForTesting}; use near_actix_test_utils::run_actix; use near_chain::Block; use near_network::types::PeerInfo; @@ -30,8 +29,7 @@ fn test_consensus_with_epoch_switches() { const HEIGHT_GOAL: u64 = 120; run_actix(async move { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let connectors1 = connectors.clone(); let validators: Vec> = [ @@ -154,7 +152,7 @@ fn test_consensus_with_epoch_switches() { } if delayed_block.header().height() <= block.header().height() + 2 { for target_ord in 0..24 { - connectors1.write().unwrap()[target_ord].0.do_send( + connectors1.write().unwrap()[target_ord].client_actor.do_send( BlockResponse { block: delayed_block.clone(), peer_id: key_pairs[0].clone().id, @@ -255,7 +253,7 @@ fn test_consensus_with_epoch_switches() { ); connectors1.write().unwrap() [epoch_id * 8 + (destination_ord + delta) % 8] - .0 + .client_actor .do_send( BlockApproval(approval, key_pairs[my_ord].id.clone()) .with_span_context(), diff --git a/chain/client/src/tests/cross_shard_tx.rs b/chain/client/src/tests/cross_shard_tx.rs index 6584b199f8b..2ed2407d9c8 100644 --- a/chain/client/src/tests/cross_shard_tx.rs +++ b/chain/client/src/tests/cross_shard_tx.rs @@ -23,7 +23,7 @@ use near_primitives::views::QueryResponseKind::ViewAccount; use near_primitives::views::{QueryRequest, QueryResponse}; use crate::adapter::{ProcessTxRequest, ProcessTxResponse}; -use crate::test_utils::{setup_mock_all_validators, BlockStats}; +use crate::test_utils::{setup_mock_all_validators, ActorHandlesForTesting, BlockStats}; use crate::{ClientActor, Query, ViewClientActor}; /// Tests that the KeyValueRuntime properly sets balances in genesis and makes them queriable @@ -32,8 +32,7 @@ fn test_keyvalue_runtime_balances() { let successful_queries = Arc::new(AtomicUsize::new(0)); init_integration_logger(); run_actix(async move { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let vs = ValidatorSchedule::new() .num_shards(4) @@ -70,7 +69,7 @@ fn test_keyvalue_runtime_balances() { let expected = (1000 + i * 100) as u128; let successful_queries2 = successful_queries.clone(); - let actor = connectors_[i].1.send( + let actor = connectors_[i].view_client_actor.send( Query::new( BlockReference::latest(), QueryRequest::ViewAccount { account_id: validators[i].clone() }, @@ -97,7 +96,7 @@ fn test_keyvalue_runtime_balances() { fn send_tx( num_validators: usize, - connectors: Arc, Addr)>>>, + connectors: Arc>>, connector_ordinal: usize, from: AccountId, to: AccountId, @@ -109,7 +108,7 @@ fn send_tx( let signer = InMemorySigner::from_seed(from.clone(), KeyType::ED25519, from.as_ref()); actix::spawn( connectors.write().unwrap()[connector_ordinal] - .0 + .client_actor .send( ProcessTxRequest { transaction: SignedTransaction::send_money( @@ -159,7 +158,7 @@ fn send_tx( fn test_cross_shard_tx_callback( res: Result, MailboxError>, account_id: AccountId, - connectors: Arc, Addr)>>>, + connectors: Arc>>, iteration: Arc, nonce: Arc, validators: Vec, @@ -193,7 +192,7 @@ fn test_cross_shard_tx_callback( let presumable_epoch1 = presumable_epoch.clone(); let actor = &connectors_[account_id_to_shard_id(&account_id, 8) as usize + (*presumable_epoch.read().unwrap() * 8) % 24] - .1; + .view_client_actor; let actor = actor.send( Query::new( BlockReference::latest(), @@ -291,7 +290,7 @@ fn test_cross_shard_tx_callback( let block_stats1 = block_stats.clone(); let actor = &connectors_[account_id_to_shard_id(&validators[i], 8) as usize + (*presumable_epoch.read().unwrap() * 8) % 24] - .1; + .view_client_actor; let actor = actor.send( Query::new( BlockReference::latest(), @@ -345,7 +344,7 @@ fn test_cross_shard_tx_callback( let presumable_epoch1 = presumable_epoch.clone(); let actor = &connectors_[account_id_to_shard_id(&account_id, 8) as usize + (*presumable_epoch.read().unwrap() * 8) % 24] - .1; + .view_client_actor; let actor = actor.send( Query::new( BlockReference::latest(), @@ -403,8 +402,7 @@ fn test_cross_shard_tx_common( ) { init_integration_logger(); run_actix(async move { - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let vs = ValidatorSchedule::new() .num_shards(8) @@ -489,7 +487,7 @@ fn test_cross_shard_tx_common( let block_stats1 = block_stats.clone(); let actor = &connectors_[account_id_to_shard_id(&validators[i], 8) as usize + *presumable_epoch.read().unwrap() * 8] - .1; + .view_client_actor; let actor = actor.send( Query::new( BlockReference::latest(), diff --git a/chain/client/src/tests/maintenance_windows.rs b/chain/client/src/tests/maintenance_windows.rs index 93703fe40d3..4fadca5f58c 100644 --- a/chain/client/src/tests/maintenance_windows.rs +++ b/chain/client/src/tests/maintenance_windows.rs @@ -12,9 +12,9 @@ use near_o11y::WithSpanContextExt; fn test_get_maintenance_windows_for_validator() { init_test_logger(); run_actix(async { - let (_, view_client) = + let actor_handles = setup_no_network(vec!["test".parse().unwrap()], "other".parse().unwrap(), true, true); - let actor = view_client.send( + let actor = actor_handles.view_client_actor.send( GetMaintenanceWindows { account_id: "test".parse().unwrap() }.with_span_context(), ); @@ -41,9 +41,9 @@ fn test_get_maintenance_windows_for_validator() { fn test_get_maintenance_windows_for_not_validator() { init_test_logger(); run_actix(async { - let (_, view_client) = + let actor_handles = setup_no_network(vec!["test".parse().unwrap()], "other".parse().unwrap(), true, true); - let actor = view_client.send( + let actor = actor_handles.view_client_actor.send( GetMaintenanceWindows { account_id: "alice".parse().unwrap() }.with_span_context(), ); let actor = actor.then(|res| { diff --git a/chain/client/src/tests/query_client.rs b/chain/client/src/tests/query_client.rs index 60fa7d87230..8590971f338 100644 --- a/chain/client/src/tests/query_client.rs +++ b/chain/client/src/tests/query_client.rs @@ -37,9 +37,9 @@ use num_rational::Ratio; fn query_client() { init_test_logger(); run_actix(async { - let (_, view_client) = + let actor_handles = setup_no_network(vec!["test".parse().unwrap()], "other".parse().unwrap(), true, true); - let actor = view_client.send( + let actor = actor_handles.view_client_actor.send( Query::new( BlockReference::latest(), QueryRequest::ViewAccount { account_id: "test".parse().unwrap() }, @@ -64,10 +64,12 @@ fn query_client() { fn query_status_not_crash() { init_test_logger(); run_actix(async { - let (client, view_client) = + let actor_handles = setup_no_network(vec!["test".parse().unwrap()], "other".parse().unwrap(), true, false); let signer = create_test_signer("test"); - let actor = view_client.send(GetBlockWithMerkleTree::latest().with_span_context()); + let actor = actor_handles + .view_client_actor + .send(GetBlockWithMerkleTree::latest().with_span_context()); let actor = actor.then(move |res| { let (block, block_merkle_tree) = res.unwrap().unwrap(); let mut block_merkle_tree = PartialMerkleTree::clone(&block_merkle_tree); @@ -100,7 +102,8 @@ fn query_status_not_crash() { next_block.mut_header().resign(&signer); actix::spawn( - client + actor_handles + .client_actor .send( BlockResponse { block: next_block, @@ -111,7 +114,8 @@ fn query_status_not_crash() { ) .then(move |_| { actix::spawn( - client + actor_handles + .client_actor .send( Status { is_health_check: true, detailed: false } .with_span_context(), @@ -135,12 +139,13 @@ fn query_status_not_crash() { fn test_execution_outcome_for_chunk() { init_test_logger(); run_actix(async { - let (client, view_client) = + let actor_handles = setup_no_network(vec!["test".parse().unwrap()], "test".parse().unwrap(), true, false); let signer = InMemorySigner::from_seed("test".parse().unwrap(), KeyType::ED25519, "test"); actix::spawn(async move { - let block_hash = view_client + let block_hash = actor_handles + .view_client_actor .send(GetBlock::latest().with_span_context()) .await .unwrap() @@ -157,7 +162,8 @@ fn test_execution_outcome_for_chunk() { block_hash, ); let tx_hash = transaction.get_hash(); - let res = client + let res = actor_handles + .client_actor .send( ProcessTxRequest { transaction, is_forwarded: false, check_only: false } .with_span_context(), @@ -167,7 +173,8 @@ fn test_execution_outcome_for_chunk() { assert!(matches!(res, ProcessTxResponse::ValidTx)); actix::clock::sleep(Duration::from_millis(500)).await; - let block_hash = view_client + let block_hash = actor_handles + .view_client_actor .send( TxStatus { tx_hash, @@ -184,7 +191,8 @@ fn test_execution_outcome_for_chunk() { .transaction_outcome .block_hash; - let mut execution_outcomes_in_block = view_client + let mut execution_outcomes_in_block = actor_handles + .view_client_actor .send(GetExecutionOutcomesForBlock { block_hash }.with_span_context()) .await .unwrap() @@ -288,15 +296,16 @@ fn test_garbage_collection() { -> (PeerManagerMessageResponse, bool) { if let NetworkRequests::Block { block } = msg.as_network_requests_ref() { if block.header().height() > target_height { - let view_client_non_archival = &conns[0].1; - let view_client_archival = &conns[1].1; + let view_client_non_archival = &conns[0].view_client_actor; + let view_client_archival = &conns[1].view_client_actor; let mut tests = vec![]; // Recent data is present on all nodes (archival or not). let prev_height = block.header().prev_height().unwrap(); - for (_, view_client) in conns.iter() { + for actor_handles in conns.iter() { tests.push(actix::spawn( - view_client + actor_handles + .view_client_actor .send( Query::new( BlockReference::BlockId(BlockId::Height( diff --git a/chain/jsonrpc/jsonrpc-tests/src/lib.rs b/chain/jsonrpc/jsonrpc-tests/src/lib.rs index 84547ad2b07..941b3fe4810 100644 --- a/chain/jsonrpc/jsonrpc-tests/src/lib.rs +++ b/chain/jsonrpc/jsonrpc-tests/src/lib.rs @@ -27,7 +27,7 @@ pub fn start_all_with_validity_period_and_no_epoch_sync( transaction_validity_period: NumBlocks, enable_doomslug: bool, ) -> (Addr, tcp::ListenerAddr) { - let (client_addr, view_client_addr) = setup_no_network_with_validity_period_and_no_epoch_sync( + let actor_handles = setup_no_network_with_validity_period_and_no_epoch_sync( vec!["test1".parse().unwrap(), "test2".parse().unwrap()], if let NodeType::Validator = node_type { "test1".parse().unwrap() @@ -43,11 +43,11 @@ pub fn start_all_with_validity_period_and_no_epoch_sync( start_http( RpcConfig::new(addr), TEST_GENESIS_CONFIG.clone(), - client_addr, - view_client_addr.clone(), + actor_handles.client_actor, + actor_handles.view_client_actor.clone(), None, ); - (view_client_addr, addr) + (actor_handles.view_client_actor, addr) } #[macro_export] diff --git a/chain/network/src/client.rs b/chain/network/src/client.rs index 78783bf617a..caf9507f714 100644 --- a/chain/network/src/client.rs +++ b/chain/network/src/client.rs @@ -1,13 +1,10 @@ -use crate::network_protocol::{ - PartialEncodedChunkForwardMsg, PartialEncodedChunkRequestMsg, PartialEncodedChunkResponseMsg, - StateResponseInfo, -}; +use crate::network_protocol::StateResponseInfo; +use crate::shards_manager::ShardsManagerAdapterForNetwork; use crate::types::{NetworkInfo, ReasonForBan}; use near_primitives::block::{Approval, Block, BlockHeader}; use near_primitives::challenge::Challenge; use near_primitives::hash::CryptoHash; use near_primitives::network::{AnnounceAccount, PeerId}; -use near_primitives::sharding::PartialEncodedChunk; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{AccountId, EpochId, ShardId}; use near_primitives::views::FinalExecutionOutcomeView; @@ -44,22 +41,6 @@ pub trait Client: Send + Sync + 'static { async fn transaction(&self, transaction: SignedTransaction, is_forwarded: bool); - async fn partial_encoded_chunk_request( - &self, - req: PartialEncodedChunkRequestMsg, - msg_hash: CryptoHash, - ); - - async fn partial_encoded_chunk_response( - &self, - resp: PartialEncodedChunkResponseMsg, - timestamp: time::Instant, - ); - - async fn partial_encoded_chunk(&self, chunk: PartialEncodedChunk); - - async fn partial_encoded_chunk_forward(&self, msg: PartialEncodedChunkForwardMsg); - async fn block_request(&self, hash: CryptoHash) -> Option>; async fn block_headers_request(&self, hashes: Vec) -> Option>; @@ -119,24 +100,6 @@ impl Client for Noop { async fn transaction(&self, _transaction: SignedTransaction, _is_forwarded: bool) {} - async fn partial_encoded_chunk_request( - &self, - _req: PartialEncodedChunkRequestMsg, - _msg_hash: CryptoHash, - ) { - } - - async fn partial_encoded_chunk_response( - &self, - _resp: PartialEncodedChunkResponseMsg, - _timestamp: time::Instant, - ) { - } - - async fn partial_encoded_chunk(&self, _chunk: PartialEncodedChunk) {} - - async fn partial_encoded_chunk_forward(&self, _msg: PartialEncodedChunkForwardMsg) {} - async fn block_request(&self, _hash: CryptoHash) -> Option> { None } @@ -166,3 +129,31 @@ impl Client for Noop { Ok(vec![]) } } + +impl ShardsManagerAdapterForNetwork for Noop { + fn process_partial_encoded_chunk( + &self, + _partial_encoded_chunk: near_primitives::sharding::PartialEncodedChunk, + ) { + } + + fn process_partial_encoded_chunk_forward( + &self, + _partial_encoded_chunk_forward: crate::types::PartialEncodedChunkForwardMsg, + ) { + } + + fn process_partial_encoded_chunk_response( + &self, + _partial_encoded_chunk_response: crate::types::PartialEncodedChunkResponseMsg, + _received_time: std::time::Instant, + ) { + } + + fn process_partial_encoded_chunk_request( + &self, + _partial_encoded_chunk_request: crate::types::PartialEncodedChunkRequestMsg, + _route_back: CryptoHash, + ) { + } +} diff --git a/chain/network/src/lib.rs b/chain/network/src/lib.rs index 29ebb31c433..1b558783814 100644 --- a/chain/network/src/lib.rs +++ b/chain/network/src/lib.rs @@ -18,6 +18,7 @@ pub mod config_json; pub mod debug; pub mod raw; pub mod routing; +pub mod shards_manager; pub mod tcp; pub mod test_utils; pub mod time; diff --git a/chain/network/src/peer/peer_actor.rs b/chain/network/src/peer/peer_actor.rs index a8c2f6118a5..aecb75ad818 100644 --- a/chain/network/src/peer/peer_actor.rs +++ b/chain/network/src/peer/peer_actor.rs @@ -916,19 +916,23 @@ impl PeerActor { None } RoutedMessageBody::PartialEncodedChunkRequest(request) => { - network_state.client.partial_encoded_chunk_request(request, msg_hash).await; + network_state + .shards_manager_adapter + .process_partial_encoded_chunk_request(request, msg_hash); None } RoutedMessageBody::PartialEncodedChunkResponse(response) => { - network_state.client.partial_encoded_chunk_response(response, clock.now()).await; + network_state + .shards_manager_adapter + .process_partial_encoded_chunk_response(response, clock.now().into()); None } RoutedMessageBody::VersionedPartialEncodedChunk(chunk) => { - network_state.client.partial_encoded_chunk(chunk).await; + network_state.shards_manager_adapter.process_partial_encoded_chunk(chunk); None } RoutedMessageBody::PartialEncodedChunkForward(msg) => { - network_state.client.partial_encoded_chunk_forward(msg).await; + network_state.shards_manager_adapter.process_partial_encoded_chunk_forward(msg); None } RoutedMessageBody::ReceiptOutcomeRequest(_) => { diff --git a/chain/network/src/peer/testonly.rs b/chain/network/src/peer/testonly.rs index 70e0bcd7144..b3b2ff039c5 100644 --- a/chain/network/src/peer/testonly.rs +++ b/chain/network/src/peer/testonly.rs @@ -106,6 +106,7 @@ impl PeerHandle { .unwrap(), network_cfg.verify().unwrap(), cfg.chain.genesis_id.clone(), + fc.clone(), fc, vec![], )); diff --git a/chain/network/src/peer_manager/network_state/mod.rs b/chain/network/src/peer_manager/network_state/mod.rs index 0b8666ab49a..cc9011bc7a8 100644 --- a/chain/network/src/peer_manager/network_state/mod.rs +++ b/chain/network/src/peer_manager/network_state/mod.rs @@ -13,6 +13,7 @@ use crate::peer_manager::peer_manager_actor::Event; use crate::peer_manager::peer_store; use crate::private_actix::RegisterPeerError; use crate::routing::route_back_cache::RouteBackCache; +use crate::shards_manager::ShardsManagerAdapterForNetwork; use crate::stats::metrics; use crate::store; use crate::tcp; @@ -88,6 +89,7 @@ pub(crate) struct NetworkState { /// GenesisId of the chain. pub genesis_id: GenesisId, pub client: Arc, + pub shards_manager_adapter: Arc, /// Network-related info about the chain. pub chain_info: ArcSwap>, @@ -148,6 +150,7 @@ impl NetworkState { config: config::VerifiedConfig, genesis_id: GenesisId, client: Arc, + shards_manager_adapter: Arc, whitelist_nodes: Vec, ) -> Self { Self { @@ -162,6 +165,7 @@ impl NetworkState { )), genesis_id, client, + shards_manager_adapter, chain_info: Default::default(), tier2: connection::Pool::new(config.node_id()), tier1: connection::Pool::new(config.node_id()), diff --git a/chain/network/src/peer_manager/peer_manager_actor.rs b/chain/network/src/peer_manager/peer_manager_actor.rs index 7242457ec71..c9d985f7460 100644 --- a/chain/network/src/peer_manager/peer_manager_actor.rs +++ b/chain/network/src/peer_manager/peer_manager_actor.rs @@ -9,6 +9,7 @@ use crate::peer::peer_actor::PeerActor; use crate::peer_manager::connection; use crate::peer_manager::network_state::{NetworkState, WhitelistNode}; use crate::peer_manager::peer_store; +use crate::shards_manager::ShardsManagerAdapterForNetwork; use crate::stats::metrics; use crate::store; use crate::tcp; @@ -163,6 +164,7 @@ impl PeerManagerActor { store: Arc, config: config::NetworkConfig, client: Arc, + shards_manager_adapter: Arc, genesis_id: GenesisId, ) -> anyhow::Result> { let config = config.verify().context("config")?; @@ -193,6 +195,7 @@ impl PeerManagerActor { config.clone(), genesis_id, client, + shards_manager_adapter, whitelist_nodes, )); arbiter.spawn({ diff --git a/chain/network/src/peer_manager/testonly.rs b/chain/network/src/peer_manager/testonly.rs index e5bc1a33a64..1a4383fae90 100644 --- a/chain/network/src/peer_manager/testonly.rs +++ b/chain/network/src/peer_manager/testonly.rs @@ -487,7 +487,7 @@ pub(crate) async fn start( let genesis_id = chain.genesis_id.clone(); let fc = Arc::new(fake_client::Fake { event_sink: send.sink().compose(Event::Client) }); cfg.event_sink = send.sink().compose(Event::PeerManager); - PeerManagerActor::spawn(clock, store, cfg, fc, genesis_id).unwrap() + PeerManagerActor::spawn(clock, store, cfg, fc.clone(), fc, genesis_id).unwrap() } }) .await; diff --git a/chain/network/src/shards_manager.rs b/chain/network/src/shards_manager.rs new file mode 100644 index 00000000000..f976d379843 --- /dev/null +++ b/chain/network/src/shards_manager.rs @@ -0,0 +1,93 @@ +use std::time::Instant; + +use actix::Message; +use near_primitives::{hash::CryptoHash, sharding::PartialEncodedChunk}; + +use crate::types::{ + MsgRecipient, PartialEncodedChunkForwardMsg, PartialEncodedChunkRequestMsg, + PartialEncodedChunkResponseMsg, +}; + +/// The interface of the ShardsManager for the networking component. Note that this is defined +/// in near-network and not in near-chunks because near-chunks has a dependency on +/// near-network. +pub trait ShardsManagerAdapterForNetwork: Send + Sync + 'static { + /// Processes a PartialEncodedChunk received from the network. + /// These are received from chunk producers, containing owned parts and tracked + /// receipt proofs. + fn process_partial_encoded_chunk(&self, partial_encoded_chunk: PartialEncodedChunk); + /// Processes a PartialEncodedChunkForwardMsg received from the network. + /// These are received from part owners as an optimization (of the otherwise + /// reactive path of requesting parts that are missing). + fn process_partial_encoded_chunk_forward( + &self, + partial_encoded_chunk_forward: PartialEncodedChunkForwardMsg, + ); + /// Processes a PartialEncodedChunkResponseMsg received from the network. + /// These are received in response to the PartialEncodedChunkRequestMsg + /// we have sent earlier. + fn process_partial_encoded_chunk_response( + &self, + partial_encoded_chunk_response: PartialEncodedChunkResponseMsg, + received_time: Instant, + ); + /// Processes a PartialEncodedChunkRequestMsg received from the network. + /// These are received from another node when they think we have the parts + /// or receipt proofs they need. + fn process_partial_encoded_chunk_request( + &self, + partial_encoded_chunk_request: PartialEncodedChunkRequestMsg, + route_back: CryptoHash, + ); +} + +#[derive(Message)] +#[rtype(result = "()")] +pub enum ShardsManagerRequestFromNetwork { + ProcessPartialEncodedChunk(PartialEncodedChunk), + ProcessPartialEncodedChunkForward(PartialEncodedChunkForwardMsg), + ProcessPartialEncodedChunkResponse { + partial_encoded_chunk_response: PartialEncodedChunkResponseMsg, + received_time: Instant, + }, + ProcessPartialEncodedChunkRequest { + partial_encoded_chunk_request: PartialEncodedChunkRequestMsg, + route_back: CryptoHash, + }, +} + +impl> ShardsManagerAdapterForNetwork for A { + fn process_partial_encoded_chunk(&self, partial_encoded_chunk: PartialEncodedChunk) { + self.do_send(ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunk( + partial_encoded_chunk, + )); + } + fn process_partial_encoded_chunk_forward( + &self, + partial_encoded_chunk_forward: PartialEncodedChunkForwardMsg, + ) { + self.do_send(ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunkForward( + partial_encoded_chunk_forward, + )); + } + fn process_partial_encoded_chunk_response( + &self, + partial_encoded_chunk_response: PartialEncodedChunkResponseMsg, + received_time: Instant, + ) { + self.do_send(ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunkResponse { + partial_encoded_chunk_response, + received_time, + }); + } + fn process_partial_encoded_chunk_request( + &self, + partial_encoded_chunk_request: PartialEncodedChunkRequestMsg, + route_back: CryptoHash, + ) { + self.do_send(ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunkRequest { + partial_encoded_chunk_request, + route_back, + }); + } +} diff --git a/chain/network/src/test_utils.rs b/chain/network/src/test_utils.rs index de0bc53edb1..e9bcf4b4e65 100644 --- a/chain/network/src/test_utils.rs +++ b/chain/network/src/test_utils.rs @@ -266,6 +266,9 @@ impl MockPeerManagerAdapter { pub fn pop_most_recent(&self) -> Option { self.requests.write().unwrap().pop_back() } + pub fn put_back_most_recent(&self, request: PeerManagerMessageRequest) { + self.requests.write().unwrap().push_back(request); + } } #[derive(Message, Clone, Debug)] diff --git a/chain/network/src/testonly/fake_client.rs b/chain/network/src/testonly/fake_client.rs index 051b8c86b1f..080977deb62 100644 --- a/chain/network/src/testonly/fake_client.rs +++ b/chain/network/src/testonly/fake_client.rs @@ -1,8 +1,11 @@ +use std::time::Instant; + use crate::client; use crate::network_protocol::{ PartialEncodedChunkForwardMsg, PartialEncodedChunkRequestMsg, PartialEncodedChunkResponseMsg, StateResponseInfo, }; +use crate::shards_manager::ShardsManagerAdapterForNetwork; use crate::sink::Sink; use crate::types::{NetworkInfo, ReasonForBan, StateResponseInfoV2}; use near_primitives::block::{Approval, Block, BlockHeader}; @@ -82,30 +85,6 @@ impl client::Client for Fake { self.event_sink.push(Event::Transaction(transaction)); } - async fn partial_encoded_chunk_request( - &self, - req: PartialEncodedChunkRequestMsg, - _msg_hash: CryptoHash, - ) { - self.event_sink.push(Event::ChunkRequest(req.chunk_hash)); - } - - async fn partial_encoded_chunk_response( - &self, - resp: PartialEncodedChunkResponseMsg, - _timestamp: time::Instant, - ) { - self.event_sink.push(Event::Chunk(resp.parts)); - } - - async fn partial_encoded_chunk(&self, _chunk: PartialEncodedChunk) { - unimplemented!(); - } - - async fn partial_encoded_chunk_forward(&self, _msg: PartialEncodedChunkForwardMsg) { - unimplemented!(); - } - async fn block_request(&self, hash: CryptoHash) -> Option> { self.event_sink.push(Event::BlockRequest(hash)); None @@ -143,3 +122,32 @@ impl client::Client for Fake { Ok(accounts.into_iter().map(|a| a.0).collect()) } } + +impl ShardsManagerAdapterForNetwork for Fake { + fn process_partial_encoded_chunk(&self, _partial_encoded_chunk: PartialEncodedChunk) { + unimplemented!() + } + + fn process_partial_encoded_chunk_forward( + &self, + _partial_encoded_chunk_forward: PartialEncodedChunkForwardMsg, + ) { + unimplemented!() + } + + fn process_partial_encoded_chunk_response( + &self, + partial_encoded_chunk_response: PartialEncodedChunkResponseMsg, + _received_time: Instant, + ) { + self.event_sink.push(Event::Chunk(partial_encoded_chunk_response.parts)); + } + + fn process_partial_encoded_chunk_request( + &self, + partial_encoded_chunk_request: PartialEncodedChunkRequestMsg, + _route_back: CryptoHash, + ) { + self.event_sink.push(Event::ChunkRequest(partial_encoded_chunk_request.chunk_hash)); + } +} diff --git a/chain/network/src/types.rs b/chain/network/src/types.rs index 898ce4b5eb8..8930c5425c9 100644 --- a/chain/network/src/types.rs +++ b/chain/network/src/types.rs @@ -421,6 +421,7 @@ impl< { } +// TODO: rename to a more generic name. pub struct NetworkRecipient { recipient: OnceCell>, } diff --git a/chain/rosetta-rpc/src/adapters/mod.rs b/chain/rosetta-rpc/src/adapters/mod.rs index 09716d7ec5a..65549bf7f56 100644 --- a/chain/rosetta-rpc/src/adapters/mod.rs +++ b/chain/rosetta-rpc/src/adapters/mod.rs @@ -703,7 +703,7 @@ mod tests { fn test_convert_block_changes_to_transactions() { run_actix(async { let runtime_config: RuntimeConfigView = RuntimeConfig::test().into(); - let (_client, view_client) = setup_no_network( + let actor_handles = setup_no_network( vec!["test".parse().unwrap()], "other".parse().unwrap(), true, @@ -793,7 +793,7 @@ mod tests { }, ); let transactions = super::transactions::convert_block_changes_to_transactions( - &view_client, + &actor_handles.view_client_actor, &runtime_config, &block_hash, accounts_changes, diff --git a/integration-tests/src/tests/client/challenges.rs b/integration-tests/src/tests/client/challenges.rs index e51b53afb0e..44aac393261 100644 --- a/integration-tests/src/tests/client/challenges.rs +++ b/integration-tests/src/tests/client/challenges.rs @@ -533,10 +533,7 @@ fn test_receive_invalid_chunk_as_chunk_producer() { one_part_receipt_proofs, &[merkle_paths[0].clone()], ); - assert!(env.clients[1] - .shards_mgr - .process_partial_encoded_chunk(partial_encoded_chunk.into()) - .is_ok()); + env.shards_manager_adapters[1].process_partial_encoded_chunk(partial_encoded_chunk); env.process_block(1, block, Provenance::NONE); // At this point we should create a challenge and send it out. diff --git a/integration-tests/src/tests/client/chunks_management.rs b/integration-tests/src/tests/client/chunks_management.rs index 75a04ee2e7c..dac82a4eaf6 100644 --- a/integration-tests/src/tests/client/chunks_management.rs +++ b/integration-tests/src/tests/client/chunks_management.rs @@ -1,5 +1,5 @@ use crate::test_helpers::heavy_test; -use actix::{Addr, System}; +use actix::System; use futures::{future, FutureExt}; use near_actix_test_utils::run_actix; use near_chain::test_utils::ValidatorSchedule; @@ -7,8 +7,8 @@ use near_chunks::{ CHUNK_REQUEST_RETRY_MS, CHUNK_REQUEST_SWITCH_TO_FULL_FETCH_MS, CHUNK_REQUEST_SWITCH_TO_OTHERS_MS, }; -use near_client::test_utils::setup_mock_all_validators; -use near_client::{ClientActor, GetBlock, ProcessTxRequest, ViewClientActor}; +use near_client::test_utils::{setup_mock_all_validators, ActorHandlesForTesting}; +use near_client::{GetBlock, ProcessTxRequest}; use near_network::types::PeerManagerMessageRequest; use near_network::types::{AccountIdOrPeerTrackingShard, PeerInfo}; use near_network::types::{NetworkRequests, NetworkResponses}; @@ -41,8 +41,7 @@ impl Test { fn run_impl(self) { init_test_logger(); - let connectors: Arc, Addr)>>> = - Arc::new(RwLock::new(vec![])); + let connectors: Arc>> = Arc::new(RwLock::new(vec![])); let heights = Arc::new(RwLock::new(HashMap::new())); let heights1 = heights; @@ -246,12 +245,12 @@ impl Test { ); *connectors.write().unwrap() = conn; - let view_client = connectors.write().unwrap()[0].1.clone(); + let view_client = connectors.write().unwrap()[0].view_client_actor.clone(); let actor = view_client.send(GetBlock::latest().with_span_context()); let actor = actor.then(move |res| { let block_hash = res.unwrap().unwrap().header.hash; let connectors_ = connectors.write().unwrap(); - connectors_[0].0.do_send( + connectors_[0].client_actor.do_send( ProcessTxRequest { transaction: SignedTransaction::empty(block_hash), is_forwarded: false, @@ -259,7 +258,7 @@ impl Test { } .with_span_context(), ); - connectors_[1].0.do_send( + connectors_[1].client_actor.do_send( ProcessTxRequest { transaction: SignedTransaction::empty(block_hash), is_forwarded: false, @@ -267,7 +266,7 @@ impl Test { } .with_span_context(), ); - connectors_[2].0.do_send( + connectors_[2].client_actor.do_send( ProcessTxRequest { transaction: SignedTransaction::empty(block_hash), is_forwarded: false, diff --git a/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs b/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs index 6a86b445328..4ee68bccc0a 100644 --- a/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs +++ b/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs @@ -5,6 +5,7 @@ use assert_matches::assert_matches; use near_chain::chain::NUM_ORPHAN_ANCESTORS_CHECK; use near_chain::{ChainGenesis, Error, Provenance, RuntimeWithEpochManagerAdapter}; use near_chain_configs::Genesis; +use near_chunks::metrics::PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER; use near_client::test_utils::{create_chunk_with_transactions, TestEnv}; use near_client::ProcessTxResponse; use near_crypto::{InMemorySigner, KeyType, Signer}; @@ -15,7 +16,7 @@ use near_primitives::account::AccessKey; use near_primitives::errors::InvalidTxError; use near_primitives::runtime::config_store::RuntimeConfigStore; use near_primitives::shard_layout::ShardLayout; -use near_primitives::sharding::{ChunkHash, PartialEncodedChunk}; +use near_primitives::sharding::ChunkHash; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{AccountId, BlockHeight}; use near_primitives::version::{ProtocolFeature, ProtocolVersion}; @@ -547,7 +548,6 @@ struct ChunkForwardingOptimizationTestData { num_part_ords_requested: usize, num_part_ords_sent_as_partial_encoded_chunk: usize, num_part_ords_forwarded: usize, - num_forwards_with_missing_chunk_header: usize, chunk_parts_that_must_be_known: HashSet<(ChunkHash, u64, usize)>, } @@ -597,7 +597,6 @@ impl ChunkForwardingOptimizationTestData { num_part_ords_requested: 0, num_part_ords_sent_as_partial_encoded_chunk: 0, num_part_ords_forwarded: 0, - num_forwards_with_missing_chunk_header: 0, chunk_parts_that_must_be_known: HashSet::new(), } } @@ -654,12 +653,8 @@ impl ChunkForwardingOptimizationTestData { self.num_part_ords_sent_as_partial_encoded_chunk += partial_encoded_chunk.parts.len(); self.env - .client(&account_id) - .shards_mgr - .process_partial_encoded_chunk( - PartialEncodedChunk::from(partial_encoded_chunk).into(), - ) - .unwrap(); + .shards_manager(&account_id) + .process_partial_encoded_chunk(partial_encoded_chunk.into()); } NetworkRequests::PartialEncodedChunkForward { account_id, forward } => { debug!( @@ -678,20 +673,7 @@ impl ChunkForwardingOptimizationTestData { )); } self.num_part_ords_forwarded += forward.parts.len(); - match self - .env - .client(&account_id) - .shards_mgr - .process_partial_encoded_chunk_forward(forward) - { - Ok(_) => {} - Err(near_chunks::Error::UnknownChunk) => { - self.num_forwards_with_missing_chunk_header += 1; - } - Err(e) => { - panic!("Unexpected error from chunk forward: {:?}", e) - } - } + self.env.shards_manager(&account_id).process_partial_encoded_chunk_forward(forward); } _ => { panic!("Unexpected network request: {:?}", requests); @@ -728,6 +710,7 @@ fn test_chunk_forwarding_optimization() { // a part that was already forwarded to it. We simulate four validator nodes, with one // block producer and four chunk producers. init_test_logger(); + PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER.reset(); let mut test = ChunkForwardingOptimizationTestData::new(); loop { let height = test.env.clients[0].chain.head().unwrap().height; @@ -779,7 +762,7 @@ fn test_chunk_forwarding_optimization() { // With very high probability we should've encountered some cases where forwarded parts // could not be applied because the chunk header is not available. Assert this did indeed // happen. - assert!(test.num_forwards_with_missing_chunk_header > 0); + assert!(PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER.get() > 0.0); debug!(target: "test", "Counters for debugging: num_part_ords_requested: {} @@ -789,7 +772,7 @@ fn test_chunk_forwarding_optimization() { test.num_part_ords_requested, test.num_part_ords_sent_as_partial_encoded_chunk, test.num_part_ords_forwarded, - test.num_forwards_with_missing_chunk_header + PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER.get(), ); } diff --git a/integration-tests/src/tests/client/features/adversarial_behaviors.rs b/integration-tests/src/tests/client/features/adversarial_behaviors.rs index 91f4aa323fe..0fce7192ad6 100644 --- a/integration-tests/src/tests/client/features/adversarial_behaviors.rs +++ b/integration-tests/src/tests/client/features/adversarial_behaviors.rs @@ -8,7 +8,6 @@ use near_o11y::testonly::init_test_logger; use near_primitives::{ runtime::config_store::RuntimeConfigStore, shard_layout::ShardLayout, - sharding::PartialEncodedChunk, types::{AccountId, EpochId, ShardId}, }; use near_store::test_utils::create_test_store; @@ -78,25 +77,11 @@ impl AdversarialBehaviorTestData { } NetworkRequests::PartialEncodedChunkMessage { account_id, partial_encoded_chunk } => { self.env - .client(&account_id) - .shards_mgr - .process_partial_encoded_chunk( - PartialEncodedChunk::from(partial_encoded_chunk).into(), - ) - .unwrap(); + .shards_manager(&account_id) + .process_partial_encoded_chunk(partial_encoded_chunk.into()); } NetworkRequests::PartialEncodedChunkForward { account_id, forward } => { - match self - .env - .client(&account_id) - .shards_mgr - .process_partial_encoded_chunk_forward(forward) - { - Ok(_) | Err(near_chunks::Error::UnknownChunk) => {} - Err(e) => { - panic!("Unexpected error from chunk forward: {:?}", e) - } - } + self.env.shards_manager(&account_id).process_partial_encoded_chunk_forward(forward); } NetworkRequests::Challenge(_) => { // challenges not enabled. diff --git a/integration-tests/src/tests/client/process_blocks.rs b/integration-tests/src/tests/client/process_blocks.rs index 06c8250a2ec..e8199140b10 100644 --- a/integration-tests/src/tests/client/process_blocks.rs +++ b/integration-tests/src/tests/client/process_blocks.rs @@ -8,7 +8,9 @@ use actix::System; use assert_matches::assert_matches; use futures::{future, FutureExt}; use near_chain::test_utils::ValidatorSchedule; -use near_chunks::test_utils::MockClientAdapterForShardsManager; +use near_chunks::test_utils::{ + MockClientAdapterForShardsManager, NoopShardsManagerAdapterForClient, +}; use near_primitives::config::{ActionCosts, ExtCosts}; use near_primitives::num_rational::{Ratio, Rational32}; @@ -23,7 +25,8 @@ use near_chain::{ use near_chain_configs::{ClientConfig, Genesis, DEFAULT_GC_NUM_EPOCHS_TO_KEEP}; use near_chunks::{ChunkStatus, ShardsManager}; use near_client::test_utils::{ - create_chunk_on_height, setup_client, setup_mock, setup_mock_all_validators, TestEnv, + create_chunk_on_height, setup_client_with_synchronous_shards_manager, setup_mock, + setup_mock_all_validators, TestEnv, }; use near_client::{ BlockApproval, BlockResponse, Client, GetBlock, GetBlockWithMerkleTree, ProcessTxRequest, @@ -286,7 +289,7 @@ fn produce_blocks_with_tx() { let mut encoded_chunks: Vec = vec![]; init_test_logger(); run_actix(async { - let (client, view_client) = setup_mock( + let actor_handles = setup_mock( vec!["test".parse().unwrap()], "test".parse().unwrap(), true, @@ -333,10 +336,10 @@ fn produce_blocks_with_tx() { }), ); near_network::test_utils::wait_or_panic(5000); - let actor = view_client.send(GetBlock::latest().with_span_context()); + let actor = actor_handles.view_client_actor.send(GetBlock::latest().with_span_context()); let actor = actor.then(move |res| { let block_hash = res.unwrap().unwrap().header.hash; - client.do_send( + actor_handles.client_actor.do_send( ProcessTxRequest { transaction: SignedTransaction::empty(block_hash), is_forwarded: false, @@ -359,7 +362,7 @@ fn receive_network_block() { // The first header announce will be when the block is received. We don't immediately endorse // it. The second header announce will happen with the endorsement a little later. let first_header_announce = Arc::new(RwLock::new(true)); - let (client, view_client) = setup_mock( + let actor_handles = setup_mock( vec!["test2".parse().unwrap(), "test1".parse().unwrap(), "test3".parse().unwrap()], "test2".parse().unwrap(), true, @@ -376,7 +379,9 @@ fn receive_network_block() { PeerManagerMessageResponse::NetworkResponses(NetworkResponses::NoResponse) }), ); - let actor = view_client.send(GetBlockWithMerkleTree::latest().with_span_context()); + let actor = actor_handles + .view_client_actor + .send(GetBlockWithMerkleTree::latest().with_span_context()); let actor = actor.then(move |res| { let (last_block, block_merkle_tree) = res.unwrap().unwrap(); let mut block_merkle_tree = PartialMerkleTree::clone(&block_merkle_tree); @@ -409,7 +414,7 @@ fn receive_network_block() { block_merkle_tree.root(), None, ); - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block, peer_id: PeerInfo::random().id, was_requested: false } .with_span_context(), ); @@ -427,7 +432,7 @@ fn produce_block_with_approvals() { let validators: Vec<_> = (1..=10).map(|i| AccountId::try_from(format!("test{}", i)).unwrap()).collect(); run_actix(async { - let (client, view_client) = setup_mock( + let actor_handles = setup_mock( validators.clone(), "test1".parse().unwrap(), true, @@ -455,7 +460,9 @@ fn produce_block_with_approvals() { PeerManagerMessageResponse::NetworkResponses(NetworkResponses::NoResponse) }), ); - let actor = view_client.send(GetBlockWithMerkleTree::latest().with_span_context()); + let actor = actor_handles + .view_client_actor + .send(GetBlockWithMerkleTree::latest().with_span_context()); let actor = actor.then(move |res| { let (last_block, block_merkle_tree) = res.unwrap().unwrap(); let mut block_merkle_tree = PartialMerkleTree::clone(&block_merkle_tree); @@ -488,7 +495,7 @@ fn produce_block_with_approvals() { block_merkle_tree.root(), None, ); - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block.clone(), peer_id: PeerInfo::random().id, @@ -511,7 +518,9 @@ fn produce_block_with_approvals() { 10, // the height at which "test1" is producing &signer, ); - client.do_send(BlockApproval(approval, PeerInfo::random().id).with_span_context()); + actor_handles + .client_actor + .do_send(BlockApproval(approval, PeerInfo::random().id).with_span_context()); } future::ready(()) @@ -559,9 +568,9 @@ fn produce_block_with_approvals_arrived_early() { match msg { NetworkRequests::Block { block } => { if block.header().height() == 3 { - for (i, (client, _)) in conns.iter().enumerate() { + for (i, actor_handles) in conns.iter().enumerate() { if i > 0 { - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block.clone(), peer_id: PeerInfo::random().id, @@ -586,7 +595,7 @@ fn produce_block_with_approvals_arrived_early() { } if approval_counter == 3 { let block = block_holder.read().unwrap().clone().unwrap(); - conns[0].0.do_send( + conns[0].client_actor.do_send( BlockResponse { block: block, peer_id: PeerInfo::random().id, @@ -613,7 +622,7 @@ fn invalid_blocks_common(is_requested: bool) { init_test_logger(); run_actix(async move { let mut ban_counter = 0; - let (client, view_client) = setup_mock( + let actor_handles = setup_mock( vec!["test".parse().unwrap()], "other".parse().unwrap(), true, @@ -663,7 +672,9 @@ fn invalid_blocks_common(is_requested: bool) { PeerManagerMessageResponse::NetworkResponses(NetworkResponses::NoResponse) }), ); - let actor = view_client.send(GetBlockWithMerkleTree::latest().with_span_context()); + let actor = actor_handles + .view_client_actor + .send(GetBlockWithMerkleTree::latest().with_span_context()); let actor = actor.then(move |res| { let (last_block, block_merkle_tree) = res.unwrap().unwrap(); let mut block_merkle_tree = PartialMerkleTree::clone(&block_merkle_tree); @@ -700,7 +711,7 @@ fn invalid_blocks_common(is_requested: bool) { let mut block = valid_block.clone(); block.mut_header().get_mut().inner_rest.chunk_mask = vec![]; block.mut_header().get_mut().init(); - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block.clone(), peer_id: PeerInfo::random().id, @@ -716,7 +727,7 @@ fn invalid_blocks_common(is_requested: bool) { block.mut_header().get_mut().inner_rest.latest_protocol_version = PROTOCOL_VERSION - 1; block.mut_header().get_mut().init(); - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block.clone(), peer_id: PeerInfo::random().id, @@ -742,7 +753,7 @@ fn invalid_blocks_common(is_requested: bool) { } }; block.set_chunks(chunks); - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block.clone(), peer_id: PeerInfo::random().id, @@ -753,7 +764,7 @@ fn invalid_blocks_common(is_requested: bool) { // Send proper block. let block2 = valid_block; - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block2.clone(), peer_id: PeerInfo::random().id, @@ -765,7 +776,7 @@ fn invalid_blocks_common(is_requested: bool) { let mut block3 = block2; block3.mut_header().get_mut().inner_rest.chunk_headers_root = hash(&[1]); block3.mut_header().get_mut().init(); - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block3.clone(), peer_id: PeerInfo::random().id, @@ -876,9 +887,9 @@ fn ban_peer_for_invalid_block_common(mode: InvalidBlockMode) { } } - for (i, (client, _)) in conns.clone().into_iter().enumerate() { + for (i, actor_handles) in conns.clone().into_iter().enumerate() { if i != block_producer_idx { - client.do_send( + actor_handles.client_actor.do_send( BlockResponse { block: block_mut.clone(), peer_id: PeerInfo::random().id, @@ -1000,7 +1011,7 @@ fn client_sync_headers() { run_actix(async { let peer_info1 = PeerInfo::random(); let peer_info2 = peer_info1.clone(); - let (client, _) = setup_mock( + let actor_handles = setup_mock( vec!["test".parse().unwrap()], "other".parse().unwrap(), false, @@ -1017,7 +1028,7 @@ fn client_sync_headers() { _ => PeerManagerMessageResponse::NetworkResponses(NetworkResponses::NoResponse), }), ); - client.do_send( + actor_handles.client_actor.do_send( SetNetworkInfo(NetworkInfo { connected_peers: vec![ConnectedPeerInfo { full_peer_info: FullPeerInfo { @@ -1115,7 +1126,7 @@ fn test_time_attack() { let chain_genesis = ChainGenesis::test(); let vs = ValidatorSchedule::new().block_producers_per_epoch(vec![vec!["test1".parse().unwrap()]]); - let mut client = setup_client( + let mut client = setup_client_with_synchronous_shards_manager( store, vs, Some("test1".parse().unwrap()), @@ -1151,7 +1162,7 @@ fn test_invalid_approvals() { let chain_genesis = ChainGenesis::test(); let vs = ValidatorSchedule::new().block_producers_per_epoch(vec![vec!["test1".parse().unwrap()]]); - let mut client = setup_client( + let mut client = setup_client_with_synchronous_shards_manager( store, vs, Some("test1".parse().unwrap()), @@ -1199,7 +1210,7 @@ fn test_invalid_gas_price() { chain_genesis.min_gas_price = 100; let vs = ValidatorSchedule::new().block_producers_per_epoch(vec![vec!["test1".parse().unwrap()]]); - let mut client = setup_client( + let mut client = setup_client_with_synchronous_shards_manager( store, vs, Some("test1".parse().unwrap()), @@ -1360,7 +1371,7 @@ fn test_bad_chunk_mask() { let vs = ValidatorSchedule::new() .num_shards(2) .block_producers_per_epoch(vec![validators.clone()]); - setup_client( + setup_client_with_synchronous_shards_manager( create_test_store(), vs, Some(account_id.clone()), @@ -2147,7 +2158,7 @@ fn test_incorrect_validator_key_produce_block() { chain_genesis, runtime_adapter, Arc::new(MockPeerManagerAdapter::default()), - Arc::new(MockClientAdapterForShardsManager::default()), + Arc::new(NoopShardsManagerAdapterForClient {}), Some(signer), false, TEST_SEED, diff --git a/integration-tests/src/tests/client/runtimes.rs b/integration-tests/src/tests/client/runtimes.rs index 366c5ce6d29..d4ed18f2158 100644 --- a/integration-tests/src/tests/client/runtimes.rs +++ b/integration-tests/src/tests/client/runtimes.rs @@ -1,23 +1,16 @@ //! Client is responsible for tracking the chain, chunks, and producing them when needed. //! This client works completely synchronously and must be operated by some async actor outside. -use assert_matches::assert_matches; use near_chain::{ChainGenesis, RuntimeWithEpochManagerAdapter}; use near_chain_configs::Genesis; -use near_chunks::test_utils::ChunkTestFixture; -use near_chunks::ProcessPartialEncodedChunkResult; use near_client::test_utils::TestEnv; use near_crypto::KeyType; use near_network::test_utils::MockPeerManagerAdapter; -use near_network::types::PartialEncodedChunkForwardMsg; use near_primitives::block::{Approval, ApprovalInner}; use near_primitives::block_header::ApprovalType; use near_primitives::hash::hash; use near_primitives::network::PeerId; -use near_primitives::sharding::ShardChunkHeaderInner; -use near_primitives::sharding::{PartialEncodedChunk, ShardChunkHeader}; use near_primitives::test_utils::create_test_signer; -use near_primitives::utils::MaybeValidated; use near_primitives::validator_signer::InMemoryValidatorSigner; use near_store::test_utils::create_test_store; use nearcore::config::GenesisExt; @@ -114,57 +107,3 @@ fn test_cap_max_gas_price() { let max_gas_price = env.clients[0].chain.block_economics_config.max_gas_price(protocol_version); assert!(max_gas_price <= 20 * min_gas_price); } - -#[test] -fn test_process_partial_encoded_chunk_with_missing_block() { - let mut env = - TestEnv::builder(ChainGenesis::test()).runtime_adapters(create_runtimes(1)).build(); - let client = &mut env.clients[0]; - let chunk_producer = ChunkTestFixture::default(); - let mut mock_chunk = chunk_producer.make_partial_encoded_chunk(&[0]); - match &mut mock_chunk { - PartialEncodedChunk::V2(mock_chunk) => { - // change the prev_block to some unknown block - match &mut mock_chunk.header { - ShardChunkHeader::V1(ref mut header) => { - header.inner.prev_block_hash = hash(b"some_prev_block"); - header.init(); - } - ShardChunkHeader::V2(ref mut header) => { - header.inner.prev_block_hash = hash(b"some_prev_block"); - header.init(); - } - ShardChunkHeader::V3(header) => { - match &mut header.inner { - ShardChunkHeaderInner::V1(inner) => { - inner.prev_block_hash = hash(b"some_prev_block") - } - ShardChunkHeaderInner::V2(inner) => { - inner.prev_block_hash = hash(b"some_prev_block") - } - } - header.init(); - } - } - } - _ => unreachable!(), - } - - let mock_forward = PartialEncodedChunkForwardMsg::from_header_and_parts( - &mock_chunk.cloned_header(), - mock_chunk.parts().to_vec(), - ); - - // process_partial_encoded_chunk should return Ok(NeedBlock) if the chunk is - // based on a missing block. - let result = - client.shards_mgr.process_partial_encoded_chunk(MaybeValidated::from(mock_chunk.clone())); - assert_matches!(result, Ok(ProcessPartialEncodedChunkResult::NeedBlock)); - let accepted_blocks = client.finish_blocks_in_processing(); - assert!(accepted_blocks.is_empty()); - - // process_partial_encoded_chunk_forward should return UnknownChunk if it is based on a - // a missing block. - let result = client.shards_mgr.process_partial_encoded_chunk_forward(mock_forward); - assert_matches!(result.unwrap_err(), near_chunks::Error::UnknownChunk); -} diff --git a/integration-tests/src/tests/network/peer_handshake.rs b/integration-tests/src/tests/network/peer_handshake.rs index 95814e938a6..4b5edb29bc9 100644 --- a/integration-tests/src/tests/network/peer_handshake.rs +++ b/integration-tests/src/tests/network/peer_handshake.rs @@ -34,6 +34,7 @@ fn make_peer_manager( near_store::db::TestDB::new(), config, Arc::new(near_network::client::Noop), + Arc::new(near_network::client::Noop), GenesisId::default(), ) .unwrap() diff --git a/integration-tests/src/tests/network/runner.rs b/integration-tests/src/tests/network/runner.rs index 06c053f7aee..7859fcd3e4a 100644 --- a/integration-tests/src/tests/network/runner.rs +++ b/integration-tests/src/tests/network/runner.rs @@ -1,8 +1,10 @@ use actix::{Actor, Addr}; use anyhow::{anyhow, bail, Context}; use near_chain::test_utils::{KeyValueRuntime, ValidatorSchedule}; +use near_chain::types::RuntimeAdapter; use near_chain::{Chain, ChainGenesis}; use near_chain_configs::ClientConfig; +use near_chunks::shards_manager_actor::start_shards_manager; use near_client::{start_client, start_view_client}; use near_network::actix::ActixSystem; use near_network::blacklist; @@ -21,6 +23,7 @@ use near_primitives::block::GenesisId; use near_primitives::network::PeerId; use near_primitives::test_utils::create_test_signer; use near_primitives::types::{AccountId, ValidatorId}; +use near_primitives::validator_signer::ValidatorSigner; use near_telemetry::{TelemetryActor, TelemetryConfig}; use std::collections::HashSet; use std::future::Future; @@ -65,6 +68,7 @@ fn setup_network_node( hash: genesis_block.header().hash().clone(), }; let network_adapter = Arc::new(NetworkRecipient::default()); + let shards_manager_adapter = Arc::new(NetworkRecipient::default()); let adv = near_client::adversarial::Controls::default(); let client_actor = start_client( client_config.clone(), @@ -72,7 +76,8 @@ fn setup_network_node( runtime.clone(), config.node_id(), network_adapter.clone(), - Some(signer), + shards_manager_adapter.clone(), + Some(signer.clone()), telemetry_actor, None, adv.clone(), @@ -84,14 +89,24 @@ fn setup_network_node( chain_genesis.clone(), runtime.clone(), network_adapter.clone(), - client_config, + client_config.clone(), adv, ); + let (shards_manager_actor, _) = start_shards_manager( + runtime.clone(), + network_adapter.clone(), + Arc::new(client_actor.clone()), + Some(signer.validator_id().clone()), + runtime.store().clone(), + client_config.chunk_request_retry_period, + ); + shards_manager_adapter.set_recipient(shards_manager_actor); let peer_manager = PeerManagerActor::spawn( time::Clock::real(), db.clone(), config, Arc::new(near_client::adapter::Adapter::new(client_actor, view_client_actor)), + shards_manager_adapter, genesis_id, ) .unwrap(); diff --git a/integration-tests/src/tests/network/stress_network.rs b/integration-tests/src/tests/network/stress_network.rs index 878b4d5aa0b..0ce4111387d 100644 --- a/integration-tests/src/tests/network/stress_network.rs +++ b/integration-tests/src/tests/network/stress_network.rs @@ -29,6 +29,7 @@ fn make_peer_manager( near_store::db::TestDB::new(), config, Arc::new(near_network::client::Noop), + Arc::new(near_network::client::Noop), GenesisId::default(), ) .unwrap() diff --git a/nearcore/src/lib.rs b/nearcore/src/lib.rs index 81311c83207..53670181842 100644 --- a/nearcore/src/lib.rs +++ b/nearcore/src/lib.rs @@ -8,6 +8,7 @@ use actix_web; use anyhow::Context; use cold_storage::ColdStoreLoopHandle; use near_chain::{Chain, ChainGenesis}; +use near_chunks::shards_manager_actor::start_shards_manager; use near_client::{start_client, start_view_client, ClientActor, ConfigUpdater, ViewClientActor}; use near_network::time; use near_network::types::NetworkRecipient; @@ -187,6 +188,8 @@ pub fn start_with_config_and_synchronization( let node_id = config.network_config.node_id(); let network_adapter = Arc::new(NetworkRecipient::default()); + let shards_manager_adapter = Arc::new(NetworkRecipient::default()); + let client_adapter_for_shards_manager = Arc::new(NetworkRecipient::default()); let adv = near_client::adversarial::Controls::new(config.client_config.archive); let view_client = start_view_client( @@ -198,17 +201,28 @@ pub fn start_with_config_and_synchronization( adv.clone(), ); let (client_actor, client_arbiter_handle) = start_client( - config.client_config, + config.client_config.clone(), chain_genesis, - runtime, + runtime.clone(), node_id, network_adapter.clone(), - config.validator_signer, + shards_manager_adapter.clone(), + config.validator_signer.clone(), telemetry, shutdown_signal.clone(), adv, config_updater, ); + client_adapter_for_shards_manager.set_recipient(client_actor.clone()); + let (shards_manager_actor, shards_manager_arbiter_handle) = start_shards_manager( + runtime, + network_adapter.clone(), + client_adapter_for_shards_manager.clone(), + config.validator_signer.as_ref().map(|signer| signer.validator_id().clone()), + store.get_store(Temperature::Hot), + config.client_config.chunk_request_retry_period, + ); + shards_manager_adapter.set_recipient(shards_manager_actor.clone()); #[allow(unused_mut)] let mut rpc_servers = Vec::new(); @@ -217,6 +231,7 @@ pub fn start_with_config_and_synchronization( store.into_inner(near_store::Temperature::Hot), config.network_config, Arc::new(near_client::adapter::Adapter::new(client_actor.clone(), view_client.clone())), + shards_manager_adapter, genesis_id, ) .context("PeerManager::spawn()")?; @@ -255,7 +270,7 @@ pub fn start_with_config_and_synchronization( client: client_actor, view_client, rpc_servers, - arbiters: vec![client_arbiter_handle], + arbiters: vec![client_arbiter_handle, shards_manager_arbiter_handle], cold_store_loop_handle, }) } diff --git a/tools/chainsync-loadtest/src/main.rs b/tools/chainsync-loadtest/src/main.rs index 6dbaccb8ddf..7b3d9f1bbaa 100644 --- a/tools/chainsync-loadtest/src/main.rs +++ b/tools/chainsync-loadtest/src/main.rs @@ -43,6 +43,7 @@ pub fn start_with_config(config: NearConfig, qps_limit: u32) -> anyhow::Result Option> { None } @@ -352,3 +333,31 @@ impl near_network::client::Client for Network { Ok(accounts.into_iter().map(|a| a.0).collect()) } } + +impl near_network::shards_manager::ShardsManagerAdapterForNetwork for Network { + fn process_partial_encoded_chunk( + &self, + _partial_encoded_chunk: near_primitives::sharding::PartialEncodedChunk, + ) { + } + + fn process_partial_encoded_chunk_forward( + &self, + _partial_encoded_chunk_forward: PartialEncodedChunkForwardMsg, + ) { + } + + fn process_partial_encoded_chunk_response( + &self, + _partial_encoded_chunk_response: PartialEncodedChunkResponseMsg, + _received_time: std::time::Instant, + ) { + } + + fn process_partial_encoded_chunk_request( + &self, + _partial_encoded_chunk_request: PartialEncodedChunkRequestMsg, + _route_back: CryptoHash, + ) { + } +} diff --git a/tools/mock-node/Cargo.toml b/tools/mock-node/Cargo.toml index ba21381640c..d4c5707aaa9 100644 --- a/tools/mock-node/Cargo.toml +++ b/tools/mock-node/Cargo.toml @@ -27,6 +27,7 @@ near-chain = { path = "../../chain/chain" } near-chain-configs = { path = "../../core/chain-configs" } near-client = { path = "../../chain/client" } near-crypto = { path = "../../core/crypto" } +near-chunks = { path = "../../chain/chunks" } near-epoch-manager = { path = "../../chain/epoch-manager"} near-jsonrpc = { path = "../../chain/jsonrpc" } near-network = { path = "../../chain/network" } diff --git a/tools/mock-node/src/lib.rs b/tools/mock-node/src/lib.rs index e22d6402b9b..1bd115937b6 100644 --- a/tools/mock-node/src/lib.rs +++ b/tools/mock-node/src/lib.rs @@ -196,6 +196,7 @@ impl IncomingRequests { pub struct MockPeerManagerActor { /// Client address for the node that we are testing client: Arc, + shards_manager_adapter: Arc, /// Access a pre-generated chain history from storage chain_history_access: ChainHistoryAccess, /// Current network state for the simulated network @@ -212,6 +213,9 @@ pub struct MockPeerManagerActor { impl MockPeerManagerActor { fn new( client: Arc, + shards_manager_adapter: Arc< + dyn near_network::shards_manager::ShardsManagerAdapterForNetwork, + >, genesis_config: &GenesisConfig, mut chain: Chain, client_start_height: BlockHeight, @@ -267,6 +271,7 @@ impl MockPeerManagerActor { ); Self { client, + shards_manager_adapter, chain_history_access: ChainHistoryAccess { chain, target_height }, network_info, block_production_delay, @@ -343,21 +348,13 @@ impl MockPeerManagerActor { fn send_chunk_request(&mut self, ctx: &mut Context) { if let Some((interval, request)) = &self.incoming_requests.chunk_request { - actix::spawn({ - let client = self.client.clone(); - let request = request.clone(); - async move { - client - .partial_encoded_chunk_request( - request.clone(), - // this can just be nonsense since the PeerManager is mocked out anyway. If/when we update the mock node - // to exercise the PeerManager code as well, then this won't matter anyway since the mock code won't be - // responsible for it. - CryptoHash::default(), - ) - .await - } - }); + self.shards_manager_adapter.process_partial_encoded_chunk_request( + request.clone(), + // this can just be nonsense since the PeerManager is mocked out anyway. If/when we update the mock node + // to exercise the PeerManager code as well, then this won't matter anyway since the mock code won't be + // responsible for it. + CryptoHash::default(), + ); run_later(ctx, *interval, move |act, ctx| { act.send_chunk_request(ctx); @@ -425,17 +422,10 @@ impl Handler> for MockPeerManagerActo .chain_history_access .retrieve_partial_encoded_chunk(&request) .unwrap(); - actix::spawn({ - let client = act.client.clone(); - async move { - client - .partial_encoded_chunk_response( - response, - Clock::instant().into(), - ) - .await - } - }); + act.shards_manager_adapter.process_partial_encoded_chunk_response( + response, + Clock::instant().into(), + ); }); } NetworkRequests::PartialEncodedChunkResponse { .. } => {} diff --git a/tools/mock-node/src/setup.rs b/tools/mock-node/src/setup.rs index 7d064619a58..d611789369d 100644 --- a/tools/mock-node/src/setup.rs +++ b/tools/mock-node/src/setup.rs @@ -7,6 +7,7 @@ use near_chain::types::RuntimeAdapter; use near_chain::ChainStoreUpdate; use near_chain::{Chain, ChainGenesis, ChainStore, ChainStoreAccess, DoomslugThresholdMode}; use near_chain_configs::GenesisConfig; +use near_chunks::shards_manager_actor::start_shards_manager; use near_client::{start_client, start_view_client, ClientActor, ViewClientActor}; use near_epoch_manager::{EpochManager, EpochManagerAdapter}; use near_network::types::NetworkRecipient; @@ -43,6 +44,7 @@ fn setup_runtime( fn setup_mock_peer_manager_actor( chain: Chain, client: Arc, + shards_manager_adapter: Arc, genesis_config: &GenesisConfig, block_production_delay: Duration, client_start_height: BlockHeight, @@ -57,6 +59,7 @@ fn setup_mock_peer_manager_actor( }; MockPeerManagerActor::new( client, + shards_manager_adapter, genesis_config, chain, client_start_height, @@ -111,6 +114,7 @@ pub fn setup_mock_node( let node_id = config.network_config.node_id(); let network_adapter = Arc::new(NetworkRecipient::default()); + let shards_manager_adapter = Arc::new(NetworkRecipient::default()); let adv = near_client::adversarial::Controls::default(); // set up client dir to be ready to process blocks from client_start_height @@ -241,6 +245,7 @@ pub fn setup_mock_node( client_runtime.clone(), node_id, network_adapter.clone(), + shards_manager_adapter.clone(), config.validator_signer.clone(), telemetry, None, @@ -251,12 +256,22 @@ pub fn setup_mock_node( let view_client = start_view_client( None, chain_genesis.clone(), - client_runtime, + client_runtime.clone(), network_adapter.clone(), config.client_config.clone(), adv, ); + let (shards_manager_actor, _) = start_shards_manager( + client_runtime.clone(), + network_adapter.clone(), + Arc::new(client.clone()), + config.validator_signer.map(|signer| signer.validator_id().clone()), + client_runtime.store().clone(), + config.client_config.chunk_request_retry_period, + ); + shards_manager_adapter.set_recipient(shards_manager_actor); + let arbiter = Arbiter::new(); let client1 = client.clone(); let view_client1 = view_client.clone(); @@ -278,6 +293,7 @@ pub fn setup_mock_node( setup_mock_peer_manager_actor( chain, Arc::new(near_client::adapter::Adapter::new(client1, view_client1)), + shards_manager_adapter, &genesis_config, block_production_delay, client_start_height,