From 9c00ad9ff2b5363076e17582034da1e9429f2f42 Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Fri, 8 Nov 2019 16:41:36 -0700 Subject: [PATCH] Remove some low-hanging TODOs (#6839) --- core/src/rpc_pubsub.rs | 8 +++++--- metrics/src/metrics.rs | 6 +----- net/gce.sh | 1 - net/scripts/colo-provider.sh | 11 ++++++----- net/scripts/colo-utils.sh | 6 +++--- net/scripts/ec2-provider.sh | 13 ++++++------- sdk/src/fee_calculator.rs | 9 ++------- 7 files changed, 23 insertions(+), 31 deletions(-) diff --git a/core/src/rpc_pubsub.rs b/core/src/rpc_pubsub.rs index c01881aa6..4cd85a4c0 100644 --- a/core/src/rpc_pubsub.rs +++ b/core/src/rpc_pubsub.rs @@ -11,7 +11,10 @@ use solana_sdk::signature::Signature; use solana_sdk::transaction; use std::sync::{atomic, Arc}; -#[allow(clippy::needless_return)] // TODO remove me when rpc is updated? +// Suppress needless_return due to +// https://github.com/paritytech/jsonrpc/blob/2d38e6424d8461cdf72e78425ce67d51af9c6586/derive/src/lib.rs#L204 +// Once https://github.com/paritytech/jsonrpc/issues/418 is resolved, try to remove this clippy allow +#[allow(clippy::needless_return)] #[rpc(server)] pub trait RpcSolPubSub { type Metadata; @@ -376,7 +379,6 @@ mod tests { let contract_funds = Keypair::new(); let contract_state = Keypair::new(); let budget_program_id = solana_budget_api::id(); - let executable = false; // TODO let bank = Bank::new(&genesis_block); let blockhash = bank.last_blockhash(); let bank_forks = Arc::new(RwLock::new(BankForks::new(0, bank))); @@ -428,7 +430,7 @@ mod tests { "owner": budget_program_id, "lamports": 51, "data": expected_data, - "executable": executable, + "executable": false, "rent_epoch": 0, }, "subscription": 0, diff --git a/metrics/src/metrics.rs b/metrics/src/metrics.rs index 93a700c26..d68be3a3b 100644 --- a/metrics/src/metrics.rs +++ b/metrics/src/metrics.rs @@ -438,11 +438,7 @@ pub fn set_panic_hook(program: &'static str) { // The 'one' field exists to give Kapacitor Alerts a numerical value // to filter on .add_field_i64("one", 1) - .add_field_str( - "message", - // TODO: use ono.message() when it becomes stable - &ono.to_string(), - ) + .add_field_str("message", &ono.to_string()) .add_field_str("location", &location) .to_owned(), Level::Error, diff --git a/net/gce.sh b/net/gce.sh index 280104315..e02483632 100755 --- a/net/gce.sh +++ b/net/gce.sh @@ -36,7 +36,6 @@ azure) # shellcheck source=net/scripts/azure-provider.sh source "$here"/scripts/azure-provider.sh - # TODO: Dial in machine types for Azure cpuBootstrapLeaderMachineType=Standard_D16s_v3 gpuBootstrapLeaderMachineType=Standard_NC12 clientMachineType=Standard_D16s_v3 diff --git a/net/scripts/colo-provider.sh b/net/scripts/colo-provider.sh index a72bd19b8..34105e7a3 100755 --- a/net/scripts/colo-provider.sh +++ b/net/scripts/colo-provider.sh @@ -5,7 +5,8 @@ # Utilities for working with Colo instances # -declare COLO_TODO_PARALLELIZE=false +# COLO_PARALLELIZE is not ready for use, disable it +declare COLO_PARALLELIZE=false __cloud_colo_here="$(dirname "${BASH_SOURCE[0]}")" # shellcheck source=net/scripts/colo-utils.sh @@ -40,7 +41,7 @@ __cloud_FindInstances() { declare filter=$1 instances=() - if ! $COLO_TODO_PARALLELIZE; then + if ! $COLO_PARALLELIZE; then colo_load_resources colo_load_availability false fi @@ -110,7 +111,7 @@ cloud_Initialize() { # networkName=$1 # unused # zone=$2 #unused colo_load_resources - if $COLO_TODO_PARALLELIZE; then + if $COLO_PARALLELIZE; then colo_load_availability fi } @@ -166,7 +167,7 @@ cloud_CreateInstances() { done fi - if $COLO_TODO_PARALLELIZE; then + if $COLO_PARALLELIZE; then declare HOST_NAME IP PRIV_IP STATUS ZONE LOCK_USER INSTNAME INDEX RES LINE declare -a AVAILABLE declare AVAILABLE_TEXT @@ -270,7 +271,7 @@ cloud_FetchFile() { cloud_StatusAll() { declare HOST_NAME IP PRIV_IP STATUS ZONE LOCK_USER INSTNAME - if ! $COLO_TODO_PARALLELIZE; then + if ! $COLO_PARALLELIZE; then colo_load_resources colo_load_availability false fi diff --git a/net/scripts/colo-utils.sh b/net/scripts/colo-utils.sh index b2ab3f023..968ff8488 100644 --- a/net/scripts/colo-utils.sh +++ b/net/scripts/colo-utils.sh @@ -178,9 +178,9 @@ colo_node_status_all() { done < <(colo_instance_run_foreach "$(__colo_node_status_script)") } -# TODO: As part of COLO_TOOD_PARALLELIZE this list will need to be maintained -# in a lockfile to work around `cloud_CreateInstance` being called in the -# background for validators +# Note: As part of enabling COLO_PARALLELIZE, this list will need to be maintained in a +# lockfile to work around `cloud_CreateInstance` being called in the background +# for validators export COLO_RES_REQUISITIONED=() colo_node_requisition() { declare IP=$1 diff --git a/net/scripts/ec2-provider.sh b/net/scripts/ec2-provider.sh index 112797e9f..f7acf3349 100755 --- a/net/scripts/ec2-provider.sh +++ b/net/scripts/ec2-provider.sh @@ -19,9 +19,7 @@ __cloud_GetRegion() { echo "$region" } -# sshPrivateKey should be globally defined whenever this function is called. -# -# TODO: Remove usage of the sshPrivateKey global +# Note: sshPrivateKey should be globally defined whenever this function is called. __cloud_SshPrivateKeyCheck() { # shellcheck disable=SC2154 if [[ -z $sshPrivateKey ]]; then @@ -174,8 +172,8 @@ cloud_CreateInstances() { # # Custom Ubuntu 18.04 LTS image with CUDA 9.2 and CUDA 10.0 installed # - # TODO: Unfortunately these AMIs are not public. When this becomes an issue, - # use the stock Ubuntu 18.04 image and programmatically install CUDA after the + # Unfortunately these AMIs are not public. When this becomes an issue, use + # the stock Ubuntu 18.04 image and programmatically install CUDA after the # instance boots # case $region in @@ -287,9 +285,10 @@ cloud_CreateInstances() { IFS=: read -r instanceId publicIp privateIp zone < <(echo "${instances[0]}") ( set -x - # TODO: Poll that the instance has moved to the 'running' state instead of - # blindly sleeping for 30 seconds... + # It would be better to poll that the instance has moved to the 'running' + # state instead of blindly sleeping for 30 seconds... sleep 30 + declare region= region=$(__cloud_GetRegion "$zone") aws ec2 associate-address \ diff --git a/sdk/src/fee_calculator.rs b/sdk/src/fee_calculator.rs index 39ff67cc4..f372865dc 100644 --- a/sdk/src/fee_calculator.rs +++ b/sdk/src/fee_calculator.rs @@ -64,9 +64,6 @@ impl FeeCalculator { if me.target_signatures_per_slot > 0 { // lamports_per_signature can range from 50% to 1000% of // target_lamports_per_signature - // - // TODO: Are these decent limits? - // me.min_lamports_per_signature = std::cmp::max(1, me.target_lamports_per_signature / 2); me.max_lamports_per_signature = me.target_lamports_per_signature * 10; @@ -91,10 +88,8 @@ impl FeeCalculator { if gap == 0 { me.lamports_per_signature = desired_lamports_per_signature; } else { - // Adjust fee by 5% of target_lamports_per_signature to produce a smooth increase/decrease in fees over time. - // - // TODO: Is this fee curve smooth enough or too smooth? - // + // Adjust fee by 5% of target_lamports_per_signature to produce a smooth + // increase/decrease in fees over time. let gap_adjust = std::cmp::max(1, me.target_lamports_per_signature / 20) as i64 * gap.signum();