From f62fba16176a99a0987720a750ce5a46e67caff3 Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Tue, 5 Mar 2019 21:12:30 -0800 Subject: [PATCH 1/4] Make room for more fields in JsonRpcConfig --- core/src/local_cluster.rs | 3 +-- core/src/rpc.rs | 46 ++++++++++++++------------------------- fullnode/src/main.rs | 2 +- tests/local_cluster.rs | 19 ++++++++-------- 4 files changed, 27 insertions(+), 43 deletions(-) diff --git a/core/src/local_cluster.rs b/core/src/local_cluster.rs index d297ab4a42328f..ebd2cfbfeaa7d1 100644 --- a/core/src/local_cluster.rs +++ b/core/src/local_cluster.rs @@ -211,7 +211,6 @@ impl Drop for LocalCluster { #[cfg(test)] mod test { use super::*; - use crate::rpc::JsonRpcConfig; #[test] fn test_local_cluster_start_and_exit() { @@ -224,7 +223,7 @@ mod test { fn test_local_cluster_start_and_exit_with_config() { solana_logger::setup(); let mut fullnode_exit = FullnodeConfig::default(); - fullnode_exit.rpc_config = JsonRpcConfig::TestOnlyAllowRpcFullnodeExit; + fullnode_exit.rpc_config.enable_fullnode_exit = true; let cluster = LocalCluster::new_with_config(1, 100, 3, &fullnode_exit); drop(cluster) } diff --git a/core/src/rpc.rs b/core/src/rpc.rs index da527541329cf1..454658d70fd0b8 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -22,14 +22,15 @@ use std::thread::sleep; use std::time::{Duration, Instant}; #[derive(Clone)] -pub enum JsonRpcConfig { - DefaultConfig, - TestOnlyAllowRpcFullnodeExit, +pub struct JsonRpcConfig { + pub enable_fullnode_exit: bool, // Enable the 'fullnodeExit' command } impl Default for JsonRpcConfig { fn default() -> Self { - JsonRpcConfig::DefaultConfig + Self { + enable_fullnode_exit: false, + } } } @@ -110,16 +111,13 @@ impl JsonRpcRequestProcessor { } pub fn fullnode_exit(&self) -> Result { - match self.config { - JsonRpcConfig::DefaultConfig => { - debug!("default config, fullnode_exit ignored"); - Ok(false) - } - JsonRpcConfig::TestOnlyAllowRpcFullnodeExit => { - warn!("TEST_ONLY JsonRPC fullnode_exit request..."); - self.fullnode_exit.store(true, Ordering::Relaxed); - Ok(true) - } + if self.config.enable_fullnode_exit { + warn!("fullnode_exit request..."); + self.fullnode_exit.store(true, Ordering::Relaxed); + Ok(true) + } else { + debug!("fullnode_exit ignored"); + Ok(false) } } } @@ -714,26 +712,14 @@ mod tests { assert_eq!(request_processor.fullnode_exit(), Ok(false)); assert_eq!(exit.load(Ordering::Relaxed), false); } - #[test] - fn test_rpc_request_processor_default_config_fullnode_exit_fails() { - let exit = Arc::new(AtomicBool::new(false)); - let request_processor = JsonRpcRequestProcessor::new( - StorageState::default(), - JsonRpcConfig::DefaultConfig, - &exit, - ); - assert_eq!(request_processor.fullnode_exit(), Ok(false)); - assert_eq!(exit.load(Ordering::Relaxed), false); - } #[test] fn test_rpc_request_processor_allow_fullnode_exit_config() { let exit = Arc::new(AtomicBool::new(false)); - let request_processor = JsonRpcRequestProcessor::new( - StorageState::default(), - JsonRpcConfig::TestOnlyAllowRpcFullnodeExit, - &exit, - ); + let mut config = JsonRpcConfig::default(); + config.enable_fullnode_exit = true; + let request_processor = + JsonRpcRequestProcessor::new(StorageState::default(), config, &exit); assert_eq!(request_processor.fullnode_exit(), Ok(true)); assert_eq!(exit.load(Ordering::Relaxed), true); } diff --git a/fullnode/src/main.rs b/fullnode/src/main.rs index 533505f513555c..c99ee969266528 100644 --- a/fullnode/src/main.rs +++ b/fullnode/src/main.rs @@ -217,7 +217,7 @@ fn main() { let use_only_bootstrap_leader = matches.is_present("no_leader_rotation"); if matches.is_present("enable_rpc_exit") { - fullnode_config.rpc_config = solana::rpc::JsonRpcConfig::TestOnlyAllowRpcFullnodeExit; + fullnode_config.rpc_config.enable_fullnode_exit = true; } let gossip_addr = { diff --git a/tests/local_cluster.rs b/tests/local_cluster.rs index 0f13bface268ac..807021a87381c5 100644 --- a/tests/local_cluster.rs +++ b/tests/local_cluster.rs @@ -3,7 +3,6 @@ extern crate solana; use solana::cluster_tests; use solana::fullnode::FullnodeConfig; use solana::local_cluster::LocalCluster; -use solana::rpc::JsonRpcConfig; #[test] fn test_spend_and_verify_all_nodes_1() { @@ -54,18 +53,18 @@ fn test_fullnode_exit_default_config_should_panic() { fn test_fullnode_exit_2() { solana_logger::setup(); let num_nodes = 2; - let mut fullnode_exit = FullnodeConfig::default(); - fullnode_exit.rpc_config = JsonRpcConfig::TestOnlyAllowRpcFullnodeExit; - let local = LocalCluster::new_with_config(num_nodes, 10_000, 100, &fullnode_exit); + let mut fullnode_config = FullnodeConfig::default(); + fullnode_config.rpc_config.enable_fullnode_exit = true; + let local = LocalCluster::new_with_config(num_nodes, 10_000, 100, &fullnode_config); cluster_tests::fullnode_exit(&local.entry_point_info, num_nodes); } #[test] fn test_leader_failure_2() { let num_nodes = 2; - let mut fullnode_exit = FullnodeConfig::default(); - fullnode_exit.rpc_config = JsonRpcConfig::TestOnlyAllowRpcFullnodeExit; - let local = LocalCluster::new_with_config(num_nodes, 10_000, 100, &fullnode_exit); + let mut fullnode_config = FullnodeConfig::default(); + fullnode_config.rpc_config.enable_fullnode_exit = true; + let local = LocalCluster::new_with_config(num_nodes, 10_000, 100, &fullnode_config); cluster_tests::kill_entry_and_spend_and_verify_rest( &local.entry_point_info, &local.funding_keypair, @@ -76,9 +75,9 @@ fn test_leader_failure_2() { #[test] fn test_leader_failure_3() { let num_nodes = 3; - let mut fullnode_exit = FullnodeConfig::default(); - fullnode_exit.rpc_config = JsonRpcConfig::TestOnlyAllowRpcFullnodeExit; - let local = LocalCluster::new_with_config(num_nodes, 10_000, 100, &fullnode_exit); + let mut fullnode_config = FullnodeConfig::default(); + fullnode_config.rpc_config.enable_fullnode_exit = true; + let local = LocalCluster::new_with_config(num_nodes, 10_000, 100, &fullnode_config); cluster_tests::kill_entry_and_spend_and_verify_rest( &local.entry_point_info, &local.funding_keypair, From b40b275973a76163a6585a83c4dede8dc2f324ea Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Wed, 6 Mar 2019 08:14:39 -0800 Subject: [PATCH 2/4] Remove dead code --- core/src/rpc.rs | 4 ---- core/src/rpc_service.rs | 1 - 2 files changed, 5 deletions(-) diff --git a/core/src/rpc.rs b/core/src/rpc.rs index 454658d70fd0b8..b81a338fd44e11 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -170,7 +170,6 @@ fn verify_signature(input: &str) -> Result { pub struct Meta { pub request_processor: Arc>, pub cluster_info: Arc>, - pub rpc_addr: SocketAddr, pub drone_addr: SocketAddr, } impl Metadata for Meta {} @@ -436,7 +435,6 @@ mod tests { cluster_info.write().unwrap().insert_info(leader.clone()); cluster_info.write().unwrap().set_leader(leader.id); - let rpc_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 0); let drone_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 0); let mut io = MetaIoHandler::default(); @@ -446,7 +444,6 @@ mod tests { request_processor, cluster_info, drone_addr, - rpc_addr, }; (io, meta, blockhash, alice) } @@ -638,7 +635,6 @@ mod tests { }, cluster_info: Arc::new(RwLock::new(ClusterInfo::new(NodeInfo::default()))), drone_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 0), - rpc_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 0), }; let req = diff --git a/core/src/rpc_service.rs b/core/src/rpc_service.rs index 1f3d34eda45f60..0651defe0a25c6 100644 --- a/core/src/rpc_service.rs +++ b/core/src/rpc_service.rs @@ -52,7 +52,6 @@ impl JsonRpcService { request_processor: request_processor_.clone(), cluster_info: info.clone(), drone_addr, - rpc_addr, }).threads(4) .cors(DomainsValidation::AllowOnly(vec![ AccessControlAllowOrigin::Any, From f5dc19f5420254b958cba345c2c32b9c73d7825a Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Wed, 6 Mar 2019 08:28:18 -0800 Subject: [PATCH 3/4] Don't fetch the working_bank twice --- core/src/fullnode.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/core/src/fullnode.rs b/core/src/fullnode.rs index eb8fca5ed209dc..583c440c99727f 100644 --- a/core/src/fullnode.rs +++ b/core/src/fullnode.rs @@ -237,10 +237,7 @@ impl Fullnode { } let bank = bank_forks_.read().unwrap().working_bank(); trace!("rpc working bank {} {}", bank.slot(), bank.last_blockhash()); - rpc_service_rp - .write() - .unwrap() - .set_bank(&bank_forks_.read().unwrap().working_bank()); + rpc_service_rp.write().unwrap().set_bank(&bank); let timer = Duration::from_millis(100); sleep(timer); }); From e29ec9aad73ecf489e2ad2485c5d07826884d03d Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Wed, 6 Mar 2019 09:26:12 -0800 Subject: [PATCH 4/4] requestAirdrop RPC API is now optional --- core/src/fullnode.rs | 14 ------------ core/src/rpc.rs | 34 +++++++++++++++++------------- core/src/rpc_service.rs | 8 +------ fullnode/src/main.rs | 21 ++++++++++++------ multinode-demo/bootstrap-leader.sh | 2 +- multinode-demo/fullnode.sh | 1 + run.sh | 1 + 7 files changed, 38 insertions(+), 43 deletions(-) diff --git a/core/src/fullnode.rs b/core/src/fullnode.rs index 583c440c99727f..140e71fcbb85c3 100644 --- a/core/src/fullnode.rs +++ b/core/src/fullnode.rs @@ -125,25 +125,11 @@ impl Fullnode { keypair.clone(), ))); - // TODO: The RPC service assumes that there is a drone running on the cluster - // entrypoint, which is a bad assumption. - // See https://github.com/solana-labs/solana/issues/1830 for the removal of drone - // from the RPC API - let drone_addr = { - let mut entrypoint_drone_addr = match entrypoint_info_option { - Some(entrypoint_info_info) => entrypoint_info_info.rpc, - None => node.info.rpc, - }; - entrypoint_drone_addr.set_port(solana_drone::drone::DRONE_PORT); - entrypoint_drone_addr - }; - let storage_state = StorageState::new(); let rpc_service = JsonRpcService::new( &cluster_info, SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), node.info.rpc.port()), - drone_addr, storage_state.clone(), config.rpc_config.clone(), &exit, diff --git a/core/src/rpc.rs b/core/src/rpc.rs index b81a338fd44e11..0277b3af18725c 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -21,15 +21,17 @@ use std::sync::{Arc, RwLock}; use std::thread::sleep; use std::time::{Duration, Instant}; -#[derive(Clone)] +#[derive(Debug, Clone)] pub struct JsonRpcConfig { pub enable_fullnode_exit: bool, // Enable the 'fullnodeExit' command + pub drone_addr: Option, } impl Default for JsonRpcConfig { fn default() -> Self { Self { enable_fullnode_exit: false, + drone_addr: None, } } } @@ -170,7 +172,6 @@ fn verify_signature(input: &str) -> Result { pub struct Meta { pub request_processor: Arc>, pub cluster_info: Arc>, - pub drone_addr: SocketAddr, } impl Metadata for Meta {} @@ -291,6 +292,14 @@ impl RpcSol for RpcSolImpl { fn request_airdrop(&self, meta: Self::Metadata, id: String, lamports: u64) -> Result { trace!("request_airdrop id={} lamports={}", id, lamports); + + let drone_addr = meta + .request_processor + .read() + .unwrap() + .config + .drone_addr + .ok_or_else(Error::invalid_request)?; let pubkey = verify_pubkey(id)?; let blockhash = meta @@ -299,13 +308,11 @@ impl RpcSol for RpcSolImpl { .unwrap() .bank()? .last_blockhash(); - let transaction = - request_airdrop_transaction(&meta.drone_addr, &pubkey, lamports, blockhash).map_err( - |err| { - info!("request_airdrop_transaction failed: {:?}", err); - Error::internal_error() - }, - )?;; + let transaction = request_airdrop_transaction(&drone_addr, &pubkey, lamports, blockhash) + .map_err(|err| { + info!("request_airdrop_transaction failed: {:?}", err); + Error::internal_error() + })?;; let data = serialize(&transaction).map_err(|err| { info!("request_airdrop: serialize error: {:?}", err); @@ -412,7 +419,7 @@ mod tests { use solana_sdk::hash::{hash, Hash}; use solana_sdk::signature::{Keypair, KeypairUtil}; use solana_sdk::system_transaction::SystemTransaction; - use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + use std::net::SocketAddr; use std::thread; fn start_rpc_handler_with_tx(pubkey: Pubkey) -> (MetaIoHandler, Meta, Hash, Keypair) { @@ -435,7 +442,6 @@ mod tests { cluster_info.write().unwrap().insert_info(leader.clone()); cluster_info.write().unwrap().set_leader(leader.id); - let drone_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 0); let mut io = MetaIoHandler::default(); let rpc = RpcSolImpl; @@ -443,7 +449,6 @@ mod tests { let meta = Meta { request_processor, cluster_info, - drone_addr, }; (io, meta, blockhash, alice) } @@ -599,14 +604,14 @@ mod tests { let bob_pubkey = Keypair::new().pubkey(); let (io, meta, _blockhash, _alice) = start_rpc_handler_with_tx(bob_pubkey); - // Expect internal error because no leader is running + // Expect internal error because no drone is available let req = format!( r#"{{"jsonrpc":"2.0","id":1,"method":"requestAirdrop","params":["{}", 50]}}"#, bob_pubkey ); let res = io.handle_request_sync(&req, meta); let expected = - r#"{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal error"},"id":1}"#; + r#"{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"},"id":1}"#; let expected: Response = serde_json::from_str(expected).expect("expected response deserialization"); let result: Response = serde_json::from_str(&res.expect("actual response")) @@ -634,7 +639,6 @@ mod tests { Arc::new(RwLock::new(request_processor)) }, cluster_info: Arc::new(RwLock::new(ClusterInfo::new(NodeInfo::default()))), - drone_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 0), }; let req = diff --git a/core/src/rpc_service.rs b/core/src/rpc_service.rs index 0651defe0a25c6..b8a39a4aba8a09 100644 --- a/core/src/rpc_service.rs +++ b/core/src/rpc_service.rs @@ -24,12 +24,12 @@ impl JsonRpcService { pub fn new( cluster_info: &Arc>, rpc_addr: SocketAddr, - drone_addr: SocketAddr, storage_state: StorageState, config: JsonRpcConfig, exit: &Arc, ) -> Self { info!("rpc bound to {:?}", rpc_addr); + info!("rpc configuration: {:?}", config); let request_processor = Arc::new(RwLock::new(JsonRpcRequestProcessor::new( storage_state, config, @@ -51,7 +51,6 @@ impl JsonRpcService { ServerBuilder::with_meta_extractor(io, move |_req: &hyper::Request| Meta { request_processor: request_processor_.clone(), cluster_info: info.clone(), - drone_addr, }).threads(4) .cors(DomainsValidation::AllowOnly(vec![ AccessControlAllowOrigin::Any, @@ -105,14 +104,9 @@ mod tests { IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), solana_netutil::find_available_port_in_range((10000, 65535)).unwrap(), ); - let drone_addr = SocketAddr::new( - IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), - solana_netutil::find_available_port_in_range((10000, 65535)).unwrap(), - ); let mut rpc_service = JsonRpcService::new( &cluster_info, rpc_addr, - drone_addr, StorageState::default(), JsonRpcConfig::default(), &exit, diff --git a/fullnode/src/main.rs b/fullnode/src/main.rs index c99ee969266528..851ea1523bcd22 100644 --- a/fullnode/src/main.rs +++ b/fullnode/src/main.rs @@ -169,6 +169,13 @@ fn main() { .takes_value(false) .help("Enable the JSON RPC 'fullnodeExit' API. Only enable in a debug environment"), ) + .arg( + Arg::with_name("rpc_drone_address") + .long("rpc-drone-address") + .value_name("HOST:PORT") + .takes_value(true) + .help("Enable the JSON RPC 'requestAirdrop' API with this drone address."), + ) .arg( Arg::with_name("signer") .short("s") @@ -219,6 +226,9 @@ fn main() { if matches.is_present("enable_rpc_exit") { fullnode_config.rpc_config.enable_fullnode_exit = true; } + fullnode_config.rpc_config.drone_addr = matches + .value_of("rpc_drone_address") + .map(|address| address.parse().expect("failed to parse drone address")); let gossip_addr = { let mut addr = solana_netutil::parse_port_or_addr( @@ -238,9 +248,10 @@ fn main() { } else { fullnode_config.account_paths = None; } - let cluster_entrypoint = matches - .value_of("network") - .map(|network| network.parse().expect("failed to parse network address")); + let cluster_entrypoint = matches.value_of("network").map(|network| { + let gossip_addr = network.parse().expect("failed to parse network address"); + NodeInfo::new_entry_point(&gossip_addr) + }); let (_signer_service, signer_addr) = if let Some(signer_addr) = matches.value_of("signer") { ( None, @@ -296,9 +307,7 @@ fn main() { &keypair, ledger_path, vote_signer, - cluster_entrypoint - .map(|i| NodeInfo::new_entry_point(&i)) - .as_ref(), + cluster_entrypoint.as_ref(), &fullnode_config, ); diff --git a/multinode-demo/bootstrap-leader.sh b/multinode-demo/bootstrap-leader.sh index 8e019b77aa6953..62b62e99e81be3 100755 --- a/multinode-demo/bootstrap-leader.sh +++ b/multinode-demo/bootstrap-leader.sh @@ -28,12 +28,12 @@ tune_system trap 'kill "$pid" && wait "$pid"' INT TERM $solana_ledger_tool --ledger "$SOLANA_CONFIG_DIR"/bootstrap-leader-ledger verify -# shellcheck disable=SC2086 # Don't want to double quote maybe_blockstream or maybe_init_complete_file $program \ --identity "$SOLANA_CONFIG_DIR"/bootstrap-leader-id.json \ --ledger "$SOLANA_CONFIG_DIR"/bootstrap-leader-ledger \ --accounts "$SOLANA_CONFIG_DIR"/bootstrap-leader-accounts \ --rpc-port 8899 \ + --rpc-drone-address 127.0.0.1:9900 \ "$@" \ > >($bootstrap_leader_logger) 2>&1 & pid=$! diff --git a/multinode-demo/fullnode.sh b/multinode-demo/fullnode.sh index 761865a1c60d47..7fae130144702c 100755 --- a/multinode-demo/fullnode.sh +++ b/multinode-demo/fullnode.sh @@ -222,6 +222,7 @@ $program \ --network "$leader_address" \ --ledger "$ledger_config_dir" \ --accounts "$accounts_config_dir" \ + --rpc-drone-address "${leader_address%:*}:9900" \ "${extra_fullnode_args[@]}" \ > >($fullnode_logger) 2>&1 & pid=$! diff --git a/run.sh b/run.sh index 1eb48493be796d..07a8525b38fb6d 100755 --- a/run.sh +++ b/run.sh @@ -55,6 +55,7 @@ args=( --identity "$dataDir"/config/leader-keypair.json --ledger "$dataDir"/ledger/ --rpc-port 8899 + --rpc-drone-address 127.0.0.1:9900 ) if [[ -n $blockstreamSocket ]]; then args+=(--blockstream "$blockstreamSocket")