Skip to content
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
22 changes: 1 addition & 21 deletions rs/cycles_account_manager/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ pub const CRITICAL_ERROR_RESPONSE_CYCLES_REFUND: &str =
pub const CRITICAL_ERROR_EXECUTION_CYCLES_REFUND: &str =
"cycles_account_manager_execution_cycles_refund_error";

/// [EXC-1168] Flag to turn on cost scaling according to a subnet replication factor.
const USE_COST_SCALING_FLAG: bool = true;
const SECONDS_PER_DAY: u128 = 24 * 60 * 60;

/// Maximum payload size of a management call to update_settings
Expand Down Expand Up @@ -174,9 +172,6 @@ pub struct CyclesAccountManager {
/// The configuration of this [`CyclesAccountManager`] controlling the fees
/// that are charged for various operations.
config: CyclesAccountManagerConfig,

/// [EXC-1168] Temporary development flag to enable cost scaling according to subnet size.
use_cost_scaling_flag: bool,
}

impl CyclesAccountManager {
Expand All @@ -193,20 +188,9 @@ impl CyclesAccountManager {
own_subnet_type,
own_subnet_id,
config,
use_cost_scaling_flag: USE_COST_SCALING_FLAG,
}
}

/// [EXC-1168] Helper function to set the flag to enable cost scaling according to subnet size.
pub fn set_using_cost_scaling(&mut self, use_cost_scaling_flag: bool) {
self.use_cost_scaling_flag = use_cost_scaling_flag;
}

/// [EXC-1168] Helper function to read the flag to enable cost scaling according to subnet size.
pub fn use_cost_scaling(&self) -> bool {
self.use_cost_scaling_flag
}

/// Returns the subnet type of this [`CyclesAccountManager`].
pub fn subnet_type(&self) -> SubnetType {
self.own_subnet_type
Expand All @@ -219,10 +203,7 @@ impl CyclesAccountManager {

// Scale cycles cost according to a subnet size.
fn scale_cost(&self, cycles: Cycles, subnet_size: usize) -> Cycles {
match self.use_cost_scaling_flag {
false => cycles,
true => (cycles * subnet_size) / self.config.reference_subnet_size,
}
(cycles * subnet_size) / self.config.reference_subnet_size
}

////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1169,7 +1150,6 @@ mod tests {
own_subnet_type: SubnetType::Application,
own_subnet_id: subnet_test_id(0),
config,
use_cost_scaling_flag: true,
}
}

