Skip to content

Commit

Permalink
fix: config cleanup (#4938)
Browse files Browse the repository at this point in the history
Description
---
Removed `max_randomx_vms` from `BaseNodeStateMachineConfig` and the config preset, now passing the initialized RandomXFactory in via the initializer, same for `bypass_range_proof_verification` but passing only the setting


Motivation and Context
---
#4909

There are a few duplicate config keys that exist in the base node config and the base node state machine config
meaning settings and behaviour could mismatch.

`base_node.max_randomx_vms` (this setting will not work)
`base_node.bypass_range_proof_verification` (this setting will not work)
`base_node.force_sync_peers` (this was fixed in #4647 but the fix is a bit hacky) 

https://github.com/tari-project/tari/blob/956b27954dda1f15f82bff0ba0ba0ee1f0880d2d/base_layer/core/src/base_node/state_machine_service/state_machine.rs#L52

https://github.com/tari-project/tari/blob/development/applications/tari_base_node/src/config.rs#L109

ref #4646

How Has This Been Tested?
---
manually
  • Loading branch information
agubarev committed Nov 23, 2022
1 parent 2af6c94 commit 68f990f
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 14 deletions.
3 changes: 3 additions & 0 deletions applications/tari_base_node/src/bootstrap.rs
Expand Up @@ -43,6 +43,7 @@ use tari_core::{
consensus::ConsensusManager,
mempool,
mempool::{service::MempoolHandle, Mempool, MempoolServiceInitializer, MempoolSyncInitializer},
proof_of_work::randomx_factory::RandomXFactory,
transactions::CryptoFactories,
};
use tari_p2p::{
Expand Down Expand Up @@ -153,6 +154,8 @@ where B: BlockchainBackend + 'static
base_node_config.state_machine.clone(),
self.rules,
self.factories,
RandomXFactory::new(self.app_config.base_node.max_randomx_vms),
self.app_config.base_node.bypass_range_proof_verification,
))
.build()
.await?;
Expand Down
2 changes: 0 additions & 2 deletions applications/tari_base_node/src/config.rs
Expand Up @@ -103,10 +103,8 @@ pub struct BaseNodeConfig {
/// The relative path to store the lmbd data
pub lmdb_path: PathBuf,
/// The maximum amount of VMs that RandomX will be use
// TODO: This is a potential conflict with 'BaseNodeStateMachineConfig::max_randomx_vms'
pub max_randomx_vms: usize,
/// Bypass range proof verification to speed up validation
// TODO: This is a potential conflict with 'BaseNodeStateMachineConfig::bypass_range_proof_verification'
pub bypass_range_proof_verification: bool,
/// The p2p config settings
pub p2p: P2pConfig,
Expand Down
13 changes: 10 additions & 3 deletions base_layer/core/src/base_node/state_machine_service/initializer.rs
Expand Up @@ -51,6 +51,8 @@ pub struct BaseNodeStateMachineInitializer<B> {
config: BaseNodeStateMachineConfig,
rules: ConsensusManager,
factories: CryptoFactories,
randomx_factory: RandomXFactory,
bypass_range_proof_verification: bool,
}

impl<B> BaseNodeStateMachineInitializer<B>
Expand All @@ -61,12 +63,16 @@ where B: BlockchainBackend + 'static
config: BaseNodeStateMachineConfig,
rules: ConsensusManager,
factories: CryptoFactories,
randomx_factory: RandomXFactory,
bypass_range_proof_verification: bool,
) -> Self {
Self {
db,
config,
rules,
factories,
randomx_factory,
bypass_range_proof_verification,
}
}
}
Expand All @@ -91,6 +97,8 @@ where B: BlockchainBackend + 'static
let rules = self.rules.clone();
let db = self.db.clone();
let config = self.config.clone();
let randomx_factory = self.randomx_factory.clone();
let bypass_range_proof_verification = self.bypass_range_proof_verification;

let mut mdc = vec![];
log_mdc::iter(|k, v| mdc.push((k.to_owned(), v.to_owned())));
Expand All @@ -105,10 +113,9 @@ where B: BlockchainBackend + 'static
db.clone(),
rules.clone(),
factories,
config.bypass_range_proof_verification,
bypass_range_proof_verification,
config.blockchain_sync_config.validation_concurrency,
);
let max_randomx_vms = config.max_randomx_vms;

let node = BaseNodeStateMachine::new(
db,
Expand All @@ -120,7 +127,7 @@ where B: BlockchainBackend + 'static
sync_validators,
status_event_sender,
state_event_publisher,
RandomXFactory::new(max_randomx_vms),
randomx_factory,
rules,
handles.get_shutdown_signal(),
);
Expand Down
Expand Up @@ -52,24 +52,17 @@ const LOG_TARGET: &str = "c::bn::base_node";
pub struct BaseNodeStateMachineConfig {
pub blockchain_sync_config: BlockchainSyncConfig,
/// The maximum amount of VMs that RandomX will be use
// TODO: This is a potential conflict with 'BaseNodeConfig::max_randomx_vms'
pub max_randomx_vms: usize,
/// The amount of blocks this node can be behind a peer before considered to be lagging (to test the block
/// propagation by delaying lagging)
pub blocks_behind_before_considered_lagging: u64,
/// Bypass range proof verification to speed up validation
// TODO: This is a potential conflict with 'BaseNodeConfig::bypass_range_proof_verification'
pub bypass_range_proof_verification: bool,
}

#[allow(clippy::derivable_impls)]
impl Default for BaseNodeStateMachineConfig {
fn default() -> Self {
Self {
blockchain_sync_config: Default::default(),
max_randomx_vms: 0,
blocks_behind_before_considered_lagging: 0,
bypass_range_proof_verification: false,
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions common/config/presets/c_base_node.toml
Expand Up @@ -141,8 +141,6 @@ track_reorgs = true
# The amount of blocks this node can be behind a peer before considered to be lagging (to test the block
# propagation by delaying lagging) (default = 0)
#blocks_behind_before_considered_lagging = 0
# Bypass range proof verification to speed up validation (default = false)
#bypass_range_proof_verification = false

[base_node.p2p]
# The node's publicly-accessible hostname. This is the host name that is advertised on the network so that
Expand Down

0 comments on commit 68f990f

Please sign in to comment.