diff --git a/Cargo.lock b/Cargo.lock index 7987d34e20b..f33d96d2452 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4450,6 +4450,7 @@ dependencies = [ "itertools", "lru 0.12.3", "near-async", + "near-chain-configs", "near-crypto", "near-fmt", "near-o11y", diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index 171d6d059dd..a201710170e 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -198,8 +198,8 @@ pub enum Error { #[error("Invalid Split Shard Ids when resharding. shard_id: {0}, parent_shard_id: {1}")] InvalidSplitShardsIds(u64, u64), /// Someone is not a validator. Usually happens in signature verification - #[error("Not A Validator")] - NotAValidator, + #[error("Not A Validator: {0}")] + NotAValidator(String), /// Someone is not a chunk validator. Happens if we're asked to validate a chunk we're not /// supposed to validate, or to verify a chunk approval signed by a validator that isn't /// supposed to validate the chunk. @@ -308,7 +308,7 @@ impl Error { | Error::InvalidRandomnessBeaconOutput | Error::InvalidBlockMerkleRoot | Error::InvalidProtocolVersion - | Error::NotAValidator + | Error::NotAValidator(_) | Error::NotAChunkValidator | Error::InvalidChallengeRoot => true, } @@ -384,7 +384,7 @@ impl Error { Error::InvalidRandomnessBeaconOutput => "invalid_randomness_beacon_output", Error::InvalidBlockMerkleRoot => "invalid_block_merkele_root", Error::InvalidProtocolVersion => "invalid_protocol_version", - Error::NotAValidator => "not_a_validator", + Error::NotAValidator(_) => "not_a_validator", Error::NotAChunkValidator => "not_a_chunk_validator", Error::InvalidChallengeRoot => "invalid_challenge_root", } @@ -396,7 +396,9 @@ impl From for Error { match error { EpochError::EpochOutOfBounds(epoch_id) => Error::EpochOutOfBounds(epoch_id), EpochError::MissingBlock(h) => Error::DBNotFoundErr(format!("epoch block: {h}")), - EpochError::NotAValidator(_account_id, _epoch_id) => Error::NotAValidator, + EpochError::NotAValidator(account_id, epoch_id) => { + Error::NotAValidator(format!("account_id: {account_id}, epoch_id: {epoch_id:?}")) + } err => Error::ValidatorError(err.to_string()), } } diff --git a/chain/chain/src/chain.rs b/chain/chain/src/chain.rs index f3e208573cc..45d18aa697c 100644 --- a/chain/chain/src/chain.rs +++ b/chain/chain/src/chain.rs @@ -87,6 +87,7 @@ use near_primitives::unwrap_or_return; #[cfg(feature = "new_epoch_sync")] use near_primitives::utils::index_to_bytes; use near_primitives::utils::MaybeValidated; +use near_primitives::validator_signer::ValidatorSigner; use near_primitives::version::{ProtocolFeature, ProtocolVersion, PROTOCOL_VERSION}; use near_primitives::views::{ BlockStatusView, DroppedReason, ExecutionOutcomeWithIdView, ExecutionStatusView, @@ -400,7 +401,7 @@ impl Chain { chain_config: ChainConfig, snapshot_callbacks: Option, apply_chunks_spawner: Arc, - validator_account_id: Option<&AccountId>, + validator: MutableConfigValue>>, ) -> Result { // Get runtime initial state and create genesis block out of it. let state_roots = get_genesis_state_roots(runtime_adapter.store())? @@ -539,7 +540,7 @@ impl Chain { .iter() .filter(|shard_uid| { shard_tracker.care_about_shard( - validator_account_id, + validator.get().map(|v| v.validator_id().clone()).as_ref(), &tip.prev_block_hash, shard_uid.shard_id(), true, diff --git a/chain/chain/src/doomslug.rs b/chain/chain/src/doomslug.rs index 20f05a01174..cd922710e3f 100644 --- a/chain/chain/src/doomslug.rs +++ b/chain/chain/src/doomslug.rs @@ -138,7 +138,6 @@ pub struct Doomslug { endorsement_pending: bool, /// Information to track the timer (see `start_timer` routine in the paper) timer: DoomslugTimer, - signer: Option>, /// How many approvals to have before producing a block. In production should be always `HalfStake`, /// but for many tests we use `NoApprovals` to invoke more forkfulness threshold_mode: DoomslugThresholdMode, @@ -362,7 +361,6 @@ impl Doomslug { min_delay: Duration, delay_step: Duration, max_delay: Duration, - signer: Option>, threshold_mode: DoomslugThresholdMode, ) -> Self { Doomslug { @@ -392,7 +390,6 @@ impl Doomslug { delay_step, max_delay, }, - signer, threshold_mode, history: VecDeque::new(), } @@ -465,7 +462,7 @@ impl Doomslug { /// A vector of approvals that need to be sent to other block producers as a result of processing /// the timers #[must_use] - pub fn process_timer(&mut self) -> Vec { + pub fn process_timer(&mut self, signer: &Option>) -> Vec { let now = self.clock.now(); let mut ret = vec![]; for _ in 0..MAX_TIMER_ITERS { @@ -486,7 +483,7 @@ impl Doomslug { if tip_height >= self.largest_target_height.get() { self.largest_target_height.set(tip_height + 1); - if let Some(approval) = self.create_approval(tip_height + 1) { + if let Some(approval) = self.create_approval(tip_height + 1, signer) { ret.push(approval); } self.update_history(ApprovalHistoryEntry { @@ -514,7 +511,7 @@ impl Doomslug { self.largest_target_height .set(std::cmp::max(self.timer.height + 1, self.largest_target_height.get())); - if let Some(approval) = self.create_approval(self.timer.height + 1) { + if let Some(approval) = self.create_approval(self.timer.height + 1, signer) { ret.push(approval); } self.update_history(ApprovalHistoryEntry { @@ -541,9 +538,13 @@ impl Doomslug { ret } - fn create_approval(&self, target_height: BlockHeight) -> Option { - self.signer.as_ref().map(|signer| { - Approval::new(self.tip.block_hash, self.tip.height, target_height, &**signer) + fn create_approval( + &self, + target_height: BlockHeight, + signer: &Option>, + ) -> Option { + signer.as_ref().map(|signer| { + Approval::new(self.tip.block_hash, self.tip.height, target_height, &*signer) }) } @@ -787,6 +788,7 @@ mod tests { #[test] fn test_endorsements_and_skips_basic() { let clock = FakeClock::new(Utc::UNIX_EPOCH); + let signer = Some(Arc::new(create_test_signer("test").into())); let mut ds = Doomslug::new( clock.clock(), 0, @@ -794,29 +796,28 @@ mod tests { Duration::milliseconds(1000), Duration::milliseconds(100), Duration::milliseconds(3000), - Some(Arc::new(create_test_signer("test"))), DoomslugThresholdMode::TwoThirds, ); // Set a new tip, must produce an endorsement ds.set_tip(hash(&[1]), 1, 1); clock.advance(Duration::milliseconds(399)); - assert_eq!(ds.process_timer().len(), 0); + assert_eq!(ds.process_timer(&signer).len(), 0); clock.advance(Duration::milliseconds(1)); - let approval = ds.process_timer().into_iter().nth(0).unwrap(); + let approval = ds.process_timer(&signer).into_iter().nth(0).unwrap(); assert_eq!(approval.inner, ApprovalInner::Endorsement(hash(&[1]))); assert_eq!(approval.target_height, 2); // Same tip => no approval - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); // The block was `ds_final` and therefore started the timer. Try checking before one second expires clock.advance(Duration::milliseconds(599)); - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); // But one second should trigger the skip clock.advance(Duration::milliseconds(1)); - match ds.process_timer() { + match ds.process_timer(&signer) { approvals if approvals.is_empty() => assert!(false), approvals => { assert_eq!(approvals[0].inner, ApprovalInner::Skip(1)); @@ -827,7 +828,7 @@ mod tests { // Not processing a block at height 2 should not produce an appoval ds.set_tip(hash(&[2]), 2, 0); clock.advance(Duration::milliseconds(400)); - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); // Go forward more so we have 1 second clock.advance(Duration::milliseconds(600)); @@ -835,7 +836,7 @@ mod tests { // But at height 3 should (also neither block has finality set, keep last final at 0 for now) ds.set_tip(hash(&[3]), 3, 0); clock.advance(Duration::milliseconds(400)); - let approval = ds.process_timer().into_iter().nth(0).unwrap(); + let approval = ds.process_timer(&signer).into_iter().nth(0).unwrap(); assert_eq!(approval.inner, ApprovalInner::Endorsement(hash(&[3]))); assert_eq!(approval.target_height, 4); @@ -843,10 +844,10 @@ mod tests { clock.advance(Duration::milliseconds(600)); clock.advance(Duration::milliseconds(199)); - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); clock.advance(Duration::milliseconds(1)); - match ds.process_timer() { + match ds.process_timer(&signer) { approvals if approvals.is_empty() => assert!(false), approvals if approvals.len() == 1 => { assert_eq!(approvals[0].inner, ApprovalInner::Skip(3)); @@ -860,10 +861,10 @@ mod tests { // Now skip 5 (the extra delay is 200+300 = 500) clock.advance(Duration::milliseconds(499)); - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); clock.advance(Duration::milliseconds(1)); - match ds.process_timer() { + match ds.process_timer(&signer) { approvals if approvals.is_empty() => assert!(false), approvals => { assert_eq!(approvals[0].inner, ApprovalInner::Skip(3)); @@ -876,10 +877,10 @@ mod tests { // Skip 6 (the extra delay is 0+200+300+400 = 900) clock.advance(Duration::milliseconds(899)); - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); clock.advance(Duration::milliseconds(1)); - match ds.process_timer() { + match ds.process_timer(&signer) { approvals if approvals.is_empty() => assert!(false), approvals => { assert_eq!(approvals[0].inner, ApprovalInner::Skip(3)); @@ -893,11 +894,11 @@ mod tests { // Accept block at 5 with finality on the prev block, expect it to not produce an approval ds.set_tip(hash(&[5]), 5, 4); clock.advance(Duration::milliseconds(400)); - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); // Skip a whole bunch of heights by moving 100 seconds ahead clock.advance(Duration::seconds(100)); - assert!(ds.process_timer().len() > 10); + assert!(ds.process_timer(&signer).len() > 10); // Add some random small number of milliseconds to test that when the next block is added, the // timer is reset @@ -906,15 +907,15 @@ mod tests { // No approval, since we skipped 6 ds.set_tip(hash(&[6]), 6, 4); clock.advance(Duration::milliseconds(400)); - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); // The block height was less than the timer height, and thus the timer was reset. // The wait time for height 7 with last ds final block at 5 is 1100 clock.advance(Duration::milliseconds(699)); - assert_eq!(ds.process_timer(), vec![]); + assert_eq!(ds.process_timer(&signer), vec![]); clock.advance(Duration::milliseconds(1)); - match ds.process_timer() { + match ds.process_timer(&signer) { approvals if approvals.is_empty() => assert!(false), approvals => { assert_eq!(approvals[0].inner, ApprovalInner::Skip(6)); @@ -942,7 +943,6 @@ mod tests { .map(|(account_id, _, _)| create_test_signer(account_id)) .collect::>(); - let signer = Arc::new(create_test_signer("test")); let clock = FakeClock::new(Utc::UNIX_EPOCH); let mut ds = Doomslug::new( clock.clock(), @@ -951,7 +951,6 @@ mod tests { Duration::milliseconds(1000), Duration::milliseconds(100), Duration::milliseconds(3000), - Some(signer), DoomslugThresholdMode::TwoThirds, ); diff --git a/chain/chain/src/runtime/tests.rs b/chain/chain/src/runtime/tests.rs index 4da872683dd..17b0d026093 100644 --- a/chain/chain/src/runtime/tests.rs +++ b/chain/chain/src/runtime/tests.rs @@ -21,8 +21,8 @@ use num_rational::Ratio; use rand::{rngs::StdRng, seq::SliceRandom, SeedableRng}; use near_chain_configs::{ - default_produce_chunk_add_transactions_time_limit, Genesis, DEFAULT_GC_NUM_EPOCHS_TO_KEEP, - NEAR_BASE, + default_produce_chunk_add_transactions_time_limit, Genesis, MutableConfigValue, + DEFAULT_GC_NUM_EPOCHS_TO_KEEP, NEAR_BASE, }; use near_crypto::{InMemorySigner, KeyType, Signer}; use near_o11y::testonly::init_test_logger; @@ -1619,7 +1619,7 @@ fn get_test_env_with_chain_and_pool() -> (TestEnv, Chain, TransactionPool) { ChainConfig::test(), None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); diff --git a/chain/chain/src/store_validator.rs b/chain/chain/src/store_validator.rs index e83f5b5a9bd..075f7171c09 100644 --- a/chain/chain/src/store_validator.rs +++ b/chain/chain/src/store_validator.rs @@ -382,7 +382,7 @@ impl StoreValidator { #[cfg(test)] mod tests { use near_async::time::Clock; - use near_chain_configs::Genesis; + use near_chain_configs::{Genesis, MutableConfigValue}; use near_epoch_manager::EpochManager; use near_store::genesis::initialize_genesis_state; use near_store::test_utils::create_test_store; @@ -418,7 +418,7 @@ mod tests { ChainConfig::test(), None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); ( diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs index 18275484dbd..e9124bcfee9 100644 --- a/chain/chain/src/test_utils.rs +++ b/chain/chain/src/test_utils.rs @@ -13,7 +13,7 @@ use crate::types::{AcceptedBlock, ChainConfig, ChainGenesis}; use crate::DoomslugThresholdMode; use crate::{BlockProcessingArtifact, Provenance}; use near_async::time::Clock; -use near_chain_configs::Genesis; +use near_chain_configs::{Genesis, MutableConfigValue}; use near_chain_primitives::Error; use near_epoch_manager::shard_tracker::ShardTracker; use near_epoch_manager::{EpochManager, EpochManagerHandle}; @@ -75,7 +75,7 @@ pub fn get_chain_with_epoch_length_and_num_shards( ChainConfig::test(), None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap() } @@ -159,7 +159,7 @@ pub fn setup_with_tx_validity_period( ChainConfig::test(), None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); diff --git a/chain/chain/src/tests/doomslug.rs b/chain/chain/src/tests/doomslug.rs index 931029c5bd7..1757c08e10c 100644 --- a/chain/chain/src/tests/doomslug.rs +++ b/chain/chain/src/tests/doomslug.rs @@ -47,24 +47,22 @@ fn one_iter( .collect::>(); let signers = account_ids .iter() - .map(|account_id| Arc::new(create_test_signer(account_id))) + .map(|account_id| Some(Arc::new(create_test_signer(account_id)))) .collect::>(); let clock = FakeClock::new(Utc::UNIX_EPOCH); - let mut doomslugs = signers - .iter() - .map(|signer| { - Doomslug::new( - clock.clock(), - 0, - Duration::milliseconds(200), - Duration::milliseconds(1000), - Duration::milliseconds(100), - delta * 20, // some arbitrary number larger than delta * 6 - Some(signer.clone()), - DoomslugThresholdMode::TwoThirds, - ) - }) - .collect::>(); + let mut doomslugs: Vec<_> = std::iter::repeat_with(|| { + Doomslug::new( + clock.clock(), + 0, + Duration::milliseconds(200), + Duration::milliseconds(1000), + Duration::milliseconds(100), + delta * 20, // some arbitrary number larger than delta * 6 + DoomslugThresholdMode::TwoThirds, + ) + }) + .take(signers.len()) + .collect(); let started = clock.now(); let gst = clock.now() + time_to_gst; @@ -149,8 +147,8 @@ fn one_iter( block_queue = new_block_queue; // 3. Process timers - for ds in doomslugs.iter_mut() { - for approval in ds.process_timer() { + for (i, ds) in doomslugs.iter_mut().enumerate() { + for approval in ds.process_timer(&signers[i]) { approval_queue.push((approval, get_msg_delivery_time(clock.now(), gst, delta))); } } diff --git a/chain/chunks/src/shards_manager_actor.rs b/chain/chunks/src/shards_manager_actor.rs index 2d4f0d4b111..567dd875a71 100644 --- a/chain/chunks/src/shards_manager_actor.rs +++ b/chain/chunks/src/shards_manager_actor.rs @@ -98,6 +98,7 @@ use near_chain::byzantine_assert; use near_chain::chunks_store::ReadOnlyChunksStore; use near_chain::near_chain_primitives::error::Error::DBNotFoundErr; use near_chain::types::EpochManagerAdapter; +use near_chain_configs::MutableConfigValue; pub use near_chunks_primitives::Error; use near_epoch_manager::shard_tracker::ShardTracker; use near_network::shards_manager::ShardsManagerRequestFromNetwork; @@ -116,8 +117,8 @@ use near_primitives::receipt::Receipt; use near_primitives::reed_solomon::{reed_solomon_decode, reed_solomon_encode}; use near_primitives::sharding::{ ChunkHash, EncodedShardChunk, EncodedShardChunkBody, PartialEncodedChunk, - PartialEncodedChunkPart, PartialEncodedChunkV2, ReceiptProof, ShardChunk, ShardChunkHeader, - ShardProof, TransactionReceipt, + PartialEncodedChunkPart, PartialEncodedChunkV2, ShardChunk, ShardChunkHeader, + TransactionReceipt, }; use near_primitives::transaction::SignedTransaction; use near_primitives::types::validator_stake::ValidatorStake; @@ -243,7 +244,10 @@ impl RequestPool { pub struct ShardsManagerActor { clock: time::Clock, - me: Option, + /// Contains validator info about this node. This field is mutable and optional. Use with caution! + /// Lock the value of mutable validator signer for the duration of a request to ensure consistency. + /// Please note that the locked value should not be stored anywhere or passed through the thread boundary. + validator_signer: MutableConfigValue>>, store: ReadOnlyChunksStore, epoch_manager: Arc, @@ -293,7 +297,7 @@ pub fn start_shards_manager( shard_tracker: ShardTracker, network_adapter: Sender, client_adapter_for_shards_manager: Sender, - me: Option, + validator_signer: MutableConfigValue>>, store: Store, chunk_request_retry_period: Duration, ) -> (actix::Addr>, actix::ArbiterHandle) { @@ -310,7 +314,7 @@ pub fn start_shards_manager( let chunks_store = ReadOnlyChunksStore::new(store); let shards_manager = ShardsManagerActor::new( Clock::real(), - me, + validator_signer, epoch_manager, shard_tracker, network_adapter, @@ -331,7 +335,7 @@ pub fn start_shards_manager( impl ShardsManagerActor { pub fn new( clock: time::Clock, - me: Option, + validator_signer: MutableConfigValue>>, epoch_manager: Arc, shard_tracker: ShardTracker, network_adapter: Sender, @@ -343,7 +347,7 @@ impl ShardsManagerActor { ) -> Self { Self { clock, - me, + validator_signer, store, epoch_manager: epoch_manager.clone(), shard_tracker, @@ -384,7 +388,7 @@ impl ShardsManagerActor { ) } - pub fn update_chain_heads(&mut self, head: Tip, header_head: Tip) { + fn update_chain_heads(&mut self, head: Tip, header_head: Tip) { self.encoded_chunks.update_largest_seen_height( head.height, &self.requested_partial_encoded_chunks.requests, @@ -402,6 +406,7 @@ impl ShardsManagerActor { force_request_full: bool, request_own_parts_from_others: bool, request_from_archival: bool, + me: Option<&AccountId>, ) -> Result<(), near_chain::Error> { let _span = tracing::debug_span!( target: "chunks", @@ -417,7 +422,7 @@ impl ShardsManagerActor { let request_full = force_request_full || cares_about_shard_this_or_next_epoch( - self.me.as_ref(), + me, ancestor_hash, shard_id, true, @@ -445,7 +450,6 @@ impl ShardsManagerActor { // from the target account or any eligible peer of the node (See comments in // AccountIdOrPeerTrackingShard for when target account is used or peer is used) - let me = self.me.as_ref(); // A account that is either the original chunk producer or a random block producer tracking // the shard let shard_representative_target = if !request_own_parts_from_others @@ -454,7 +458,7 @@ impl ShardsManagerActor { { Some(chunk_producer_account_id) } else { - self.get_random_target_tracking_shard(ancestor_hash, shard_id)? + self.get_random_target_tracking_shard(ancestor_hash, shard_id, me)? }; let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(ancestor_hash)?; @@ -491,7 +495,7 @@ impl ShardsManagerActor { let shards_to_fetch_receipts = // TODO: only keep shards for which we don't have receipts yet - if request_full { HashSet::new() } else { self.get_tracking_shards(ancestor_hash) }; + if request_full { HashSet::new() } else { self.get_tracking_shards(ancestor_hash, me) }; // The loop below will be sending PartialEncodedChunkRequestMsg to various block producers. // We need to send such a message to the original chunk producer if we do not have the receipts @@ -555,6 +559,7 @@ impl ShardsManagerActor { &self, parent_hash: &CryptoHash, shard_id: ShardId, + me: Option<&AccountId>, ) -> Result, near_chain::Error> { let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(parent_hash).unwrap(); let block_producers = self @@ -571,7 +576,7 @@ impl ShardsManagerActor { false, &self.shard_tracker, ) - && self.me.as_ref() != Some(&account_id) + && me != Some(&account_id) { Some(account_id) } else { @@ -582,7 +587,11 @@ impl ShardsManagerActor { Ok(block_producers.choose(&mut rand::thread_rng())) } - fn get_tracking_shards(&self, parent_hash: &CryptoHash) -> HashSet { + fn get_tracking_shards( + &self, + parent_hash: &CryptoHash, + me: Option<&AccountId>, + ) -> HashSet { let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(parent_hash).unwrap(); self.epoch_manager .shard_ids(&epoch_id) @@ -590,7 +599,7 @@ impl ShardsManagerActor { .into_iter() .filter(|chunk_shard_id| { cares_about_shard_this_or_next_epoch( - self.me.as_ref(), + me, parent_hash, *chunk_shard_id, true, @@ -610,9 +619,10 @@ impl ShardsManagerActor { prev_hash: &CryptoHash, shard_id: ShardId, next_chunk_height: BlockHeight, + me: Option<&AccountId>, ) -> Result { // chunks will not be forwarded to non-validators - let me = match self.me.as_ref() { + let me = match me { None => return Ok(false), Some(it) => it, }; @@ -634,8 +644,12 @@ impl ShardsManagerActor { /// 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(), true) + fn request_chunk_single_mark_only( + &mut self, + chunk_header: &ShardChunkHeader, + me: Option<&AccountId>, + ) { + self.request_chunk_single(chunk_header, *chunk_header.prev_block_hash(), true, me) } /// send partial chunk requests for one chunk @@ -651,6 +665,7 @@ impl ShardsManagerActor { chunk_header: &ShardChunkHeader, ancestor_hash: CryptoHash, mark_only: bool, + me: Option<&AccountId>, ) { let height = chunk_header.height_created(); let shard_id = chunk_header.shard_id(); @@ -710,7 +725,7 @@ impl ShardsManagerActor { && 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(|_| { + self.should_wait_for_chunk_forwarding(&ancestor_hash, chunk_header.shard_id(), chunk_header.height_created()+1, me).unwrap_or_else(|_| { // ancestor_hash must be accepted because we don't request missing chunks through this // this function for orphans debug_assert!(false, "{:?} must be accepted", ancestor_hash); @@ -733,6 +748,7 @@ impl ShardsManagerActor { false, old_block, fetch_from_archival, + me, ); if let Err(err) = request_result { error!(target: "chunks", "Error during requesting partial encoded chunk: {}", err); @@ -746,10 +762,11 @@ impl ShardsManagerActor { /// `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 - pub fn request_chunks( + fn request_chunks( &mut self, chunks_to_request: Vec, prev_hash: CryptoHash, + me: Option<&AccountId>, ) { let _span = debug_span!( target: "chunks", @@ -758,7 +775,7 @@ impl ShardsManagerActor { num_chunks_to_request = chunks_to_request.len()) .entered(); for chunk_header in chunks_to_request { - self.request_chunk_single(&chunk_header, prev_hash, false); + self.request_chunk_single(&chunk_header, prev_hash, false, me); } } @@ -770,11 +787,12 @@ impl ShardsManagerActor { /// 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( + fn request_chunks_for_orphan( &mut self, chunks_to_request: Vec, epoch_id: &EpochId, ancestor_hash: CryptoHash, + me: Option<&AccountId>, ) { let _span = debug_span!( target: "chunks", @@ -790,7 +808,7 @@ impl ShardsManagerActor { } for chunk_header in chunks_to_request { - self.request_chunk_single(&chunk_header, ancestor_hash, false) + self.request_chunk_single(&chunk_header, ancestor_hash, false, me) } } @@ -802,6 +820,7 @@ impl ShardsManagerActor { header_head_height = self.chain_header_head.height, pool_size = self.requested_partial_encoded_chunks.len()) .entered(); + let me = self.validator_signer.get().map(|signer| signer.validator_id().clone()); // Process chunk one part requests. let requests = self.requested_partial_encoded_chunks.fetch(self.clock.now().into()); for (chunk_hash, chunk_request) in requests { @@ -826,6 +845,7 @@ impl ShardsManagerActor { || self.clock.now() - chunk_request.added >= self.requested_partial_encoded_chunks.switch_to_others_duration, fetch_from_archival, + me.as_ref(), ) { Ok(()) => {} Err(err) => { @@ -836,34 +856,11 @@ impl ShardsManagerActor { } } - pub fn receipts_recipient_filter( - from_shard_id: ShardId, - tracking_shards: T, - receipts_by_shard: &HashMap>, - proofs: &[MerklePath], - ) -> Vec - where - T: IntoIterator, - { - tracking_shards - .into_iter() - .map(|to_shard_id| { - let receipts = - receipts_by_shard.get(&to_shard_id).cloned().unwrap_or_else(Vec::new); - let shard_proof = ShardProof { - from_shard_id, - to_shard_id, - proof: proofs[to_shard_id as usize].clone(), - }; - ReceiptProof(receipts, shard_proof) - }) - .collect() - } - - pub fn process_partial_encoded_chunk_request( + fn process_partial_encoded_chunk_request( &self, request: PartialEncodedChunkRequestMsg, route_back: CryptoHash, + me: Option<&AccountId>, ) { let _span = tracing::debug_span!( target: "chunks", @@ -874,7 +871,7 @@ impl ShardsManagerActor { chunk_hash = %request.chunk_hash.0, part_ords = ?request.part_ords, shards = ?request.tracking_shards, - account = ?self.me.as_ref()); + account = ?me); let started = self.clock.now(); let (source, response) = self.prepare_partial_encoded_chunk_response(request); @@ -998,7 +995,7 @@ impl ShardsManagerActor { /// Looks up the given part_ords and tracking_shards from the partial chunks /// storage, appending any we have found into the response, and deleting those we /// have found from part_ords and tracking_shards. - pub fn lookup_partial_encoded_chunk_from_partial_chunk_storage( + fn lookup_partial_encoded_chunk_from_partial_chunk_storage( part_ords: HashSet, tracking_shards: HashSet, response: &mut PartialEncodedChunkResponseMsg, @@ -1152,6 +1149,7 @@ impl ShardsManagerActor { fn decode_encoded_chunk_if_complete( &mut self, mut encoded_chunk: EncodedShardChunk, + me: Option<&AccountId>, ) -> Result, Error> { match self.check_chunk_complete(&mut encoded_chunk) { ChunkStatus::Complete(merkle_paths) => { @@ -1159,7 +1157,7 @@ impl ShardsManagerActor { match decode_encoded_chunk( &encoded_chunk, merkle_paths, - self.me.as_ref(), + me, self.epoch_manager.as_ref(), &self.shard_tracker, ) { @@ -1266,9 +1264,10 @@ impl ShardsManagerActor { } } - pub fn process_partial_encoded_chunk_forward( + fn process_partial_encoded_chunk_forward( &mut self, forward: PartialEncodedChunkForwardMsg, + me: Option<&AccountId>, ) -> Result<(), Error> { let maybe_header = self .validate_partial_encoded_chunk_forward(&forward) @@ -1305,7 +1304,7 @@ impl ShardsManagerActor { parts: forward.parts, prev_outgoing_receipts: Vec::new(), }); - self.process_partial_encoded_chunk(MaybeValidated::from_validated(partial_chunk))?; + self.process_partial_encoded_chunk(MaybeValidated::from_validated(partial_chunk), me)?; Ok(()) } @@ -1449,9 +1448,10 @@ impl ShardsManagerActor { /// are needed for processing the full chunk /// ProcessPartialEncodedChunkResult::HaveAllPartsAndReceipts: if all parts and /// receipts in the chunk are received and the chunk has been processed. - pub fn process_partial_encoded_chunk( + fn process_partial_encoded_chunk( &mut self, partial_encoded_chunk: MaybeValidated, + me: Option<&AccountId>, ) -> Result { let partial_encoded_chunk = partial_encoded_chunk.map(|chunk| PartialEncodedChunkV2::from(chunk)); @@ -1558,6 +1558,7 @@ impl ShardsManagerActor { new_part_ords, &epoch_id, &partial_encoded_chunk.header.prev_block_hash(), + me, )?; } else { let epoch_id = self @@ -1568,6 +1569,7 @@ impl ShardsManagerActor { new_part_ords, &epoch_id, &self.chain_head.last_block_hash.clone(), + me, )?; }; @@ -1575,39 +1577,41 @@ impl ShardsManagerActor { self.insert_header_if_not_exists_and_process_cached_chunk_forwards(header); // 5. Check if the chunk is complete; requesting more if not. - let result = self.try_process_chunk_parts_and_receipts(header)?; + let result = self.try_process_chunk_parts_and_receipts(header, me)?; match result { ProcessPartialEncodedChunkResult::NeedMorePartsOrReceipts => { // This may be the first time we see this chunk, so mark it in the request pool. // If it's not already requested for, next time we resend requests we would // request the chunk. - self.request_chunk_single_mark_only(header); + self.request_chunk_single_mark_only(header, me); } _ => {} } Ok(result) } - pub fn process_partial_encoded_chunk_response( + fn process_partial_encoded_chunk_response( &mut self, response: PartialEncodedChunkResponseMsg, + me: Option<&AccountId>, ) -> Result<(), Error> { let header = self.get_partial_encoded_chunk_header(&response.chunk_hash)?; let partial_chunk = PartialEncodedChunk::new(header, response.parts, response.receipts); // We already know the header signature is valid because we read it from the // shard manager. - self.process_partial_encoded_chunk(MaybeValidated::from_validated(partial_chunk))?; + self.process_partial_encoded_chunk(MaybeValidated::from_validated(partial_chunk), me)?; Ok(()) } /// Let the ShardsManager know about the chunk header, when encountering that chunk header /// from the block and the chunk is possibly not yet known to the ShardsManager. - pub fn process_chunk_header_from_block( + fn process_chunk_header_from_block( &mut self, header: &ShardChunkHeader, + me: Option<&AccountId>, ) -> Result<(), Error> { if self.insert_header_if_not_exists_and_process_cached_chunk_forwards(header) { - self.try_process_chunk_parts_and_receipts(header)?; + self.try_process_chunk_parts_and_receipts(header, me)?; } Ok(()) } @@ -1619,6 +1623,7 @@ impl ShardsManagerActor { fn try_process_chunk_parts_and_receipts( &mut self, header: &ShardChunkHeader, + me: Option<&AccountId>, ) -> Result { let chunk_hash = header.chunk_hash(); let _span = debug_span!( @@ -1676,8 +1681,8 @@ impl ShardsManagerActor { // chunk. See comments in has_all_parts and has_all_receipts to see the conditions. // we can safely unwrap here because we already checked that chunk_hash exist in encoded_chunks let entry = self.encoded_chunks.get(&chunk_hash).unwrap(); - let have_all_parts = self.has_all_parts(&prev_block_hash, entry)?; - let have_all_receipts = self.has_all_receipts(&prev_block_hash, entry)?; + let have_all_parts = self.has_all_parts(&prev_block_hash, entry, me)?; + let have_all_receipts = self.has_all_receipts(&prev_block_hash, entry, me)?; let can_reconstruct = entry.parts.len() >= self.epoch_manager.num_data_parts(); let chunk_producer = self.epoch_manager.get_chunk_producer( @@ -1698,7 +1703,7 @@ impl ShardsManagerActor { let entry = self.encoded_chunks.get(&chunk_hash).unwrap(); let cares_about_shard = cares_about_shard_this_or_next_epoch( - self.me.as_ref(), + me, &prev_block_hash, header.shard_id(), true, @@ -1713,7 +1718,7 @@ impl ShardsManagerActor { header, entry.parts.values(), entry.receipts.values(), - self.me.as_ref(), + me, self.epoch_manager.as_ref(), &self.shard_tracker, ); @@ -1738,7 +1743,7 @@ impl ShardsManagerActor { } let (shard_chunk, partial_chunk) = self - .decode_encoded_chunk_if_complete(encoded_chunk)? + .decode_encoded_chunk_if_complete(encoded_chunk, me)? .expect("decoding shouldn't fail"); // For consistency, only persist shard_chunk if we actually care about the shard. @@ -1775,7 +1780,7 @@ impl ShardsManagerActor { /// This function is needed because chunks in chunk cache will only be marked as complete after /// the previous block is accepted. So we need to check if there are any chunks can be marked as /// complete when a new block is accepted. - pub fn check_incomplete_chunks(&mut self, prev_block_hash: &CryptoHash) { + fn check_incomplete_chunks(&mut self, prev_block_hash: &CryptoHash, me: Option<&AccountId>) { let _span = debug_span!(target: "chunks", "check_incomplete_chunks", ?prev_block_hash).entered(); let mut chunks_to_process = vec![]; @@ -1792,13 +1797,13 @@ impl ShardsManagerActor { chunk_hash = ?header.chunk_hash(), ?prev_block_hash, "try to process incomplete chunk"); - if let Err(err) = self.try_process_chunk_parts_and_receipts(&header) { + if let Err(err) = self.try_process_chunk_parts_and_receipts(&header, me) { error!(target:"chunks", "unexpected error processing orphan chunk {:?}", err) } } } - /// Send the parts of the partial_encoded_chunk that are owned by `self.me` to the + /// Send the parts of the partial_encoded_chunk that are owned by `self.me()` to the /// other validators that are tracking the shard. fn send_partial_encoded_chunk_to_chunk_trackers( &mut self, @@ -1806,8 +1811,9 @@ impl ShardsManagerActor { part_ords: HashSet, epoch_id: &EpochId, latest_block_hash: &CryptoHash, + me: Option<&AccountId>, ) -> Result<(), Error> { - let me = match self.me.as_ref() { + let me = match me { Some(me) => me, None => return Ok(()), }; @@ -1929,11 +1935,12 @@ impl ShardsManagerActor { &self, prev_block_hash: &CryptoHash, chunk_entry: &EncodedChunksCacheEntry, + me: Option<&AccountId>, ) -> Result { let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(prev_block_hash)?; for shard_id in self.epoch_manager.shard_ids(&epoch_id)? { if !chunk_entry.receipts.contains_key(&shard_id) { - if need_receipt(prev_block_hash, shard_id, self.me.as_ref(), &self.shard_tracker) { + if need_receipt(prev_block_hash, shard_id, me, &self.shard_tracker) { return Ok(false); } } @@ -1948,16 +1955,12 @@ impl ShardsManagerActor { &self, prev_block_hash: &CryptoHash, chunk_entry: &EncodedChunksCacheEntry, + me: Option<&AccountId>, ) -> Result { for part_ord in 0..self.epoch_manager.num_total_parts() { let part_ord = part_ord as u64; if !chunk_entry.parts.contains_key(&part_ord) { - if need_part( - prev_block_hash, - part_ord, - self.me.as_ref(), - self.epoch_manager.as_ref(), - )? { + if need_part(prev_block_hash, part_ord, me, self.epoch_manager.as_ref())? { return Ok(false); } } @@ -2006,12 +2009,13 @@ impl ShardsManagerActor { .map_err(|err| err.into()) } - pub fn distribute_encoded_chunk( + fn distribute_encoded_chunk( &mut self, partial_chunk: PartialEncodedChunk, encoded_chunk: EncodedShardChunk, merkle_paths: &Vec, outgoing_receipts: Vec, + me: Option<&AccountId>, ) -> Result<(), Error> { let shard_id = encoded_chunk.shard_id(); let _timer = metrics::DISTRIBUTE_ENCODED_CHUNK_TIME @@ -2067,7 +2071,7 @@ impl ShardsManagerActor { &merkle_paths, ); - if Some(&to_whom) != self.me.as_ref() { + if Some(&to_whom) != me { self.peer_manager_adapter.send(PeerManagerMessageRequest::NetworkRequests( NetworkRequests::PartialEncodedChunkMessage { account_id: to_whom.clone(), @@ -2091,9 +2095,11 @@ impl ShardsManagerActor { "type" = <&'static str>::from(&request) ) .entered(); + let me = self.validator_signer.get().map(|signer| signer.validator_id().clone()); + let me = me.as_ref(); match request { ShardsManagerRequestFromClient::ProcessChunkHeaderFromBlock(chunk_header) => { - if let Err(e) = self.process_chunk_header_from_block(&chunk_header) { + if let Err(e) = self.process_chunk_header_from_block(&chunk_header, me) { warn!(target: "chunks", "Error processing chunk header from block: {:?}", e); } } @@ -2111,29 +2117,30 @@ impl ShardsManagerActor { encoded_chunk, &merkle_paths, outgoing_receipts, + me, ) { warn!(target: "chunks", "Error distributing encoded chunk: {:?}", e); } } ShardsManagerRequestFromClient::RequestChunks { chunks_to_request, prev_hash } => { - self.request_chunks(chunks_to_request, prev_hash) + self.request_chunks(chunks_to_request, prev_hash, me) } ShardsManagerRequestFromClient::RequestChunksForOrphan { chunks_to_request, epoch_id, ancestor_hash, - } => self.request_chunks_for_orphan(chunks_to_request, &epoch_id, ancestor_hash), + } => self.request_chunks_for_orphan(chunks_to_request, &epoch_id, ancestor_hash, me), ShardsManagerRequestFromClient::CheckIncompleteChunks(prev_block_hash) => { - self.check_incomplete_chunks(&prev_block_hash) + self.check_incomplete_chunks(&prev_block_hash, me) } ShardsManagerRequestFromClient::ProcessOrRequestChunk { candidate_chunk, request_header, prev_hash, } => { - if let Err(err) = self.process_partial_encoded_chunk(candidate_chunk.into()) { + if let Err(err) = self.process_partial_encoded_chunk(candidate_chunk.into(), me) { warn!(target: "chunks", ?err, "Error processing partial encoded chunk"); - self.request_chunk_single(&request_header, prev_hash, false); + self.request_chunk_single(&request_header, prev_hash, false, me); } } ShardsManagerRequestFromClient::ProcessOrRequestChunkForOrphan { @@ -2142,9 +2149,14 @@ impl ShardsManagerActor { ancestor_hash, epoch_id, } => { - if let Err(e) = self.process_partial_encoded_chunk(candidate_chunk.into()) { + if let Err(e) = self.process_partial_encoded_chunk(candidate_chunk.into(), me) { warn!(target: "chunks", "Error processing partial encoded chunk: {:?}", e); - self.request_chunks_for_orphan(vec![request_header], &epoch_id, ancestor_hash); + self.request_chunks_for_orphan( + vec![request_header], + &epoch_id, + ancestor_hash, + me, + ); } } } @@ -2157,9 +2169,12 @@ impl ShardsManagerActor { "type" = <&'static str>::from(&request) ) .entered(); + let me = self.validator_signer.get().map(|signer| signer.validator_id().clone()); + let me = me.as_ref(); match request { ShardsManagerRequestFromNetwork::ProcessPartialEncodedChunk(partial_encoded_chunk) => { - if let Err(e) = self.process_partial_encoded_chunk(partial_encoded_chunk.into()) { + if let Err(e) = self.process_partial_encoded_chunk(partial_encoded_chunk.into(), me) + { warn!(target: "chunks", "Error processing partial encoded chunk: {:?}", e); } } @@ -2167,7 +2182,7 @@ impl ShardsManagerActor { partial_encoded_chunk_forward, ) => { if let Err(e) = - self.process_partial_encoded_chunk_forward(partial_encoded_chunk_forward) + self.process_partial_encoded_chunk_forward(partial_encoded_chunk_forward, me) { warn!(target: "chunks", "Error processing partial encoded chunk forward: {:?}", e); } @@ -2180,7 +2195,7 @@ impl ShardsManagerActor { (self.clock.now().signed_duration_since(received_time)).as_seconds_f64(), ); if let Err(e) = - self.process_partial_encoded_chunk_response(partial_encoded_chunk_response) + self.process_partial_encoded_chunk_response(partial_encoded_chunk_response, me) { warn!(target: "chunks", "Error processing partial encoded chunk response: {:?}", e); } @@ -2192,6 +2207,7 @@ impl ShardsManagerActor { self.process_partial_encoded_chunk_request( partial_encoded_chunk_request, route_back, + me, ); } } @@ -2236,6 +2252,7 @@ mod test { use near_primitives::block::Tip; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::types::EpochId; + use near_primitives::validator_signer::EmptyValidatorSigner; use near_store::test_utils::create_test_store; use std::sync::Arc; @@ -2243,6 +2260,15 @@ mod test { use crate::logic::persist_chunk; use crate::test_utils::*; + fn mutable_validator_signer( + account_id: &AccountId, + ) -> MutableConfigValue>> { + MutableConfigValue::new( + Some(Arc::new(EmptyValidatorSigner::new(account_id.clone()))), + "validator_signer", + ) + } + /// should not request partial encoded chunk from self #[test] fn test_request_partial_encoded_chunk_from_self() { @@ -2268,7 +2294,7 @@ mod test { let clock = FakeClock::default(); let mut shards_manager = ShardsManagerActor::new( clock.clock(), - Some("test".parse().unwrap()), + mutable_validator_signer(&"test".parse().unwrap()), epoch_manager, shard_tracker, network_adapter.as_sender(), @@ -2312,7 +2338,7 @@ mod test { let clock = FakeClock::default(); let mut shards_manager = ShardsManagerActor::new( clock.clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2325,7 +2351,10 @@ mod test { // process chunk part 0 let partial_encoded_chunk = fixture.make_partial_encoded_chunk(&[0]); let result = shards_manager - .process_partial_encoded_chunk(MaybeValidated::from(partial_encoded_chunk)) + .process_partial_encoded_chunk( + MaybeValidated::from(partial_encoded_chunk), + Some(&fixture.mock_shard_tracker), + ) .unwrap(); assert_matches!(result, ProcessPartialEncodedChunkResult::NeedBlock); @@ -2334,6 +2363,7 @@ mod test { &fixture.mock_chunk_header, CryptoHash::default(), false, + Some(&fixture.mock_shard_tracker), ); let collect_request_parts = |fixture: &mut ChunkTestFixture| -> HashSet { let mut parts = HashSet::new(); @@ -2358,7 +2388,10 @@ mod test { // process chunk part 1 let partial_encoded_chunk = fixture.make_partial_encoded_chunk(&[1]); let result = shards_manager - .process_partial_encoded_chunk(MaybeValidated::from(partial_encoded_chunk)) + .process_partial_encoded_chunk( + MaybeValidated::from(partial_encoded_chunk), + Some(&fixture.mock_shard_tracker), + ) .unwrap(); assert_matches!(result, ProcessPartialEncodedChunkResult::NeedBlock); @@ -2385,7 +2418,7 @@ mod test { let fixture = ChunkTestFixture::default(); let mut shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2401,8 +2434,10 @@ mod test { if let PartialEncodedChunk::V2(ref mut chunk) = partial_encoded_chunk { chunk.parts[0].part_ord = fixture.mock_chunk_parts.len() as u64; } - let result = shards_manager - .process_partial_encoded_chunk(MaybeValidated::from(partial_encoded_chunk)); + let result = shards_manager.process_partial_encoded_chunk( + MaybeValidated::from(partial_encoded_chunk), + Some(&fixture.mock_shard_tracker), + ); assert_matches!(result, Err(Error::InvalidChunkPartId)); // TODO: add more test cases @@ -2414,7 +2449,7 @@ mod test { let fixture = ChunkTestFixture::default(); let mut shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_chunk_part_owner.clone()), + mutable_validator_signer(&fixture.mock_chunk_part_owner), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2451,15 +2486,24 @@ mod test { .make_partial_encoded_chunk(&[non_owned_part_ords[2], fixture.mock_part_ords[0]]), ]; shards_manager - .process_partial_encoded_chunk(MaybeValidated::from(partial_encoded_chunks.remove(0))) + .process_partial_encoded_chunk( + MaybeValidated::from(partial_encoded_chunks.remove(0)), + Some(&fixture.mock_chunk_part_owner), + ) .unwrap(); let num_forward_msgs_after_first_receiving = count_num_forward_msgs(&fixture); assert!(num_forward_msgs_after_first_receiving > 0); shards_manager - .process_partial_encoded_chunk(MaybeValidated::from(partial_encoded_chunks.remove(0))) + .process_partial_encoded_chunk( + MaybeValidated::from(partial_encoded_chunks.remove(0)), + Some(&fixture.mock_chunk_part_owner), + ) .unwrap(); shards_manager - .process_partial_encoded_chunk(MaybeValidated::from(partial_encoded_chunks.remove(0))) + .process_partial_encoded_chunk( + MaybeValidated::from(partial_encoded_chunks.remove(0)), + Some(&fixture.mock_chunk_part_owner), + ) .unwrap(); let num_forward_msgs_after_receiving_duplicates = count_num_forward_msgs(&fixture); assert_eq!( @@ -2481,9 +2525,13 @@ mod test { account_id: Option, ) -> RequestChunksResult { let clock = FakeClock::default(); + let validator = MutableConfigValue::new( + account_id.clone().map(|id| Arc::new(EmptyValidatorSigner::new(id))), + "validator_signer", + ); let mut shards_manager = ShardsManagerActor::new( clock.clock(), - account_id, + validator, Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2499,6 +2547,7 @@ mod test { shards_manager.request_chunks( vec![fixture.mock_chunk_header.clone()], *fixture.mock_chunk_header.prev_block_hash(), + account_id.as_ref(), ); let marked_as_requested = shards_manager .requested_partial_encoded_chunks @@ -2572,7 +2621,7 @@ mod test { let clock = FakeClock::default(); let mut shards_manager = ShardsManagerActor::new( clock.clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2593,7 +2642,9 @@ mod test { most_parts, ); // The validator receives the chunk forward - assert!(shards_manager.process_partial_encoded_chunk_forward(forward).is_ok()); + assert!(shards_manager + .process_partial_encoded_chunk_forward(forward, Some(&fixture.mock_shard_tracker)) + .is_ok()); let partial_encoded_chunk = PartialEncodedChunk::V2(PartialEncodedChunkV2 { header: fixture.mock_chunk_header.clone(), parts: other_parts, @@ -2601,7 +2652,10 @@ mod test { }); // The validator receives a chunk header with the rest of the parts it needed let result = shards_manager - .process_partial_encoded_chunk(MaybeValidated::from(partial_encoded_chunk)) + .process_partial_encoded_chunk( + MaybeValidated::from(partial_encoded_chunk), + Some(&fixture.mock_shard_tracker), + ) .unwrap(); match result { @@ -2615,6 +2669,7 @@ mod test { vec![fixture.mock_chunk_header.clone()], &EpochId::default(), CryptoHash::default(), + Some(&fixture.mock_shard_tracker), ); clock.advance(CHUNK_REQUEST_RETRY * 2); shards_manager.resend_chunk_requests(); @@ -2642,7 +2697,7 @@ mod test { let clock = FakeClock::default(); let mut shards_manager = ShardsManagerActor::new( clock.clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2657,7 +2712,9 @@ mod test { fixture.mock_chunk_parts.clone(), ); // The validator receives the chunk forward - assert!(shards_manager.process_partial_encoded_chunk_forward(forward).is_ok(),); + assert!(shards_manager + .process_partial_encoded_chunk_forward(forward, Some(&fixture.mock_shard_tracker)) + .is_ok(),); // The validator then receives the block, which is missing chunks; it notifies the // ShardsManager of the chunk header, and ShardsManager is able to complete the chunk // because of the forwarded parts.shards_manager @@ -2665,7 +2722,10 @@ mod test { &fixture.mock_chunk_header, ); let process_result = shards_manager - .try_process_chunk_parts_and_receipts(&fixture.mock_chunk_header) + .try_process_chunk_parts_and_receipts( + &fixture.mock_chunk_header, + Some(&fixture.mock_shard_tracker), + ) .unwrap(); match process_result { ProcessPartialEncodedChunkResult::HaveAllPartsAndReceipts => {} @@ -2680,6 +2740,7 @@ mod test { &fixture.mock_chunk_header, *fixture.mock_chunk_header.prev_block_hash(), false, + Some(&fixture.mock_shard_tracker), ); clock.advance(CHUNK_REQUEST_RETRY * 2); @@ -2704,7 +2765,7 @@ mod test { let fixture = ChunkTestFixture::default(); let mut shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2721,6 +2782,7 @@ mod test { fixture.mock_encoded_chunk.clone(), &fixture.mock_merkle_paths, fixture.mock_outgoing_receipts.clone(), + Some(&fixture.mock_shard_tracker), ) .unwrap(); @@ -2739,7 +2801,7 @@ mod test { let fixture = ChunkTestFixture::default(); let mut shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2753,6 +2815,7 @@ mod test { shards_manager .process_partial_encoded_chunk( fixture.make_partial_encoded_chunk(&fixture.all_part_ords).into(), + Some(&fixture.mock_shard_tracker), ) .unwrap(); @@ -2771,7 +2834,7 @@ mod test { let mut fixture = ChunkTestFixture::default(); let shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2804,7 +2867,7 @@ mod test { let mut fixture = ChunkTestFixture::default(); let shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2838,7 +2901,7 @@ mod test { let mut fixture = ChunkTestFixture::default(); let mut shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2854,7 +2917,10 @@ mod test { fixture.all_part_ords.split_at(fixture.all_part_ords.len() / 2); shards_manager - .process_partial_encoded_chunk(fixture.make_partial_encoded_chunk(cache_ords).into()) + .process_partial_encoded_chunk( + fixture.make_partial_encoded_chunk(cache_ords).into(), + Some(&fixture.mock_shard_tracker), + ) .unwrap(); persist_chunk( @@ -2878,7 +2944,7 @@ mod test { let mut fixture = ChunkTestFixture::default(); let mut shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2898,6 +2964,7 @@ mod test { &fixture.all_part_ords[0..fixture.all_part_ords.len() / 2], ) .into(), + Some(&fixture.mock_shard_tracker), ) .unwrap(); @@ -2924,7 +2991,7 @@ mod test { let mut fixture = ChunkTestFixture::default(); let mut shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2942,7 +3009,10 @@ mod test { let partial_ords = &fixture.all_part_ords.as_slice()[n / 3..(n * 2 / 3)]; shards_manager - .process_partial_encoded_chunk(fixture.make_partial_encoded_chunk(cache_ords).into()) + .process_partial_encoded_chunk( + fixture.make_partial_encoded_chunk(cache_ords).into(), + Some(&fixture.mock_shard_tracker), + ) .unwrap(); persist_chunk( @@ -2966,7 +3036,7 @@ mod test { let fixture = ChunkTestFixture::default(); let shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -2991,7 +3061,7 @@ mod test { let fixture = ChunkTestFixture::default(); let shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -3016,7 +3086,7 @@ mod test { let mut fixture = ChunkTestFixture::default(); let shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -3051,7 +3121,7 @@ mod test { let mut fixture = ChunkTestFixture::default(); let shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_shard_tracker.clone()), + mutable_validator_signer(&fixture.mock_shard_tracker), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -3084,7 +3154,7 @@ mod test { let fixture = ChunkTestFixture::default(); let mut shards_manager = ShardsManagerActor::new( FakeClock::default().clock(), - Some(fixture.mock_chunk_part_owner.clone()), + mutable_validator_signer(&fixture.mock_chunk_part_owner), Arc::new(fixture.epoch_manager.clone()), fixture.shard_tracker.clone(), fixture.mock_network.as_sender(), @@ -3095,11 +3165,18 @@ mod test { Duration::hours(1), ); let part = fixture.make_partial_encoded_chunk(&fixture.mock_part_ords); - shards_manager.process_partial_encoded_chunk(part.clone().into()).unwrap(); + shards_manager + .process_partial_encoded_chunk( + part.clone().into(), + Some(&fixture.mock_chunk_part_owner), + ) + .unwrap(); assert_eq!(fixture.count_chunk_ready_for_inclusion_messages(), 1); // test that chunk inclusion message is only sent once. - shards_manager.process_partial_encoded_chunk(part.into()).unwrap(); + shards_manager + .process_partial_encoded_chunk(part.into(), Some(&fixture.mock_chunk_part_owner)) + .unwrap(); assert_eq!(fixture.count_chunk_ready_for_inclusion_messages(), 0); } } diff --git a/chain/client/src/client.rs b/chain/client/src/client.rs index d0bdc6d2497..a7dc79abf81 100644 --- a/chain/client/src/client.rs +++ b/chain/client/src/client.rs @@ -148,7 +148,9 @@ pub struct Client { pub sharded_tx_pool: ShardedTransactionPool, /// Network adapter. pub network_adapter: PeerManagerAdapter, - /// Signer for block producer (if present). + /// Signer for block producer (if present). This field is mutable and optional. Use with caution! + /// Lock the value of mutable validator signer for the duration of a request to ensure consistency. + /// Please note that the locked value should not be stored anywhere or passed through the thread boundary. pub validator_signer: MutableConfigValue>>, /// Approvals for which we do not have the block yet pub pending_approvals: @@ -234,7 +236,7 @@ impl Client { runtime_adapter: Arc, network_adapter: PeerManagerAdapter, shards_manager_adapter: Sender, - validator_signer: Option>, + validator_signer: MutableConfigValue>>, enable_doomslug: bool, rng_seed: RngSeed, snapshot_callbacks: Option, @@ -261,7 +263,7 @@ impl Client { chain_config.clone(), snapshot_callbacks, async_computation_spawner.clone(), - validator_signer.as_ref().map(|x| x.validator_id()), + validator_signer.clone(), )?; // Create flat storage or initiate migration to flat storage. let flat_storage_creator = FlatStorageCreator::new( @@ -340,7 +342,6 @@ impl Client { config.max_block_production_delay, config.max_block_production_delay / 10, config.max_block_wait_delay, - validator_signer.clone(), doomslug_threshold_mode, ); let chunk_endorsement_tracker = @@ -349,7 +350,6 @@ impl Client { let panic_on_validation_error = config.chain_id != near_primitives::chains::MAINNET && config.chain_id != near_primitives::chains::TESTNET; let chunk_validator = ChunkValidator::new( - validator_signer.clone(), epoch_manager.clone(), network_adapter.clone().into_sender(), runtime_adapter.clone(), @@ -382,7 +382,7 @@ impl Client { shards_manager_adapter, sharded_tx_pool, network_adapter, - validator_signer: MutableConfigValue::new(validator_signer, "validator_signer"), + validator_signer, pending_approvals: lru::LruCache::new( NonZeroUsize::new(num_block_producer_seats).unwrap(), ), @@ -822,16 +822,17 @@ impl Client { last_header: ShardChunkHeader, next_height: BlockHeight, shard_id: ShardId, + signer: Option<&Arc>, ) -> Result, Error> { - let validator_signer = self.validator_signer.get().ok_or_else(|| { + let signer = signer.ok_or_else(|| { Error::ChunkProducer("Called without block producer info.".to_string()) })?; let chunk_proposer = self.epoch_manager.get_chunk_producer(epoch_id, next_height, shard_id).unwrap(); - if validator_signer.validator_id() != &chunk_proposer { + if signer.validator_id() != &chunk_proposer { debug!(target: "client", - me = ?validator_signer.validator_id(), + me = ?signer.as_ref().validator_id(), ?chunk_proposer, next_height, shard_id, @@ -839,14 +840,7 @@ impl Client { return Ok(None); } - self.produce_chunk( - prev_block, - epoch_id, - last_header, - next_height, - shard_id, - validator_signer, - ) + self.produce_chunk(prev_block, epoch_id, last_header, next_height, shard_id, signer) } #[instrument(target = "client", level = "debug", "produce_chunk", skip_all, fields( @@ -862,7 +856,7 @@ impl Client { last_header: ShardChunkHeader, next_height: BlockHeight, shard_id: ShardId, - validator_signer: Arc, + validator_signer: &Arc, ) -> Result, Error> { let span = tracing::Span::current(); let timer = Instant::now(); @@ -1079,8 +1073,12 @@ impl Client { Ok(prepared_transactions) } - pub fn send_challenges(&mut self, challenges: Vec) { - if let Some(validator_signer) = &self.validator_signer.get() { + fn send_challenges( + &mut self, + challenges: Vec, + signer: &Option>, + ) { + if let Some(validator_signer) = &signer { for body in challenges { let challenge = Challenge::produce(body, &**validator_signer); self.challenges.insert(challenge.hash, challenge.clone()); @@ -1099,13 +1097,14 @@ impl Client { peer_id: PeerId, was_requested: bool, apply_chunks_done_sender: Option>, + signer: &Option>, ) { let hash = *block.hash(); let prev_hash = *block.header().prev_hash(); let _span = tracing::debug_span!( target: "client", "receive_block", - me = ?self.validator_signer.get().map(|vs| vs.validator_id().clone()), + me = ?signer.as_ref().map(|vs| vs.validator_id()), %prev_hash, %hash, height = block.header().height(), @@ -1113,7 +1112,13 @@ impl Client { was_requested) .entered(); - let res = self.receive_block_impl(block, peer_id, was_requested, apply_chunks_done_sender); + let res = self.receive_block_impl( + block, + peer_id, + was_requested, + apply_chunks_done_sender, + signer, + ); // Log the errors here. Note that the real error handling logic is already // done within process_block_impl, this is just for logging. if let Err(err) = res { @@ -1148,6 +1153,7 @@ impl Client { peer_id: PeerId, was_requested: bool, apply_chunks_done_sender: Option>, + signer: &Option>, ) -> Result<(), near_chain::Error> { let _span = debug_span!(target: "chain", "receive_block_impl", was_requested, ?peer_id).entered(); @@ -1176,7 +1182,7 @@ impl Client { self.verify_and_rebroadcast_block(&block, was_requested, &peer_id)?; let provenance = if was_requested { near_chain::Provenance::SYNC } else { near_chain::Provenance::NONE }; - let res = self.start_process_block(block, provenance, apply_chunks_done_sender); + let res = self.start_process_block(block, provenance, apply_chunks_done_sender, signer); match &res { Err(near_chain::Error::Orphan) => { debug!(target: "chain", ?prev_hash, "Orphan error"); @@ -1281,6 +1287,7 @@ impl Client { block: MaybeValidated, provenance: Provenance, apply_chunks_done_sender: Option>, + signer: &Option>, ) -> Result<(), near_chain::Error> { let _span = debug_span!( target: "chain", @@ -1291,10 +1298,7 @@ impl Client { let mut block_processing_artifacts = BlockProcessingArtifact::default(); let result = { - let me = self - .validator_signer - .get() - .map(|validator_signer| validator_signer.validator_id().clone()); + let me = signer.as_ref().map(|vs| vs.validator_id().clone()); self.chain.start_process_block_async( &me, block, @@ -1304,17 +1308,17 @@ impl Client { ) }; - self.process_block_processing_artifact(block_processing_artifacts); + self.process_block_processing_artifact(block_processing_artifacts, &signer); // Send out challenge if the block was found to be invalid. - if let Some(validator_signer) = self.validator_signer.get() { + if let Some(signer) = signer { if let Err(e) = &result { match e { near_chain::Error::InvalidChunkProofs(chunk_proofs) => { self.network_adapter.send(PeerManagerMessageRequest::NetworkRequests( NetworkRequests::Challenge(Challenge::produce( ChallengeBody::ChunkProofs(*chunk_proofs.clone()), - &*validator_signer, + &*signer, )), )); } @@ -1322,7 +1326,7 @@ impl Client { self.network_adapter.send(PeerManagerMessageRequest::NetworkRequests( NetworkRequests::Challenge(Challenge::produce( ChallengeBody::ChunkState(*chunk_state.clone()), - &*validator_signer, + &*signer, )), )); } @@ -1340,13 +1344,11 @@ impl Client { &mut self, apply_chunks_done_sender: Option>, should_produce_chunk: bool, + signer: &Option>, ) -> (Vec, HashMap) { let _span = debug_span!(target: "client", "postprocess_ready_blocks", should_produce_chunk) .entered(); - let me = self - .validator_signer - .get() - .map(|validator_signer| validator_signer.validator_id().clone()); + let me = signer.as_ref().map(|signer| signer.validator_id().clone()); let mut block_processing_artifacts = BlockProcessingArtifact::default(); let (accepted_blocks, errors) = self.chain.postprocess_ready_blocks( &me, @@ -1359,7 +1361,7 @@ impl Client { header_head: self.chain.header_head().unwrap(), }); } - self.process_block_processing_artifact(block_processing_artifacts); + self.process_block_processing_artifact(block_processing_artifacts, signer); let accepted_blocks_hashes = accepted_blocks.iter().map(|accepted_block| accepted_block.hash).collect(); for accepted_block in accepted_blocks { @@ -1368,6 +1370,7 @@ impl Client { accepted_block.status, accepted_block.provenance, !should_produce_chunk, + signer, ); } self.last_time_head_progress_made = @@ -1382,6 +1385,7 @@ impl Client { pub(crate) fn process_block_processing_artifact( &mut self, block_processing_artifacts: BlockProcessingArtifact, + signer: &Option>, ) { let BlockProcessingArtifact { orphans_missing_chunks, @@ -1390,7 +1394,7 @@ impl Client { invalid_chunks, } = block_processing_artifacts; // Send out challenges that accumulated via on_challenge. - self.send_challenges(challenges); + self.send_challenges(challenges, &signer); // 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 @@ -1449,6 +1453,7 @@ impl Client { partial_chunk: PartialEncodedChunk, shard_chunk: Option, apply_chunks_done_sender: Option>, + signer: &Option>, ) { let chunk_header = partial_chunk.cloned_header(); self.chain.blocks_delay_tracker.mark_chunk_completed(&chunk_header); @@ -1463,7 +1468,7 @@ impl Client { // 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. - self.process_blocks_with_missing_chunks(apply_chunks_done_sender) + self.process_blocks_with_missing_chunks(apply_chunks_done_sender, &signer); } /// Called asynchronously when the ShardsManager finishes processing a chunk but the chunk @@ -1479,10 +1484,11 @@ impl Client { pub fn sync_block_headers( &mut self, headers: Vec, + signer: &Option>, ) -> Result<(), near_chain::Error> { let mut challenges = vec![]; self.chain.sync_block_headers(headers, &mut challenges)?; - self.send_challenges(challenges); + self.send_challenges(challenges, signer); self.shards_manager_adapter.send(ShardsManagerRequestFromClient::UpdateChainHeads { head: self.chain.head().unwrap(), header_head: self.chain.header_head().unwrap(), @@ -1548,14 +1554,14 @@ impl Client { &mut self, parent_hash: &CryptoHash, approval: Approval, + signer: &Option>, ) -> Result<(), Error> { let next_epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(parent_hash)?; let next_block_producer = self.epoch_manager.get_block_producer(&next_epoch_id, approval.target_height)?; - let validator_signer = self.validator_signer.get(); - let next_block_producer_id = validator_signer.as_ref().map(|x| x.validator_id()); + let next_block_producer_id = signer.as_ref().map(|x| x.validator_id()); if Some(&next_block_producer) == next_block_producer_id { - self.collect_block_approval(&approval, ApprovalType::SelfApproval); + self.collect_block_approval(&approval, ApprovalType::SelfApproval, signer); } else { debug!(target: "client", approval_inner = ?approval.inner, @@ -1575,12 +1581,13 @@ impl Client { /// Gets called when block got accepted. /// Only produce chunk if `skip_produce_chunk` is false. /// `skip_produce_chunk` is set to true to simulate when there are missing chunks in a block - pub fn on_block_accepted_with_optional_chunk_produce( + fn on_block_accepted_with_optional_chunk_produce( &mut self, block_hash: CryptoHash, status: BlockStatus, provenance: Provenance, skip_produce_chunk: bool, + signer: &Option>, ) { let _span = tracing::debug_span!( target: "client", @@ -1617,7 +1624,7 @@ impl Client { for (_account_id, (approval, approval_type)) in endorsements.into_iter().chain(skips.into_iter()) { - self.collect_block_approval(&approval, approval_type); + self.collect_block_approval(&approval, approval_type, signer); } } @@ -1658,10 +1665,10 @@ impl Client { } } - if let Some(validator_signer) = self.validator_signer.get() { - let validator_id = validator_signer.validator_id().clone(); + if let Some(signer) = signer.clone() { + let validator_id = signer.validator_id().clone(); - if !self.reconcile_transaction_pool(validator_id.clone(), status, &block) { + if !self.reconcile_transaction_pool(validator_id, status, &block) { return; } @@ -1669,7 +1676,7 @@ impl Client { && !self.sync_status.is_syncing() && !skip_produce_chunk { - self.produce_chunks(&block, validator_id); + self.produce_chunks(&block, &signer); } else { info!(target: "client", "not producing a chunk"); } @@ -1692,7 +1699,7 @@ impl Client { self.shards_manager_adapter .send(ShardsManagerRequestFromClient::CheckIncompleteChunks(*block.hash())); - self.process_ready_orphan_witnesses_and_clean_old(&block); + self.process_ready_orphan_witnesses_and_clean_old(&block, signer); } /// Reconcile the transaction pool after processing a block. @@ -1764,7 +1771,8 @@ impl Client { } // Produce new chunks - fn produce_chunks(&mut self, block: &Block, validator_id: AccountId) { + fn produce_chunks(&mut self, block: &Block, signer: &Arc) { + let validator_id = signer.validator_id().clone(); let _span = debug_span!( target: "client", "produce_chunks", @@ -1812,6 +1820,7 @@ impl Client { last_header.clone(), next_height, shard_id, + Some(signer), ) { Ok(Some(result)) => { let shard_chunk = self @@ -1828,6 +1837,7 @@ impl Client { &last_header, &shard_chunk, result.transactions_storage_proof, + &Some(signer.clone()), ) { tracing::error!(target: "client", ?err, "Failed to send chunk state witness to chunk validators"); } @@ -1955,21 +1965,26 @@ impl Client { pub fn process_blocks_with_missing_chunks( &mut self, apply_chunks_done_sender: Option>, + signer: &Option>, ) { let _span = debug_span!(target: "client", "process_blocks_with_missing_chunks").entered(); - let validator_signer = self.validator_signer.get(); - let me = validator_signer.as_ref().map(|validator_signer| validator_signer.validator_id()); + let me = signer.as_ref().map(|signer| signer.validator_id()); let mut blocks_processing_artifacts = BlockProcessingArtifact::default(); self.chain.check_blocks_with_missing_chunks( &me.map(|x| x.clone()), &mut blocks_processing_artifacts, apply_chunks_done_sender, ); - self.process_block_processing_artifact(blocks_processing_artifacts); + self.process_block_processing_artifact(blocks_processing_artifacts, signer); } - pub fn is_validator(&self, epoch_id: &EpochId, block_hash: &CryptoHash) -> bool { - match self.validator_signer.get() { + pub fn is_validator( + &self, + epoch_id: &EpochId, + block_hash: &CryptoHash, + signer: &Option>, + ) -> bool { + match signer { None => false, Some(signer) => { let account_id = signer.validator_id(); @@ -2036,7 +2051,12 @@ impl Client { /// * `approval` - the approval to be collected /// * `approval_type` - whether the approval was just produced by us (in which case skip validation, /// only check whether we are the next block producer and store in Doomslug) - pub fn collect_block_approval(&mut self, approval: &Approval, approval_type: ApprovalType) { + pub fn collect_block_approval( + &mut self, + approval: &Approval, + approval_type: ApprovalType, + signer: &Option>, + ) { let Approval { inner, account_id, target_height, signature } = approval; let parent_hash = match inner { @@ -2119,9 +2139,7 @@ impl Client { match self.epoch_manager.get_block_producer(&next_block_epoch_id, *target_height) { Err(_) => false, Ok(target_block_producer) => { - let validator_signer = self.validator_signer.get(); - Some(&target_block_producer) - == validator_signer.as_ref().map(|x| x.validator_id()) + Some(&target_block_producer) == signer.as_ref().map(|x| x.validator_id()) } }; @@ -2153,7 +2171,12 @@ impl Client { } /// Forwards given transaction to upcoming validators. - fn forward_tx(&self, epoch_id: &EpochId, tx: &SignedTransaction) -> Result<(), Error> { + fn forward_tx( + &self, + epoch_id: &EpochId, + tx: &SignedTransaction, + signer: &Option>, + ) -> Result<(), Error> { let shard_id = self.epoch_manager.account_id_to_shard_id(tx.transaction.signer_id(), epoch_id)?; // Use the header head to make sure the list of validators is as @@ -2182,12 +2205,11 @@ impl Client { } } - let validator_signer = self.validator_signer.get(); - if let Some(account_id) = validator_signer.as_ref().map(|bp| bp.validator_id()) { + if let Some(account_id) = signer.as_ref().map(|bp| bp.validator_id()) { validators.remove(account_id); } for validator in validators { - trace!(target: "client", me = ?validator_signer.as_ref().map(|bp| bp.validator_id()), ?tx, ?validator, shard_id, "Routing a transaction"); + trace!(target: "client", me = ?signer.as_ref().map(|bp| bp.validator_id()), ?tx, ?validator, shard_id, "Routing a transaction"); // Send message to network to actually forward transaction. self.network_adapter.send(PeerManagerMessageRequest::NetworkRequests( @@ -2209,9 +2231,9 @@ impl Client { is_forwarded: bool, check_only: bool, ) -> ProcessTxResponse { - unwrap_or_return!(self.process_tx_internal(&tx, is_forwarded, check_only), { - let validator_signer = self.validator_signer.get(); - let me = validator_signer.as_ref().map(|vs| vs.validator_id()); + let signer = self.validator_signer.get(); + unwrap_or_return!(self.process_tx_internal(&tx, is_forwarded, check_only, &signer), { + let me = signer.as_ref().map(|signer| signer.validator_id()); warn!(target: "client", ?me, ?tx, "Dropping tx"); ProcessTxResponse::NoResponse }) @@ -2239,12 +2261,16 @@ impl Client { /// If we're a validator in one of the next few chunks, but epoch switch could happen soon, /// we forward to a validator from next epoch. - fn possibly_forward_tx_to_next_epoch(&mut self, tx: &SignedTransaction) -> Result<(), Error> { + fn possibly_forward_tx_to_next_epoch( + &mut self, + tx: &SignedTransaction, + signer: &Option>, + ) -> Result<(), Error> { let head = self.chain.head()?; if let Some(next_epoch_id) = self.get_next_epoch_id_if_at_boundary(&head)? { - self.forward_tx(&next_epoch_id, tx)?; + self.forward_tx(&next_epoch_id, tx, signer)?; } else { - self.forward_tx(&head.epoch_id, tx)?; + self.forward_tx(&head.epoch_id, tx, signer)?; } Ok(()) } @@ -2255,10 +2281,10 @@ impl Client { tx: &SignedTransaction, is_forwarded: bool, check_only: bool, + signer: &Option>, ) -> Result { let head = self.chain.head()?; - let validator_signer = self.validator_signer.get(); - let me = validator_signer.as_ref().map(|vs| vs.validator_id()); + let me = signer.as_ref().map(|vs| vs.validator_id()); let cur_block = self.chain.get_head_block()?; let cur_block_header = cur_block.header(); let transaction_validity_period = self.chain.transaction_validity_period; @@ -2316,7 +2342,7 @@ impl Client { if is_forwarded { return Err(Error::Other("Node has not caught up yet".to_string())); } else { - self.forward_tx(&epoch_id, tx)?; + self.forward_tx(&epoch_id, tx, signer)?; return Ok(ProcessTxResponse::RequestRouted); } } @@ -2364,18 +2390,18 @@ impl Client { // Not active validator: // forward to current epoch validators, // possibly forward to next epoch validators - if self.active_validator(shard_id)? { + if self.active_validator(shard_id, signer)? { trace!(target: "client", account = ?me, shard_id, tx_hash = ?tx.get_hash(), is_forwarded, "Recording a transaction."); metrics::TRANSACTION_RECEIVED_VALIDATOR.inc(); if !is_forwarded { - self.possibly_forward_tx_to_next_epoch(tx)?; + self.possibly_forward_tx_to_next_epoch(tx, signer)?; } Ok(ProcessTxResponse::ValidTx) } else if !is_forwarded { trace!(target: "client", shard_id, tx_hash = ?tx.get_hash(), "Forwarding a transaction."); metrics::TRANSACTION_RECEIVED_NON_VALIDATOR.inc(); - self.forward_tx(&epoch_id, tx)?; + self.forward_tx(&epoch_id, tx, signer)?; Ok(ProcessTxResponse::RequestRouted) } else { trace!(target: "client", shard_id, tx_hash = ?tx.get_hash(), "Non-validator received a forwarded transaction, dropping it."); @@ -2391,18 +2417,21 @@ impl Client { Ok(ProcessTxResponse::NoResponse) } else { // We are not tracking this shard, so there is no way to validate this tx. Just rerouting. - self.forward_tx(&epoch_id, tx)?; + self.forward_tx(&epoch_id, tx, signer)?; Ok(ProcessTxResponse::RequestRouted) } } /// Determine if I am a validator in next few blocks for specified shard, assuming epoch doesn't change. - fn active_validator(&self, shard_id: ShardId) -> Result { + fn active_validator( + &self, + shard_id: ShardId, + signer: &Option>, + ) -> Result { let head = self.chain.head()?; let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(&head.last_block_hash)?; - let validator_signer = self.validator_signer.get(); - let account_id = if let Some(vs) = validator_signer.as_ref() { + let account_id = if let Some(vs) = signer.as_ref() { vs.validator_id() } else { return Ok(false); @@ -2428,16 +2457,17 @@ impl Client { resharding_scheduler: &Sender, apply_chunks_done_sender: Option>, state_parts_future_spawner: &dyn FutureSpawner, + signer: &Option>, ) -> Result<(), Error> { let _span = debug_span!(target: "sync", "run_catchup").entered(); let mut notify_state_sync = false; - let me = &self.validator_signer.get().map(|x| x.validator_id().clone()); + let me = signer.as_ref().map(|x| x.validator_id().clone()); for (sync_hash, state_sync_info) in self.chain.chain_store().iterate_state_sync_infos()? { assert_eq!(sync_hash, state_sync_info.epoch_tail_hash); let network_adapter = self.network_adapter.clone(); - let shards_to_split = self.get_shards_to_split(sync_hash, &state_sync_info, me)?; + let shards_to_split = self.get_shards_to_split(sync_hash, &state_sync_info, &me)?; let state_sync_timeout = self.config.state_sync_timeout; let block_header = self.chain.get_block(&sync_hash)?.header().clone(); let epoch_id = block_header.epoch_id(); @@ -2506,7 +2536,7 @@ impl Client { // for other shards later. let new_shard_sync = shards_to_split; match state_sync.run( - me, + &me, sync_hash, new_shard_sync, &mut self.chain, @@ -2524,7 +2554,7 @@ impl Client { StateSyncResult::Completed => { debug!(target: "catchup", "state sync completed now catch up blocks"); self.chain.catchup_blocks_step( - me, + &me, &sync_hash, blocks_catch_up_state, block_catch_up_task_scheduler, @@ -2534,14 +2564,14 @@ impl Client { let mut block_processing_artifacts = BlockProcessingArtifact::default(); self.chain.finish_catchup_blocks( - me, + &me, &sync_hash, &mut block_processing_artifacts, apply_chunks_done_sender.clone(), &blocks_catch_up_state.done_blocks, )?; - self.process_block_processing_artifact(block_processing_artifacts); + self.process_block_processing_artifact(block_processing_artifacts, &signer); } } } diff --git a/chain/client/src/client_actor.rs b/chain/client/src/client_actor.rs index fda5546b076..78f9b07ac05 100644 --- a/chain/client/src/client_actor.rs +++ b/chain/client/src/client_actor.rs @@ -41,7 +41,7 @@ use near_chain::{ byzantine_assert, near_chain_primitives, Block, BlockHeader, BlockProcessingArtifact, ChainGenesis, Provenance, }; -use near_chain_configs::{ClientConfig, LogSummaryStyle, ReshardingHandle}; +use near_chain_configs::{ClientConfig, LogSummaryStyle, MutableConfigValue, ReshardingHandle}; use near_chain_primitives::error::EpochErrorResultToChainError; use near_chunks::adapter::ShardsManagerRequestFromClient; use near_chunks::client::ShardsManagerResponse; @@ -134,7 +134,7 @@ pub fn start_client( state_sync_adapter: Arc>, network_adapter: PeerManagerAdapter, shards_manager_adapter: Sender, - validator_signer: Option>, + validator_signer: MutableConfigValue>>, telemetry_sender: Sender, snapshot_callbacks: Option, sender: Option>, @@ -158,7 +158,7 @@ pub fn start_client( runtime.clone(), network_adapter.clone(), shards_manager_adapter, - validator_signer.clone(), + validator_signer, enable_doomslug, seed.unwrap_or_else(random_seed_from_thread), snapshot_callbacks, @@ -182,7 +182,6 @@ pub fn start_client( client_config, node_id, network_adapter, - validator_signer, telemetry_sender, sender, adv, @@ -254,7 +253,6 @@ pub struct ClientActorInner { // Last time when log_summary method was called. log_summary_timer_next_attempt: near_async::time::Utc, - block_production_started: bool, doomslug_timer_next_attempt: near_async::time::Utc, sync_timer_next_attempt: near_async::time::Utc, sync_started: bool, @@ -310,7 +308,6 @@ impl ClientActorInner { config: ClientConfig, node_id: PeerId, network_adapter: PeerManagerAdapter, - validator_signer: Option>, telemetry_sender: Sender, shutdown_signal: Option>, adv: crate::adversarial::Controls, @@ -318,11 +315,10 @@ impl ClientActorInner { sync_jobs_sender: SyncJobsSenderForClient, state_parts_future_spawner: Box, ) -> Result { - if let Some(vs) = &validator_signer { + if let Some(vs) = &client.validator_signer.get() { info!(target: "client", "Starting validator node: {}", vs.validator_id()); } - let info_helper = - InfoHelper::new(clock.clone(), telemetry_sender, &config, validator_signer); + let info_helper = InfoHelper::new(clock.clone(), telemetry_sender, &config); let now = clock.now_utc(); Ok(ClientActorInner { @@ -348,7 +344,6 @@ impl ClientActorInner { info_helper, block_production_next_attempt: now, log_summary_timer_next_attempt: now, - block_production_started: false, doomslug_timer_next_attempt: now, sync_timer_next_attempt: now, sync_started: false, @@ -409,6 +404,7 @@ impl Handler for ClientActorInner { } let start_height = self.client.chain.mut_chain_store().get_latest_known().unwrap().height + 1; + let signer = self.client.validator_signer.get(); let mut blocks_produced = 0; for height in start_height.. { let block = @@ -425,6 +421,7 @@ impl Handler for ClientActorInner { block.into(), Provenance::PRODUCED, Some(self.myself_sender.apply_chunks_done.clone()), + &signer, ); blocks_produced += 1; if blocks_produced == num_blocks { @@ -508,11 +505,13 @@ impl Handler for ClientActorInner { // blocks other than the few special ones that State Sync expects. return; } + let signer = self.client.validator_signer.get(); self.client.receive_block( block, peer_id, was_requested, Some(self.myself_sender.apply_chunks_done.clone()), + &signer, ); } else { match self.client.epoch_manager.get_epoch_id_from_prev_block(block.header().prev_hash()) @@ -533,7 +532,8 @@ impl Handler for ClientActorInner { impl Handler for ClientActorInner { fn handle(&mut self, msg: BlockHeadersResponse) -> Result<(), ReasonForBan> { let BlockHeadersResponse(headers, peer_id) = msg; - if self.receive_headers(headers, peer_id) { + let validator_signer = self.client.validator_signer.get(); + if self.receive_headers(headers, peer_id, &validator_signer) { Ok(()) } else { warn!(target: "client", "Banning node for sending invalid block headers"); @@ -546,7 +546,12 @@ impl Handler for ClientActorInner { fn handle(&mut self, msg: BlockApproval) { let BlockApproval(approval, peer_id) = msg; debug!(target: "client", "Receive approval {:?} from peer {:?}", approval, peer_id); - self.client.collect_block_approval(&approval, ApprovalType::PeerApproval(peer_id)); + let validator_signer = self.client.validator_signer.get(); + self.client.collect_block_approval( + &approval, + ApprovalType::PeerApproval(peer_id), + &validator_signer, + ); } } @@ -825,7 +830,8 @@ impl Handler for ClientActorInner { impl Handler for ClientActorInner { fn handle(&mut self, _msg: ApplyChunksDoneMessage) { - self.try_process_unfinished_blocks(); + let validator_signer = self.client.validator_signer.get(); + self.try_process_unfinished_blocks(&validator_signer); } } @@ -887,11 +893,6 @@ impl ClientActorInner { // Start syncing job. self.start_sync(ctx); - // Start block production tracking if have block producer info. - if self.client.validator_signer.get().is_some() { - self.block_production_started = true; - } - // Start triggers self.schedule_triggers(ctx); @@ -905,7 +906,11 @@ impl ClientActorInner { /// Check if client Account Id should be sent and send it. /// Account Id is sent when is not current a validator but are becoming a validator soon. - fn check_send_announce_account(&mut self, prev_block_hash: CryptoHash) { + fn check_send_announce_account( + &mut self, + prev_block_hash: CryptoHash, + validator_signer: &Option>, + ) { // If no peers, there is no one to announce to. if self.network_info.num_connected_peers == 0 { debug!(target: "client", "No peers: skip account announce"); @@ -913,7 +918,7 @@ impl ClientActorInner { } // First check that we currently have an AccountId - let validator_signer = match self.client.validator_signer.get() { + let signer = match validator_signer { None => return, Some(signer) => signer, }; @@ -928,7 +933,7 @@ impl ClientActorInner { } } - debug!(target: "client", "Check announce account for {}, last announce time {:?}", validator_signer.validator_id(), self.last_validator_announce_time); + debug!(target: "client", "Check announce account for {}, last announce time {:?}", signer.validator_id(), self.last_validator_announce_time); // Announce AccountId if client is becoming a validator soon. let next_epoch_id = unwrap_or_return!(self @@ -937,18 +942,15 @@ impl ClientActorInner { .get_next_epoch_id_from_prev_block(&prev_block_hash)); // Check client is part of the futures validators - if self.client.is_validator(&next_epoch_id, &prev_block_hash) { - debug!(target: "client", "Sending announce account for {}", validator_signer.validator_id()); + if self.client.is_validator(&next_epoch_id, &prev_block_hash, validator_signer) { + debug!(target: "client", "Sending announce account for {}", signer.validator_id()); self.last_validator_announce_time = Some(now); - let signature = validator_signer.sign_account_announce( - validator_signer.validator_id(), - &self.node_id, - &next_epoch_id, - ); + let signature = + signer.sign_account_announce(signer.validator_id(), &self.node_id, &next_epoch_id); self.network_adapter.send(PeerManagerMessageRequest::NetworkRequests( NetworkRequests::AnnounceAccount(AnnounceAccount { - account_id: validator_signer.validator_id().clone(), + account_id: signer.validator_id().clone(), peer_id: self.node_id.clone(), epoch_id: next_epoch_id, signature, @@ -1037,7 +1039,10 @@ impl ClientActorInner { /// Retrieves latest height, and checks if must produce next block. /// Otherwise wait for block arrival or suggest to skip after timeout. - fn handle_block_production(&mut self) -> Result<(), Error> { + fn handle_block_production( + &mut self, + signer: &Option>, + ) -> Result<(), Error> { let _span = tracing::debug_span!(target: "client", "handle_block_production").entered(); // If syncing, don't try to produce blocks. if self.client.sync_status.is_syncing() { @@ -1077,7 +1082,7 @@ impl ClientActorInner { debug!(target: "client", "Cannot produce any block: not enough approvals beyond {}", latest_known.height); } - let me = if let Some(me) = &self.client.validator_signer.get() { + let me = if let Some(me) = signer { me.validator_id().clone() } else { return Ok(()); @@ -1122,7 +1127,7 @@ impl ClientActorInner { self.client .chunk_inclusion_tracker .record_endorsement_metrics(&head.last_block_hash); - if let Err(err) = self.produce_block(height) { + if let Err(err) = self.produce_block(height, signer) { // If there is an error, report it and let it retry on the next loop step. error!(target: "client", height, "Block production failed: {}", err); } else { @@ -1155,7 +1160,7 @@ impl ClientActorInner { /// /// Returns the delay before the next time `check_triggers` should be called, which is /// min(time until the closest trigger, 1 second). - pub(crate) fn check_triggers(&mut self, ctx: &mut dyn DelayedActionRunner) -> Duration { + fn check_triggers(&mut self, ctx: &mut dyn DelayedActionRunner) -> Duration { let _span = tracing::debug_span!(target: "client", "check_triggers").entered(); if let Some(config_updater) = &mut self.config_updater { config_updater.try_update(&|updateable_client_config| { @@ -1175,7 +1180,8 @@ impl ClientActorInner { } } - self.try_process_unfinished_blocks(); + let validator_signer = self.client.validator_signer.get(); + self.try_process_unfinished_blocks(&validator_signer); let mut delay = near_async::time::Duration::seconds(1); let now = self.clock.now_utc(); @@ -1201,7 +1207,7 @@ impl ClientActorInner { ); delay = core::cmp::min(delay, self.doomslug_timer_next_attempt - now) } - if self.block_production_started { + if validator_signer.is_some() { self.block_production_next_attempt = self.run_timer( self.client.config.block_production_tracking_delay, self.block_production_next_attempt, @@ -1239,20 +1245,23 @@ impl ClientActorInner { /// calls this function to finish processing the unfinished blocks. ClientActor also calls /// this function in `check_triggers`, because the actix queue may be blocked by other messages /// and we want to prioritize block processing. - fn try_process_unfinished_blocks(&mut self) { + fn try_process_unfinished_blocks(&mut self, signer: &Option>) { let _span = debug_span!(target: "client", "try_process_unfinished_blocks").entered(); - let (accepted_blocks, errors) = self - .client - .postprocess_ready_blocks(Some(self.myself_sender.apply_chunks_done.clone()), true); + let (accepted_blocks, errors) = self.client.postprocess_ready_blocks( + Some(self.myself_sender.apply_chunks_done.clone()), + true, + signer, + ); if !errors.is_empty() { error!(target: "client", ?errors, "try_process_unfinished_blocks got errors"); } - self.process_accepted_blocks(accepted_blocks); + self.process_accepted_blocks(accepted_blocks, signer); } fn try_handle_block_production(&mut self) { let _span = debug_span!(target: "client", "try_handle_block_production").entered(); - if let Err(err) = self.handle_block_production() { + let signer = self.client.validator_signer.get(); + if let Err(err) = self.handle_block_production(&signer) { tracing::error!(target: "client", ?err, "Handle block production failed") } } @@ -1260,7 +1269,8 @@ impl ClientActorInner { fn try_doomslug_timer(&mut self) { let _span = tracing::debug_span!(target: "client", "try_doomslug_timer").entered(); let _ = self.client.check_and_update_doomslug_tip(); - let approvals = self.client.doomslug.process_timer(); + let signer = self.client.validator_signer.get(); + let approvals = self.client.doomslug.process_timer(&signer); // Important to save the largest approval target height before sending approvals, so // that if the node crashes in the meantime, we cannot get slashed on recovery @@ -1271,13 +1281,15 @@ impl ClientActorInner { match chain_store_update.commit() { Ok(_) => { let head = unwrap_or_return!(self.client.chain.head()); - if self.client.is_validator(&head.epoch_id, &head.last_block_hash) - || self.client.is_validator(&head.next_epoch_id, &head.last_block_hash) + if self.client.is_validator(&head.epoch_id, &head.last_block_hash, &signer) + || self.client.is_validator(&head.next_epoch_id, &head.last_block_hash, &signer) { for approval in approvals { - if let Err(e) = - self.client.send_approval(&self.client.doomslug.get_tip().0, approval) - { + if let Err(e) = self.client.send_approval( + &self.client.doomslug.get_tip().0, + approval, + &signer, + ) { error!("Error while sending an approval {:?}", e); } } @@ -1289,7 +1301,11 @@ impl ClientActorInner { /// Produce block if we are block producer for given `next_height` height. /// Can return error, should be called with `produce_block` to handle errors and reschedule. - fn produce_block(&mut self, next_height: BlockHeight) -> Result<(), Error> { + fn produce_block( + &mut self, + next_height: BlockHeight, + signer: &Option>, + ) -> Result<(), Error> { let _span = tracing::debug_span!(target: "client", "produce_block", next_height).entered(); let Some(block) = self.client.produce_block_on_head(next_height, false)? else { return Ok(()); @@ -1305,6 +1321,7 @@ impl ClientActorInner { block, Provenance::PRODUCED, Some(self.myself_sender.apply_chunks_done.clone()), + signer, ); let Err(error) = res else { return Ok(()); @@ -1380,7 +1397,11 @@ impl ClientActorInner { } /// Process all blocks that were accepted by calling other relevant services. - fn process_accepted_blocks(&mut self, accepted_blocks: Vec) { + fn process_accepted_blocks( + &mut self, + accepted_blocks: Vec, + signer: &Option>, + ) { let _span = tracing::debug_span!( target: "client", "process_accepted_blocks", @@ -1390,11 +1411,16 @@ impl ClientActorInner { let block = self.client.chain.get_block(&accepted_block).unwrap().clone(); self.send_chunks_metrics(&block); self.send_block_metrics(&block); - self.check_send_announce_account(*block.header().last_final_block()); + self.check_send_announce_account(*block.header().last_final_block(), signer); } } - fn receive_headers(&mut self, headers: Vec, peer_id: PeerId) -> bool { + fn receive_headers( + &mut self, + headers: Vec, + peer_id: PeerId, + signer: &Option>, + ) -> bool { let _span = debug_span!(target: "client", "receive_headers", num_headers = headers.len(), ?peer_id) .entered(); @@ -1402,7 +1428,7 @@ impl ClientActorInner { info!(target: "client", "Received an empty set of block headers"); return true; } - match self.client.sync_block_headers(headers) { + match self.client.sync_block_headers(headers, signer) { Ok(_) => true, Err(err) => { if err.is_bad_data() { @@ -1532,6 +1558,7 @@ impl ClientActorInner { { // An extra scope to limit the lifetime of the span. let _span = tracing::debug_span!(target: "client", "catchup").entered(); + let validator_signer = self.client.validator_signer.get(); if let Err(err) = self.client.run_catchup( &self.network_info.highest_height_peers, &self.sync_jobs_sender.apply_state_parts, @@ -1540,8 +1567,8 @@ impl ClientActorInner { &self.sync_jobs_sender.resharding, Some(self.myself_sender.apply_chunks_done.clone()), self.state_parts_future_spawner.as_ref(), + &validator_signer, ) { - let validator_signer = self.client.validator_signer.get(); error!(target: "client", "{:?} Error occurred during catchup for the next epoch: {:?}", validator_signer.as_ref().map(|vs| vs.validator_id()), err); } } @@ -1600,6 +1627,7 @@ impl ClientActorInner { /// finishing state part job fn run_sync_step(&mut self) { let _span = tracing::debug_span!(target: "client", "run_sync_step").entered(); + let signer = self.client.validator_signer.get(); let currently_syncing = self.client.sync_status.is_syncing(); let sync = unwrap_and_report_state_sync_result!(self.syncing_info()); @@ -1615,7 +1643,7 @@ impl ClientActorInner { self.client.sync_status.update(SyncStatus::NoSync); // Announce this client's account id if their epoch is coming up. let head = unwrap_and_report_state_sync_result!(self.client.chain.head()); - self.check_send_announce_account(head.prev_block_hash); + self.check_send_announce_account(head.prev_block_hash, &signer); } } @@ -1624,7 +1652,7 @@ impl ClientActorInner { info!(target: "client", ?sync, "enabling sync"); } - self.handle_sync_needed(highest_height); + self.handle_sync_needed(highest_height, &signer); } } } @@ -1632,7 +1660,7 @@ impl ClientActorInner { /// Handle the SyncRequirement::SyncNeeded. /// /// This method runs the header sync, the block sync - fn handle_sync_needed(&mut self, highest_height: u64) { + fn handle_sync_needed(&mut self, highest_height: u64, signer: &Option>) { // Run each step of syncing separately. let header_sync_result = self.client.header_sync.run( &mut self.client.sync_status, @@ -1659,7 +1687,7 @@ impl ClientActorInner { _ => unreachable!("Sync status should have been StateSync!"), }; - let me = self.client.validator_signer.get().map(|x| x.validator_id().clone()); + let me = signer.as_ref().map(|x| x.validator_id().clone()); let block_header = self.client.chain.get_block_header(&sync_hash); let block_header = unwrap_and_report_state_sync_result!(block_header); let epoch_id = block_header.epoch_id().clone(); @@ -1724,7 +1752,7 @@ impl ClientActorInner { ); unwrap_and_report_state_sync_result!(reset_heads_result); - self.client.process_block_processing_artifact(block_processing_artifacts); + self.client.process_block_processing_artifact(block_processing_artifacts, signer); self.client.sync_status.update(SyncStatus::BlockSync { start_height: 0, current_height: 0, @@ -1944,11 +1972,13 @@ impl ClientActorInner { /// Print current summary. fn log_summary(&mut self) { let _span = tracing::debug_span!(target: "client", "log_summary").entered(); + let signer = self.client.validator_signer.get(); self.info_helper.log_summary( &self.client, &self.node_id, &self.network_info, &self.config_updater, + &signer, ) } @@ -2081,10 +2111,12 @@ impl Handler for ClientActorInner { fn handle(&mut self, msg: ShardsManagerResponse) { match msg { ShardsManagerResponse::ChunkCompleted { partial_chunk, shard_chunk } => { + let signer = self.client.validator_signer.get(); self.client.on_chunk_completed( partial_chunk, shard_chunk, Some(self.myself_sender.apply_chunks_done.clone()), + &signer, ); } ShardsManagerResponse::InvalidChunk(encoded_chunk) => { @@ -2120,7 +2152,10 @@ impl Handler for ClientActorInner { #[perf] fn handle(&mut self, msg: ChunkStateWitnessMessage) { let ChunkStateWitnessMessage { witness, raw_witness_size } = msg; - if let Err(err) = self.client.process_chunk_state_witness(witness, raw_witness_size, None) { + let signer = self.client.validator_signer.get(); + if let Err(err) = + self.client.process_chunk_state_witness(witness, raw_witness_size, None, signer) + { tracing::error!(target: "client", ?err, "Error processing chunk state witness"); } } diff --git a/chain/client/src/info.rs b/chain/client/src/info.rs index b2a6d3c4c80..0b20e16c3cd 100644 --- a/chain/client/src/info.rs +++ b/chain/client/src/info.rs @@ -59,8 +59,6 @@ pub struct InfoHelper { num_chunks_in_blocks_processed: u64, /// Total gas used during period. gas_used: u64, - /// Sign telemetry with block producer key if available. - validator_signer: Option>, /// Telemetry event sender. telemetry_sender: Sender, /// Log coloring enabled. @@ -82,7 +80,6 @@ impl InfoHelper { clock: Clock, telemetry_sender: Sender, client_config: &ClientConfig, - validator_signer: Option>, ) -> Self { set_open_files_limit(0); metrics::export_version(&client_config.version); @@ -96,7 +93,6 @@ impl InfoHelper { num_chunks_in_blocks_processed: 0, gas_used: 0, telemetry_sender, - validator_signer, log_summary_style: client_config.log_summary_style, boot_time_seconds: clock.now_utc().unix_timestamp(), epoch_id: None, @@ -319,6 +315,7 @@ impl InfoHelper { node_id: &PeerId, network_info: &NetworkInfo, config_updater: &Option, + signer: &Option>, ) { let is_syncing = client.sync_status.is_syncing(); let head = unwrap_or_return!(client.chain.head()); @@ -328,8 +325,7 @@ impl InfoHelper { &head.epoch_id, &head.last_block_hash, ); - let validator_signer = client.validator_signer.get(); - let account_id = validator_signer.as_ref().map(|x| x.validator_id()); + let account_id = signer.as_ref().map(|x| x.validator_id()); let is_validator = if let Some(account_id) = account_id { match client.epoch_manager.get_validator_by_account_id( &head.epoch_id, @@ -395,6 +391,7 @@ impl InfoHelper { .unwrap_or(0), &client.config, config_updater, + signer, ); self.log_chain_processing_info(client, &head.epoch_id); } @@ -411,6 +408,7 @@ impl InfoHelper { protocol_upgrade_block_height: BlockHeight, client_config: &ClientConfig, config_updater: &Option, + signer: &Option>, ) { let use_color = matches!(self.log_summary_style, LogSummaryStyle::Colored); let paint = |color: yansi::Color, text: Option| match text { @@ -530,6 +528,7 @@ impl InfoHelper { cpu_usage, memory_usage, is_validator, + signer, ), }; self.telemetry_sender.send(telemetry_event); @@ -545,6 +544,7 @@ impl InfoHelper { cpu_usage: f32, memory_usage: u64, is_validator: bool, + signer: &Option>, ) -> serde_json::Value { let info = TelemetryInfo { agent: TelemetryAgentInfo { @@ -563,7 +563,7 @@ impl InfoHelper { chain: TelemetryChainInfo { chain_id: client_config.chain_id.clone(), node_id: node_id.to_string(), - account_id: self.validator_signer.as_ref().map(|bp| bp.validator_id().clone()), + account_id: signer.as_ref().map(|bp| bp.validator_id().clone()), is_validator, status: sync_status.as_variant_name().to_string(), latest_block_hash: head.last_block_hash, @@ -583,8 +583,8 @@ impl InfoHelper { extra_info: serde_json::to_string(&extra_telemetry_info(client_config)).unwrap(), }; // Sign telemetry if there is a signer present. - if let Some(vs) = self.validator_signer.as_ref() { - vs.sign_telemetry(&info) + if let Some(signer) = signer { + signer.sign_telemetry(&info) } else { serde_json::to_value(&info).expect("Telemetry must serialize to json") } @@ -917,7 +917,7 @@ mod tests { use near_chain::runtime::NightshadeRuntime; use near_chain::types::ChainConfig; use near_chain::{Chain, ChainGenesis, DoomslugThresholdMode}; - use near_chain_configs::Genesis; + use near_chain_configs::{Genesis, MutableConfigValue}; use near_epoch_manager::shard_tracker::ShardTracker; use near_epoch_manager::test_utils::*; use near_epoch_manager::EpochManager; @@ -947,7 +947,8 @@ mod tests { #[test] fn test_telemetry_info() { let config = ClientConfig::test(false, 1230, 2340, 50, false, true, true, true); - let info_helper = InfoHelper::new(Clock::real(), noop().into_sender(), &config, None); + let validator = MutableConfigValue::new(None, "validator_signer"); + let info_helper = InfoHelper::new(Clock::real(), noop().into_sender(), &config); let store = near_store::test_utils::create_test_store(); let mut genesis = Genesis::test(vec!["test".parse::().unwrap()], 1); @@ -970,7 +971,7 @@ mod tests { ChainConfig::test(), None, Arc::new(RayonAsyncComputationSpawner), - None, + validator.clone(), ) .unwrap(); @@ -994,6 +995,7 @@ mod tests { 0.0, 0, false, + &validator.get(), ); println!("Got telemetry info: {:?}", telemetry); assert_matches!( @@ -1052,8 +1054,7 @@ mod tests { // Then check that get_num_validators returns the correct number of validators. let client_config = ClientConfig::test(false, 1230, 2340, 50, false, true, true, true); - let mut info_helper = - InfoHelper::new(Clock::real(), noop().into_sender(), &client_config, None); + let mut info_helper = InfoHelper::new(Clock::real(), noop().into_sender(), &client_config); assert_eq!( num_validators, info_helper.get_num_validators(&epoch_manager_adapter, &epoch_id, &last_block_hash) diff --git a/chain/client/src/stateless_validation/chunk_validator/mod.rs b/chain/client/src/stateless_validation/chunk_validator/mod.rs index 9f3e41713f7..7abef6fb317 100644 --- a/chain/client/src/stateless_validation/chunk_validator/mod.rs +++ b/chain/client/src/stateless_validation/chunk_validator/mod.rs @@ -36,8 +36,6 @@ const NUM_NEXT_BLOCK_PRODUCERS_TO_SEND_CHUNK_ENDORSEMENT: u64 = 5; /// witness is correct, and then send chunk endorsements to the block producer /// so that the chunk can be included in the block. pub struct ChunkValidator { - /// The signer for our own node, if we are a validator. If not, this is None. - my_signer: Option>, epoch_manager: Arc, network_sender: Sender, runtime_adapter: Arc, @@ -54,7 +52,6 @@ pub struct ChunkValidator { impl ChunkValidator { pub fn new( - my_signer: Option>, epoch_manager: Arc, network_sender: Sender, runtime_adapter: Arc, @@ -64,7 +61,6 @@ impl ChunkValidator { panic_on_validation_error: bool, ) -> Self { Self { - my_signer, epoch_manager, network_sender, runtime_adapter, @@ -82,11 +78,12 @@ impl ChunkValidator { /// happens in a separate thread. /// The chunk is validated asynchronously, if you want to wait for the processing to finish /// you can use the `processing_done_tracker` argument (but it's optional, it's safe to pass None there). - pub fn start_validating_chunk( + fn start_validating_chunk( &self, state_witness: ChunkStateWitness, chain: &Chain, processing_done_tracker: Option, + signer: &Arc, ) -> Result<(), Error> { let prev_block_hash = state_witness.chunk_header.prev_block_hash(); let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(prev_block_hash)?; @@ -106,7 +103,6 @@ impl ChunkValidator { let chunk_header = state_witness.chunk_header.clone(); let network_sender = self.network_sender.clone(); - let signer = self.my_signer.as_ref().ok_or(Error::NotAValidator)?.clone(); let chunk_endorsement_tracker = self.chunk_endorsement_tracker.clone(); let epoch_manager = self.epoch_manager.clone(); // If we have the chunk extra for the previous block, we can validate the chunk without state witness. @@ -135,7 +131,7 @@ impl ChunkValidator { send_chunk_endorsement_to_block_producers( &chunk_header, epoch_manager.as_ref(), - signer.as_ref(), + signer, &network_sender, chunk_endorsement_tracker.as_ref(), ); @@ -157,6 +153,7 @@ impl ChunkValidator { let runtime_adapter = self.runtime_adapter.clone(); let cache = self.main_state_transition_result_cache.clone(); + let signer = signer.clone(); self.validation_spawner.spawn("stateless_validation", move || { // processing_done_tracker must survive until the processing is finished. let _processing_done_tracker_capture: Option = @@ -250,6 +247,7 @@ impl Client { witness: ChunkStateWitness, raw_witness_size: ChunkStateWitnessSize, processing_done_tracker: Option, + signer: Option>, ) -> Result<(), Error> { tracing::debug!( target: "client", @@ -258,10 +256,18 @@ impl Client { "process_chunk_state_witness", ); + // Chunk producers should not receive state witness from themselves. + log_assert!( + signer.is_some(), + "Received a chunk state witness but this is not a validator node. Witness={:?}", + witness + ); + let signer = signer.unwrap(); + // Send the acknowledgement for the state witness back to the chunk producer. // This is currently used for network roundtrip time measurement, so we do not need to // wait for validation to finish. - self.send_state_witness_ack(&witness); + self.send_state_witness_ack(&witness, &signer); if self.config.save_latest_witnesses { self.chain.chain_store.save_latest_chunk_state_witness(&witness)?; @@ -272,6 +278,7 @@ impl Client { witness, &block, processing_done_tracker, + &signer, ), Err(Error::DBNotFoundErr(_)) => { // Previous block isn't available at the moment, add this witness to the orphan pool. @@ -282,21 +289,15 @@ impl Client { } } - fn send_state_witness_ack(&self, witness: &ChunkStateWitness) { - // Chunk producers should not receive state witness from themselves. - log_assert!( - self.validator_signer.get().is_some(), - "Received a chunk state witness but this is not a validator node. Witness={:?}", - witness - ); + fn send_state_witness_ack(&self, witness: &ChunkStateWitness, signer: &Arc) { // In production PartialWitnessActor does not forward a state witness to the chunk producer that // produced the witness. However some tests bypass PartialWitnessActor, thus when a chunk producer // receives its own state witness, we log a warning instead of panicking. // TODO: Make sure all tests run with "test_features" and panic for non-test builds. - if self.validator_signer.get().unwrap().validator_id() == &witness.chunk_producer { + if signer.validator_id() == &witness.chunk_producer { tracing::warn!( "Validator {:?} received state witness from itself. Witness={:?}", - self.validator_signer.get().unwrap().validator_id(), + signer.validator_id(), witness ); return; @@ -314,6 +315,7 @@ impl Client { witness: ChunkStateWitness, prev_block: &Block, processing_done_tracker: Option, + signer: &Arc, ) -> Result<(), Error> { if witness.chunk_header.prev_block_hash() != prev_block.hash() { return Err(Error::Other(format!( @@ -323,6 +325,11 @@ impl Client { ))); } - self.chunk_validator.start_validating_chunk(witness, &self.chain, processing_done_tracker) + self.chunk_validator.start_validating_chunk( + witness, + &self.chain, + processing_done_tracker, + signer, + ) } } diff --git a/chain/client/src/stateless_validation/chunk_validator/orphan_witness_handling.rs b/chain/client/src/stateless_validation/chunk_validator/orphan_witness_handling.rs index c3840ac29a1..96bf4229291 100644 --- a/chain/client/src/stateless_validation/chunk_validator/orphan_witness_handling.rs +++ b/chain/client/src/stateless_validation/chunk_validator/orphan_witness_handling.rs @@ -11,7 +11,9 @@ use near_chain_primitives::Error; use near_primitives::hash::CryptoHash; use near_primitives::stateless_validation::ChunkStateWitness; use near_primitives::types::BlockHeight; +use near_primitives::validator_signer::ValidatorSigner; use std::ops::Range; +use std::sync::Arc; /// We keep only orphan witnesses that are within this distance of /// the current chain head. This helps to reduce the size of @@ -75,10 +77,7 @@ impl Client { Ok(HandleOrphanWitnessOutcome::SavedToPool) } - /// Once a new block arrives, we can process the orphaned chunk state witnesses that were waiting - /// for this block. This function takes the ready witnesses out of the orhan pool and process them. - /// It also removes old witnesses (below final height) from the orphan pool to save memory. - pub fn process_ready_orphan_witnesses_and_clean_old(&mut self, new_block: &Block) { + fn process_ready_orphan_witnesses(&mut self, new_block: &Block, signer: &Arc) { let ready_witnesses = self .chunk_validator .orphan_witness_pool @@ -94,11 +93,26 @@ impl Client { "Processing an orphaned ChunkStateWitness, its previous block has arrived." ); if let Err(err) = - self.process_chunk_state_witness_with_prev_block(witness, new_block, None) + self.process_chunk_state_witness_with_prev_block(witness, new_block, None, signer) { tracing::error!(target: "client", ?err, "Error processing orphan chunk state witness"); } } + } + + /// Once a new block arrives, we can process the orphaned chunk state witnesses that were waiting + /// for this block. This function takes the ready witnesses out of the orhan pool and process them. + /// It also removes old witnesses (below final height) from the orphan pool to save memory. + pub fn process_ready_orphan_witnesses_and_clean_old( + &mut self, + new_block: &Block, + signer: &Option>, + ) { + if let Some(signer) = signer { + self.process_ready_orphan_witnesses(new_block, signer); + } else { + tracing::error!(target: "client", new_block=?new_block.hash(), "Cannot process ready orphan witnesses - not a validator"); + } // Remove all orphan witnesses that are below the last final block of the new block. // They won't be used, so we can remove them from the pool to save memory. diff --git a/chain/client/src/stateless_validation/partial_witness/partial_witness_actor.rs b/chain/client/src/stateless_validation/partial_witness/partial_witness_actor.rs index 8c7afe59012..3c4adbf6650 100644 --- a/chain/client/src/stateless_validation/partial_witness/partial_witness_actor.rs +++ b/chain/client/src/stateless_validation/partial_witness/partial_witness_actor.rs @@ -5,6 +5,7 @@ use near_async::messaging::{Actor, CanSend, Handler, Sender}; use near_async::time::Clock; use near_async::{MultiSend, MultiSenderFrom}; use near_chain::Error; +use near_chain_configs::MutableConfigValue; use near_epoch_manager::EpochManagerAdapter; use near_network::state_witness::{ ChunkStateWitnessAckMessage, PartialEncodedStateWitnessForwardMessage, @@ -32,8 +33,10 @@ use super::partial_witness_tracker::PartialEncodedStateWitnessTracker; pub struct PartialWitnessActor { /// Adapter to send messages to the network. network_adapter: PeerManagerAdapter, - /// Validator signer to sign the state witness. - my_signer: Arc, + /// Validator signer to sign the state witness. This field is mutable and optional. Use with caution! + /// Lock the value of mutable validator signer for the duration of a request to ensure consistency. + /// Please note that the locked value should not be stored anywhere or passed through the thread boundary. + my_signer: MutableConfigValue>>, /// Epoch manager to get the set of chunk validators epoch_manager: Arc, /// Tracks the parts of the state witness sent from chunk producers to chunk validators. @@ -103,7 +106,7 @@ impl PartialWitnessActor { clock: Clock, network_adapter: PeerManagerAdapter, client_sender: ClientSenderForPartialWitness, - my_signer: Arc, + my_signer: MutableConfigValue>>, epoch_manager: Arc, store: Store, ) -> Self { @@ -132,9 +135,16 @@ impl PartialWitnessActor { "distribute_chunk_state_witness", ); + let signer = match self.my_signer.get() { + Some(signer) => signer, + None => { + return Err(Error::NotAValidator(format!("distribute state witness"))); + } + }; + let witness_bytes = compress_witness(&state_witness)?; - self.send_state_witness_parts(epoch_id, chunk_header, witness_bytes)?; + self.send_state_witness_parts(epoch_id, chunk_header, witness_bytes, &signer)?; Ok(()) } @@ -145,6 +155,7 @@ impl PartialWitnessActor { epoch_id: EpochId, chunk_header: ShardChunkHeader, witness_bytes: EncodedChunkStateWitness, + signer: &ValidatorSigner, ) -> Result, Error> { let chunk_validators = self .epoch_manager @@ -180,7 +191,7 @@ impl PartialWitnessActor { part_ord, part.unwrap().to_vec(), encoded_length, - self.my_signer.as_ref(), + signer, ); (chunk_validator.clone(), partial_witness) }) @@ -195,6 +206,7 @@ impl PartialWitnessActor { epoch_id: EpochId, chunk_header: ShardChunkHeader, witness_bytes: EncodedChunkStateWitness, + signer: &ValidatorSigner, ) -> Result<(), Error> { // Capture these values first, as the sources are consumed before calling record_witness_sent. let chunk_hash = chunk_header.chunk_hash(); @@ -206,18 +218,18 @@ impl PartialWitnessActor { .with_label_values(&[shard_id_label.as_str()]) .start_timer(); let mut validator_witness_tuple = - self.generate_state_witness_parts(epoch_id, chunk_header, witness_bytes)?; + self.generate_state_witness_parts(epoch_id, chunk_header, witness_bytes, signer)?; encode_timer.observe_duration(); // Since we can't send network message to ourselves, we need to send the PartialEncodedStateWitnessForward // message for our part. if let Some(index) = validator_witness_tuple .iter() - .position(|(validator, _)| validator == self.my_signer.validator_id()) + .position(|(validator, _)| validator == signer.validator_id()) { // This also removes this validator from the list, since we do not need to send our own witness part to self. let (_, partial_witness) = validator_witness_tuple.swap_remove(index); - self.forward_state_witness_part(partial_witness)?; + self.forward_state_witness_part(partial_witness, signer)?; } // Record the witness in order to match the incoming acks for measuring round-trip times. @@ -240,6 +252,7 @@ impl PartialWitnessActor { fn forward_state_witness_part( &self, partial_witness: PartialEncodedStateWitness, + signer: &ValidatorSigner, ) -> Result<(), Error> { let chunk_producer = self.epoch_manager.get_chunk_producer( partial_witness.epoch_id(), @@ -258,9 +271,7 @@ impl PartialWitnessActor { // (1) the current validator and (2) validator that produced the chunk and witness. let target_chunk_validators = ordered_chunk_validators .into_iter() - .filter(|validator| { - validator != self.my_signer.validator_id() && *validator != chunk_producer - }) + .filter(|validator| validator != signer.validator_id() && *validator != chunk_producer) .collect(); self.network_adapter.send(PeerManagerMessageRequest::NetworkRequests( NetworkRequests::PartialEncodedStateWitnessForward( @@ -278,13 +289,20 @@ impl PartialWitnessActor { ) -> Result<(), Error> { tracing::debug!(target: "client", ?partial_witness, "Receive PartialEncodedStateWitnessMessage"); + let signer = match self.my_signer.get() { + Some(signer) => signer, + None => { + return Err(Error::NotAValidator(format!("handle partial encoded state witness"))); + } + }; + // Validate the partial encoded state witness. - if self.validate_partial_encoded_state_witness(&partial_witness)? { + if self.validate_partial_encoded_state_witness(&partial_witness, &signer)? { // Store the partial encoded state witness for self. self.partial_witness_tracker .store_partial_encoded_state_witness(partial_witness.clone())?; // Forward the part to all the chunk validators. - self.forward_state_witness_part(partial_witness)?; + self.forward_state_witness_part(partial_witness, &signer)?; } Ok(()) @@ -295,8 +313,17 @@ impl PartialWitnessActor { &mut self, partial_witness: PartialEncodedStateWitness, ) -> Result<(), Error> { + let signer = match self.my_signer.get() { + Some(signer) => signer, + None => { + return Err(Error::NotAValidator(format!( + "handle partial encoded state witness forward" + ))); + } + }; + // Validate the partial encoded state witness. - if self.validate_partial_encoded_state_witness(&partial_witness)? { + if self.validate_partial_encoded_state_witness(&partial_witness, &signer)? { // Store the partial encoded state witness for self. self.partial_witness_tracker.store_partial_encoded_state_witness(partial_witness)?; } @@ -322,6 +349,7 @@ impl PartialWitnessActor { fn validate_partial_encoded_state_witness( &self, partial_witness: &PartialEncodedStateWitness, + signer: &ValidatorSigner, ) -> Result { if !self .epoch_manager @@ -342,7 +370,7 @@ impl PartialWitnessActor { partial_witness.shard_id(), partial_witness.height_created(), )?; - if !chunk_validator_assignments.contains(self.my_signer.validator_id()) { + if !chunk_validator_assignments.contains(signer.validator_id()) { return Err(Error::NotAChunkValidator); } diff --git a/chain/client/src/stateless_validation/state_witness_producer.rs b/chain/client/src/stateless_validation/state_witness_producer.rs index f4b4000b9a4..c658c8feefb 100644 --- a/chain/client/src/stateless_validation/state_witness_producer.rs +++ b/chain/client/src/stateless_validation/state_witness_producer.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::sync::Arc; use near_async::messaging::{CanSend, IntoSender}; use near_chain::{BlockHeader, Chain, ChainStoreAccess}; @@ -13,6 +14,7 @@ use near_primitives::stateless_validation::{ ChunkStateTransition, ChunkStateWitness, StoredChunkStateTransitionData, }; use near_primitives::types::{AccountId, EpochId}; +use near_primitives::validator_signer::ValidatorSigner; use crate::stateless_validation::chunk_validator::send_chunk_endorsement_to_block_producers; use crate::Client; @@ -29,6 +31,7 @@ impl Client { prev_chunk_header: &ShardChunkHeader, chunk: &ShardChunk, transactions_storage_proof: Option, + validator_signer: &Option>, ) -> Result<(), Error> { let protocol_version = self.epoch_manager.get_epoch_protocol_version(epoch_id)?; if !checked_feature!("stable", StatelessValidationV0, protocol_version) { @@ -38,7 +41,8 @@ impl Client { let shard_id = chunk_header.shard_id(); let _span = tracing::debug_span!(target: "client", "send_chunk_state_witness", chunk_hash=?chunk_header.chunk_hash(), ?shard_id).entered(); - let my_signer = self.validator_signer.get().ok_or(Error::NotAValidator)?; + let my_signer = + validator_signer.as_ref().ok_or(Error::NotAValidator(format!("send state witness")))?; let state_witness = self.create_state_witness( my_signer.validator_id().clone(), prev_block_header, diff --git a/chain/client/src/test_utils/client.rs b/chain/client/src/test_utils/client.rs index 588fa80aa74..fba18e39c24 100644 --- a/chain/client/src/test_utils/client.rs +++ b/chain/client/src/test_utils/client.rs @@ -44,9 +44,11 @@ impl Client { should_produce_chunk: bool, allow_errors: bool, ) -> Result, near_chain::Error> { - self.start_process_block(block, provenance, None)?; + let signer = self.validator_signer.get(); + self.start_process_block(block, provenance, None, &signer)?; wait_for_all_blocks_in_processing(&mut self.chain); - let (accepted_blocks, errors) = self.postprocess_ready_blocks(None, should_produce_chunk); + let (accepted_blocks, errors) = + self.postprocess_ready_blocks(None, should_produce_chunk, &signer); if !allow_errors { assert!(errors.is_empty(), "unexpected errors when processing blocks: {errors:#?}"); } @@ -79,9 +81,10 @@ impl Client { /// This function finishes processing all blocks that started being processed. pub fn finish_blocks_in_processing(&mut self) -> Vec { + let signer = self.validator_signer.get(); let mut accepted_blocks = vec![]; while wait_for_all_blocks_in_processing(&mut self.chain) { - accepted_blocks.extend(self.postprocess_ready_blocks(None, true).0); + accepted_blocks.extend(self.postprocess_ready_blocks(None, true, &signer).0); } accepted_blocks } @@ -90,7 +93,8 @@ impl Client { /// has started. pub fn finish_block_in_processing(&mut self, hash: &CryptoHash) -> Vec { if let Ok(()) = wait_for_block_in_processing(&mut self.chain, hash) { - let (accepted_blocks, _) = self.postprocess_ready_blocks(None, true); + let signer = self.validator_signer.get(); + let (accepted_blocks, _) = self.postprocess_ready_blocks(None, true, &signer); return accepted_blocks; } vec![] @@ -104,12 +108,13 @@ impl Client { receipts, transactions_storage_proof, } = create_chunk_on_height_for_shard(self, height, shard_id); + let signer = self.validator_signer.get(); let shard_chunk = self .persist_and_distribute_encoded_chunk( encoded_chunk, merkle_paths, receipts, - self.validator_signer.get().unwrap().validator_id().clone(), + signer.as_ref().unwrap().validator_id().clone(), ) .unwrap(); let prev_block = self.chain.get_block(shard_chunk.prev_block()).unwrap(); @@ -125,6 +130,7 @@ impl Client { &prev_chunk_header, &shard_chunk, transactions_storage_proof, + &signer, ) .unwrap(); shard_chunk @@ -138,6 +144,7 @@ fn create_chunk_on_height_for_shard( ) -> ProduceChunkResult { let last_block_hash = client.chain.head().unwrap().last_block_hash; let last_block = client.chain.get_block(&last_block_hash).unwrap(); + let signer = client.validator_signer.get(); client .try_produce_chunk( &last_block, @@ -146,6 +153,7 @@ fn create_chunk_on_height_for_shard( .unwrap(), next_height, shard_id, + signer.as_ref(), ) .unwrap() .unwrap() @@ -171,6 +179,7 @@ pub fn create_chunk( ) -> (ProduceChunkResult, Block) { let last_block = client.chain.get_block_by_height(client.chain.head().unwrap().height).unwrap(); let next_height = last_block.header().height() + 1; + let signer = client.validator_signer.get(); let ProduceChunkResult { mut chunk, encoded_chunk_parts_paths: mut merkle_paths, @@ -183,6 +192,7 @@ pub fn create_chunk( last_block.chunks()[0].clone(), next_height, 0, + signer.as_ref(), ) .unwrap() .unwrap(); @@ -201,7 +211,7 @@ pub fn create_chunk( let parity_parts = total_parts - data_parts; let rs = ReedSolomon::new(data_parts, parity_parts).unwrap(); - let signer = client.validator_signer.get().unwrap(); + let signer = signer.unwrap(); let header = chunk.cloned_header(); let (mut encoded_chunk, mut new_merkle_paths) = EncodedShardChunk::new( *header.prev_block_hash(), @@ -296,6 +306,7 @@ pub fn run_catchup( let _ = System::new(); let state_parts_future_spawner = ActixArbiterHandleFutureSpawner(Arbiter::new().handle()); loop { + let signer = client.validator_signer.get(); client.run_catchup( highest_height_peers, &noop().into_sender(), @@ -304,6 +315,7 @@ pub fn run_catchup( &resharding, None, &state_parts_future_spawner, + &signer, )?; let mut catchup_done = true; for msg in block_messages.write().unwrap().drain(..) { diff --git a/chain/client/src/test_utils/setup.rs b/chain/client/src/test_utils/setup.rs index 98d2db34e59..bd6833ce752 100644 --- a/chain/client/src/test_utils/setup.rs +++ b/chain/client/src/test_utils/setup.rs @@ -55,7 +55,7 @@ use near_primitives::hash::{hash, CryptoHash}; use near_primitives::network::PeerId; use near_primitives::test_utils::create_test_signer; use near_primitives::types::{AccountId, BlockHeightDelta, EpochId, NumBlocks, NumSeats}; -use near_primitives::validator_signer::ValidatorSigner; +use near_primitives::validator_signer::{EmptyValidatorSigner, ValidatorSigner}; use near_primitives::version::PROTOCOL_VERSION; use near_store::test_utils::create_test_store; use near_telemetry::TelemetryActor; @@ -115,7 +115,10 @@ pub fn setup( protocol_version: PROTOCOL_VERSION, }; - let signer = Arc::new(create_test_signer(account_id.as_str())); + let signer = MutableConfigValue::new( + Some(Arc::new(create_test_signer(account_id.as_str()))), + "validator_signer", + ); let telemetry = ActixWrapper::new(TelemetryActor::default()).start(); let config = { let mut base = ClientConfig::test( @@ -136,7 +139,7 @@ pub fn setup( let view_client_addr = ViewClientActorInner::spawn_actix_actor( clock.clone(), - Some(signer.validator_id().clone()), + signer.clone(), chain_genesis.clone(), epoch_manager.clone(), shard_tracker.clone(), @@ -175,7 +178,7 @@ pub fn setup( state_sync_adapter, network_adapter.clone(), shards_manager_adapter_for_client.as_sender(), - Some(signer), + signer, telemetry.with_auto_span_context().into_sender(), None, None, @@ -185,13 +188,13 @@ pub fn setup( enable_doomslug, Some(TEST_SEED), ); - + let validator_signer = Some(Arc::new(EmptyValidatorSigner::new(account_id))); let (shards_manager_addr, _) = start_shards_manager( epoch_manager, shard_tracker, network_adapter.into_sender(), client_actor.clone().with_auto_span_context().into_sender(), - Some(account_id), + MutableConfigValue::new(validator_signer, "validator_signer"), store, config.chunk_request_retry_period, ); @@ -263,11 +266,14 @@ pub fn setup_only_view( }, None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); - let signer = Arc::new(create_test_signer(account_id.as_str())); + let signer = MutableConfigValue::new( + Some(Arc::new(create_test_signer(account_id.as_str()))), + "validator_signer", + ); ActixWrapper::new(TelemetryActor::default()).start(); let config = ClientConfig::test( skip_sync_wait, @@ -284,7 +290,7 @@ pub fn setup_only_view( ViewClientActorInner::spawn_actix_actor( clock, - Some(signer.validator_id().clone()), + signer, chain_genesis, epoch_manager, shard_tracker, @@ -1012,7 +1018,7 @@ pub fn setup_client_with_runtime( runtime, network_adapter, shards_manager_adapter.into_sender(), - Some(validator_signer), + MutableConfigValue::new(Some(validator_signer), "validator_signer"), enable_doomslug, rng_seed, snapshot_callbacks, @@ -1056,14 +1062,15 @@ pub fn setup_synchronous_shards_manager( }, // irrelevant None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); let chain_head = chain.head().unwrap(); let chain_header_head = chain.header_head().unwrap(); + let validator_signer = account_id.map(|id| Arc::new(EmptyValidatorSigner::new(id))); let shards_manager = ShardsManagerActor::new( clock, - account_id, + MutableConfigValue::new(validator_signer, "validator_signer"), epoch_manager, shard_tracker, network_adapter.request_sender, diff --git a/chain/client/src/test_utils/test_env.rs b/chain/client/src/test_utils/test_env.rs index e289854e56c..fdff540c003 100644 --- a/chain/client/src/test_utils/test_env.rs +++ b/chain/client/src/test_utils/test_env.rs @@ -297,7 +297,8 @@ impl TestEnv { while let Some(msg) = self.client_adapters[id].pop() { match msg { ShardsManagerResponse::ChunkCompleted { partial_chunk, shard_chunk } => { - self.clients[id].on_chunk_completed(partial_chunk, shard_chunk, None); + let signer = self.clients[id].validator_signer.get(); + self.clients[id].on_chunk_completed(partial_chunk, shard_chunk, None, &signer); } ShardsManagerResponse::InvalidChunk(encoded_chunk) => { self.clients[id].on_invalid_chunk(encoded_chunk); @@ -374,10 +375,12 @@ impl TestEnv { let processing_done_tracker = ProcessingDoneTracker::new(); witness_processing_done_waiters.push(processing_done_tracker.make_waiter()); - let processing_result = self.client(&account_id).process_chunk_state_witness( + let client = self.client(&account_id); + let processing_result = client.process_chunk_state_witness( state_witness.clone(), raw_witness_size, Some(processing_done_tracker), + client.validator_signer.get(), ); if !allow_errors { processing_result.unwrap(); diff --git a/chain/client/src/tests/doomslug.rs b/chain/client/src/tests/doomslug.rs index 9bae9a719c1..0a400199cbe 100644 --- a/chain/client/src/tests/doomslug.rs +++ b/chain/client/src/tests/doomslug.rs @@ -32,6 +32,7 @@ fn test_processing_skips_on_forks() { let validator_signer = InMemoryValidatorSigner::from_seed("test1".parse().unwrap(), KeyType::ED25519, "test1"); let approval = Approval::new(CryptoHash::default(), 1, 3, &validator_signer.into()); - env.clients[1].collect_block_approval(&approval, ApprovalType::SelfApproval); + let client_signer = env.clients[1].validator_signer.get(); + env.clients[1].collect_block_approval(&approval, ApprovalType::SelfApproval, &client_signer); assert!(!env.clients[1].doomslug.approval_status_at_height(&3).approvals.is_empty()); } diff --git a/chain/client/src/tests/process_blocks.rs b/chain/client/src/tests/process_blocks.rs index 398f0d5dbfe..6d06874e7ac 100644 --- a/chain/client/src/tests/process_blocks.rs +++ b/chain/client/src/tests/process_blocks.rs @@ -33,6 +33,7 @@ fn test_not_process_height_twice() { duplicate_block.mut_header().get_mut().inner_rest.prev_validator_proposals = proposals; duplicate_block.mut_header().resign(&validator_signer); let dup_block_hash = *duplicate_block.hash(); + let signer = env.clients[0].validator_signer.get(); // we should have dropped the block before we even tried to process it, so the result should be ok env.clients[0] .receive_block_impl( @@ -40,6 +41,7 @@ fn test_not_process_height_twice() { PeerId::new(PublicKey::empty(KeyType::ED25519)), false, None, + &signer, ) .unwrap(); // check that the second block is not being processed @@ -112,9 +114,16 @@ fn test_bad_block_content_vrf() { let block = env.clients[0].produce_block(2).unwrap().unwrap(); let mut bad_block = block.clone(); bad_block.set_vrf_value(Value([0u8; 32])); + let signer = env.clients[0].validator_signer.get(); let err = env.clients[0] - .receive_block_impl(bad_block, PeerId::new(PublicKey::empty(KeyType::ED25519)), false, None) + .receive_block_impl( + bad_block, + PeerId::new(PublicKey::empty(KeyType::ED25519)), + false, + None, + &signer, + ) .unwrap_err(); assert_matches!(err, near_chain::Error::InvalidSignature); @@ -131,9 +140,16 @@ fn test_bad_block_signature() { let block = env.clients[0].produce_block(2).unwrap().unwrap(); let mut bad_block = block.clone(); bad_block.mut_header().get_mut().signature = Signature::default(); + let signer = env.clients[0].validator_signer.get(); let err = env.clients[0] - .receive_block_impl(bad_block, PeerId::new(PublicKey::empty(KeyType::ED25519)), false, None) + .receive_block_impl( + bad_block, + PeerId::new(PublicKey::empty(KeyType::ED25519)), + false, + None, + &signer, + ) .unwrap_err(); assert_matches!(err, near_chain::Error::InvalidSignature); diff --git a/chain/client/src/view_client_actor.rs b/chain/client/src/view_client_actor.rs index 20c1bf374da..6a6ca2b499a 100644 --- a/chain/client/src/view_client_actor.rs +++ b/chain/client/src/view_client_actor.rs @@ -13,7 +13,7 @@ use near_chain::types::{RuntimeAdapter, Tip}; use near_chain::{ get_epoch_block_producers_view, Chain, ChainGenesis, ChainStoreAccess, DoomslugThresholdMode, }; -use near_chain_configs::{ClientConfig, ProtocolConfigView}; +use near_chain_configs::{ClientConfig, MutableConfigValue, ProtocolConfigView}; use near_chain_primitives::error::EpochErrorResultToChainError; use near_client_primitives::types::{ Error, GetBlock, GetBlockError, GetBlockProof, GetBlockProofError, GetBlockProofResponse, @@ -51,6 +51,7 @@ use near_primitives::types::{ AccountId, BlockHeight, BlockId, BlockReference, EpochReference, Finality, MaybeBlockId, ShardId, SyncCheckpoint, TransactionOrReceiptId, ValidatorInfoIdentifier, }; +use near_primitives::validator_signer::ValidatorSigner; use near_primitives::views::validator_stake_view::ValidatorStakeView; use near_primitives::views::{ BlockView, ChunkView, EpochValidatorInfo, ExecutionOutcomeWithIdView, ExecutionStatusView, @@ -90,8 +91,10 @@ pub struct ViewClientActorInner { clock: Clock, pub adv: crate::adversarial::Controls, - /// Validator account (if present). - validator_account_id: Option, + /// Validator account (if present). This field is mutable and optional. Use with caution! + /// Lock the value of mutable validator signer for the duration of a request to ensure consistency. + /// Please note that the locked value should not be stored anywhere or passed through the thread boundary. + validator: MutableConfigValue>>, chain: Chain, epoch_manager: Arc, shard_tracker: ShardTracker, @@ -117,7 +120,7 @@ impl ViewClientActorInner { pub fn spawn_actix_actor( clock: Clock, - validator_account_id: Option, + validator: MutableConfigValue>>, chain_genesis: ChainGenesis, epoch_manager: Arc, shard_tracker: ShardTracker, @@ -142,7 +145,7 @@ impl ViewClientActorInner { let view_client_actor = ViewClientActorInner { clock: clock.clone(), adv: adv.clone(), - validator_account_id: validator_account_id.clone(), + validator: validator.clone(), chain, epoch_manager: epoch_manager.clone(), shard_tracker: shard_tracker.clone(), @@ -505,6 +508,7 @@ impl ViewClientActorInner { tx_hash: CryptoHash, signer_account_id: AccountId, fetch_receipt: bool, + validator_signer: &Option>, ) -> Result { { // TODO(telezhnaya): take into account `fetch_receipt()` @@ -529,7 +533,7 @@ impl ViewClientActorInner { .map_err(|err| TxStatusError::InternalError(err.to_string()))?; // Check if we are tracking this shard. if self.shard_tracker.care_about_shard( - self.validator_account_id.as_ref(), + validator_signer.as_ref().map(|v| v.validator_id()), &head.prev_block_hash, target_shard_id, true, @@ -778,7 +782,8 @@ impl Handler for ViewClientActorInner { tracing::debug!(target: "client", ?msg); let _timer = metrics::VIEW_CLIENT_MESSAGE_TIME.with_label_values(&["TxStatus"]).start_timer(); - self.get_tx_status(msg.tx_hash, msg.signer_account_id, msg.fetch_receipt) + let validator_signer = self.validator.get(); + self.get_tx_status(msg.tx_hash, msg.signer_account_id, msg.fetch_receipt, &validator_signer) } } @@ -1061,7 +1066,7 @@ impl Handler for ViewClientActorInner { .account_id_to_shard_id(&account_id, &head.epoch_id) .into_chain_error()?; if self.shard_tracker.care_about_shard( - self.validator_account_id.as_ref(), + self.validator.get().map(|v| v.validator_id().clone()).as_ref(), &head.last_block_hash, target_shard_id, true, @@ -1196,8 +1201,10 @@ impl Handler for ViewClientActorInner { let _timer = metrics::VIEW_CLIENT_MESSAGE_TIME.with_label_values(&["TxStatusRequest"]).start_timer(); let TxStatusRequest { tx_hash, signer_account_id } = msg; - if let Ok(Some(result)) = - self.get_tx_status(tx_hash, signer_account_id, false).map(|s| s.execution_outcome) + let validator_signer = self.validator.get(); + if let Ok(Some(result)) = self + .get_tx_status(tx_hash, signer_account_id, false, &validator_signer) + .map(|s| s.execution_outcome) { Some(Box::new(result.into_outcome())) } else { diff --git a/chain/epoch-manager/src/adapter.rs b/chain/epoch-manager/src/adapter.rs index f55b94f2313..e2ad3659863 100644 --- a/chain/epoch-manager/src/adapter.rs +++ b/chain/epoch-manager/src/adapter.rs @@ -900,7 +900,7 @@ impl EpochManagerAdapter for EpochManagerHandle { data, signature, ) { - Err(Error::NotAValidator) => { + Err(Error::NotAValidator(_)) => { let (fisherman, is_slashed) = self.get_fisherman_by_account_id(epoch_id, last_known_block_hash, account_id)?; if is_slashed { @@ -1048,7 +1048,7 @@ impl EpochManagerAdapter for EpochManagerHandle { chunk_header.height_created(), )?; if !chunk_validator_assignments.contains(&endorsement.account_id) { - return Err(Error::NotAValidator); + return Err(Error::NotAValidator(format!("verify chunk endorsement"))); } let validator = epoch_manager.get_validator_by_account_id(&epoch_id, &endorsement.account_id)?; diff --git a/chain/epoch-manager/src/tests/mod.rs b/chain/epoch-manager/src/tests/mod.rs index f0d0663fcbd..3d906c2e817 100644 --- a/chain/epoch-manager/src/tests/mod.rs +++ b/chain/epoch-manager/src/tests/mod.rs @@ -2848,7 +2848,7 @@ fn test_verify_chunk_endorsements() { let err = epoch_manager.verify_chunk_endorsement(&chunk_header, &chunk_endorsement).unwrap_err(); match err { - Error::NotAValidator => (), + Error::NotAValidator(_) => (), _ => assert!(false, "Expected NotAValidator error but got {:?}", err), } } diff --git a/chain/network/Cargo.toml b/chain/network/Cargo.toml index 3d28bcb5254..d6972fc4097 100644 --- a/chain/network/Cargo.toml +++ b/chain/network/Cargo.toml @@ -54,6 +54,7 @@ time.workspace = true near-async.workspace = true near-fmt.workspace = true near-o11y.workspace = true +near-chain-configs.workspace = true near-crypto.workspace = true near-performance-metrics.workspace = true near-performance-metrics-macros.workspace = true @@ -74,6 +75,7 @@ webrtc-util.workspace = true [features] nightly_protocol = [ "near-async/nightly_protocol", + "near-chain-configs/nightly_protocol", "near-fmt/nightly_protocol", "near-o11y/nightly_protocol", "near-primitives/nightly_protocol", @@ -81,6 +83,7 @@ nightly_protocol = [ ] nightly = [ "near-async/nightly", + "near-chain-configs/nightly", "near-fmt/nightly", "near-o11y/nightly", "near-primitives/nightly", diff --git a/chain/network/src/config.rs b/chain/network/src/config.rs index 88c65053bd3..a34e9cfdb94 100644 --- a/chain/network/src/config.rs +++ b/chain/network/src/config.rs @@ -9,6 +9,7 @@ use crate::tcp; use crate::types::ROUTED_MESSAGE_TTL; use anyhow::Context; use near_async::time; +use near_chain_configs::MutableConfigValue; use near_crypto::{KeyType, SecretKey}; use near_primitives::network::PeerId; use near_primitives::test_utils::create_test_signer; @@ -56,13 +57,26 @@ pub enum ValidatorProxies { #[derive(Clone)] pub struct ValidatorConfig { - pub signer: Arc, + /// Contains signer key for this node. This field is mutable and optional. Use with caution! + /// Lock the value of mutable validator signer for the duration of a request to ensure consistency. + /// Please note that the locked value should not be stored anywhere or passed through the thread boundary. + pub signer: MutableConfigValue>>, pub proxies: ValidatorProxies, } +/// A snapshot of ValidatorConfig. Use to freeze the value of the mutable validator signer field. +pub struct FrozenValidatorConfig<'a> { + pub signer: Option>, + pub proxies: &'a ValidatorProxies, +} + impl ValidatorConfig { - pub fn account_id(&self) -> AccountId { - self.signer.validator_id().clone() + pub fn account_id(&self) -> Option { + self.signer.get().map(|s| s.validator_id().clone()) + } + + pub fn frozen_view(&self) -> FrozenValidatorConfig { + FrozenValidatorConfig { signer: self.signer.get(), proxies: &self.proxies } } } @@ -104,7 +118,7 @@ impl SocketOptions { pub struct NetworkConfig { pub node_addr: Option, pub node_key: SecretKey, - pub validator: Option, + pub validator: ValidatorConfig, pub peer_store: peer_store::Config, pub snapshot_hosts: snapshot_hosts::Config, @@ -227,7 +241,7 @@ impl NetworkConfig { pub fn new( cfg: crate::config_json::Config, node_key: SecretKey, - validator_signer: Option>, + validator_signer: MutableConfigValue>>, archive: bool, ) -> anyhow::Result { if cfg.public_addrs.len() > MAX_PEER_ADDRS { @@ -263,14 +277,14 @@ impl NetworkConfig { } let mut this = Self { node_key, - validator: validator_signer.map(|signer| ValidatorConfig { - signer, + validator: ValidatorConfig { + signer: validator_signer, proxies: if !cfg.public_addrs.is_empty() { ValidatorProxies::Static(cfg.public_addrs) } else { ValidatorProxies::Dynamic(cfg.trusted_stun_servers) }, - }), + }, node_addr: match cfg.addr.as_str() { "" => None, addr => Some(tcp::ListenerAddr::new( @@ -373,7 +387,10 @@ impl NetworkConfig { pub fn from_seed(seed: &str, node_addr: tcp::ListenerAddr) -> Self { let node_key = SecretKey::from_seed(KeyType::ED25519, seed); let validator = ValidatorConfig { - signer: Arc::new(create_test_signer(seed)), + signer: MutableConfigValue::new( + Some(Arc::new(create_test_signer(seed))), + "validator_signer", + ), proxies: ValidatorProxies::Static(vec![PeerAddr { addr: *node_addr, peer_id: PeerId::new(node_key.public_key()), @@ -382,7 +399,7 @@ impl NetworkConfig { NetworkConfig { node_addr: Some(node_addr), node_key, - validator: Some(validator), + validator, peer_store: peer_store::Config { boot_nodes: vec![], blacklist: blacklist::Blacklist::default(), diff --git a/chain/network/src/peer/peer_actor.rs b/chain/network/src/peer/peer_actor.rs index c822cb657c7..72305926812 100644 --- a/chain/network/src/peer/peer_actor.rs +++ b/chain/network/src/peer/peer_actor.rs @@ -307,7 +307,9 @@ impl PeerActor { let my_node_info = PeerInfo { id: network_state.config.node_id(), addr: network_state.config.node_addr.as_ref().map(|a| **a), - account_id: network_state.config.validator.as_ref().map(|v| v.account_id()), + // TODO(validator-key-hot-swap) Consider using mutable validator signer instead of PeerInfo.account_id ? + // That likely requires bigger changes and account_id here is later used for debug / logging purposes only. + account_id: network_state.config.validator.account_id(), }; // recv is the HandshakeSignal returned by this spawn_inner() call. let (send, recv): (HandshakeSignalSender, HandshakeSignal) = @@ -464,13 +466,13 @@ impl PeerActor { archival: self.network_state.config.archive, }, partial_edge_info: spec.partial_edge_info, - owned_account: self.network_state.config.validator.as_ref().map(|vc| { + owned_account: self.network_state.config.validator.signer.get().map(|signer| { OwnedAccount { - account_key: vc.signer.public_key(), + account_key: signer.public_key(), peer_id: self.network_state.config.node_id(), timestamp: self.clock.now_utc(), } - .sign(vc.signer.as_ref()) + .sign(&signer) }), }; let msg = match spec.tier { diff --git a/chain/network/src/peer_manager/network_state/mod.rs b/chain/network/src/peer_manager/network_state/mod.rs index c6fb4fe552d..b4090fa9fe3 100644 --- a/chain/network/src/peer_manager/network_state/mod.rs +++ b/chain/network/src/peer_manager/network_state/mod.rs @@ -503,7 +503,7 @@ impl NetworkState { // Check if the message is for myself and don't try to send it in that case. if let PeerIdOrHash::PeerId(target) = &msg.target { if target == &my_peer_id { - tracing::debug!(target: "network", account_id = ?self.config.validator.as_ref().map(|v|v.account_id()), ?my_peer_id, ?msg, "Drop signed message to myself"); + tracing::debug!(target: "network", account_id = ?self.config.validator.account_id(), ?my_peer_id, ?msg, "Drop signed message to myself"); metrics::CONNECTED_TO_MYSELF.inc(); return false; } @@ -540,7 +540,7 @@ impl NetworkState { metrics::MessageDropped::NoRouteFound.inc(&msg.body); tracing::debug!(target: "network", - account_id = ?self.config.validator.as_ref().map(|v|v.account_id()), + account_id = ?self.config.validator.account_id(), to = ?msg.target, reason = ?find_route_error, known_peers = ?self.graph.routing_table.reachable_peers(), @@ -610,7 +610,7 @@ impl NetworkState { // TODO(MarX, #1369): Message is dropped here. Define policy for this case. metrics::MessageDropped::UnknownAccount.inc(&msg); tracing::debug!(target: "network", - account_id = ?self.config.validator.as_ref().map(|v|v.account_id()), + account_id = ?self.config.validator.account_id(), to = ?account_id, ?msg,"Drop message: unknown account", ); diff --git a/chain/network/src/peer_manager/network_state/routing.rs b/chain/network/src/peer_manager/network_state/routing.rs index f01c4cb9435..0fe045fcdad 100644 --- a/chain/network/src/peer_manager/network_state/routing.rs +++ b/chain/network/src/peer_manager/network_state/routing.rs @@ -45,7 +45,7 @@ impl NetworkState { let this = self.clone(); self.spawn(async move { let new_accounts = this.account_announcements.add_accounts(accounts); - tracing::debug!(target: "network", account_id = ?this.config.validator.as_ref().map(|v|v.account_id()), ?new_accounts, "Received new accounts"); + tracing::debug!(target: "network", account_id = ?this.config.validator.account_id(), ?new_accounts, "Received new accounts"); #[cfg(test)] this.config.event_sink.send(crate::peer_manager::peer_manager_actor::Event::AccountsAdded(new_accounts.clone())); this.broadcast_routing_table_update(RoutingTableUpdate::from_accounts( diff --git a/chain/network/src/peer_manager/network_state/tier1.rs b/chain/network/src/peer_manager/network_state/tier1.rs index e2f7d16bbd0..2aa677bc922 100644 --- a/chain/network/src/peer_manager/network_state/tier1.rs +++ b/chain/network/src/peer_manager/network_state/tier1.rs @@ -1,5 +1,5 @@ use crate::accounts_data::{AccountDataCacheSnapshot, LocalAccountData}; -use crate::config; +use crate::config::{self, FrozenValidatorConfig}; use crate::network_protocol::{ AccountData, PeerAddr, PeerInfo, PeerMessage, SignedAccountData, SyncAccountsData, }; @@ -18,18 +18,23 @@ use std::collections::{HashMap, HashSet}; use std::sync::Arc; impl super::NetworkState { - // Returns ValidatorConfig of this node iff it belongs to TIER1 according to `accounts_data`. - pub fn tier1_validator_config( + // Returns a snapshot of ValidatorConfig of this node iff it belongs to TIER1 according to `accounts_data`. + fn tier1_validator_config( &self, accounts_data: &AccountDataCacheSnapshot, - ) -> Option<&config::ValidatorConfig> { + ) -> Option { if self.config.tier1.is_none() { return None; } - self.config - .validator + let signer = self.config.validator.signer.get(); + if signer .as_ref() - .filter(|cfg| accounts_data.keys.contains(&cfg.signer.public_key())) + .filter(|signer| accounts_data.keys.contains(&signer.public_key())) + .is_none() + { + return None; + } + Some(FrozenValidatorConfig { signer, proxies: &self.config.validator.proxies }) } async fn tier1_connect_to_my_proxies( @@ -95,6 +100,7 @@ impl super::NetworkState { let accounts_data = self.accounts_data.load(); let vc = self.tier1_validator_config(&accounts_data)?; + let signer = vc.signer?; let proxies = match (&self.config.node_addr, &vc.proxies) { (None, _) => vec![], (_, config::ValidatorProxies::Static(peer_addrs)) => peer_addrs.clone(), @@ -191,7 +197,7 @@ impl super::NetworkState { let new_data = self.accounts_data.set_local( clock, LocalAccountData { - signer: vc.signer.clone(), + signer, data: Arc::new(AccountData { peer_id: self.config.node_id(), proxies: my_proxies }), }, ); @@ -274,7 +280,7 @@ impl super::NetworkState { // Construct a safe set of connections. let mut safe_set: HashSet = safe.values().map(|v| (*v).clone()).collect(); // Add proxies of our node to the safe set. - if let Some(vc) = validator_cfg { + if let Some(vc) = validator_cfg.as_ref() { match &vc.proxies { config::ValidatorProxies::Dynamic(_) => { safe_set.insert(self.config.node_id()); @@ -294,6 +300,7 @@ impl super::NetworkState { } } if let Some(vc) = validator_cfg { + let validator_signer = if let Some(v) = vc.signer { v } else { return }; // Try to establish new TIER1 connections to accounts in random order. let mut handles = vec![]; let mut account_keys: Vec<_> = proxies_by_account.keys().copied().collect(); @@ -302,7 +309,7 @@ impl super::NetworkState { // tier1_connect() is responsible for connecting to proxies // of this node. tier1_connect() connects only to proxies // of other TIER1 nodes. - if account_key == &vc.signer.public_key() { + if account_key == &validator_signer.public_key() { continue; } // Bound the number of connections established at a single call to diff --git a/chain/network/src/peer_manager/testonly.rs b/chain/network/src/peer_manager/testonly.rs index f7d49f8ad18..5ef9abce822 100644 --- a/chain/network/src/peer_manager/testonly.rs +++ b/chain/network/src/peer_manager/testonly.rs @@ -110,7 +110,7 @@ pub(crate) fn make_chain_info( let mut chain_info = chain.get_chain_info(); let mut account_keys = AccountKeys::new(); for cfg in validators { - let s = &cfg.validator.as_ref().unwrap().signer; + let s = &cfg.validator.signer.get().unwrap(); account_keys.entry(s.validator_id().clone()).or_default().insert(s.public_key()); } chain_info.tier1_accounts = Arc::new(account_keys); diff --git a/chain/network/src/peer_manager/tests/accounts_data.rs b/chain/network/src/peer_manager/tests/accounts_data.rs index bf8350c7507..660ddb78545 100644 --- a/chain/network/src/peer_manager/tests/accounts_data.rs +++ b/chain/network/src/peer_manager/tests/accounts_data.rs @@ -322,7 +322,7 @@ async fn validator_node_restart() { // Now pm0 should learn from pm1 about the conflicting version and should broadcast // new AccountData (with higher version) to override the old AccountData. - let pm0_account_key = cfg.validator.as_ref().unwrap().signer.public_key(); + let pm0_account_key = cfg.validator.signer.get().unwrap().public_key(); pm1.wait_for_accounts_data_pred(|accounts_data| { let data = match accounts_data.data.get(&pm0_account_key) { Some(it) => it, diff --git a/chain/network/src/peer_manager/tests/connection_pool.rs b/chain/network/src/peer_manager/tests/connection_pool.rs index 30d07d1148a..79e00807f20 100644 --- a/chain/network/src/peer_manager/tests/connection_pool.rs +++ b/chain/network/src/peer_manager/tests/connection_pool.rs @@ -153,7 +153,7 @@ async fn owned_account_mismatch() { let mut events = pm.events.from_now(); let mut stream = Stream::new(Some(Encoding::Proto), stream); let cfg = chain.make_config(rng); - let vc = cfg.validator.clone().unwrap(); + let signer = cfg.validator.signer.get().unwrap(); stream .write(&PeerMessage::Tier2Handshake(Handshake { protocol_version: PROTOCOL_VERSION, @@ -170,12 +170,12 @@ async fn owned_account_mismatch() { ), owned_account: Some( OwnedAccount { - account_key: vc.signer.public_key().clone(), + account_key: signer.public_key(), // random peer_id, different than the expected cfg.node_id(). peer_id: data::make_peer_id(rng), timestamp: clock.now_utc(), } - .sign(vc.signer.as_ref()), + .sign(&signer), ), })) .await; @@ -223,7 +223,7 @@ async fn owned_account_conflict() { ClosingReason::RejectedByPeerManager(RegisterPeerError::PoolError( connection::PoolError::AlreadyConnectedAccount { peer_id: cfg1.node_id(), - account_key: cfg1.validator.as_ref().unwrap().signer.public_key(), + account_key: cfg1.validator.signer.get().unwrap().public_key(), } )) ); @@ -282,7 +282,7 @@ async fn invalid_edge() { let port = stream.local_addr.port(); let mut events = pm.events.from_now(); let mut stream = Stream::new(Some(Encoding::Proto), stream); - let vc = cfg.validator.clone().unwrap(); + let signer = cfg.validator.signer.get().unwrap(); let handshake = Handshake { protocol_version: PROTOCOL_VERSION, oldest_supported_version: PROTOCOL_VERSION, @@ -293,11 +293,11 @@ async fn invalid_edge() { partial_edge_info: edge.clone(), owned_account: Some( OwnedAccount { - account_key: vc.signer.public_key().clone(), + account_key: signer.public_key(), peer_id: cfg.node_id(), timestamp: clock.now_utc(), } - .sign(vc.signer.as_ref()), + .sign(&signer), ), }; let handshake = match tier { diff --git a/chain/network/src/peer_manager/tests/tier1.rs b/chain/network/src/peer_manager/tests/tier1.rs index aaa3c6ff030..2a7945ddb59 100644 --- a/chain/network/src/peer_manager/tests/tier1.rs +++ b/chain/network/src/peer_manager/tests/tier1.rs @@ -55,8 +55,8 @@ async fn send_tier1_message( from: &peer_manager::testonly::ActorHandler, to: &peer_manager::testonly::ActorHandler, ) -> Option { - let from_signer = from.cfg.validator.as_ref().unwrap().signer.clone(); - let to_signer = to.cfg.validator.as_ref().unwrap().signer.clone(); + let from_signer = from.cfg.validator.signer.get().unwrap(); + let to_signer = to.cfg.validator.signer.get().unwrap(); let target = to_signer.validator_id().clone(); let want = RoutedMessageBody::BlockApproval(make_block_approval(rng, from_signer.as_ref())); let clock = clock.clone(); @@ -211,11 +211,10 @@ async fn proxy_connections() { let mut validators = vec![]; for i in 0..N { let mut cfg = chain.make_config(rng); - cfg.validator.as_mut().unwrap().proxies = - config::ValidatorProxies::Static(vec![PeerAddr { - peer_id: proxies[i].cfg.node_id(), - addr: **proxies[i].cfg.node_addr.as_ref().unwrap(), - }]); + cfg.validator.proxies = config::ValidatorProxies::Static(vec![PeerAddr { + peer_id: proxies[i].cfg.node_id(), + addr: **proxies[i].cfg.node_addr.as_ref().unwrap(), + }]); validators .push(start_pm(clock.clock(), near_store::db::TestDB::new(), cfg, chain.clone()).await); } @@ -307,12 +306,12 @@ async fn proxy_change() { let p0cfg = chain.make_config(rng); let p1cfg = chain.make_config(rng); let mut v0cfg = chain.make_config(rng); - v0cfg.validator.as_mut().unwrap().proxies = config::ValidatorProxies::Static(vec![ + v0cfg.validator.proxies = config::ValidatorProxies::Static(vec![ PeerAddr { peer_id: p0cfg.node_id(), addr: **p0cfg.node_addr.as_ref().unwrap() }, PeerAddr { peer_id: p1cfg.node_id(), addr: **p1cfg.node_addr.as_ref().unwrap() }, ]); let mut v1cfg = chain.make_config(rng); - v1cfg.validator.as_mut().unwrap().proxies = config::ValidatorProxies::Static(vec![]); + v1cfg.validator.proxies = config::ValidatorProxies::Static(vec![]); tracing::info!(target:"test", "Start all nodes."); let p0 = start_pm(clock.clock(), TestDB::new(), p0cfg.clone(), chain.clone()).await; @@ -401,8 +400,7 @@ async fn stun_self_discovery() { let stun_server1 = stun::testonly::Server::new().await; let stun_server2 = stun::testonly::Server::new().await; let mut cfg = chain.make_config(rng); - let vc = cfg.validator.as_mut().unwrap(); - vc.proxies = config::ValidatorProxies::Dynamic(vec![ + cfg.validator.proxies = config::ValidatorProxies::Dynamic(vec![ stun_server1.addr().to_string(), stun_server2.addr().to_string(), ]); diff --git a/core/chain-configs/src/updateable_config.rs b/core/chain-configs/src/updateable_config.rs index 2b6b64cb9f6..ed0cb4cd8fa 100644 --- a/core/chain-configs/src/updateable_config.rs +++ b/core/chain-configs/src/updateable_config.rs @@ -58,15 +58,18 @@ impl MutableConfigValue { self.value.lock().unwrap().clone() } - pub fn update(&self, val: T) { + /// Attempts to update the value and returns whether the value changed. + pub fn update(&self, val: T) -> bool { let mut lock = self.value.lock().unwrap(); if *lock != val { tracing::info!(target: "config", "Updated config field '{}' from {:?} to {:?}", self.field_name, *lock, val); self.set_metric_value(lock.clone(), 0); *lock = val.clone(); self.set_metric_value(val, 1); + true } else { tracing::info!(target: "config", "Mutable config field '{}' remains the same: {:?}", self.field_name, val); + false } } diff --git a/core/primitives/src/validator_signer.rs b/core/primitives/src/validator_signer.rs index aa92a5a8d84..1ccf530db97 100644 --- a/core/primitives/src/validator_signer.rs +++ b/core/primitives/src/validator_signer.rs @@ -193,6 +193,10 @@ pub struct EmptyValidatorSigner { } impl EmptyValidatorSigner { + pub fn new(account_id: AccountId) -> ValidatorSigner { + ValidatorSigner::Empty(Self { account_id }) + } + fn validator_id(&self) -> &AccountId { &self.account_id } diff --git a/integration-tests/src/node/mod.rs b/integration-tests/src/node/mod.rs index 65d53f7660e..e33ae3ffd5f 100644 --- a/integration-tests/src/node/mod.rs +++ b/integration-tests/src/node/mod.rs @@ -6,6 +6,7 @@ pub use crate::node::runtime_node::RuntimeNode; pub use crate::node::thread_node::ThreadNode; use crate::user::{AsyncUser, User}; use near_chain_configs::Genesis; +use near_chain_configs::MutableConfigValue; use near_crypto::{InMemorySigner, Signer}; use near_jsonrpc_primitives::errors::ServerError; use near_primitives::num_rational::Ratio; @@ -133,7 +134,10 @@ fn near_configs_to_node_configs( configs[i].clone(), genesis.clone(), (&network_signers[i]).into(), - Some(Arc::new(validator_signers[i].clone())), + MutableConfigValue::new( + Some(Arc::new(validator_signers[i].clone())), + "validator_signer", + ), ) .unwrap(), )) diff --git a/integration-tests/src/node/process_node.rs b/integration-tests/src/node/process_node.rs index b5fd308d111..f1420eaf74c 100644 --- a/integration-tests/src/node/process_node.rs +++ b/integration-tests/src/node/process_node.rs @@ -39,7 +39,7 @@ impl Node for ProcessNode { } fn account_id(&self) -> Option { - self.config.validator_signer.as_ref().map(|vs| vs.validator_id().clone()) + self.config.validator_signer.get().map(|vs| vs.validator_id().clone()) } fn start(&mut self) { @@ -112,7 +112,7 @@ impl ProcessNode { pub fn new(config: NearConfig) -> ProcessNode { let mut rng = rand::thread_rng(); let work_dir = env::temp_dir().join(format!("process_node_{}", rng.gen::())); - let account_id = config.validator_signer.as_ref().unwrap().validator_id().clone(); + let account_id = config.validator_signer.get().unwrap().validator_id().clone(); let signer = Arc::new( InMemorySigner::from_seed(account_id.clone(), KeyType::ED25519, account_id.as_ref()) .into(), diff --git a/integration-tests/src/node/thread_node.rs b/integration-tests/src/node/thread_node.rs index 60329c0a941..d3d5d631abc 100644 --- a/integration-tests/src/node/thread_node.rs +++ b/integration-tests/src/node/thread_node.rs @@ -36,7 +36,7 @@ impl Node for ThreadNode { } fn account_id(&self) -> Option { - self.config.validator_signer.as_ref().map(|vs| vs.validator_id().clone()) + self.config.validator_signer.get().map(|vs| vs.validator_id().clone()) } fn start(&mut self) { @@ -86,7 +86,7 @@ impl Node for ThreadNode { impl ThreadNode { /// Side effects: create storage, open database, lock database pub fn new(config: NearConfig) -> ThreadNode { - let account_id = config.validator_signer.as_ref().unwrap().validator_id().clone(); + let account_id = config.validator_signer.get().unwrap().validator_id().clone(); let signer = InMemorySigner::from_seed(account_id.clone(), KeyType::ED25519, account_id.as_ref()); ThreadNode { diff --git a/integration-tests/src/test_loop/builder.rs b/integration-tests/src/test_loop/builder.rs index 95dc2944076..3b40bb27c3c 100644 --- a/integration-tests/src/test_loop/builder.rs +++ b/integration-tests/src/test_loop/builder.rs @@ -14,7 +14,7 @@ use near_chain::types::RuntimeAdapter; use near_chain::ChainGenesis; use near_chain_configs::{ ClientConfig, DumpConfig, ExternalStorageConfig, ExternalStorageLocation, Genesis, - StateSyncConfig, SyncConfig, + MutableConfigValue, StateSyncConfig, SyncConfig, }; use near_chunks::shards_manager_actor::ShardsManagerActor; use near_client::client_actor::ClientActorInner; @@ -188,7 +188,10 @@ impl TestLoopBuilder { let snapshot_callbacks = SnapshotCallbacks { make_snapshot_callback, delete_snapshot_callback }; - let validator_signer = Arc::new(create_test_signer(self.clients[idx].as_str())); + let validator_signer = MutableConfigValue::new( + Some(Arc::new(create_test_signer(self.clients[idx].as_str()))), + "validator_signer", + ); let client = Client::new( self.test_loop.clock(), client_config.clone(), @@ -199,7 +202,7 @@ impl TestLoopBuilder { runtime_adapter.clone(), network_adapter.as_multi_sender(), shards_manager_adapter.as_sender(), - Some(validator_signer.clone()), + validator_signer.clone(), true, [0; 32], Some(snapshot_callbacks), @@ -210,7 +213,7 @@ impl TestLoopBuilder { let shards_manager = ShardsManagerActor::new( self.test_loop.clock(), - Some(self.clients[idx].clone()), + validator_signer.clone(), epoch_manager.clone(), shard_tracker.clone(), network_adapter.as_sender(), @@ -228,7 +231,6 @@ impl TestLoopBuilder { client_config.clone(), PeerId::random(), network_adapter.as_multi_sender(), - None, noop().into_sender(), None, Default::default(), @@ -242,7 +244,7 @@ impl TestLoopBuilder { self.test_loop.clock(), network_adapter.as_multi_sender(), client_adapter.as_multi_sender(), - validator_signer, + validator_signer.clone(), epoch_manager.clone(), store, ); @@ -268,7 +270,7 @@ impl TestLoopBuilder { epoch_manager, shard_tracker, runtime: runtime_adapter, - account_id: Some(self.clients[idx].clone()), + validator: validator_signer, dump_future_runner: Box::new(move |future| { future_spawner.spawn_boxed("state_sync_dumper", future); Box::new(|| {}) diff --git a/integration-tests/src/test_loop/tests/simple_test_loop_example.rs b/integration-tests/src/test_loop/tests/simple_test_loop_example.rs index d1b371e2df8..5b5410b3021 100644 --- a/integration-tests/src/test_loop/tests/simple_test_loop_example.rs +++ b/integration-tests/src/test_loop/tests/simple_test_loop_example.rs @@ -4,7 +4,7 @@ use near_async::time::Duration; use near_chain::chunks_store::ReadOnlyChunksStore; use near_chain::ChainGenesis; use near_chain_configs::test_genesis::TestGenesisBuilder; -use near_chain_configs::ClientConfig; +use near_chain_configs::{ClientConfig, MutableConfigValue}; use near_chunks::shards_manager_actor::ShardsManagerActor; use near_client::client_actor::ClientActorInner; use near_client::sync_jobs_actor::SyncJobsActor; @@ -74,6 +74,10 @@ fn test_client_with_simple_test_loop() { &genesis.config, epoch_manager.clone(), ); + let validator_signer = MutableConfigValue::new( + Some(Arc::new(create_test_signer(accounts[0].as_str()))), + "validator_signer", + ); let shards_manager_adapter = LateBoundSender::new(); let sync_jobs_adapter = LateBoundSender::new(); @@ -97,7 +101,7 @@ fn test_client_with_simple_test_loop() { runtime_adapter, noop().into_multi_sender(), shards_manager_adapter.as_sender(), - Some(Arc::new(create_test_signer(accounts[0].as_str()))), + validator_signer.clone(), true, [0; 32], None, @@ -108,7 +112,7 @@ fn test_client_with_simple_test_loop() { let shards_manager = ShardsManagerActor::new( test_loop.clock(), - Some(accounts[0].clone()), + validator_signer, epoch_manager, shard_tracker, noop().into_sender(), @@ -126,7 +130,6 @@ fn test_client_with_simple_test_loop() { client_config, PeerId::random(), noop().into_multi_sender(), - None, noop().into_sender(), None, Default::default(), diff --git a/integration-tests/src/tests/client/cold_storage.rs b/integration-tests/src/tests/client/cold_storage.rs index a9a871b9d2e..96573f3dcd5 100644 --- a/integration-tests/src/tests/client/cold_storage.rs +++ b/integration-tests/src/tests/client/cold_storage.rs @@ -1,6 +1,6 @@ use borsh::BorshDeserialize; use near_chain::Provenance; -use near_chain_configs::Genesis; +use near_chain_configs::{Genesis, MutableConfigValue}; use near_client::test_utils::TestEnv; use near_client::ProcessTxResponse; use near_crypto::{InMemorySigner, KeyType, Signer}; @@ -488,7 +488,7 @@ fn test_cold_loop_on_gc_boundary() { public_key: signer.public_key, secret_key: signer.secret_key, }, - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); near_config.client_config = env.clients[0].config.clone(); 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 6e4275a0a9f..a50b3da39b5 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 @@ -500,10 +500,12 @@ fn test_processing_chunks_sanity() { let mut next_blocks: Vec<_> = (3 * i..3 * i + 3).collect(); next_blocks.shuffle(&mut rng); for ind in next_blocks { + let signer = env.clients[1].validator_signer.get(); let _ = env.clients[1].start_process_block( blocks[ind].clone().into(), Provenance::NONE, None, + &signer, ); if rng.gen_bool(0.5) { env.process_shards_manager_responses_and_finish_processing_blocks(1); @@ -724,10 +726,12 @@ fn test_chunk_forwarding_optimization() { // The block producer of course has the complete block so we can process that. for i in 0..test.num_validators { debug!(target: "test", "Processing block {} as validator #{}", block.header().height(), i); + let signer = test.env.clients[i].validator_signer.get(); let _ = test.env.clients[i].start_process_block( block.clone().into(), if i == 0 { Provenance::PRODUCED } else { Provenance::NONE }, None, + &signer, ); let mut accepted_blocks = test.env.clients[i].finish_block_in_processing(block.header().hash()); @@ -813,8 +817,13 @@ fn test_processing_blocks_async() { let mut rng = thread_rng(); blocks.shuffle(&mut rng); for ind in 0..blocks.len() { - let _ = - env.clients[1].start_process_block(blocks[ind].clone().into(), Provenance::NONE, None); + let signer = env.clients[1].validator_signer.get(); + let _ = env.clients[1].start_process_block( + blocks[ind].clone().into(), + Provenance::NONE, + None, + &signer, + ); } env.process_shards_manager_responses_and_finish_processing_blocks(1); diff --git a/integration-tests/src/tests/client/features/adversarial_behaviors.rs b/integration-tests/src/tests/client/features/adversarial_behaviors.rs index ddb020a595f..186992450d6 100644 --- a/integration-tests/src/tests/client/features/adversarial_behaviors.rs +++ b/integration-tests/src/tests/client/features/adversarial_behaviors.rs @@ -171,10 +171,12 @@ fn test_non_adversarial_case() { for i in 0..test.num_validators { debug!(target: "test", "Processing block {} as validator #{}", height, i); + let signer = test.env.clients[i].validator_signer.get(); let _ = test.env.clients[i].start_process_block( block.clone().into(), if i == 0 { Provenance::PRODUCED } else { Provenance::NONE }, None, + &signer, ); let mut accepted_blocks = test.env.clients[i].finish_block_in_processing(block.header().hash()); @@ -305,10 +307,12 @@ fn test_banning_chunk_producer_when_seeing_invalid_chunk_base( // The block producer of course has the complete block so we can process that. for i in 0..test.num_validators { debug!(target: "test", "Processing block {} as validator #{}", height, i); + let signer = test.env.clients[i].validator_signer.get(); let _ = test.env.clients[i].start_process_block( block.clone().into(), if i == 0 { Provenance::PRODUCED } else { Provenance::NONE }, None, + &signer, ); let mut accepted_blocks = test.env.clients[i].finish_block_in_processing(block.header().hash()); diff --git a/integration-tests/src/tests/client/features/orphan_chunk_state_witness.rs b/integration-tests/src/tests/client/features/orphan_chunk_state_witness.rs index b6621f14a53..19772b777e6 100644 --- a/integration-tests/src/tests/client/features/orphan_chunk_state_witness.rs +++ b/integration-tests/src/tests/client/features/orphan_chunk_state_witness.rs @@ -151,11 +151,13 @@ fn setup_orphan_witness_test() -> OrphanWitnessTestEnv { for account_id in chunk_validators.into_iter().filter(|acc| *acc != excluded_validator) { let processing_done_tracker = ProcessingDoneTracker::new(); witness_processing_done_waiters.push(processing_done_tracker.make_waiter()); - env.client(&account_id) + let client = env.client(&account_id); + client .process_chunk_state_witness( state_witness.clone(), raw_witness_size, Some(processing_done_tracker), + client.validator_signer.get(), ) .unwrap(); } @@ -222,8 +224,9 @@ fn test_orphan_witness_valid() { // `excluded_validator` receives witness for chunk belonging to `block2`, but it doesn't have `block1`. // The witness should become an orphaned witness and it should be saved to the orphan pool. let witness_size = borsh_size(&witness); - env.client(&excluded_validator) - .process_chunk_state_witness(witness, witness_size, None) + let client = env.client(&excluded_validator); + client + .process_chunk_state_witness(witness, witness_size, None, client.validator_signer.get()) .unwrap(); let block_processed = env @@ -320,8 +323,9 @@ fn test_orphan_witness_not_fully_validated() { // There is no way to fully validate an orphan witness, so this is the correct behavior. // The witness will later be fully validated when the required block arrives. let witness_size = borsh_size(&witness); - env.client(&excluded_validator) - .process_chunk_state_witness(witness, witness_size, None) + let client = env.client(&excluded_validator); + client + .process_chunk_state_witness(witness, witness_size, None, client.validator_signer.get()) .unwrap(); } diff --git a/integration-tests/src/tests/client/features/stateless_validation.rs b/integration-tests/src/tests/client/features/stateless_validation.rs index d1585b218ca..38daf726866 100644 --- a/integration-tests/src/tests/client/features/stateless_validation.rs +++ b/integration-tests/src/tests/client/features/stateless_validation.rs @@ -405,7 +405,8 @@ fn test_chunk_state_witness_bad_shard_id() { // Client should reject this ChunkStateWitness and the error message should mention "shard" tracing::info!(target: "test", "Processing invalid ChunkStateWitness"); - let res = env.clients[0].process_chunk_state_witness(witness, witness_size, None); + let signer = env.clients[0].validator_signer.get(); + let res = env.clients[0].process_chunk_state_witness(witness, witness_size, None, signer); let error = res.unwrap_err(); let error_message = format!("{}", error).to_lowercase(); tracing::info!(target: "test", "error message: {}", error_message); @@ -521,6 +522,7 @@ fn test_invalid_transactions() { &prev_chunk_header, &shard_chunk, transactions_storage_proof, + &client.validator_signer.get(), ) .unwrap(); diff --git a/integration-tests/src/tests/client/process_blocks.rs b/integration-tests/src/tests/client/process_blocks.rs index fb476eb393c..1ac99d5b6b3 100644 --- a/integration-tests/src/tests/client/process_blocks.rs +++ b/integration-tests/src/tests/client/process_blocks.rs @@ -1872,7 +1872,8 @@ fn test_gc_tail_update() { blocks.push(block); } let headers = blocks.iter().map(|b| b.header().clone()).collect::>(); - env.clients[1].sync_block_headers(headers).unwrap(); + let signer = env.clients[1].validator_signer.get(); + env.clients[1].sync_block_headers(headers, &signer).unwrap(); // simulate save sync hash block let prev_sync_block = blocks[blocks.len() - 3].clone(); let prev_sync_hash = *prev_sync_block.hash(); @@ -2290,12 +2291,13 @@ fn test_validate_chunk_extra() { let mut chain_store = ChainStore::new(env.clients[0].chain.chain_store().store().clone(), genesis_height, true); let chunk_header = encoded_chunk.cloned_header(); - let validator_id = env.clients[0].validator_signer.get().unwrap().validator_id().clone(); + let signer = env.clients[0].validator_signer.get(); + let validator_id = signer.as_ref().unwrap().validator_id().clone(); env.clients[0] .persist_and_distribute_encoded_chunk(encoded_chunk, merkle_paths, receipts, validator_id) .unwrap(); env.clients[0].chain.blocks_with_missing_chunks.accept_chunk(&chunk_header.chunk_hash()); - env.clients[0].process_blocks_with_missing_chunks(None); + env.clients[0].process_blocks_with_missing_chunks(None, &signer); let accepted_blocks = env.clients[0].finish_block_in_processing(block1.hash()); assert_eq!(accepted_blocks.len(), 1); env.resume_block_processing(block2.hash()); diff --git a/integration-tests/src/tests/client/resharding.rs b/integration-tests/src/tests/client/resharding.rs index aa1ac7f78f0..acbd897409a 100644 --- a/integration-tests/src/tests/client/resharding.rs +++ b/integration-tests/src/tests/client/resharding.rs @@ -352,12 +352,14 @@ impl TestReshardingEnv { // because we want to call run_catchup before finish processing this block. This simulates // that catchup and block processing run in parallel. let block = MaybeValidated::from(block.clone()); - client.start_process_block(block, Provenance::NONE, None).unwrap(); + let signer = client.validator_signer.get(); + client.start_process_block(block, Provenance::NONE, None, &signer).unwrap(); if should_catchup { run_catchup(client, &[])?; } while wait_for_all_blocks_in_processing(&mut client.chain) { - let (_, errors) = client.postprocess_ready_blocks(None, should_produce_chunk); + let (_, errors) = + client.postprocess_ready_blocks(None, should_produce_chunk, &signer); assert!(errors.is_empty(), "unexpected errors: {:?}", errors); } // manually invoke gc diff --git a/integration-tests/src/tests/client/runtimes.rs b/integration-tests/src/tests/client/runtimes.rs index 5f09f98ab0e..e7cd36318e6 100644 --- a/integration-tests/src/tests/client/runtimes.rs +++ b/integration-tests/src/tests/client/runtimes.rs @@ -23,7 +23,12 @@ fn test_pending_approvals() { let parent_hash = hash(&[1]); let approval = Approval::new(parent_hash, 0, 1, &signer); let peer_id = PeerId::random(); - env.clients[0].collect_block_approval(&approval, ApprovalType::PeerApproval(peer_id.clone())); + let client_signer = env.clients[0].validator_signer.get(); + env.clients[0].collect_block_approval( + &approval, + ApprovalType::PeerApproval(peer_id.clone()), + &client_signer, + ); let approvals = env.clients[0].pending_approvals.pop(&ApprovalInner::Endorsement(parent_hash)); let expected = vec![("test0".parse().unwrap(), (approval, ApprovalType::PeerApproval(peer_id)))] @@ -45,7 +50,12 @@ fn test_invalid_approvals() { // Approval not from a validator. Should be dropped let approval = Approval::new(parent_hash, 1, 3, &signer); let peer_id = PeerId::random(); - env.clients[0].collect_block_approval(&approval, ApprovalType::PeerApproval(peer_id.clone())); + let client_signer = env.clients[0].validator_signer.get(); + env.clients[0].collect_block_approval( + &approval, + ApprovalType::PeerApproval(peer_id.clone()), + &client_signer, + ); assert_eq!(env.clients[0].pending_approvals.len(), 0); // Approval with invalid signature. Should be dropped let signer = @@ -53,7 +63,11 @@ fn test_invalid_approvals() { .into(); let genesis_hash = *env.clients[0].chain.genesis().hash(); let approval = Approval::new(genesis_hash, 0, 1, &signer); - env.clients[0].collect_block_approval(&approval, ApprovalType::PeerApproval(peer_id)); + env.clients[0].collect_block_approval( + &approval, + ApprovalType::PeerApproval(peer_id), + &client_signer, + ); assert_eq!(env.clients[0].pending_approvals.len(), 0); } diff --git a/integration-tests/src/tests/client/state_dump.rs b/integration-tests/src/tests/client/state_dump.rs index ff71c7c0963..89eaff5a7f2 100644 --- a/integration-tests/src/tests/client/state_dump.rs +++ b/integration-tests/src/tests/client/state_dump.rs @@ -4,7 +4,7 @@ use near_async::time::{Clock, Duration}; use near_chain::near_chain_primitives::error::QueryError; use near_chain::{ChainGenesis, ChainStoreAccess, Provenance}; use near_chain_configs::ExternalStorageLocation::Filesystem; -use near_chain_configs::{DumpConfig, Genesis, NEAR_BASE}; +use near_chain_configs::{DumpConfig, Genesis, MutableConfigValue, NEAR_BASE}; use near_client::sync::external::{external_storage_location, StateFileType}; use near_client::test_utils::TestEnv; use near_client::ProcessTxResponse; @@ -18,6 +18,7 @@ use near_primitives::state_part::PartId; use near_primitives::state_sync::StatePartKey; use near_primitives::transaction::SignedTransaction; use near_primitives::types::BlockHeight; +use near_primitives::validator_signer::{EmptyValidatorSigner, InMemoryValidatorSigner}; use near_primitives::views::{QueryRequest, QueryResponseKind}; use near_store::flat::store_helper; use near_store::DBCol; @@ -57,6 +58,10 @@ fn test_state_dump() { credentials_file: None, }); + let validator = MutableConfigValue::new( + Some(Arc::new(EmptyValidatorSigner::new("test0".parse().unwrap()))), + "validator_signer", + ); let mut state_sync_dumper = StateSyncDumper { clock: Clock::real(), client_config: config.clone(), @@ -64,7 +69,7 @@ fn test_state_dump() { epoch_manager: epoch_manager.clone(), shard_tracker, runtime, - account_id: Some("test0".parse().unwrap()), + validator, dump_future_runner: StateSyncDumper::arbiter_dump_future_runner(), handle: None, }; @@ -144,8 +149,11 @@ fn run_state_sync_with_dumped_parts( .nightshade_runtimes(&genesis) .build(); - let signer: Signer = - InMemorySigner::from_seed("test0".parse().unwrap(), KeyType::ED25519, "test0").into(); + let signer = InMemorySigner::from_seed("test0".parse().unwrap(), KeyType::ED25519, "test0"); + let validator = MutableConfigValue::new( + Some(Arc::new(InMemoryValidatorSigner::from_signer(signer.clone()).into())), + "validator_signer", + ); let genesis_block = env.clients[0].chain.get_block_by_height(0).unwrap(); let genesis_hash = *genesis_block.hash(); @@ -169,7 +177,7 @@ fn run_state_sync_with_dumped_parts( epoch_manager: epoch_manager.clone(), shard_tracker, runtime, - account_id: Some("test0".parse().unwrap()), + validator, dump_future_runner: StateSyncDumper::arbiter_dump_future_runner(), handle: None, }; @@ -183,6 +191,7 @@ fn run_state_sync_with_dumped_parts( account_creation_at_epoch_height * epoch_length + 1 }; + let signer: Signer = signer.into(); for i in 1..=dump_node_head_height { if i == account_creation_at_height { let tx = SignedTransaction::create_account( diff --git a/integration-tests/src/tests/genesis_helpers.rs b/integration-tests/src/tests/genesis_helpers.rs index a652752d598..7f109ad33c2 100644 --- a/integration-tests/src/tests/genesis_helpers.rs +++ b/integration-tests/src/tests/genesis_helpers.rs @@ -2,7 +2,7 @@ use near_async::time::Clock; use near_chain::rayon_spawner::RayonAsyncComputationSpawner; use near_chain::types::ChainConfig; use near_chain::{Chain, ChainGenesis, DoomslugThresholdMode}; -use near_chain_configs::Genesis; +use near_chain_configs::{Genesis, MutableConfigValue}; use near_epoch_manager::shard_tracker::ShardTracker; use near_epoch_manager::EpochManager; use near_primitives::block::{Block, BlockHeader}; @@ -38,7 +38,7 @@ fn genesis_header(genesis: &Genesis) -> BlockHeader { ChainConfig::test(), None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); chain.genesis().clone() @@ -64,7 +64,7 @@ pub fn genesis_block(genesis: &Genesis) -> Block { ChainConfig::test(), None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); chain.get_block(&chain.genesis().hash().clone()).unwrap() diff --git a/integration-tests/src/tests/nearcore/stake_nodes.rs b/integration-tests/src/tests/nearcore/stake_nodes.rs index a83153fe2bd..17b5c1742ef 100644 --- a/integration-tests/src/tests/nearcore/stake_nodes.rs +++ b/integration-tests/src/tests/nearcore/stake_nodes.rs @@ -131,7 +131,7 @@ fn test_stake_nodes() { // &*test_nodes[1].config.block_producer.as_ref().unwrap().signer, &(*test_nodes[1].signer).clone().into(), TESTING_INIT_STAKE, - test_nodes[1].config.validator_signer.as_ref().unwrap().public_key(), + test_nodes[1].config.validator_signer.get().unwrap().public_key(), test_nodes[1].genesis_hash, ); actix::spawn( @@ -227,7 +227,7 @@ fn test_validator_kickout() { test_node.account_id.clone(), &*signer, stake, - test_node.config.validator_signer.as_ref().unwrap().public_key(), + test_node.config.validator_signer.get().unwrap().public_key(), test_node.genesis_hash, ) }); @@ -381,7 +381,7 @@ fn test_validator_join() { test_nodes[1].account_id.clone(), &*signer, 0, - test_nodes[1].config.validator_signer.as_ref().unwrap().public_key(), + test_nodes[1].config.validator_signer.get().unwrap().public_key(), test_nodes[1].genesis_hash, ); @@ -398,7 +398,7 @@ fn test_validator_join() { test_nodes[2].account_id.clone(), &*signer, TESTING_INIT_STAKE, - test_nodes[2].config.validator_signer.as_ref().unwrap().public_key(), + test_nodes[2].config.validator_signer.get().unwrap().public_key(), test_nodes[2].genesis_hash, ); diff --git a/integration-tests/src/tests/nearcore/sync_nodes.rs b/integration-tests/src/tests/nearcore/sync_nodes.rs index 82542ede4cd..40a7c9c3131 100644 --- a/integration-tests/src/tests/nearcore/sync_nodes.rs +++ b/integration-tests/src/tests/nearcore/sync_nodes.rs @@ -179,7 +179,7 @@ fn sync_state_stake_change() { "test1".parse().unwrap(), &*signer, TESTING_INIT_STAKE / 2, - near1.validator_signer.unwrap().public_key(), + near1.validator_signer.get().unwrap().public_key(), genesis_hash, ); actix::spawn( diff --git a/integration-tests/src/tests/network/runner.rs b/integration-tests/src/tests/network/runner.rs index b71634551f5..9c7ab737c74 100644 --- a/integration-tests/src/tests/network/runner.rs +++ b/integration-tests/src/tests/network/runner.rs @@ -6,7 +6,7 @@ use near_async::messaging::{noop, IntoMultiSender, IntoSender, LateBoundSender}; use near_async::time::{self, Clock}; use near_chain::types::RuntimeAdapter; use near_chain::{Chain, ChainGenesis}; -use near_chain_configs::{ClientConfig, Genesis, GenesisConfig}; +use near_chain_configs::{ClientConfig, Genesis, GenesisConfig, MutableConfigValue}; use near_chunks::shards_manager_actor::start_shards_manager; use near_client::adapter::client_sender_for_network; use near_client::{start_client, PartialWitnessActor, SyncAdapter, ViewClientActorInner}; @@ -65,7 +65,10 @@ fn setup_network_node( &genesis.config, epoch_manager.clone(), ); - let signer = Arc::new(create_test_signer(account_id.as_str())); + let validator_signer = MutableConfigValue::new( + Some(Arc::new(create_test_signer(account_id.as_str()))), + "validator_signer", + ); let telemetry_actor = ActixWrapper::new(TelemetryActor::new(TelemetryConfig::default())).start(); @@ -100,7 +103,7 @@ fn setup_network_node( state_sync_adapter, network_adapter.as_multi_sender(), shards_manager_adapter.as_sender(), - Some(signer.clone()), + validator_signer.clone(), telemetry_actor.with_auto_span_context().into_sender(), None, None, @@ -113,7 +116,7 @@ fn setup_network_node( .client_actor; let view_client_addr = ViewClientActorInner::spawn_actix_actor( Clock::real(), - config.validator.as_ref().map(|v| v.account_id()), + validator_signer.clone(), chain_genesis, epoch_manager.clone(), shard_tracker.clone(), @@ -127,7 +130,7 @@ fn setup_network_node( shard_tracker, network_adapter.as_sender(), client_actor.clone().with_auto_span_context().into_sender(), - Some(signer.validator_id().clone()), + validator_signer.clone(), runtime.store().clone(), client_config.chunk_request_retry_period, ); @@ -135,7 +138,7 @@ fn setup_network_node( Clock::real(), network_adapter.as_multi_sender(), client_actor.clone().with_auto_span_context().into_multi_sender(), - signer, + validator_signer, epoch_manager, runtime.store().clone(), )); diff --git a/nearcore/src/config.rs b/nearcore/src/config.rs index f8a2abe1e42..e86f0fbfa4b 100644 --- a/nearcore/src/config.rs +++ b/nearcore/src/config.rs @@ -504,7 +504,10 @@ pub struct NearConfig { pub rosetta_rpc_config: Option, pub telemetry_config: TelemetryConfig, pub genesis: Genesis, - pub validator_signer: Option>, + /// Contains validator key for this node. This field is mutable and optional. Use with caution! + /// Lock the value of mutable validator signer for the duration of a request to ensure consistency. + /// Please note that the locked value should not be stored anywhere or passed through the thread boundary. + pub validator_signer: MutableConfigValue>>, } impl NearConfig { @@ -512,7 +515,7 @@ impl NearConfig { config: Config, genesis: Genesis, network_key_pair: KeyFile, - validator_signer: Option>, + validator_signer: MutableConfigValue>>, ) -> anyhow::Result { Ok(NearConfig { config: config.clone(), @@ -618,7 +621,7 @@ impl NearConfig { self.config.write_to_file(&dir.join(CONFIG_FILENAME)).expect("Error writing config"); - if let Some(validator_signer) = &self.validator_signer { + if let Some(validator_signer) = &self.validator_signer.get() { validator_signer .write_to_file(&dir.join(&self.config.validator_key_file)) .expect("Error writing validator key file"); @@ -1221,6 +1224,20 @@ impl From for KeyFile { } } +pub fn load_validator_key(validator_file: &Path) -> anyhow::Result>> { + if !validator_file.exists() { + return Ok(None); + } + match InMemoryValidatorSigner::from_file(&validator_file) { + Ok(signer) => Ok(Some(Arc::new(signer.into()))), + Err(_) => { + let error_message = + format!("Failed initializing validator signer from {}", validator_file.display()); + Err(anyhow!(error_message)) + } + } +} + pub fn load_config( dir: &Path, genesis_validation: GenesisValidationMode, @@ -1234,21 +1251,13 @@ pub fn load_config( validation_errors.push_errors(e) }; - let validator_file = dir.join(&config.validator_key_file); - let validator_signer = if validator_file.exists() { - match InMemoryValidatorSigner::from_file(&validator_file) { - Ok(signer) => Some(Arc::new(signer.into())), - Err(_) => { - let error_message = format!( - "Failed initializing validator signer from {}", - validator_file.display() - ); - validation_errors.push_validator_key_file_error(error_message); - None - } + let validator_file: PathBuf = dir.join(&config.validator_key_file); + let validator_signer = match load_validator_key(&validator_file) { + Ok(validator_signer) => validator_signer, + Err(e) => { + validation_errors.push_validator_key_file_error(e.to_string()); + None } - } else { - None }; let node_key_path = dir.join(&config.node_key_file); @@ -1309,7 +1318,7 @@ pub fn load_config( config, genesis.unwrap(), network_signer.unwrap().into(), - validator_signer, + MutableConfigValue::new(validator_signer, "validator_signer"), )?; Ok(near_config) } @@ -1332,7 +1341,13 @@ pub fn load_test_config(seed: &str, addr: tcp::ListenerAddr, genesis: Genesis) - let validator_signer = Arc::new(create_test_signer(seed)) as Arc; (signer, Some(validator_signer)) }; - NearConfig::new(config, genesis, signer.into(), validator_signer).unwrap() + NearConfig::new( + config, + genesis, + signer.into(), + MutableConfigValue::new(validator_signer, "validator_signer"), + ) + .unwrap() } #[cfg(test)] diff --git a/nearcore/src/dyn_config.rs b/nearcore/src/dyn_config.rs index b02e24a3b02..0daf5ed1a94 100644 --- a/nearcore/src/dyn_config.rs +++ b/nearcore/src/dyn_config.rs @@ -19,19 +19,18 @@ pub fn read_updateable_configs( None } }; - let updateable_client_config = - match Config::from_file(&home_dir.join(crate::config::CONFIG_FILENAME)) - .map(get_updateable_client_config) - { - Ok(config) => Some(config), - Err(err) => { - errs.push(UpdateableConfigLoaderError::ConfigFileError { - file: PathBuf::from(crate::config::CONFIG_FILENAME), - err: err.into(), - }); - None - } - }; + let config = match Config::from_file(&home_dir.join(crate::config::CONFIG_FILENAME)) { + Ok(config) => Some(config), + Err(err) => { + errs.push(UpdateableConfigLoaderError::ConfigFileError { + file: PathBuf::from(crate::config::CONFIG_FILENAME), + err: err.into(), + }); + None + } + }; + let updateable_client_config = config.as_ref().map(get_updateable_client_config); + if errs.is_empty() { crate::metrics::CONFIG_CORRECT.set(1); Ok(UpdateableConfigs { log_config, client_config: updateable_client_config }) @@ -42,7 +41,7 @@ pub fn read_updateable_configs( } } -pub fn get_updateable_client_config(config: Config) -> UpdateableClientConfig { +pub fn get_updateable_client_config(config: &Config) -> UpdateableClientConfig { // All fields that can be updated while the node is running should be explicitly set here. // Keep this list in-sync with `core/dyn-configs/README.md`. UpdateableClientConfig { diff --git a/nearcore/src/lib.rs b/nearcore/src/lib.rs index 266b94482e5..00879b25c26 100644 --- a/nearcore/src/lib.rs +++ b/nearcore/src/lib.rs @@ -11,7 +11,7 @@ use anyhow::Context; use cold_storage::ColdStoreLoopHandle; use near_async::actix::AddrWithAutoSpanContextExt; use near_async::actix_wrapper::{spawn_actix_actor, ActixWrapper}; -use near_async::messaging::{noop, IntoMultiSender, IntoSender, LateBoundSender}; +use near_async::messaging::{IntoMultiSender, IntoSender, LateBoundSender}; use near_async::time::{self, Clock}; pub use near_chain::runtime::NightshadeRuntime; use near_chain::state_snapshot_actor::{ @@ -331,7 +331,7 @@ pub fn start_with_config_and_synchronization( let view_client_addr = ViewClientActorInner::spawn_actix_actor( Clock::real(), - config.validator_signer.as_ref().map(|signer| signer.validator_id().clone()), + config.validator_signer.clone(), chain_genesis.clone(), view_epoch_manager.clone(), view_shard_tracker, @@ -360,21 +360,15 @@ pub fn start_with_config_and_synchronization( ); let snapshot_callbacks = SnapshotCallbacks { make_snapshot_callback, delete_snapshot_callback }; - let (partial_witness_actor, partial_witness_arbiter) = if config.validator_signer.is_some() { - let my_signer = config.validator_signer.clone().unwrap(); - let (partial_witness_actor, partial_witness_arbiter) = - spawn_actix_actor(PartialWitnessActor::new( - Clock::real(), - network_adapter.as_multi_sender(), - client_adapter_for_partial_witness_actor.as_multi_sender(), - my_signer, - epoch_manager.clone(), - storage.get_hot_store(), - )); - (Some(partial_witness_actor), Some(partial_witness_arbiter)) - } else { - (None, None) - }; + let (partial_witness_actor, partial_witness_arbiter) = + spawn_actix_actor(PartialWitnessActor::new( + Clock::real(), + network_adapter.as_multi_sender(), + client_adapter_for_partial_witness_actor.as_multi_sender(), + config.validator_signer.clone(), + epoch_manager.clone(), + storage.get_hot_store(), + )); let (_gc_actor, gc_arbiter) = spawn_actix_actor(GCActor::new( runtime.store().clone(), @@ -402,10 +396,7 @@ pub fn start_with_config_and_synchronization( shutdown_signal, adv, config_updater, - partial_witness_actor - .clone() - .map(|actor| actor.with_auto_span_context().into_multi_sender()) - .unwrap_or_else(|| noop().into_multi_sender()), + partial_witness_actor.clone().with_auto_span_context().into_multi_sender(), true, None, ); @@ -419,7 +410,7 @@ pub fn start_with_config_and_synchronization( shard_tracker.clone(), network_adapter.as_sender(), client_adapter_for_shards_manager.as_sender(), - config.validator_signer.as_ref().map(|signer| signer.validator_id().clone()), + config.validator_signer.clone(), split_store.unwrap_or_else(|| storage.get_hot_store()), config.client_config.chunk_request_retry_period, ); @@ -439,7 +430,7 @@ pub fn start_with_config_and_synchronization( epoch_manager, shard_tracker, runtime, - account_id: config.validator_signer.map(|signer| signer.validator_id().clone()), + validator: config.validator_signer.clone(), dump_future_runner: StateSyncDumper::arbiter_dump_future_runner(), handle: None, }; @@ -454,9 +445,7 @@ pub fn start_with_config_and_synchronization( config.network_config, client_sender_for_network(client_actor.clone(), view_client_addr.clone()), shards_manager_adapter.as_sender(), - partial_witness_actor - .map(|actor| actor.with_auto_span_context().into_multi_sender()) - .unwrap_or_else(|| noop().into_multi_sender()), + partial_witness_actor.with_auto_span_context().into_multi_sender(), genesis_id, ) .context("PeerManager::spawn()")?; @@ -507,13 +496,11 @@ pub fn start_with_config_and_synchronization( trie_metrics_arbiter, state_snapshot_arbiter, gc_arbiter, + partial_witness_arbiter, ]; if let Some(db_metrics_arbiter) = db_metrics_arbiter { arbiters.push(db_metrics_arbiter); } - if let Some(partial_witness_arbiter) = partial_witness_arbiter { - arbiters.push(partial_witness_arbiter); - } Ok(NearNode { client: client_actor, diff --git a/nearcore/src/state_sync.rs b/nearcore/src/state_sync.rs index df96c111699..085e5a1734a 100644 --- a/nearcore/src/state_sync.rs +++ b/nearcore/src/state_sync.rs @@ -7,7 +7,7 @@ use futures::FutureExt; use near_async::time::{Clock, Duration, Instant}; use near_chain::types::RuntimeAdapter; use near_chain::{Chain, ChainGenesis, ChainStoreAccess, DoomslugThresholdMode, Error}; -use near_chain_configs::{ClientConfig, ExternalStorageLocation}; +use near_chain_configs::{ClientConfig, ExternalStorageLocation, MutableConfigValue}; use near_client::sync::external::{ create_bucket_readwrite, external_storage_location, StateFileType, }; @@ -22,6 +22,7 @@ use near_primitives::hash::CryptoHash; use near_primitives::state_part::PartId; use near_primitives::state_sync::{StatePartKey, StateSyncDumpProgress}; use near_primitives::types::{AccountId, EpochHeight, EpochId, ShardId, StateRoot}; +use near_primitives::validator_signer::ValidatorSigner; use near_store::DBCol; use rand::{thread_rng, Rng}; use std::collections::HashSet; @@ -35,7 +36,10 @@ pub struct StateSyncDumper { pub epoch_manager: Arc, pub shard_tracker: ShardTracker, pub runtime: Arc, - pub account_id: Option, + /// Contains validator key for this node. This field is mutable and optional. Use with caution! + /// Lock the value of mutable validator signer for the duration of a request to ensure consistency. + /// Please note that the locked value should not be stored anywhere or passed through the thread boundary. + pub validator: MutableConfigValue>>, pub dump_future_runner: Box) -> Box>, pub handle: Option, } @@ -125,7 +129,7 @@ impl StateSyncDumper { dump_config.restart_dump_for_shards.clone().unwrap_or_default(), external.clone(), dump_config.iteration_delay.unwrap_or(Duration::seconds(10)), - self.account_id.clone(), + self.validator.clone(), keep_running.clone(), ) .boxed(), @@ -334,7 +338,7 @@ async fn state_sync_dump( restart_dump_for_shards: Vec, external: ExternalConnection, iteration_delay: Duration, - account_id: Option, + validator: MutableConfigValue>>, keep_running: Arc, ) { tracing::info!(target: "state_sync_dump", shard_id, "Running StateSyncDump loop"); @@ -348,6 +352,7 @@ async fn state_sync_dump( // Note that without this check the state dumping thread is unstoppable, i.e. non-interruptable. while keep_running.load(std::sync::atomic::Ordering::Relaxed) { tracing::debug!(target: "state_sync_dump", shard_id, "Running StateSyncDump loop iteration"); + let account_id = validator.get().map(|v| v.validator_id().clone()); let current_state = get_current_state( &chain, &shard_id, diff --git a/neard/src/cli.rs b/neard/src/cli.rs index 2d43c6bc635..7aef39f7260 100644 --- a/neard/src/cli.rs +++ b/neard/src/cli.rs @@ -537,11 +537,7 @@ impl RunCmd { o11y_opts, near_config.client_config.chain_id.clone(), near_config.network_config.node_key.public_key().clone(), - near_config - .network_config - .validator - .as_ref() - .map(|validator| validator.account_id()), + near_config.network_config.validator.account_id(), ) .await .global(); diff --git a/test-utils/store-validator/src/main.rs b/test-utils/store-validator/src/main.rs index 14a60da94b9..326c446c729 100644 --- a/test-utils/store-validator/src/main.rs +++ b/test-utils/store-validator/src/main.rs @@ -52,7 +52,7 @@ fn main() { ) .expect("could not create transaction runtime"); let mut store_validator = StoreValidator::new( - near_config.validator_signer.as_ref().map(|x| x.validator_id().clone()), + near_config.validator_signer.get().map(|x| x.validator_id().clone()), near_config.genesis.config, epoch_manager, shard_tracker, diff --git a/tools/chainsync-loadtest/src/main.rs b/tools/chainsync-loadtest/src/main.rs index 03490f94928..c47e642767a 100644 --- a/tools/chainsync-loadtest/src/main.rs +++ b/tools/chainsync-loadtest/src/main.rs @@ -10,6 +10,7 @@ use near_async::messaging::IntoSender; use near_async::messaging::LateBoundSender; use near_async::time; use near_chain_configs::Genesis; +use near_chain_configs::MutableConfigValue; use near_network::concurrency::ctx; use near_network::concurrency::scope; use near_network::PeerManagerActor; @@ -69,7 +70,8 @@ fn download_configs(chain_id: &str, dir: &std::path::Path) -> anyhow::Result anyhow::Result<()> { let home_dir = Path::new(&args.chain_history_home_dir); let mut near_config = nearcore::config::load_config(home_dir, GenesisValidationMode::Full) .context("Error loading config")?; - near_config.validator_signer = None; + near_config.validator_signer = MutableConfigValue::new(None, "validator_signer"); near_config.client_config.min_num_peers = 1; let signer = InMemorySigner::from_random("mock_node".parse().unwrap(), KeyType::ED25519); near_config.network_config.node_key = signer.secret_key; diff --git a/tools/speedy_sync/src/main.rs b/tools/speedy_sync/src/main.rs index 7a872637fa9..d1320a5b6e9 100644 --- a/tools/speedy_sync/src/main.rs +++ b/tools/speedy_sync/src/main.rs @@ -255,7 +255,7 @@ fn load_snapshot(load_cmd: LoadCmd) { }, None, Arc::new(RayonAsyncComputationSpawner), - None, + MutableConfigValue::new(None, "validator_signer"), ) .unwrap(); diff --git a/tools/state-viewer/src/commands.rs b/tools/state-viewer/src/commands.rs index fb6f54c70f6..288f3e65019 100644 --- a/tools/state-viewer/src/commands.rs +++ b/tools/state-viewer/src/commands.rs @@ -1463,7 +1463,7 @@ impl std::fmt::Debug for StateStatsAccount { #[cfg(test)] mod tests { use near_chain::types::RuntimeAdapter; - use near_chain_configs::Genesis; + use near_chain_configs::{Genesis, MutableConfigValue}; use near_client::test_utils::TestEnv; use near_crypto::{InMemorySigner, KeyFile, KeyType}; use near_epoch_manager::EpochManager; @@ -1529,8 +1529,13 @@ mod tests { // Check that `send_money()` actually changed state. assert_ne!(chunk_extras[0].state_root(), chunk_extras[1].state_root()); - let near_config = - NearConfig::new(Config::default(), genesis, KeyFile::from(&signer), None).unwrap(); + let near_config = NearConfig::new( + Config::default(), + genesis, + KeyFile::from(&signer), + MutableConfigValue::new(None, "validator_signer"), + ) + .unwrap(); let (_epoch_manager, _runtime, state_roots, block_header) = crate::commands::load_trie(store, home_dir, &near_config); assert_eq!(&state_roots[0], chunk_extras[1].state_root()); diff --git a/tools/state-viewer/src/state_dump.rs b/tools/state-viewer/src/state_dump.rs index 9df1dda58a9..7612702f4b4 100644 --- a/tools/state-viewer/src/state_dump.rs +++ b/tools/state-viewer/src/state_dump.rs @@ -297,7 +297,7 @@ mod test { use near_chain::{ChainStoreAccess, Provenance}; use near_chain_configs::genesis_validate::validate_genesis; use near_chain_configs::test_utils::TESTING_INIT_STAKE; - use near_chain_configs::{Genesis, GenesisChangeConfig}; + use near_chain_configs::{Genesis, GenesisChangeConfig, MutableConfigValue}; use near_client::test_utils::TestEnv; use near_client::ProcessTxResponse; use near_crypto::{InMemorySigner, KeyFile, KeyType, PublicKey, SecretKey}; @@ -356,10 +356,13 @@ mod test { public_key: PublicKey::empty(KeyType::ED25519), secret_key: SecretKey::from_random(KeyType::ED25519), }, - Some(Arc::new( - InMemoryValidatorSigner::from_random("test".parse().unwrap(), KeyType::ED25519) - .into(), - )), + MutableConfigValue::new( + Some(Arc::new( + InMemoryValidatorSigner::from_random("test".parse().unwrap(), KeyType::ED25519) + .into(), + )), + "validator_signer", + ), ) .unwrap(); @@ -747,10 +750,13 @@ mod test { public_key: PublicKey::empty(KeyType::ED25519), secret_key: SecretKey::from_random(KeyType::ED25519), }, - Some(Arc::new( - InMemoryValidatorSigner::from_random("test".parse().unwrap(), KeyType::ED25519) - .into(), - )), + MutableConfigValue::new( + Some(Arc::new( + InMemoryValidatorSigner::from_random("test".parse().unwrap(), KeyType::ED25519) + .into(), + )), + "validator_signer", + ), ) .unwrap(); @@ -816,10 +822,13 @@ mod test { public_key: PublicKey::empty(KeyType::ED25519), secret_key: SecretKey::from_random(KeyType::ED25519), }, - Some(Arc::new( - InMemoryValidatorSigner::from_random("test".parse().unwrap(), KeyType::ED25519) - .into(), - )), + MutableConfigValue::new( + Some(Arc::new( + InMemoryValidatorSigner::from_random("test".parse().unwrap(), KeyType::ED25519) + .into(), + )), + "validator_signer", + ), ) .unwrap(); let head = env.clients[0].chain.head().unwrap();