Skip to content

Commit

Permalink
chore: make DTS in PocketIC always enabled (#2446)
Browse files Browse the repository at this point in the history
This PR makes DTS always enabled in PocketIC and removes the enumeration
`DtsFlag` and its associated builder patterns. The motivation for this
change is that DTS is enabled on all subnets in production and making it
always enabled in PocketIC also simplifies PocketIC.
  • Loading branch information
mraszyk authored Nov 11, 2024
1 parent d10ff79 commit 14093af
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 138 deletions.
1 change: 1 addition & 0 deletions packages/pocket-ic/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Removed
- Functions `PocketIc::from_config`, `PocketIc::from_config_and_max_request_time`, and `PocketIc::from_config_and_server_url`.
Use the `PocketIcBuilder` instead.
- The enumeration `DtsFlag` and its associated builder patterns: DTS is always enabled in PocketIC.

### Changed
- The type `Topology` becomes a struct with two fields: `subnet_configs` contains an association of subnet ids to their configurations
Expand Down
51 changes: 0 additions & 51 deletions packages/pocket-ic/src/common/rest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,6 @@ pub struct ExtendedSubnetConfigSet {
pub struct SubnetSpec {
state_config: SubnetStateConfig,
instruction_config: SubnetInstructionConfig,
dts_flag: DtsFlag,
}

impl SubnetSpec {
Expand All @@ -496,29 +495,15 @@ impl SubnetSpec {
self
}

/// DTS is disabled on benchmarking subnet by default
/// since running update calls with very high instruction counts and DTS enabled
/// is very slow.
/// You can enable DTS by using `.with_dts_flag(DtsConfig::Enabled)`.
pub fn with_benchmarking_instruction_config(mut self) -> SubnetSpec {
self.instruction_config = SubnetInstructionConfig::Benchmarking;
self.dts_flag = DtsFlag::Disabled;
self
}

pub fn with_dts_flag(mut self, dts_flag: DtsFlag) -> SubnetSpec {
self.dts_flag = dts_flag;
self
}

pub fn get_state_path(&self) -> Option<PathBuf> {
self.state_config.get_path()
}

pub fn get_dts_flag(&self) -> DtsFlag {
self.dts_flag
}

pub fn get_subnet_id(&self) -> Option<RawSubnetId> {
match &self.state_config {
SubnetStateConfig::New => None,
Expand All @@ -545,7 +530,6 @@ impl Default for SubnetSpec {
Self {
state_config: SubnetStateConfig::New,
instruction_config: SubnetInstructionConfig::Production,
dts_flag: DtsFlag::Enabled,
}
}
}
Expand All @@ -561,13 +545,6 @@ pub enum SubnetInstructionConfig {
Benchmarking,
}

/// Specifies whether DTS should be disabled on this subnet.
#[derive(Debug, Clone, Copy, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema)]
pub enum DtsFlag {
Enabled,
Disabled,
}

/// Specifies whether the subnet should be created from scratch or loaded
/// from a path.
#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema)]
Expand Down Expand Up @@ -609,7 +586,6 @@ impl ExtendedSubnetConfigSet {
Option<PathBuf>,
Option<RawSubnetId>,
SubnetInstructionConfig,
DtsFlag,
)> {
use SubnetKind::*;
vec![
Expand All @@ -628,7 +604,6 @@ impl ExtendedSubnetConfigSet {
spec.get_state_path(),
spec.get_subnet_id(),
spec.get_instruction_config(),
spec.get_dts_flag(),
)
})
.collect()
Expand All @@ -648,32 +623,6 @@ impl ExtendedSubnetConfigSet {
}
Err("ExtendedSubnetConfigSet must contain at least one subnet".to_owned())
}

pub fn with_dts_flag(mut self, dts_flag: DtsFlag) -> ExtendedSubnetConfigSet {
self.nns = self.nns.map(|nns| nns.with_dts_flag(dts_flag));
self.sns = self.sns.map(|sns| sns.with_dts_flag(dts_flag));
self.ii = self.ii.map(|ii| ii.with_dts_flag(dts_flag));
self.fiduciary = self
.fiduciary
.map(|fiduciary| fiduciary.with_dts_flag(dts_flag));
self.bitcoin = self.bitcoin.map(|bitcoin| bitcoin.with_dts_flag(dts_flag));
self.system = self
.system
.into_iter()
.map(|conf| conf.with_dts_flag(dts_flag))
.collect();
self.application = self
.application
.into_iter()
.map(|conf| conf.with_dts_flag(dts_flag))
.collect();
self.verified_application = self
.verified_application
.into_iter()
.map(|conf| conf.with_dts_flag(dts_flag))
.collect();
self
}
}

/// Configuration details for a subnet, returned by PocketIc server
Expand Down
12 changes: 3 additions & 9 deletions packages/pocket-ic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
pub use crate::management_canister::CanisterSettings;
use crate::{
common::rest::{
BlobCompression, BlobId, CanisterHttpRequest, DtsFlag, ExtendedSubnetConfigSet,
HttpsConfig, InstanceId, MockCanisterHttpResponse, RawEffectivePrincipal, RawMessageId,
SubnetId, SubnetKind, SubnetSpec, Topology,
BlobCompression, BlobId, CanisterHttpRequest, ExtendedSubnetConfigSet, HttpsConfig,
InstanceId, MockCanisterHttpResponse, RawEffectivePrincipal, RawMessageId, SubnetId,
SubnetKind, SubnetSpec, Topology,
},
management_canister::{CanisterId, CanisterStatusResult},
nonblocking::PocketIc as PocketIcAsync,
Expand Down Expand Up @@ -326,12 +326,6 @@ impl PocketIcBuilder {
self.config = Some(config);
self
}

pub fn with_dts_flag(mut self, dts_flag: DtsFlag) -> Self {
let config = self.config.unwrap_or_default().with_dts_flag(dts_flag);
self.config = Some(config);
self
}
}

/// Main entry point for interacting with PocketIC.
Expand Down
57 changes: 14 additions & 43 deletions packages/pocket-ic/tests/slow.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use candid::{Encode, Principal};
use pocket_ic::{common::rest::DtsFlag, PocketIc, PocketIcBuilder, UserError, WasmResult};
use pocket_ic::{PocketIc, PocketIcBuilder, UserError, WasmResult};
use std::time::Duration;

// 200T cycles
Expand Down Expand Up @@ -39,7 +39,10 @@ fn execute_many_instructions(
Encode!(&instructions).unwrap(),
);
let t1 = pic.get_time();
// testing for the exact number n of rounds could lead to flakiness
// so we test for [n, n + 1] instead
assert!(t1 >= t0 + Duration::from_nanos(dts_rounds));
assert!(t1 <= t0 + Duration::from_nanos(dts_rounds + 1));

if system_subnet {
let cycles = pic.cycle_balance(can_id);
Expand All @@ -56,7 +59,7 @@ fn test_benchmarking_app_subnet() {
.build();

let instructions = 42_000_000_000_u64;
let dts_rounds = 1; // DTS is disabled on benchmarking subnets
let dts_rounds = 1; // DTS slice limit is very high on benchmarking subnets
execute_many_instructions(&pic, instructions, dts_rounds, false).unwrap();
}

Expand All @@ -67,59 +70,27 @@ fn test_benchmarking_system_subnet() {
.build();

let instructions = 42_000_000_000_u64;
let dts_rounds = 1; // DTS is disabled on benchmarking subnets
let dts_rounds = 1; // DTS slice limit is very high on benchmarking subnets
execute_many_instructions(&pic, instructions, dts_rounds, true).unwrap();
}

fn test_dts(dts_flag: DtsFlag) {
let pic = PocketIcBuilder::new()
.with_application_subnet()
.with_dts_flag(dts_flag)
.build();
#[test]
fn test_dts() {
let pic = PocketIcBuilder::new().with_application_subnet().build();

let instructions = 4_000_000_000_u64;
let dts_rounds = if let DtsFlag::Enabled = dts_flag {
instructions / 2_000_000_000
} else {
1
};
let instructions = 8_000_000_000_u64;
let dts_rounds = instructions / 2_000_000_000;
execute_many_instructions(&pic, instructions, dts_rounds, false).unwrap();
}

#[test]
fn test_dts_enabled() {
test_dts(DtsFlag::Enabled);
}

#[test]
fn test_dts_disabled() {
test_dts(DtsFlag::Disabled);
}

fn instruction_limit_exceeded(dts_flag: DtsFlag) {
let pic = PocketIcBuilder::new()
.with_application_subnet()
.with_dts_flag(dts_flag)
.build();
fn instruction_limit_exceeded() {
let pic = PocketIcBuilder::new().with_application_subnet().build();

let instructions = 42_000_000_000_u64;
let dts_rounds = if let DtsFlag::Enabled = dts_flag {
20 // instruction limit exceeded after 20 rounds
} else {
1
};
let dts_rounds = 20; // instruction limit exceeded after 20 rounds
let res = execute_many_instructions(&pic, instructions, dts_rounds, false).unwrap_err();
assert!(res.description.contains(
"Canister exceeded the limit of 40000000000 instructions for single message execution."
));
}

#[test]
fn test_instruction_limit_exceeded_dts_enabled() {
instruction_limit_exceeded(DtsFlag::Enabled);
}

#[test]
fn test_instruction_limit_exceeded_dts_disabled() {
instruction_limit_exceeded(DtsFlag::Disabled);
}
20 changes: 3 additions & 17 deletions rs/pocket_ic_server/src/pocket_ic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ use ic_validator_ingress_message::StandaloneIngressSigVerifier;
use itertools::Itertools;
use pocket_ic::common::rest::{
self, BinaryBlob, BlobCompression, CanisterHttpHeader, CanisterHttpMethod, CanisterHttpRequest,
CanisterHttpResponse, DtsFlag, ExtendedSubnetConfigSet, MockCanisterHttpResponse, RawAddCycles,
CanisterHttpResponse, ExtendedSubnetConfigSet, MockCanisterHttpResponse, RawAddCycles,
RawCanisterCall, RawCanisterId, RawEffectivePrincipal, RawMessageId, RawSetStableMemory,
SubnetInstructionConfig, SubnetKind, SubnetSpec, Topology,
};
Expand Down Expand Up @@ -168,7 +168,6 @@ struct SubnetConfigInternal {
pub subnet_id: SubnetId,
pub subnet_kind: SubnetKind,
pub instruction_config: SubnetInstructionConfig,
pub dts_flag: DtsFlag,
pub ranges: Vec<CanisterIdRange>,
pub alloc_range: Option<CanisterIdRange>,
}
Expand Down Expand Up @@ -523,6 +522,7 @@ impl PocketIc {
subnet_config.scheduler_config.max_instructions_per_round = instruction_limit;
}
subnet_config.scheduler_config.max_instructions_per_message = instruction_limit;
subnet_config.scheduler_config.max_instructions_per_slice = instruction_limit;
subnet_config
.scheduler_config
.max_instructions_per_message_without_dts = instruction_limit;
Expand Down Expand Up @@ -600,7 +600,6 @@ impl PocketIc {
subnet_kind: config.subnet_config.subnet_kind,
subnet_seed: hex::decode(subnet_seed).unwrap().try_into().unwrap(),
instruction_config: config.subnet_config.instruction_config,
dts_flag: config.subnet_config.dts_flag,
time: config.time,
})
.collect()
Expand All @@ -613,7 +612,6 @@ impl PocketIc {
spec.get_state_path(),
spec.get_subnet_id(),
spec.get_instruction_config(),
spec.get_dts_flag(),
)
});
let app = subnet_configs.application.iter().map(|spec| {
Expand All @@ -622,7 +620,6 @@ impl PocketIc {
spec.get_state_path(),
spec.get_subnet_id(),
spec.get_instruction_config(),
spec.get_dts_flag(),
)
});
let verified_app = subnet_configs.verified_application.iter().map(|spec| {
Expand All @@ -631,7 +628,6 @@ impl PocketIc {
spec.get_state_path(),
spec.get_subnet_id(),
spec.get_instruction_config(),
spec.get_dts_flag(),
)
});
sys.chain(app).chain(verified_app)
Expand All @@ -641,7 +637,7 @@ impl PocketIc {

let ii_subnet_split = subnet_configs.ii.is_some();

for (subnet_kind, subnet_state_dir, subnet_id, instruction_config, dts_flag) in
for (subnet_kind, subnet_state_dir, subnet_id, instruction_config) in
fixed_range_subnets.into_iter().chain(flexible_subnets)
{
let RangeConfig {
Expand All @@ -667,7 +663,6 @@ impl PocketIc {
subnet_kind,
subnet_seed,
instruction_config,
dts_flag,
time: GENESIS.into(),
});
}
Expand All @@ -690,7 +685,6 @@ impl PocketIc {
subnet_kind,
subnet_seed,
instruction_config,
dts_flag,
time,
} in subnet_config_info.into_iter()
{
Expand All @@ -714,10 +708,6 @@ impl PocketIc {
bitcoin_adapter_uds_path.clone(),
);

if let DtsFlag::Disabled = dts_flag {
builder = builder.no_dts();
};

if subnet_kind == SubnetKind::NNS {
builder = builder.with_root_subnet_config();
}
Expand Down Expand Up @@ -784,7 +774,6 @@ impl PocketIc {
instruction_config,
ranges,
alloc_range,
dts_flag,
};
subnet_configs.insert(subnet_seed, subnet_config_internal);
}
Expand Down Expand Up @@ -1098,7 +1087,6 @@ struct SubnetConfigInfo {
pub subnet_kind: SubnetKind,
pub subnet_seed: [u8; 32],
pub instruction_config: SubnetInstructionConfig,
pub dts_flag: DtsFlag,
pub time: SystemTime,
}

Expand Down Expand Up @@ -2479,7 +2467,6 @@ fn route(
return Err(format!("The effective canister ID {canister_id} belongs to the NNS or II subnet on the IC mainnet for which PocketIC provides a `SubnetKind`: please set up your PocketIC instance with a subnet of that `SubnetKind`."));
}
let instruction_config = SubnetInstructionConfig::Production;
let dts_flag = DtsFlag::Enabled;
// The binary representation of canister IDs on the IC mainnet consists of exactly 10 bytes.
let canister_id_slice: &[u8] = canister_id.as_ref();
if canister_id_slice.len() != 10 {
Expand Down Expand Up @@ -2552,7 +2539,6 @@ fn route(
instruction_config,
ranges: vec![range],
alloc_range: Some(canister_allocation_range),
dts_flag,
};
pic.topology
.subnet_configs
Expand Down
2 changes: 0 additions & 2 deletions rs/pocket_ic_server/tests/spec_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::common::raw_canister_id_range_into;
use candid::Principal;
use ic_registry_routing_table::{canister_id_into_u64, CanisterIdRange};
use ic_registry_subnet_type::SubnetType;
use pocket_ic::common::rest::DtsFlag;
use pocket_ic::PocketIcBuilder;
use rcgen::{CertificateParams, KeyPair};
use spec_compliance::run_ic_ref_test;
Expand Down Expand Up @@ -112,7 +111,6 @@ fn setup_and_run_ic_ref_test(
let mut pic = PocketIcBuilder::new()
.with_nns_subnet()
.with_application_subnet()
.with_dts_flag(DtsFlag::Disabled)
.build();
let endpoint = pic.make_live(None);
let topo = pic.topology();
Expand Down
Loading

0 comments on commit 14093af

Please sign in to comment.