Skip to content

Commit

Permalink
remove(node): remove advance_to_highest_supported_protocol_version …
Browse files Browse the repository at this point in the history 
…feature flag (#3601)

* remove(node): remove `advance_to_highest_supported_protocol_version` feature flag

* fix(node): `test_choose_next_system_packages` needs to be a simtest

* fix(node): add async keyword to `sim_test` test func
  • Loading branch information
@muXxer
muXxer authored Oct 24, 2024
1 parent 517f2d9 commit 86dc9f4
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 82 deletions.
24 changes: 0 additions & 24 deletions crates/iota-core/src/authority.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4356,19 +4356,11 @@ impl AuthorityState {
/// has voted to upgrade to. If the proposed protocol version is not
/// supported, None is returned.
fn is_protocol_version_supported_v1(
current_protocol_version: ProtocolVersion,
proposed_protocol_version: ProtocolVersion,
protocol_config: &ProtocolConfig,
committee: &Committee,
capabilities: Vec<AuthorityCapabilitiesV1>,
mut buffer_stake_bps: u64,
) -> Option<(ProtocolVersion, Vec<ObjectRef>)> {
if proposed_protocol_version > current_protocol_version + 1
&& !protocol_config.advance_to_highest_supported_protocol_version()
{
return None;
}

if buffer_stake_bps > 10000 {
warn!("clamping buffer_stake_bps to 10000");
buffer_stake_bps = 10000;
Expand Down Expand Up @@ -4444,19 +4436,11 @@ impl AuthorityState {
}

fn is_protocol_version_supported_v2(
current_protocol_version: ProtocolVersion,
proposed_protocol_version: ProtocolVersion,
protocol_config: &ProtocolConfig,
committee: &Committee,
capabilities: Vec<AuthorityCapabilitiesV2>,
mut buffer_stake_bps: u64,
) -> Option<(ProtocolVersion, Vec<ObjectRef>)> {
if proposed_protocol_version > current_protocol_version + 1
&& !protocol_config.advance_to_highest_supported_protocol_version()
{
return None;
}

if buffer_stake_bps > 10000 {
warn!("clamping buffer_stake_bps to 10000");
buffer_stake_bps = 10000;
Expand Down Expand Up @@ -4538,7 +4522,6 @@ impl AuthorityState {
/// returns the current protocol version and system packages.
fn choose_protocol_version_and_system_packages_v1(
current_protocol_version: ProtocolVersion,
protocol_config: &ProtocolConfig,
committee: &Committee,
capabilities: Vec<AuthorityCapabilitiesV1>,
buffer_stake_bps: u64,
Expand All @@ -4547,9 +4530,7 @@ impl AuthorityState {
let mut system_packages = vec![];

while let Some((version, packages)) = Self::is_protocol_version_supported_v1(
current_protocol_version,
next_protocol_version + 1,
protocol_config,
committee,
capabilities.clone(),
buffer_stake_bps,
Expand All @@ -4563,7 +4544,6 @@ impl AuthorityState {

fn choose_protocol_version_and_system_packages_v2(
current_protocol_version: ProtocolVersion,
protocol_config: &ProtocolConfig,
committee: &Committee,
capabilities: Vec<AuthorityCapabilitiesV2>,
buffer_stake_bps: u64,
Expand All @@ -4575,9 +4555,7 @@ impl AuthorityState {
// incrementing the proposed protocol version by one until no further
// upgrades are supported.
while let Some((version, packages)) = Self::is_protocol_version_supported_v2(
current_protocol_version,
next_protocol_version + 1,
protocol_config,
committee,
capabilities.clone(),
buffer_stake_bps,
Expand Down Expand Up @@ -4716,7 +4694,6 @@ impl AuthorityState {
if epoch_store.protocol_config().authority_capabilities_v2() {
Self::choose_protocol_version_and_system_packages_v2(
epoch_store.protocol_version(),
epoch_store.protocol_config(),
epoch_store.committee(),
epoch_store
.get_capabilities_v2()
Expand All @@ -4726,7 +4703,6 @@ impl AuthorityState {
} else {
Self::choose_protocol_version_and_system_packages_v1(
epoch_store.protocol_version(),
epoch_store.protocol_config(),
epoch_store.committee(),
epoch_store
.get_capabilities_v1()
Expand Down
45 changes: 5 additions & 40 deletions crates/iota-core/src/unit_tests/authority_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4975,9 +4975,8 @@ async fn test_consensus_message_processed() {
);
}

#[ignore = "https://github.com/iotaledger/iota/issues/2793"]
#[test]
fn test_choose_next_system_packages() {
#[sim_test]
async fn test_choose_next_system_packages() {
telemetry_subscribers::init_for_testing();
let o1 = random_object_ref();
let o2 = random_object_ref();
Expand Down Expand Up @@ -5023,7 +5022,6 @@ fn test_choose_next_system_packages() {
let committee = Committee::new_simple_test_committee().0;
let v = &committee.voting_rights;
let mut protocol_config = ProtocolConfig::get_for_max_version_UNSAFE();
protocol_config.set_advance_to_highest_supported_protocol_version_for_testing(false);
protocol_config.set_buffer_stake_for_protocol_upgrade_bps_for_testing(7500);

// all validators agree on new system packages, but without a new protocol
Expand All @@ -5039,7 +5037,6 @@ fn test_choose_next_system_packages() {
(ver(1), vec![]),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5058,7 +5055,6 @@ fn test_choose_next_system_packages() {
(ver(1), vec![]),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities.clone(),
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5072,7 +5068,6 @@ fn test_choose_next_system_packages() {
(ver(2), sort(vec![o1, o2])),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5091,7 +5086,6 @@ fn test_choose_next_system_packages() {
(ver(1), vec![]),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5110,7 +5104,6 @@ fn test_choose_next_system_packages() {
(ver(2), sort(vec![o1, o2])),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5129,27 +5122,24 @@ fn test_choose_next_system_packages() {
(ver(1), vec![]),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
)
);

// all validators support 3, but with this protocol config we cannot advance
// multiple versions at once.
// all validators support 3, so we advance by multiple versions at once.
let capabilities = vec![
make_capabilities!(3, v[0].0, vec![o1, o2]),
make_capabilities!(3, v[1].0, vec![o1, o2]),
make_capabilities!(3, v[2].0, vec![o1, o2]),
make_capabilities!(3, v[3].0, vec![o1, o2]),
make_capabilities!(3, v[3].0, vec![o1, o3]),
];

assert_eq!(
(ver(2), sort(vec![o1, o2])),
(ver(3), sort(vec![o1, o2])),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5168,28 +5158,6 @@ fn test_choose_next_system_packages() {
(ver(1), vec![]),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
)
);

protocol_config.set_advance_to_highest_supported_protocol_version_for_testing(true);

// skip straight to version 3
let capabilities = vec![
make_capabilities!(3, v[0].0, vec![o1, o2]),
make_capabilities!(3, v[1].0, vec![o1, o2]),
make_capabilities!(3, v[2].0, vec![o1, o2]),
make_capabilities!(3, v[3].0, vec![o1, o3]),
];

assert_eq!(
(ver(3), sort(vec![o1, o2])),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5209,7 +5177,6 @@ fn test_choose_next_system_packages() {
(ver(3), sort(vec![o1, o2])),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5231,7 +5198,6 @@ fn test_choose_next_system_packages() {
(ver(1), sort(vec![])),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand All @@ -5253,7 +5219,6 @@ fn test_choose_next_system_packages() {
(ver(1), sort(vec![])),
AuthorityState::choose_protocol_version_and_system_packages_v2(
ProtocolVersion::MIN,
&protocol_config,
&committee,
capabilities,
protocol_config.buffer_stake_for_protocol_upgrade_bps(),
Expand Down
24 changes: 6 additions & 18 deletions crates/iota-protocol-config/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ impl ProtocolVersion {
#[cfg(not(msim))]
const MAX_ALLOWED: Self = Self::MAX;

// We create one additional "fake" version in simulator builds so that we can
// We create 4 additional "fake" versions in simulator builds so that we can
// test upgrades.
#[cfg(msim)]
pub const MAX_ALLOWED: Self = Self(MAX_PROTOCOL_VERSION + 1);
pub const MAX_ALLOWED: Self = Self(MAX_PROTOCOL_VERSION + 4);

pub fn new(v: u64) -> Self {
Self(v)
Expand Down Expand Up @@ -112,10 +112,6 @@ struct FeatureFlags {
// Disables unnecessary invariant check in the Move VM when swapping the value out of a local
#[serde(skip_serializing_if = "is_false")]
disable_invariant_violation_check_in_swap_loc: bool,
// advance to highest supported protocol version at epoch change, instead of the next
// consecutive protocol version.
#[serde(skip_serializing_if = "is_false")]
advance_to_highest_supported_protocol_version: bool,
// If true, checks no extra bytes in a compiled module
#[serde(skip_serializing_if = "is_false")]
no_extraneous_module_bytes: bool,
Expand Down Expand Up @@ -970,11 +966,6 @@ impl ProtocolConfig {
.disable_invariant_violation_check_in_swap_loc
}

pub fn advance_to_highest_supported_protocol_version(&self) -> bool {
self.feature_flags
.advance_to_highest_supported_protocol_version
}

pub fn no_extraneous_module_bytes(&self) -> bool {
self.feature_flags.no_extraneous_module_bytes
}
Expand Down Expand Up @@ -1624,8 +1615,6 @@ impl ProtocolConfig {
cfg.feature_flags
.disable_invariant_violation_check_in_swap_loc = true;
cfg.feature_flags.no_extraneous_module_bytes = true;
cfg.feature_flags
.advance_to_highest_supported_protocol_version = true;
cfg.feature_flags.consensus_transaction_ordering = ConsensusTransactionOrdering::ByGasPrice;

cfg.feature_flags.hardened_otw_check = true;
Expand Down Expand Up @@ -1679,8 +1668,11 @@ impl ProtocolConfig {
cfg.feature_flags.authority_capabilities_v2 = true;
}

// TODO: remove the never_loop attribute when the version 2 is added.
// Ignore this check for the fake versions for
// `test_choose_next_system_packages`. TODO: remove the never_loop
// attribute when the version 2 is added.
#[allow(clippy::never_loop)]
#[cfg(not(msim))]
for cur in 2..=version.0 {
match cur {
1 => unreachable!(),
Expand Down Expand Up @@ -1767,10 +1759,6 @@ impl ProtocolConfig {
// `_for_testing`. Non-feature_flags should already have test setters defined
// through macros.
impl ProtocolConfig {
pub fn set_advance_to_highest_supported_protocol_version_for_testing(&mut self, val: bool) {
self.feature_flags
.advance_to_highest_supported_protocol_version = val
}
pub fn set_zklogin_auth_for_testing(&mut self, val: bool) {
self.feature_flags.zklogin_auth = val
}
Expand Down

0 comments on commit 86dc9f4

Please sign in to comment.