Remove/address some TODOs (#6923)

cli/src/cli.rs

@@ -1122,7 +1122,6 @@ pub fn process_command(config: &CliConfig) -> ProcessResult {
 
 // Quick and dirty Keypair that assumes the client will do retries but not update the
 // blockhash. If the client updates the blockhash, the signature will be invalid.
-// TODO: Parse `msg` and use that data to make a new airdrop request.
 struct DroneKeypair {
     transaction: Transaction,
 }

client/src/rpc_client.rs

@@ -775,8 +775,13 @@ impl RpcClient {
                 break;
             }
             if now.elapsed().as_secs() > 15 {
-                // TODO: Return a better error.
+                return Err(io::Error::new(
-                return Err(io::Error::new(io::ErrorKind::Other, "signature not found"));
+                    io::ErrorKind::Other,
+                    format!(
+                        "signature not found after {} seconds",
+                        now.elapsed().as_secs()
+                    ),
+                ));
             }
             sleep(Duration::from_millis(250));
         }
@@ -859,8 +864,13 @@ impl RpcClient {
                 if confirmed_blocks > 0 {
                     return Ok(confirmed_blocks);
                 } else {
-                    // TODO: Return a better error.
+                    return Err(io::Error::new(
-                    return Err(io::Error::new(io::ErrorKind::Other, "signature not found"));
+                        io::ErrorKind::Other,
+                        format!(
+                            "signature not found after {} seconds",
+                            now.elapsed().as_secs()
+                        ),
+                    ));
                 }
             }
             sleep(Duration::from_millis(250));
@@ -919,8 +929,7 @@ impl RpcClient {
         })
     }
 
