From 655e3bc41828e1db9b9ff848dd076d8f145f13c0 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Tue, 28 Apr 2020 11:20:43 -0600 Subject: [PATCH] Rpc: Use cluster largest_confirmed_root as commitment max (#9750) automerge --- core/src/commitment.rs | 5 ++ core/src/rpc.rs | 102 +++++++++++++++++++++++++++-------------- 2 files changed, 72 insertions(+), 35 deletions(-) diff --git a/core/src/commitment.rs b/core/src/commitment.rs index 3c940208cb..ad4062d78e 100644 --- a/core/src/commitment.rs +++ b/core/src/commitment.rs @@ -161,6 +161,11 @@ impl BlockCommitmentCache { root: Slot::default(), } } + + #[cfg(test)] + pub(crate) fn set_get_largest_confirmed_root(&mut self, root: Slot) { + self.largest_confirmed_root = root; + } } pub struct CommitmentAggregationData { diff --git a/core/src/rpc.rs b/core/src/rpc.rs index 2f1d0a73d5..03538d2e34 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -8,7 +8,7 @@ use crate::{ validator::ValidatorExit, }; use bincode::serialize; -use jsonrpc_core::{Error, Metadata, Result}; +use jsonrpc_core::{Error, ErrorCode, Metadata, Result}; use jsonrpc_derive::rpc; use solana_client::{ rpc_request::{ @@ -45,6 +45,8 @@ use std::{ time::{Duration, Instant}, }; +const JSON_RPC_SERVER_ERROR_0: i64 = -32000; + type RpcResponse = Result>; fn new_response(bank: &Bank, value: T) -> RpcResponse { @@ -78,17 +80,32 @@ pub struct JsonRpcRequestProcessor { } impl JsonRpcRequestProcessor { - fn bank(&self, commitment: Option) -> Arc { + fn bank(&self, commitment: Option) -> Result> { debug!("RPC commitment_config: {:?}", commitment); let r_bank_forks = self.bank_forks.read().unwrap(); if commitment.is_some() && commitment.unwrap().commitment == CommitmentLevel::Recent { let bank = r_bank_forks.working_bank(); debug!("RPC using working_bank: {:?}", bank.slot()); - bank + Ok(bank) } else { - let slot = r_bank_forks.root(); - debug!("RPC using block: {:?}", slot); - r_bank_forks.get(slot).cloned().unwrap() + let cluster_root = self + .block_commitment_cache + .read() + .unwrap() + .largest_confirmed_root(); + debug!("RPC using block: {:?}", cluster_root); + r_bank_forks + .get(cluster_root) + .cloned() + .ok_or_else(|| Error { + code: ErrorCode::ServerError(JSON_RPC_SERVER_ERROR_0), + message: format!( + "Cluster largest_confirmed_root {} does not exist on node. Node root: {}", + cluster_root, + r_bank_forks.root(), + ), + data: None, + }) } } @@ -115,7 +132,7 @@ impl JsonRpcRequestProcessor { pubkey: Result, commitment: Option, ) -> RpcResponse> { - let bank = &*self.bank(commitment); + let bank = &*self.bank(commitment)?; pubkey.and_then(|key| new_response(bank, bank.get_account(&key).map(RpcAccount::encode))) } @@ -125,7 +142,7 @@ impl JsonRpcRequestProcessor { commitment: Option, ) -> Result { Ok(self - .bank(commitment) + .bank(commitment)? .get_minimum_balance_for_rent_exemption(data_len)) } @@ -135,7 +152,7 @@ impl JsonRpcRequestProcessor { commitment: Option, ) -> Result> { Ok(self - .bank(commitment) + .bank(commitment)? .get_program_accounts(Some(&program_id)) .into_iter() .map(|(pubkey, account)| RpcKeyedAccount { @@ -146,13 +163,13 @@ impl JsonRpcRequestProcessor { } pub fn get_inflation(&self, commitment: Option) -> Result { - Ok(self.bank(commitment).inflation()) + Ok(self.bank(commitment)?.inflation()) } pub fn get_epoch_schedule(&self) -> Result { // Since epoch schedule data comes from the genesis config, any commitment level should be // fine - Ok(*self.bank(None).epoch_schedule()) + Ok(*self.bank(None)?.epoch_schedule()) } pub fn get_balance( @@ -160,7 +177,7 @@ impl JsonRpcRequestProcessor { pubkey: Result, commitment: Option, ) -> RpcResponse { - let bank = &*self.bank(commitment); + let bank = &*self.bank(commitment)?; pubkey.and_then(|key| new_response(bank, bank.get_balance(&key))) } @@ -168,7 +185,7 @@ impl JsonRpcRequestProcessor { &self, commitment: Option, ) -> RpcResponse { - let bank = &*self.bank(commitment); + let bank = &*self.bank(commitment)?; let (blockhash, fee_calculator) = bank.confirmed_last_blockhash(); new_response( bank, @@ -183,7 +200,7 @@ impl JsonRpcRequestProcessor { &self, blockhash: &Hash, ) -> RpcResponse> { - let bank = &*self.bank(None); + let bank = &*self.bank(None)?; let fee_calculator = bank.get_fee_calculator(blockhash); new_response( bank, @@ -192,7 +209,7 @@ impl JsonRpcRequestProcessor { } fn get_fee_rate_governor(&self) -> RpcResponse { - let bank = &*self.bank(None); + let bank = &*self.bank(None)?; let fee_rate_governor = bank.get_fee_rate_governor(); new_response( bank, @@ -207,7 +224,7 @@ impl JsonRpcRequestProcessor { signature: Result, commitment: Option, ) -> RpcResponse { - let bank = &*self.bank(commitment); + let bank = &*self.bank(commitment)?; match signature { Err(e) => Err(e), Ok(sig) => { @@ -231,11 +248,11 @@ impl JsonRpcRequestProcessor { } fn get_slot(&self, commitment: Option) -> Result { - Ok(self.bank(commitment).slot()) + Ok(self.bank(commitment)?.slot()) } fn get_slot_leader(&self, commitment: Option) -> Result { - Ok(self.bank(commitment).collector_id().to_string()) + Ok(self.bank(commitment)?.collector_id().to_string()) } fn minimum_ledger_slot(&self) -> Result { @@ -252,18 +269,18 @@ impl JsonRpcRequestProcessor { } fn get_transaction_count(&self, commitment: Option) -> Result { - Ok(self.bank(commitment).transaction_count() as u64) + Ok(self.bank(commitment)?.transaction_count() as u64) } fn get_total_supply(&self, commitment: Option) -> Result { - Ok(self.bank(commitment).capitalization()) + Ok(self.bank(commitment)?.capitalization()) } fn get_vote_accounts( &self, commitment: Option, ) -> Result { - let bank = self.bank(commitment); + let bank = self.bank(commitment)?; let vote_accounts = bank.vote_accounts(); let epoch_vote_accounts = bank .epoch_vote_accounts(bank.get_epoch_and_slot_index(bank.slot()).0) @@ -325,7 +342,7 @@ impl JsonRpcRequestProcessor { } fn get_slots_per_segment(&self, commitment: Option) -> Result { - Ok(self.bank(commitment).slots_per_segment()) + Ok(self.bank(commitment)?.slots_per_segment()) } fn get_storage_pubkeys_for_slot(&self, slot: Slot) -> Result> { @@ -375,7 +392,11 @@ impl JsonRpcRequestProcessor { start_slot: Slot, end_slot: Option, ) -> Result> { - let end_slot = end_slot.unwrap_or_else(|| self.bank(None).slot()); + let end_slot = if let Some(end_slot) = end_slot { + end_slot + } else { + self.bank(None)?.slot() + }; if end_slot < start_slot { return Ok(vec![]); } @@ -393,7 +414,7 @@ impl JsonRpcRequestProcessor { // queried). If these values will be variable in the future, those timing parameters will // need to be stored persistently, and the slot_duration calculation will likely need to be // moved upstream into blockstore. Also, an explicit commitment level will need to be set. - let bank = self.bank(None); + let bank = self.bank(None)?; let slot_duration = slot_duration_from_slots_per_year(bank.slots_per_year()); let epoch = bank.epoch_schedule().get_epoch(slot); let stakes = HashMap::new(); @@ -411,7 +432,7 @@ impl JsonRpcRequestProcessor { signature: Signature, commitment: Option, ) -> Option { - self.get_transaction_status(signature, &self.bank(commitment)) + self.get_transaction_status(signature, &self.bank(commitment).ok()?) .map( |TransactionStatus { status, @@ -430,6 +451,7 @@ impl JsonRpcRequestProcessor { commitment: Option, ) -> Option> { self.bank(commitment) + .ok()? .get_signature_status_slot(&signature) .map(|(_, status)| status) } @@ -444,7 +466,7 @@ impl JsonRpcRequestProcessor { let search_transaction_history = config .map(|x| x.search_transaction_history) .unwrap_or(false); - let bank = self.bank(Some(CommitmentConfig::recent())); + let bank = self.bank(Some(CommitmentConfig::recent()))?; for signature in signatures { let status = if let Some(status) = self.get_transaction_status(signature, &bank) { @@ -939,7 +961,7 @@ impl RpcSol for RpcSolImpl { meta: Self::Metadata, commitment: Option, ) -> Result { - let bank = meta.request_processor.read().unwrap().bank(commitment); + let bank = meta.request_processor.read().unwrap().bank(commitment)?; let epoch_schedule = bank.epoch_schedule(); let slot = bank.slot(); @@ -975,7 +997,7 @@ impl RpcSol for RpcSolImpl { slot: Option, commitment: Option, ) -> Result> { - let bank = meta.request_processor.read().unwrap().bank(commitment); + let bank = meta.request_processor.read().unwrap().bank(commitment)?; let slot = slot.unwrap_or_else(|| bank.slot()); let epoch = bank.epoch_schedule().get_epoch(slot); @@ -1139,7 +1161,7 @@ impl RpcSol for RpcSolImpl { .request_processor .read() .unwrap() - .bank(commitment.clone()) + .bank(commitment.clone())? .confirmed_last_blockhash() .0; let transaction = request_airdrop_transaction(&faucet_addr, &pubkey, lamports, blockhash) @@ -1522,15 +1544,13 @@ pub mod tests { let mut roots = blockstore_roots.clone(); if !roots.is_empty() { - roots.retain(|&x| x > 1); + roots.retain(|&x| x > 0); let mut parent_bank = bank; for (i, root) in roots.iter().enumerate() { let new_bank = Bank::new_from_parent(&parent_bank, parent_bank.collector_id(), *root); parent_bank = bank_forks.write().unwrap().insert(new_bank); - parent_bank.squash(); - bank_forks.write().unwrap().set_root(*root, &None, None); - let parent = if i > 0 { roots[i - 1] } else { 1 }; + let parent = if i > 0 { roots[i - 1] } else { 0 }; fill_blockstore_slot_with_ticks(&blockstore, 5, *root, parent, Hash::default()); } blockstore.set_roots(&roots).unwrap(); @@ -1540,6 +1560,10 @@ pub mod tests { roots.iter().max().unwrap() + 1, ); bank_forks.write().unwrap().insert(new_bank); + + for root in roots.iter() { + bank_forks.write().unwrap().set_root(*root, &None, Some(0)); + } } let bank = bank_forks.read().unwrap().working_bank(); @@ -2649,8 +2673,16 @@ pub mod tests { fn test_get_confirmed_blocks() { let bob_pubkey = Pubkey::new_rand(); let roots = vec![0, 1, 3, 4, 8]; - let RpcHandler { io, meta, .. } = - start_rpc_handler_with_tx_and_blockstore(&bob_pubkey, roots.clone(), 0); + let RpcHandler { + io, + meta, + block_commitment_cache, + .. + } = start_rpc_handler_with_tx_and_blockstore(&bob_pubkey, roots.clone(), 0); + block_commitment_cache + .write() + .unwrap() + .set_get_largest_confirmed_root(8); let req = format!(r#"{{"jsonrpc":"2.0","id":1,"method":"getConfirmedBlocks","params":[0]}}"#);