Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Node Startup Config] Add ability to skip optimizer/sanitizer. #11502

Merged
merged 2 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 44 additions & 1 deletion config/src/config/config_optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ impl ConfigOptimizer for NodeConfig {
node_type: NodeType,
chain_id: Option<ChainId>,
) -> Result<bool, Error> {
// If config optimization is disabled, don't do anything!
if node_config.node_startup.skip_config_optimizer {
return Ok(false);
}

// Optimize only the relevant sub-configs
let mut optimizers_with_modifications = vec![];
if IndexerConfig::optimize(node_config, local_config_yaml, node_type, chain_id)? {
Expand Down Expand Up @@ -288,9 +293,47 @@ fn build_seed_peer(
#[cfg(test)]
mod tests {
use super::*;
use crate::{config::NetworkConfig, network_id::NetworkId};
use crate::{
config::{node_startup_config::NodeStartupConfig, NetworkConfig},
network_id::NetworkId,
};
use aptos_types::account_address::AccountAddress;

#[test]
fn test_disable_optimizer() {
// Create a default node config (with optimization enabled)
let mut node_config = NodeConfig::default();

// Optimize the node config for mainnet VFNs and verify modifications are made
let modified_config = NodeConfig::optimize(
&mut node_config,
&serde_yaml::from_str("{}").unwrap(), // An empty local config
NodeType::ValidatorFullnode,
Some(ChainId::mainnet()),
)
.unwrap();
assert!(modified_config);

// Create a node config with the optimizer disabled
let mut node_config = NodeConfig {
node_startup: NodeStartupConfig {
skip_config_optimizer: true,
..Default::default()
},
..Default::default()
};

// Optimize the node config for mainnet VFNs and verify no modifications are made
let modified_config = NodeConfig::optimize(
&mut node_config,
&serde_yaml::from_str("{}").unwrap(), // An empty local config
NodeType::ValidatorFullnode,
Some(ChainId::mainnet()),
)
.unwrap();
assert!(!modified_config);
}

#[test]
fn test_optimize_public_network_config_mainnet() {
// Create a public network config with no seeds
Expand Down
40 changes: 39 additions & 1 deletion config/src/config/config_sanitizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ impl ConfigSanitizer for NodeConfig {
node_type: NodeType,
chain_id: Option<ChainId>,
) -> Result<(), Error> {
// If config sanitization is disabled, don't do anything!
if node_config.node_startup.skip_config_sanitizer {
return Ok(());
}

// Sanitize all of the sub-configs
AdminServiceConfig::sanitize(node_config, node_type, chain_id)?;
ApiConfig::sanitize(node_config, node_type, chain_id)?;
Expand Down Expand Up @@ -197,7 +202,40 @@ fn sanitize_validator_network_config(
#[cfg(test)]
mod tests {
use super::*;
use crate::{config::NetworkConfig, network_id::NetworkId};
use crate::{
config::{node_startup_config::NodeStartupConfig, NetworkConfig},
network_id::NetworkId,
};

#[test]
fn test_disable_config_sanitizer() {
// Create a default node config (with sanitization enabled)
let mut node_config = NodeConfig::default();

// Set a bad node config for mainnet
node_config.execution.paranoid_hot_potato_verification = false;

// Sanitize the config and verify the sanitizer fails
let error =
NodeConfig::sanitize(&node_config, NodeType::Validator, Some(ChainId::mainnet()))
.unwrap_err();
assert!(matches!(error, Error::ConfigSanitizerFailed(_, _)));

// Create a node config with the sanitizer disabled
let mut node_config = NodeConfig {
node_startup: NodeStartupConfig {
skip_config_sanitizer: true,
..Default::default()
},
..Default::default()
};

// Set a bad node config for mainnet
node_config.execution.paranoid_hot_potato_verification = false;

// Sanitize the config and verify the sanitizer passes
NodeConfig::sanitize(&node_config, NodeType::Validator, Some(ChainId::mainnet())).unwrap();
}

#[test]
fn test_sanitize_missing_pfn_network_configs() {
Expand Down
5 changes: 3 additions & 2 deletions config/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ mod inspection_service_config;
mod jwk_consensus_config;
mod logger_config;
mod mempool_config;
mod netbench;
mod netbench_config;
mod network_config;
mod node_config;
mod node_config_loader;
mod node_startup_config;
mod override_node_config;
mod peer_monitoring_config;
mod persistable_config;
Expand Down Expand Up @@ -53,7 +54,7 @@ pub use indexer_table_info_config::*;
pub use inspection_service_config::*;
pub use logger_config::*;
pub use mempool_config::*;
pub use netbench::*;
pub use netbench_config::*;
pub use network_config::*;
pub use node_config::*;
pub use node_config_loader::sanitize_node_config;
Expand Down
File renamed without changes.
7 changes: 5 additions & 2 deletions config/src/config/node_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
use super::{DagConsensusConfig, IndexerTableInfoConfig};
use crate::{
config::{
dkg_config::DKGConfig, jwk_consensus_config::JWKConsensusConfig, netbench::NetbenchConfig,
node_config_loader::NodeConfigLoader, persistable_config::PersistableConfig,
dkg_config::DKGConfig, jwk_consensus_config::JWKConsensusConfig,
netbench_config::NetbenchConfig, node_config_loader::NodeConfigLoader,
node_startup_config::NodeStartupConfig, persistable_config::PersistableConfig,
utils::RootPath, AdminServiceConfig, ApiConfig, BaseConfig, ConsensusConfig, Error,
ExecutionConfig, IndexerConfig, IndexerGrpcConfig, InspectionServiceConfig, LoggerConfig,
MempoolConfig, NetworkConfig, PeerMonitoringServiceConfig, SafetyRulesTestConfig,
Expand Down Expand Up @@ -67,6 +68,8 @@ pub struct NodeConfig {
#[serde(default)]
pub netbench: Option<NetbenchConfig>,
#[serde(default)]
pub node_startup: NodeStartupConfig,
#[serde(default)]
pub peer_monitoring_service: PeerMonitoringServiceConfig,
#[serde(default)]
pub state_sync: StateSyncConfig,
Expand Down
36 changes: 36 additions & 0 deletions config/src/config/node_startup_config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

use serde::{Deserialize, Serialize};

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, Serialize)]
#[serde(default, deny_unknown_fields)]
pub struct NodeStartupConfig {
pub skip_config_optimizer: bool, // Whether or not to skip the config optimizer at startup
pub skip_config_sanitizer: bool, // Whether or not to skip the config sanitizer at startup
}

#[allow(clippy::derivable_impls)] // Derive default manually (this is safer than guessing defaults)
impl Default for NodeStartupConfig {
fn default() -> Self {
Self {
skip_config_optimizer: false,
skip_config_sanitizer: false,
}
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_node_startup_config_default() {
// Create the default config
let config = NodeStartupConfig::default();

// Verify both fields are set to false
assert!(!config.skip_config_optimizer);
assert!(!config.skip_config_sanitizer);
}
}
Loading