-    // TODO: Remove
+    pub fn send(
-    pub fn retry_make_rpc_request(
         &self,
         request: &RpcRequest,
         params: Option<Value>,
@@ -955,7 +964,7 @@ mod tests {
     use std::{sync::mpsc::channel, thread};
 
     #[test]
-    fn test_make_rpc_request() {
+    fn test_send() {
         let (sender, receiver) = channel();
         thread::spawn(move || {
             let rpc_addr = "0.0.0.0:0".parse().unwrap();
@@ -989,7 +998,7 @@ mod tests {
         let rpc_addr = receiver.recv().unwrap();
         let rpc_client = RpcClient::new_socket(rpc_addr);
 
-        let balance = rpc_client.retry_make_rpc_request(
+        let balance = rpc_client.send(
             &RpcRequest::GetBalance,
             Some(json!(["deadbeefXjn8o3yroDHxUtKsZZgoy4GPkPPXfouKNHhx"])),
             0,
@@ -997,15 +1006,14 @@ mod tests {
         );
         assert_eq!(balance.unwrap().as_u64().unwrap(), 50);
 
-        let blockhash =
+        let blockhash = rpc_client.send(&RpcRequest::GetRecentBlockhash, None, 0, None);
-            rpc_client.retry_make_rpc_request(&RpcRequest::GetRecentBlockhash, None, 0, None);
         assert_eq!(
             blockhash.unwrap().as_str().unwrap(),
             "deadbeefXjn8o3yroDHxUtKsZZgoy4GPkPPXfouKNHhx"
         );
 
         // Send erroneous parameter
-        let blockhash = rpc_client.retry_make_rpc_request(
+        let blockhash = rpc_client.send(
             &RpcRequest::GetRecentBlockhash,
             Some(json!("parameter")),
             0,
@@ -1015,7 +1023,7 @@ mod tests {
     }
 
     #[test]
-    fn test_retry_make_rpc_request() {
+    fn test_retry_send() {
         solana_logger::setup();
         let (sender, receiver) = channel();
         thread::spawn(move || {
@@ -1044,7 +1052,7 @@ mod tests {
         let rpc_addr = receiver.recv().unwrap();
         let rpc_client = RpcClient::new_socket(rpc_addr);
 
-        let balance = rpc_client.retry_make_rpc_request(
+        let balance = rpc_client.send(
             &RpcRequest::GetBalance,
             Some(json!(["deadbeefXjn8o3yroDHxUtKsZZgoy4GPkPPXfouKNHhw"])),
             10,

client/src/rpc_client_request.rs

@@ -67,10 +67,7 @@ impl GenericRpcClientRequest for RpcClientRequest {
                     return Ok(json["result"].clone());
                 }
                 Err(e) => {
-                    info!(
+                    info!("{:?} failed, {} retries left: {:?}", request, retries, e);
-                        "make_rpc_request({:?}) failed, {} retries left: {:?}",
-                        request, retries, e
-                    );
                     if retries == 0 {
                         return Err(e.into());
                     }

core/src/archiver.rs

@@ -747,7 +747,7 @@ impl Archiver {
                 RpcClient::new_socket(rpc_peers[node_index].rpc)
             };
             Ok(rpc_client
-                .retry_make_rpc_request(
+                .send(
                     &RpcRequest::GetSlotsPerSegment,
                     None,
                     0,
@@ -804,7 +804,7 @@ impl Archiver {
                     RpcClient::new_socket(rpc_peers[node_index].rpc)
                 };
                 let response = rpc_client
-                    .retry_make_rpc_request(&RpcRequest::GetStorageTurn, None, 0, None)
+                    .send(&RpcRequest::GetStorageTurn, None, 0, None)
                     .map_err(|err| {
                         warn!("Error while making rpc request {:?}", err);
                         Error::IO(io::Error::new(ErrorKind::Other, "rpc error"))

drone/src/drone.rs

@@ -188,7 +188,7 @@ pub fn request_airdrop_transaction(
         "request_airdrop_transaction: drone_addr={} id={} lamports={} blockhash={}",
         drone_addr, id, lamports, blockhash
     );
-    // TODO: make this async tokio client
+
     let mut stream = TcpStream::connect_timeout(drone_addr, Duration::new(3, 0))?;
     stream.set_read_timeout(Some(Duration::new(10, 0)))?;
     let req = DroneRequest::GetAirdrop {

install/src/update_manifest.rs

@@ -56,8 +56,6 @@ impl SignedUpdateManifest {
 
 impl ConfigState for SignedUpdateManifest {
     fn max_space() -> u64 {
-        // TODO: Use a fully populated manifest to compute a better value
+        256 // Enough space for a fully populated SignedUpdateManifest
-        //      bincode::serialized_size(&Self::default()).unwrap()
-        256
     }
 }

ledger/src/bank_forks.rs

@@ -165,7 +165,6 @@ impl BankForks {
         bank
     }
 
-    // TODO: really want to kill this...
     pub fn working_bank(&self) -> Arc<Bank> {
         self.working_bank.clone()
     }

ledger/src/blocktree_processor.rs

@@ -428,7 +428,6 @@ fn process_next_slots(
     }
 
     // Reverse sort by slot, so the next slot to be processed can be popped
-    // TODO: remove me once leader_scheduler can hang with out-of-order slots?
     pending_slots.sort_by(|a, b| b.0.cmp(&a.0));
     Ok(())
 }

programs/stake_api/src/stake_instruction.rs

@@ -328,7 +328,6 @@ pub fn process_instruction(
     let keyed_accounts = &mut keyed_accounts.iter_mut();
     let me = &mut next_keyed_account(keyed_accounts)?;
 
-    // TODO: data-driven unpack and dispatch of KeyedAccounts
     match limited_deserialize(data)? {
         StakeInstruction::Initialize(authorized, lockup) => me.initialize(
             &authorized,

programs/vote_api/src/vote_instruction.rs

@@ -176,7 +176,6 @@ pub fn process_instruction(
     let (me, rest) = &mut keyed_accounts.split_at_mut(1);
     let me = &mut me[0];
 
-    // TODO: data-driven unpack and dispatch of KeyedAccounts
     match limited_deserialize(data)? {
         VoteInstruction::InitializeAccount(vote_init) => {
             if rest.is_empty() {

runtime/src/bank.rs

@@ -514,7 +514,6 @@ impl Bank {
         }
         // if I'm the first Bank in an epoch, count, claim, disburse rewards from Inflation
 
-        // TODO: on-chain wallclock?
         //  years_elapsed =         slots_elapsed                             /     slots/year
         let year = (self.epoch_schedule.get_last_slot_in_epoch(epoch)) as f64 / self.slots_per_year;
 
@@ -1584,7 +1583,6 @@ impl Bank {
         let dbhq = dbank.blockhash_queue.read().unwrap();
         assert_eq!(*bhq, *dbhq);
 
-        // TODO: Uncomment once status cache serialization is done
         let sc = self.src.status_cache.read().unwrap();
         let dsc = dbank.src.status_cache.read().unwrap();
         assert_eq!(*sc, *dsc);

runtime/src/bank_client.rs

@@ -191,10 +191,12 @@ impl SyncClient for BankClient {
                 }
             };
             if now.elapsed().as_secs() > 15 {
-                // TODO: Return a better error.
                 return Err(TransportError::IoError(io::Error::new(
                     io::ErrorKind::Other,
-                    "signature not found",
+                    format!(
+                        "signature not found after {} seconds",
+                        now.elapsed().as_secs()
+                    ),
                 )));
             }
             sleep(Duration::from_millis(250));
@@ -212,10 +214,12 @@ impl SyncClient for BankClient {
                 }
             }
             if now.elapsed().as_secs() > 15 {
-                // TODO: Return a better error.
                 return Err(TransportError::IoError(io::Error::new(
                     io::ErrorKind::Other,
-                    "signature not found",
+                    format!(
+                        "signature not found after {} seconds",
+                        now.elapsed().as_secs()
+                    ),
                 )));
             }
             sleep(Duration::from_millis(250));

runtime/src/genesis_utils.rs

@@ -29,12 +29,11 @@ pub fn create_genesis_config_with_leader(
     bootstrap_leader_stake_lamports: u64,
 ) -> GenesisConfigInfo {
     let mint_keypair = Keypair::new();
-    let voting_keypair = Keypair::new();
+    let bootstrap_leader_voting_keypair = Keypair::new();
-    let staking_keypair = Keypair::new();
+    let bootstrap_leader_staking_keypair = Keypair::new();
 
-    // TODO: de-duplicate the stake... passive staking is fully implemented
+    let bootstrap_leader_vote_account = vote_state::create_account(
-    let vote_account = vote_state::create_account(
+        &bootstrap_leader_voting_keypair.pubkey(),
-        &voting_keypair.pubkey(),
         &bootstrap_leader_pubkey,
         0,
         bootstrap_leader_stake_lamports,
@@ -42,30 +41,31 @@ pub fn create_genesis_config_with_leader(
 
     let rent = Rent::free();
 
-    let stake_account = stake_state::create_account(
+    let bootstrap_leader_stake_account = stake_state::create_account(
-        &staking_keypair.pubkey(),
+        &bootstrap_leader_staking_keypair.pubkey(),
-        &voting_keypair.pubkey(),
+        &bootstrap_leader_voting_keypair.pubkey(),
-        &vote_account,
+        &bootstrap_leader_vote_account,
         &rent,
         bootstrap_leader_stake_lamports,
     );
 
     let accounts = vec![
-        // the mint
         (
             mint_keypair.pubkey(),
             Account::new(mint_lamports, 0, &system_program::id()),
         ),
-        // node needs an account to issue votes and storage proofs from, this will require
-        //  airdrops at some point to cover fees...
         (
             *bootstrap_leader_pubkey,
             Account::new(BOOTSTRAP_LEADER_LAMPORTS, 0, &system_program::id()),
         ),
-        // where votes go to
+        (
-        (voting_keypair.pubkey(), vote_account),
+            bootstrap_leader_voting_keypair.pubkey(),
-        // passive bootstrap leader stake, duplicates above temporarily
+            bootstrap_leader_vote_account,
-        (staking_keypair.pubkey(), stake_account),
+        ),
+        (
+            bootstrap_leader_staking_keypair.pubkey(),
+            bootstrap_leader_stake_account,
+        ),
     ];
 
     // Bare minimum program set
@@ -75,8 +75,8 @@ pub fn create_genesis_config_with_leader(
         solana_vote_program!(),
         solana_stake_program!(),
     ];
-    let fee_calculator = FeeCalculator::new(0, 0); // most tests don't want fees
 
+    let fee_calculator = FeeCalculator::new(0, 0); // most tests can't handle transaction fees
     let mut genesis_config = GenesisConfig {
         accounts,
         native_instruction_processors,
@@ -91,6 +91,6 @@ pub fn create_genesis_config_with_leader(
     GenesisConfigInfo {
         genesis_config,
         mint_keypair,
-        voting_keypair,
+        voting_keypair: bootstrap_leader_voting_keypair,
     }
 }

runtime/src/status_cache.rs

@@ -93,7 +93,6 @@ impl<T: Serialize + Clone> StatusCache<T> {
         None
     }
 
-    /// TODO: wallets should send the Transactions recent blockhash as well
     pub fn get_signature_status_slow(
         &self,
         sig: &Signature,

system-test/testnet-performance/testnet-automation.sh

@@ -238,7 +238,6 @@ function launchTestnet() {
 
 cd "$(dirname "$0")/../.."
 
-# TODO: Make sure a dB named $TESTNET_TAG exists in the influxDB host, or can be created
 [[ -n $TESTNET_TAG ]] || TESTNET_TAG=testnet-automation
 [[ -n $INFLUX_HOST ]] || INFLUX_HOST=https://metrics.solana.com:8086
 [[ -n $RAMP_UP_TIME ]] || RAMP_UP_TIME=0