From 63a2913bf2259af4a61743e9b860d572186c647b Mon Sep 17 00:00:00 2001 From: Daniel Porteous Date: Thu, 8 Dec 2022 20:59:40 +0000 Subject: [PATCH] [NHC] Add Fetchers, overhaul Evaluator inputs --- .../framework/aptos-framework/doc/voting.md | 7 +- .../node-checker/src/checker/minimum_peers.rs | 213 ++++++++++ ecosystem/node-checker/src/checker/mod.rs | 40 ++ ecosystem/node-checker/src/checker/traits.rs | 48 +++ ecosystem/node-checker/src/checker/types.rs | 69 ++++ ecosystem/node-checker/src/common/clients.rs | 101 +++++ .../src/{ => common}/common_args.rs | 0 ecosystem/node-checker/src/common/mod.rs | 8 + .../node-checker/src/configuration/common.rs | 22 +- .../node-checker/src/configuration/create.rs | 39 -- .../node-checker/src/configuration/mod.rs | 13 +- .../node-checker/src/configuration/types.rs | 371 +++++------------- .../src/configuration/validate.rs | 9 +- .../node-checker/src/evaluator/traits.rs | 123 +----- ecosystem/node-checker/src/evaluator/types.rs | 32 +- ecosystem/node-checker/src/fetcher/metrics.rs | 135 +++++++ ecosystem/node-checker/src/fetcher/mod.rs | 7 + ecosystem/node-checker/src/fetcher/traits.rs | 47 +++ ecosystem/node-checker/src/handler/mod.rs | 1 + .../src/handler/synchronous_handler.rs | 9 + ecosystem/node-checker/src/lib.rs | 13 +- .../reqwest_metric_collector.rs | 3 +- .../node-checker/src/provider/helpers.rs | 43 ++ .../node-checker/src/provider/metrics.rs | 150 +++++++ ecosystem/node-checker/src/provider/mod.rs | 13 + .../src/provider/provider_collection.rs | 41 ++ ecosystem/node-checker/src/provider/traits.rs | 46 +++ .../src/runner/blocking_runner.rs | 274 ------------- ecosystem/node-checker/src/runner/mod.rs | 4 +- .../node-checker/src/runner/sync_runner.rs | 122 ++++++ ecosystem/node-checker/src/runner/traits.rs | 37 +- ecosystem/node-checker/src/server/api.rs | 237 +++-------- ecosystem/node-checker/src/server/build.rs | 96 +++++ ecosystem/node-checker/src/server/common.rs | 4 +- .../src/server/configurations_manager.rs | 88 ----- ecosystem/node-checker/src/server/errors.rs | 1 + ecosystem/node-checker/src/server/mod.rs | 3 +- ecosystem/node-checker/src/server/run.rs | 49 +-- 38 files changed, 1416 insertions(+), 1102 deletions(-) create mode 100644 ecosystem/node-checker/src/checker/minimum_peers.rs create mode 100644 ecosystem/node-checker/src/checker/mod.rs create mode 100644 ecosystem/node-checker/src/checker/traits.rs create mode 100644 ecosystem/node-checker/src/checker/types.rs create mode 100644 ecosystem/node-checker/src/common/clients.rs rename ecosystem/node-checker/src/{ => common}/common_args.rs (100%) create mode 100644 ecosystem/node-checker/src/common/mod.rs delete mode 100644 ecosystem/node-checker/src/configuration/create.rs create mode 100644 ecosystem/node-checker/src/fetcher/metrics.rs create mode 100644 ecosystem/node-checker/src/fetcher/mod.rs create mode 100644 ecosystem/node-checker/src/fetcher/traits.rs create mode 100644 ecosystem/node-checker/src/handler/mod.rs create mode 100644 ecosystem/node-checker/src/handler/synchronous_handler.rs create mode 100644 ecosystem/node-checker/src/provider/helpers.rs create mode 100644 ecosystem/node-checker/src/provider/metrics.rs create mode 100644 ecosystem/node-checker/src/provider/mod.rs create mode 100644 ecosystem/node-checker/src/provider/provider_collection.rs create mode 100644 ecosystem/node-checker/src/provider/traits.rs delete mode 100644 ecosystem/node-checker/src/runner/blocking_runner.rs create mode 100644 ecosystem/node-checker/src/runner/sync_runner.rs create mode 100644 ecosystem/node-checker/src/server/build.rs delete mode 100644 ecosystem/node-checker/src/server/configurations_manager.rs create mode 100644 ecosystem/node-checker/src/server/errors.rs diff --git a/aptos-move/framework/aptos-framework/doc/voting.md b/aptos-move/framework/aptos-framework/doc/voting.md index 74cdfaf2021c0..f92be497e9c45 100644 --- a/aptos-move/framework/aptos-framework/doc/voting.md +++ b/aptos-move/framework/aptos-framework/doc/voting.md @@ -107,8 +107,9 @@ Extra metadata (e.g. description, code url) can be part of the ProposalType stru Currently, we have three attributes that are used by the voting flow. 1. RESOLVABLE_TIME_METADATA_KEY: this is uesed to record the resolvable time to ensure that resolution has to be done non-atomically. 2. IS_MULTI_STEP_PROPOSAL_KEY: this is used to track if a proposal is single-step or multi-step. - 3. IS_MULTI_STEP_PROPOSAL_IN_EXECUTION_KEY: this attribute only exists for and applies to multi-step proposals. The value is used to - indicate if a multi-step proposal is in execution. If yes, we will disable further voting for this multi-step proposal. + 3. IS_MULTI_STEP_PROPOSAL_IN_EXECUTION_KEY: this attribute only applies to multi-step proposals. A single-step proposal will not have + this field in its metadata map. The value is used to indicate if a multi-step proposal is in execution. If yes, we will disable further + voting for this multi-step proposal.
creation_time_secs: u64 @@ -889,6 +890,7 @@ This guarantees that voting eligibility and voting power are controlled by the r ## Function `is_proposal_resolvable` +Common checks on if a proposal is resolvable, regardless if the proposal is single-step or multi-step.
fun is_proposal_resolvable<ProposalType: store>(voting_forum_address: address, proposal_id: u64)
@@ -1019,6 +1021,7 @@ there are more yes votes than no. If either of these conditions is not met, this
     let voting_forum = borrow_global_mut<VotingForum<ProposalType>>(voting_forum_address);
     let proposal = table::borrow_mut(&mut voting_forum.proposals, proposal_id);
 
