Skip to content

Commit

Permalink
Add extra consistency checks to pallet_configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
@tmpolaczyk
tmpolaczyk committed Sep 27, 2023
1 parent 9e2393c commit 991780b
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 48 deletions.
10 changes: 10 additions & 0 deletions pallets/configuration/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ impl Default for HostConfiguration {
pub enum InconsistentError {
/// `max_orchestrator_collators` is lower than `min_orchestrator_collators`
MaxCollatorsLowerThanMinCollators,
/// `min_orchestrator_collators` must be at least 1
MinOrchestratorCollatorsTooLow,
/// `max_collators` must be at least 1
MaxCollatorsTooLow,
}

impl HostConfiguration {
Expand All @@ -88,6 +92,12 @@ impl HostConfiguration {
///
/// This function returns an error if the configuration is inconsistent.
pub fn check_consistency(&self) -> Result<(), InconsistentError> {
if self.max_collators < 1 {
return Err(InconsistentError::MaxCollatorsTooLow);
}
if self.min_orchestrator_collators < 1 {
return Err(InconsistentError::MinOrchestratorCollatorsTooLow);
}
if self.max_orchestrator_collators < self.min_orchestrator_collators {
return Err(InconsistentError::MaxCollatorsLowerThanMinCollators);
}
Expand Down
121 changes: 73 additions & 48 deletions pallets/configuration/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
// along with Tanssi. If not, see <http://www.gnu.org/licenses/>

use {
crate::{mock::*, HostConfiguration, PendingConfigs},
frame_support::{assert_ok, dispatch::GetDispatchInfo},
crate::{mock::*, Error, HostConfiguration, PendingConfigs},
frame_support::{assert_noop, assert_ok, dispatch::GetDispatchInfo},
sp_std::vec,
};

Expand Down Expand Up @@ -51,14 +51,14 @@ fn config_sets_default_values() {
#[test]
fn config_set_value() {
new_test_ext_with_genesis(HostConfiguration {
max_collators: 0,
min_orchestrator_collators: 0,
max_orchestrator_collators: 0,
collators_per_container: 0,
max_collators: 100,
min_orchestrator_collators: 2,
max_orchestrator_collators: 5,
collators_per_container: 2,
})
.execute_with(|| {
run_to_block(1);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().max_collators, 100);
assert_ok!(
Configuration::set_max_collators(RuntimeOrigin::root(), 50),
()
Expand All @@ -70,23 +70,23 @@ fn config_set_value() {
2,
HostConfiguration {
max_collators: 50,
min_orchestrator_collators: 0,
max_orchestrator_collators: 0,
collators_per_container: 0,
min_orchestrator_collators: 2,
max_orchestrator_collators: 5,
collators_per_container: 2,
}
)]
);

// The session delay is set to 2, and one session is 5 blocks,
// so the change should not happen until block 11
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().max_collators, 100);
run_to_block(2);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().max_collators, 100);
// First block of session 1
run_to_block(6);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().max_collators, 100);
run_to_block(10);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().max_collators, 100);
// First block of session 2
run_to_block(11);
assert_eq!(Configuration::config().max_collators, 50);
Expand All @@ -96,16 +96,16 @@ fn config_set_value() {
#[test]
fn config_set_many_values_same_block() {
new_test_ext_with_genesis(HostConfiguration {
max_collators: 0,
min_orchestrator_collators: 0,
max_orchestrator_collators: 0,
collators_per_container: 0,
max_collators: 100,
min_orchestrator_collators: 2,
max_orchestrator_collators: 5,
collators_per_container: 2,
})
.execute_with(|| {
run_to_block(1);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().collators_per_container, 0);
assert_eq!(Configuration::config().min_orchestrator_collators, 0);
assert_eq!(Configuration::config().max_collators, 100);
assert_eq!(Configuration::config().collators_per_container, 2);
assert_eq!(Configuration::config().min_orchestrator_collators, 2);
assert_ok!(
Configuration::set_max_collators(RuntimeOrigin::root(), 50),
()
Expand Down Expand Up @@ -135,9 +135,9 @@ fn config_set_many_values_same_block() {
// The session delay is set to 2, and one session is 5 blocks,
// so the change should not happen until block 11
run_to_block(10);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().collators_per_container, 0);
assert_eq!(Configuration::config().min_orchestrator_collators, 0);
assert_eq!(Configuration::config().max_collators, 100);
assert_eq!(Configuration::config().collators_per_container, 2);
assert_eq!(Configuration::config().min_orchestrator_collators, 2);
// First block of session 2
run_to_block(11);
assert_eq!(Configuration::config().max_collators, 50);
Expand All @@ -149,16 +149,16 @@ fn config_set_many_values_same_block() {
#[test]
fn config_set_many_values_different_blocks() {
new_test_ext_with_genesis(HostConfiguration {
max_collators: 0,
min_orchestrator_collators: 0,
max_orchestrator_collators: 0,
collators_per_container: 0,
max_collators: 100,
min_orchestrator_collators: 2,
max_orchestrator_collators: 5,
collators_per_container: 2,
})
.execute_with(|| {
run_to_block(1);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().collators_per_container, 0);
assert_eq!(Configuration::config().min_orchestrator_collators, 0);
assert_eq!(Configuration::config().max_collators, 100);
assert_eq!(Configuration::config().collators_per_container, 2);
assert_eq!(Configuration::config().min_orchestrator_collators, 2);
assert_ok!(
Configuration::set_max_collators(RuntimeOrigin::root(), 50),
()
Expand Down Expand Up @@ -190,9 +190,9 @@ fn config_set_many_values_different_blocks() {
// The session delay is set to 2, and one session is 5 blocks,
// so the change should not happen until block 11
run_to_block(10);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().min_orchestrator_collators, 0);
assert_eq!(Configuration::config().collators_per_container, 0);
assert_eq!(Configuration::config().max_collators, 100);
assert_eq!(Configuration::config().min_orchestrator_collators, 2);
assert_eq!(Configuration::config().collators_per_container, 2);
// First block of session 2
run_to_block(11);
assert_eq!(Configuration::config().max_collators, 50);
Expand All @@ -204,16 +204,16 @@ fn config_set_many_values_different_blocks() {
#[test]
fn config_set_many_values_different_sessions() {
new_test_ext_with_genesis(HostConfiguration {
max_collators: 0,
min_orchestrator_collators: 0,
max_orchestrator_collators: 0,
collators_per_container: 0,
max_collators: 100,
min_orchestrator_collators: 2,
max_orchestrator_collators: 5,
collators_per_container: 2,
})
.execute_with(|| {
run_to_block(1);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().min_orchestrator_collators, 0);
assert_eq!(Configuration::config().collators_per_container, 0);
assert_eq!(Configuration::config().max_collators, 100);
assert_eq!(Configuration::config().min_orchestrator_collators, 2);
assert_eq!(Configuration::config().collators_per_container, 2);
assert_ok!(
Configuration::set_max_collators(RuntimeOrigin::root(), 50),
()
Expand All @@ -223,9 +223,9 @@ fn config_set_many_values_different_sessions() {
Configuration::set_min_orchestrator_collators(RuntimeOrigin::root(), 20),
()
);
assert_eq!(Configuration::config().max_collators, 0);
assert_eq!(Configuration::config().min_orchestrator_collators, 0);
assert_eq!(Configuration::config().collators_per_container, 0);
assert_eq!(Configuration::config().max_collators, 100);
assert_eq!(Configuration::config().min_orchestrator_collators, 2);
assert_eq!(Configuration::config().collators_per_container, 2);
run_to_block(11);
assert_ok!(
Configuration::set_collators_per_container(RuntimeOrigin::root(), 10),
Expand All @@ -241,7 +241,7 @@ fn config_set_many_values_different_sessions() {
max_collators: 50,
min_orchestrator_collators: 20,
max_orchestrator_collators: 20,
collators_per_container: 0,
collators_per_container: 2,
}
),
(
Expand All @@ -257,19 +257,44 @@ fn config_set_many_values_different_sessions() {
);

assert_eq!(Configuration::config().max_collators, 50);
assert_eq!(Configuration::config().min_orchestrator_collators, 0);
assert_eq!(Configuration::config().collators_per_container, 0);
assert_eq!(Configuration::config().min_orchestrator_collators, 2);
assert_eq!(Configuration::config().collators_per_container, 2);
run_to_block(16);
assert_eq!(Configuration::config().max_collators, 50);
assert_eq!(Configuration::config().min_orchestrator_collators, 20);
assert_eq!(Configuration::config().collators_per_container, 0);
assert_eq!(Configuration::config().collators_per_container, 2);
run_to_block(21);
assert_eq!(Configuration::config().max_collators, 50);
assert_eq!(Configuration::config().min_orchestrator_collators, 20);
assert_eq!(Configuration::config().collators_per_container, 10);
});
}

#[test]
fn config_cannot_set_invalid_values() {
new_test_ext_with_genesis(HostConfiguration {
max_collators: 100,
min_orchestrator_collators: 2,
max_orchestrator_collators: 5,
collators_per_container: 2,
})
.execute_with(|| {
run_to_block(1);
assert_noop!(
Configuration::set_max_collators(RuntimeOrigin::root(), 0),
Error::<Test>::InvalidNewValue
);
assert_noop!(
Configuration::set_min_orchestrator_collators(RuntimeOrigin::root(), 0),
Error::<Test>::InvalidNewValue
);
assert_noop!(
Configuration::set_max_orchestrator_collators(RuntimeOrigin::root(), 0),
Error::<Test>::InvalidNewValue
);
});
}

#[test]
fn weights_assigned_to_extrinsics_are_correct() {
new_test_ext().execute_with(|| {
Expand Down

0 comments on commit 991780b

Please sign in to comment.