Expand Down
8 changes: 0 additions & 8 deletions rs/execution_environment/tests/subnet_size_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ fn simulate_one_gib_per_second_cost(
let one_second = Duration::from_secs(1);

let env = StateMachineBuilder::new()
.with_use_cost_scaling_flag(true)
.with_subnet_type(subnet_type)
.with_subnet_size(subnet_size)
.build();
Expand Down Expand Up @@ -318,7 +317,6 @@ fn filtered_subnet_config(subnet_type: SubnetType, filter: KeepFeesFilter) -> Su
/// eg. ingress induction cost.
fn simulate_execute_install_code_cost(subnet_type: SubnetType, subnet_size: usize) -> Cycles {
let env = StateMachineBuilder::new()
.with_use_cost_scaling_flag(true)
.with_subnet_type(subnet_type)
.with_subnet_size(subnet_size)
.with_config(Some(StateMachineConfig::new(
Expand Down Expand Up @@ -356,7 +354,6 @@ fn simulate_execute_ingress_cost(
filter: KeepFeesFilter,
) -> Cycles {
let env = StateMachineBuilder::new()
.with_use_cost_scaling_flag(true)
.with_subnet_type(subnet_type)
.with_subnet_size(subnet_size)
.with_config(Some(StateMachineConfig::new(
Expand Down Expand Up @@ -389,7 +386,6 @@ fn simulate_execute_message_cost(subnet_type: SubnetType, subnet_size: usize) ->
/// including charging and refunding execution cycles.
fn simulate_execute_canister_heartbeat_cost(subnet_type: SubnetType, subnet_size: usize) -> Cycles {
let env = StateMachineBuilder::new()
.with_use_cost_scaling_flag(true)
.with_subnet_type(subnet_type)
.with_subnet_size(subnet_size)
.build();
Expand Down Expand Up @@ -420,7 +416,6 @@ fn simulate_sign_with_ecdsa_cost(
name: "key_id_secp256k1".to_string(),
};
let env = StateMachineBuilder::new()
.with_use_cost_scaling_flag(true)
.with_subnet_type(subnet_type)
.with_subnet_size(subnet_size)
.with_nns_subnet_id(nns_subnet_id)
Expand Down Expand Up @@ -475,7 +470,6 @@ fn simulate_sign_with_ecdsa_cost(
/// after executing the message.
fn simulate_http_request_cost(subnet_type: SubnetType, subnet_size: usize) -> Cycles {
let env = StateMachineBuilder::new()
.with_use_cost_scaling_flag(true)
.with_subnet_type(subnet_type)
.with_subnet_size(subnet_size)
.with_features(SubnetFeatures::from_str("http_requests").unwrap())
Expand Down Expand Up @@ -537,7 +531,6 @@ fn simulate_http_request_cost(subnet_type: SubnetType, subnet_size: usize) -> Cy
/// Filtered subnet config is used to avoid dealing with irrelevant costs.
fn simulate_xnet_call_cost(subnet_type: SubnetType, subnet_size: usize) -> Cycles {
let env = StateMachineBuilder::new()
.with_use_cost_scaling_flag(true)
.with_subnet_type(subnet_type)
.with_subnet_size(subnet_size)
.with_config(Some(StateMachineConfig::new(
Expand Down Expand Up @@ -583,7 +576,6 @@ fn simulate_xnet_call_cost(subnet_type: SubnetType, subnet_size: usize) -> Cycle
/// Simulates creating canister B from canister A to get a canister creation cost.
fn simulate_create_canister_cost(subnet_type: SubnetType, subnet_size: usize) -> Cycles {
let env = StateMachineBuilder::new()
.with_use_cost_scaling_flag(true)
.with_subnet_type(subnet_type)
.with_subnet_size(subnet_size)
.build();
Expand Down
1 change: 0 additions & 1 deletion rs/pocket_ic_server/src/pocket_ic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ impl PocketIc {
.with_time(time)
.with_state_machine_state_dir(state_machine_state_dir)
.with_registry_data_provider(registry_data_provider.clone())
.with_use_cost_scaling_flag(true)
}

pub(crate) fn new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ fn should_consume_expected_amount_of_cycles() {
initial_interval: Some(DEFAULT_MAX_WAIT_TIME_IN_SECS),
upgrade_interval: Some(DEFAULT_MAX_WAIT_TIME_IN_SECS),
assert_cost: |initial, upgrade| {
const EXPECTED_LEDGER_CYCLES_CONSUMPTION: i128 = 57_473_930;
const EXPECTED_INDEX_CYCLES_CONSUMPTION: i128 = 207_959_396;
const EXPECTED_LEDGER_CYCLES_CONSUMPTION: i128 = 124_445_767;
Comment thread
maksymar marked this conversation as resolved.
const EXPECTED_INDEX_CYCLES_CONSUMPTION: i128 = 449_388_554;
for ledger_consumption in [initial.ledger, upgrade.ledger] {
assert!(
abs_relative_difference(
Expand Down
17 changes: 2 additions & 15 deletions rs/state_machine_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,6 @@ pub struct StateMachineBuilder {
nns_subnet_id: Option<SubnetId>,
subnet_id: Option<SubnetId>,
routing_table: RoutingTable,
use_cost_scaling_flag: bool,
enable_canister_snapshots: bool,
idkg_keys_signing_enabled_status: BTreeMap<MasterPublicKeyId, bool>,
ecdsa_signature_fee: Option<Cycles>,
Expand All @@ -737,7 +736,6 @@ impl StateMachineBuilder {
checkpoint_interval_length: None,
subnet_type: SubnetType::System,
enable_canister_snapshots: false,
use_cost_scaling_flag: false,
subnet_size: SMALL_APP_SUBNET_MAX_SIZE,
nns_subnet_id: None,
subnet_id: None,
Expand Down Expand Up @@ -850,13 +848,6 @@ impl StateMachineBuilder {
}
}

pub fn with_use_cost_scaling_flag(self, use_cost_scaling_flag: bool) -> Self {
Self {
use_cost_scaling_flag,
..self
}
}

pub fn with_canister_snapshots(self, enable_canister_snapshots: bool) -> Self {
Self {
enable_canister_snapshots,
Expand Down Expand Up @@ -956,7 +947,6 @@ impl StateMachineBuilder {
self.subnet_type,
self.subnet_size,
self.subnet_id,
self.use_cost_scaling_flag,
self.enable_canister_snapshots,
self.idkg_keys_signing_enabled_status,
self.ecdsa_signature_fee,
Expand Down Expand Up @@ -1252,7 +1242,6 @@ impl StateMachine {
subnet_type: SubnetType,
subnet_size: usize,
subnet_id: Option<SubnetId>,
use_cost_scaling_flag: bool,
enable_canister_snapshots: bool,
idkg_keys_signing_enabled_status: BTreeMap<MasterPublicKeyId, bool>,
ecdsa_signature_fee: Option<Cycles>,
Expand Down Expand Up @@ -1332,14 +1321,12 @@ impl StateMachine {
..Default::default()
};

let mut cycles_account_manager = CyclesAccountManager::new(
let cycles_account_manager = Arc::new(CyclesAccountManager::new(
subnet_config.scheduler_config.max_instructions_per_message,
subnet_type,
subnet_id,
subnet_config.cycles_account_manager_config,
);
cycles_account_manager.set_using_cost_scaling(use_cost_scaling_flag);
let cycles_account_manager = Arc::new(cycles_account_manager);
));
let state_manager = Arc::new(StateManagerImpl::new(
Arc::new(FakeVerifier),
subnet_id,
Expand Down
13 changes: 2 additions & 11 deletions rs/tests/src/nns_tests/cycles_minting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ use slog::info;
use std::sync::atomic::{AtomicU64, Ordering};
use url::Url;

/// [EXC-1168] Flag to turn on cost scaling according to a subnet replication factor.
const USE_COST_SCALING_FLAG: bool = true;

fn make_user_ed25519(seed: u64) -> (ic_canister_client_sender::Ed25519KeyPair, PrincipalId) {
let mut rng = StdRng::seed_from_u64(seed);
let kp = ic_canister_client_sender::Ed25519KeyPair::generate(&mut rng);
Expand Down Expand Up @@ -97,15 +94,9 @@ pub fn config_with_multiple_app_subnets(env: TestEnv) {
});
}

// TODO(EXC-1168): cleanup after cost scaling is fully implemented.
fn scale_cycles(cycles: Cycles) -> Cycles {
match USE_COST_SCALING_FLAG {
false => cycles,
true => {
let subnet_size: usize = 1; // Subnet has only a single node, see usage of `add_fast_single_node_subnet` in `config()`.
(cycles * subnet_size) / SMALL_APP_SUBNET_MAX_SIZE
}
}
let subnet_size: usize = 1; // Subnet has only a single node, see usage of `add_fast_single_node_subnet` in `config()`.
(cycles * subnet_size) / SMALL_APP_SUBNET_MAX_SIZE
}

pub fn test(env: TestEnv) {
Expand Down
12 changes: 2 additions & 10 deletions rs/tests/src/tecdsa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ pub(crate) const KEY_ID1: &str = "secp256k1";
/// passed to execution.
pub(crate) const DKG_INTERVAL: u64 = 19;

/// [EXC-1168] Flag to turn on cost scaling according to a subnet replication factor.
const USE_COST_SCALING_FLAG: bool = true;
pub(crate) const NUMBER_OF_NODES: usize = 4;

pub(crate) fn make_key(name: &str) -> EcdsaKeyId {
Expand Down Expand Up @@ -128,15 +126,9 @@ pub(crate) fn empty_subnet_update() -> UpdateSubnetPayload {
}
}

// TODO(EXC-1168): cleanup after cost scaling is fully implemented.
pub(crate) fn scale_cycles(cycles: Cycles) -> Cycles {
match USE_COST_SCALING_FLAG {
false => cycles,
true => {
// Subnet is constructed with `NUMBER_OF_NODES`, see `config()` and `config_without_ecdsa_on_nns()`.
(cycles * NUMBER_OF_NODES) / SMALL_APP_SUBNET_MAX_SIZE
}
}
// Subnet is constructed with `NUMBER_OF_NODES`, see `config()` and `config_without_ecdsa_on_nns()`.
(cycles * NUMBER_OF_NODES) / SMALL_APP_SUBNET_MAX_SIZE
}

pub(crate) async fn get_public_key_and_test_signature(
Expand Down