+    // Update the IS_MULTI_STEP_PROPOSAL_IN_EXECUTION_KEY key to indicate that the multi-step proposal is in execution.
     let multi_step_in_execution_key = utf8(IS_MULTI_STEP_PROPOSAL_IN_EXECUTION_KEY);
     if (simple_map::contains_key(&proposal.metadata, &multi_step_in_execution_key)) {
         let is_multi_step_proposal_in_execution_value = simple_map::borrow_mut(&mut proposal.metadata, &multi_step_in_execution_key);
diff --git a/ecosystem/node-checker/src/checker/minimum_peers.rs b/ecosystem/node-checker/src/checker/minimum_peers.rs
new file mode 100644
index 0000000000000..0cd110914e91a
--- /dev/null
+++ b/ecosystem/node-checker/src/checker/minimum_peers.rs
@@ -0,0 +1,213 @@
+// Copyright (c) Aptos
+// SPDX-License-Identifier: Apache-2.0
+
+/// These evaluators are only valuable in certain contexts. For example, this is
+/// not a useful evaluator for node registration for the AITs, since each node
+/// is running in their own isolated network, where no consensus is occurring.
+/// This is useful for the AIT itself though, where the nodes are participating
+/// in a real network.
+use crate::{
+    checker::{CheckResult, Checker},
+    get_provider,
+    provider::{
+        metrics::{get_metric, GetMetricResult, Label, MetricsProvider},
+        Provider, ProviderCollection,
+    },
+};
+use anyhow::Result;
+use once_cell::sync::Lazy;
+use prometheus_parse::Scrape;
+use serde::{Deserialize, Serialize};
+
+use super::traits::CheckerError;
+
+/// Evaluator for minimum number of peers.
+#[derive(Clone, Debug, Deserialize, Serialize)]
+pub struct MinimumPeersCheckerConfig {
+    #[serde(default)]
+    pub required: bool,
+
+    /// The minimum number of inbound connections required to be able to pass.
+    /// For fullnodes, it only matters that this is greater than zero if the
+    /// node operator wants to seed data to other nodes.
+    #[serde(default = "MinimumPeersCheckerConfig::default_minimum_peers_inbound")]
+    pub minimum_peers_inbound: u64,
+
+    /// The minimum number of outbound connections required to be able to pass.
+    /// This must be greater than zero for the node to be able to synchronize.
+    #[serde(default = "MinimumPeersCheckerConfig::default_minimum_peers_outbound")]
+    pub minimum_peers_outbound: u64,
+}
+
+impl MinimumPeersCheckerConfig {
+    pub fn default_minimum_peers_inbound() -> u64 {
+        0
+    }
+
+    pub fn default_minimum_peers_outbound() -> u64 {
+        1
+    }
+}
+
+#[derive(Debug)]
+pub struct MinimumPeersChecker {
+    config: MinimumPeersCheckerConfig,
+}
+
+impl MinimumPeersChecker {
+    pub fn new(config: MinimumPeersCheckerConfig) -> Self {
+        Self { config }
+    }
+
+    #[allow(clippy::comparison_chain)]
+    fn build_evaluation(
+        &self,
+        connections: u64,
+        minimum: u64,
+        connection_type: &ConnectionType,
+    ) -> CheckResult {
+        let name = connection_type.get_name();
+        let particle = connection_type.get_particle();
+        let opposite_particle = connection_type.get_opposite_particle();
+        let explanation = format!(
+            "There are {} {} connections {} other nodes {} the target node (the minimum is {}).",
+            connections, name, particle, opposite_particle, minimum
+        );
+        if connections >= minimum {
+            CheckResult::new(
+                format!(
+                    "There are sufficient {} connections {} the target node",
+                    name, particle
+                ),
+                100,
+                explanation,
+            )
+        } else {
+            CheckResult::new(
+                format!(
+                    "There are not enough {} connections {} the target node",
+                    name, particle
+                ),
+                50,
+                format!("{} Try setting explicit peers.", explanation),
+            )
+            .links(vec![
+                "https://aptos.dev/nodes/full-node/troubleshooting-fullnode-setup".to_string(),
+            ])
+        }
+    }
+
+    fn default_minimum_inbound() -> u64 {
+        0
+    }
+
+    fn default_minimum_outbound() -> u64 {
+        1
+    }
+}
+
+#[async_trait::async_trait]
+impl Checker for MinimumPeersChecker {
+    async fn check(&self, input: &ProviderCollection) -> Result, CheckerError> {
+        let target_metrics_provider = get_provider!(
+            input.target_metrics_provider,
+            self.config.required,
+            MetricsProvider
+        );
+        let scrape = target_metrics_provider.provide().await?;
+        let (inbound_connections, outbound_connections) = match get_metrics(&scrape) {
+            Ok((inbound_connections, outbound_connections)) => {
+                (inbound_connections, outbound_connections)
+            }
+            Err(evaluation_results) => return Ok(evaluation_results),
+        };
+
+        Ok(vec![
+            self.build_evaluation(
+                inbound_connections,
+                self.config.minimum_peers_inbound,
+                &ConnectionType::Inbound,
+            ),
+            self.build_evaluation(
+                outbound_connections,
+                self.config.minimum_peers_outbound,
+                &ConnectionType::Outbound,
+            ),
+        ])
+    }
+}
+
+//////////////////////////////////////////////////////////////////////////////
+// Helpers.
+//////////////////////////////////////////////////////////////////////////////
+
+const METRIC: &str = "aptos_connections";
+
+static INBOUND_LABEL: Lazy