Skip to content

Commit

Permalink
feat: Add wasm_memory_threshold to ProposeToUpdateCanisterSettingsCmd (
Browse files Browse the repository at this point in the history
…#2864)

To support `on_low_wasm_memory` hook we are adding
`wasm_memory_threshold` to `NNS` so it allows users to update its value
using `NNS` proposal. We are as well adding `wasm_memory_threshold` to
canister settings to canisters in `IC` repo.

The `wasm_memory_threshold` will be added to the interface specification
in dfinity/portal#3761

---------

Co-authored-by: Andre Popovitch <[email protected]>
Co-authored-by: Arshavir Ter-Gabrielyan <[email protected]>
  • Loading branch information
3 people authored Dec 18, 2024
1 parent 5c084ec commit 4c775db
Show file tree
Hide file tree
Showing 34 changed files with 98 additions and 2 deletions.
16 changes: 16 additions & 0 deletions rs/nervous_system/clients/src/canister_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ pub struct DefiniteCanisterSettings {
pub reserved_cycles_limit: Option<candid::Nat>,
pub wasm_memory_limit: Option<candid::Nat>,
pub log_visibility: Option<LogVisibility>,
pub wasm_memory_threshold: Option<candid::Nat>,
}

/// Partial copy-paste of ic-types::ic_00::CanisterStatusResult.
Expand Down Expand Up @@ -111,6 +112,7 @@ pub struct DefiniteCanisterSettingsFromManagementCanister {
pub reserved_cycles_limit: candid::Nat,
pub wasm_memory_limit: candid::Nat,
pub log_visibility: LogVisibility,
pub wasm_memory_threshold: candid::Nat,
}

impl From<CanisterStatusResultFromManagementCanister> for CanisterStatusResult {
Expand Down Expand Up @@ -152,6 +154,7 @@ impl From<DefiniteCanisterSettingsFromManagementCanister> for DefiniteCanisterSe
reserved_cycles_limit,
wasm_memory_limit,
log_visibility,
wasm_memory_threshold,
} = value;

let compute_allocation = Some(compute_allocation);
Expand All @@ -160,6 +163,7 @@ impl From<DefiniteCanisterSettingsFromManagementCanister> for DefiniteCanisterSe
let reserved_cycles_limit = Some(reserved_cycles_limit);
let wasm_memory_limit = Some(wasm_memory_limit);
let log_visibility = Some(log_visibility);
let wasm_memory_threshold = Some(wasm_memory_threshold);

DefiniteCanisterSettings {
controllers,
Expand All @@ -169,6 +173,7 @@ impl From<DefiniteCanisterSettingsFromManagementCanister> for DefiniteCanisterSe
reserved_cycles_limit,
wasm_memory_limit,
log_visibility,
wasm_memory_threshold,
}
}
}
Expand All @@ -193,6 +198,7 @@ impl CanisterStatusResultFromManagementCanister {
reserved_cycles_limit: candid::Nat::from(47_u32),
wasm_memory_limit: candid::Nat::from(48_u32),
log_visibility: LogVisibility::Controllers,
wasm_memory_threshold: candid::Nat::from(49_u32),
},
cycles: candid::Nat::from(47_u32),
idle_cycles_burned_per_day: candid::Nat::from(48_u32),
Expand Down Expand Up @@ -237,6 +243,7 @@ impl CanisterStatusResultV2 {
freezing_threshold: u64,
idle_cycles_burned_per_day: u128,
wasm_memory_limit: u64,
wasm_memory_threshold: u64,
) -> Self {
Self {
status,
Expand All @@ -251,6 +258,7 @@ impl CanisterStatusResultV2 {
memory_allocation,
freezing_threshold,
Some(wasm_memory_limit),
Some(wasm_memory_threshold),
),
idle_cycles_burned_per_day: candid::Nat::from(idle_cycles_burned_per_day),
}
Expand Down Expand Up @@ -297,6 +305,7 @@ impl CanisterStatusResultV2 {
45, // freezing_threshold
46, // idle_cycles_burned_per_day
47, // wasm_memory_limit
41, // wasm_memory_threshold
)
}

Expand All @@ -318,6 +327,7 @@ pub struct DefiniteCanisterSettingsArgs {
pub memory_allocation: candid::Nat,
pub freezing_threshold: candid::Nat,
pub wasm_memory_limit: Option<candid::Nat>,
pub wasm_memory_threshold: Option<candid::Nat>,
}

impl From<ic_management_canister_types::DefiniteCanisterSettingsArgs>
Expand All @@ -330,6 +340,7 @@ impl From<ic_management_canister_types::DefiniteCanisterSettingsArgs>
memory_allocation: settings.memory_allocation(),
freezing_threshold: settings.freezing_threshold(),
wasm_memory_limit: Some(settings.wasm_memory_limit()),
wasm_memory_threshold: Some(settings.wasm_memory_threshold()),
}
}
}
Expand All @@ -341,6 +352,7 @@ impl DefiniteCanisterSettingsArgs {
memory_allocation: Option<u64>,
freezing_threshold: u64,
wasm_memory_limit: Option<u64>,
wasm_memory_threshold: Option<u64>,
) -> Self {
let memory_allocation = match memory_allocation {
None => candid::Nat::from(0_u32),
Expand All @@ -352,6 +364,7 @@ impl DefiniteCanisterSettingsArgs {
memory_allocation,
freezing_threshold: candid::Nat::from(freezing_threshold),
wasm_memory_limit: wasm_memory_limit.map(candid::Nat::from),
wasm_memory_threshold: wasm_memory_threshold.map(candid::Nat::from),
}
}

Expand Down Expand Up @@ -383,6 +396,7 @@ impl From<CanisterStatusResultFromManagementCanister> for CanisterStatusResultV2
memory_allocation: value.settings.memory_allocation,
freezing_threshold: value.settings.freezing_threshold,
wasm_memory_limit: Some(value.settings.wasm_memory_limit),
wasm_memory_threshold: Some(value.settings.wasm_memory_threshold),
},
memory_size: value.memory_size,
cycles: value.cycles,
Expand Down Expand Up @@ -417,6 +431,7 @@ mod tests {
reserved_cycles_limit: candid::Nat::from(96_u32),
wasm_memory_limit: candid::Nat::from(95_u32),
log_visibility: LogVisibility::Controllers,
wasm_memory_threshold: candid::Nat::from(94_u32),
},
cycles: candid::Nat::from(999_u32),
idle_cycles_burned_per_day: candid::Nat::from(998_u32),
Expand All @@ -435,6 +450,7 @@ mod tests {
reserved_cycles_limit: Some(candid::Nat::from(96_u32)),
wasm_memory_limit: Some(candid::Nat::from(95_u32)),
log_visibility: Some(LogVisibility::Controllers),
wasm_memory_threshold: Some(candid::Nat::from(94_u32)),
},
cycles: candid::Nat::from(999_u32),
idle_cycles_burned_per_day: Some(candid::Nat::from(998_u32)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ async fn test_limit_outstanding_calls() {
reserved_cycles_limit: zero.clone(),
wasm_memory_limit: zero.clone(),
log_visibility: LogVisibility::Controllers,
wasm_memory_threshold: zero.clone(),
},
status: CanisterStatusType::Running,
reserved_cycles: zero.clone(),
Expand Down
1 change: 1 addition & 0 deletions rs/nervous_system/clients/src/update_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub struct CanisterSettings {
pub reserved_cycles_limit: Option<candid::Nat>,
pub log_visibility: Option<LogVisibility>,
pub wasm_memory_limit: Option<candid::Nat>,
pub wasm_memory_threshold: Option<candid::Nat>,
}

/// A wrapper call to the management canister `update_settings` API.
Expand Down
2 changes: 2 additions & 0 deletions rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2713,6 +2713,8 @@ pub mod update_canister_settings {
pub log_visibility: Option<i32>,
#[prost(uint64, optional, tag = "6")]
pub wasm_memory_limit: Option<u64>,
#[prost(uint64, optional, tag = "7")]
pub wasm_memory_threshold: Option<u64>,
}
/// Log visibility of a canister.
#[derive(
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/canister/governance.did
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type CanisterSettings = record {
wasm_memory_limit : opt nat64;
memory_allocation : opt nat64;
compute_allocation : opt nat64;
wasm_memory_threshold : opt nat64;
};

type CanisterStatusResultV2 = record {
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/canister/governance_test.did
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type CanisterSettings = record {
wasm_memory_limit : opt nat64;
memory_allocation : opt nat64;
compute_allocation : opt nat64;
wasm_memory_threshold : opt nat64;
};

type CanisterStatusResultV2 = record {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2257,6 +2257,7 @@ message UpdateCanisterSettings {
optional uint64 freezing_threshold = 4;
optional LogVisibility log_visibility = 5;
optional uint64 wasm_memory_limit = 6;
optional uint64 wasm_memory_threshold = 7;
}

// The settings to update. Required.
Expand Down
2 changes: 2 additions & 0 deletions rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3263,6 +3263,8 @@ pub mod update_canister_settings {
pub log_visibility: ::core::option::Option<i32>,
#[prost(uint64, optional, tag = "6")]
pub wasm_memory_limit: ::core::option::Option<u64>,
#[prost(uint64, optional, tag = "7")]
pub wasm_memory_threshold: ::core::option::Option<u64>,
}
/// Log visibility of a canister.
#[derive(
Expand Down
2 changes: 2 additions & 0 deletions rs/nns/governance/src/pb/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3060,6 +3060,7 @@ impl From<pb::update_canister_settings::CanisterSettings>
freezing_threshold: item.freezing_threshold,
log_visibility: item.log_visibility,
wasm_memory_limit: item.wasm_memory_limit,
wasm_memory_threshold: item.wasm_memory_threshold,
}
}
}
Expand All @@ -3075,6 +3076,7 @@ impl From<pb_api::update_canister_settings::CanisterSettings>
freezing_threshold: item.freezing_threshold,
log_visibility: item.log_visibility,
wasm_memory_limit: item.wasm_memory_limit,
wasm_memory_threshold: item.wasm_memory_threshold,
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions rs/nns/governance/src/proposals/update_canister_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ impl UpdateCanisterSettings {
&& settings.freezing_threshold.is_none()
&& settings.log_visibility.is_none()
&& settings.wasm_memory_limit.is_none()
&& settings.wasm_memory_threshold.is_none()
{
return Err(invalid_proposal_error(
"At least one setting must be provided",
Expand All @@ -75,6 +76,7 @@ impl UpdateCanisterSettings {
let memory_allocation = settings.memory_allocation.map(Nat::from);
let freezing_threshold = settings.freezing_threshold.map(Nat::from);
let wasm_memory_limit = settings.wasm_memory_limit.map(Nat::from);
let wasm_memory_threshold = settings.wasm_memory_threshold.map(Nat::from);
let log_visibility = match settings.log_visibility {
Some(log_visibility) => Some(Self::valid_log_visibility(log_visibility)?),
None => None,
Expand All @@ -89,6 +91,7 @@ impl UpdateCanisterSettings {
wasm_memory_limit,
log_visibility,
reserved_cycles_limit,
wasm_memory_threshold,
})
}

Expand Down Expand Up @@ -223,6 +226,7 @@ mod tests {
}),
memory_allocation: Some(1 << 32),
wasm_memory_limit: Some(1 << 31),
wasm_memory_threshold: Some(1 << 30),
compute_allocation: Some(10),
freezing_threshold: Some(100),
log_visibility: Some(LogVisibility::Public as i32),
Expand Down Expand Up @@ -257,6 +261,7 @@ mod tests {
freezing_threshold: Some(Nat::from(100u64)),
log_visibility: Some(RootLogVisibility::Public),
reserved_cycles_limit: None,
wasm_memory_threshold: Some(Nat::from(1u64 << 30)),
}
}
);
Expand All @@ -273,6 +278,7 @@ mod tests {
}),
memory_allocation: Some(1 << 32),
wasm_memory_limit: Some(1 << 31),
wasm_memory_threshold: Some(1 << 30),
compute_allocation: Some(10),
freezing_threshold: Some(100),
log_visibility: Some(LogVisibility::Public as i32),
Expand Down Expand Up @@ -305,6 +311,7 @@ mod tests {
freezing_threshold: Some(Nat::from(100u64)),
log_visibility: Some(RootLogVisibility::Public),
reserved_cycles_limit: None,
wasm_memory_threshold: Some(Nat::from(1u64 << 30)),
}
);
}
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/tests/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12644,6 +12644,7 @@ lazy_static! {
448076, // freezing_threshold
268693, // idle_cycles_burned_per_day
(3.5 * (1 << 30) as f32) as u64, // wasm_memory_limit (3.5gb)
123478, // wasm_memory_threshold
)),
}),
governance: Some(ic_sns_root::CanisterSummary {
Expand Down
1 change: 1 addition & 0 deletions rs/nns/handlers/lifeline/impl/lifeline.did
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type CanisterSettings = record {
reserved_cycles_limit: opt nat;
wasm_memory_limit: opt nat;
log_visibility: opt LogVisibility;
wasm_memory_threshold: opt nat;
};
type LogVisibility = variant { controllers; public };

Expand Down
1 change: 1 addition & 0 deletions rs/nns/handlers/lifeline/impl/lifeline.mo
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ actor {
reserved_cycles_limit: ?Nat;
wasm_memory_limit: ?Nat;
log_visibility: ?LogVisibility;
wasm_memory_threshold: ?Nat;
};

// IC00 is the management canister. We rely on it for the four
Expand Down
2 changes: 2 additions & 0 deletions rs/nns/handlers/root/impl/canister/root.did
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type CanisterSettings = record {
wasm_memory_limit : opt nat;
memory_allocation : opt nat;
compute_allocation : opt nat;
wasm_memory_threshold : opt nat;
};

type CanisterStatusResult = record {
Expand Down Expand Up @@ -85,6 +86,7 @@ type DefiniteCanisterSettings = record {
wasm_memory_limit : opt nat;
memory_allocation : opt nat;
compute_allocation : opt nat;
wasm_memory_threshold : opt nat;
};

type LogVisibility = variant {
Expand Down
1 change: 1 addition & 0 deletions rs/nns/handlers/root/interface/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ impl SpyNnsRootCanisterClientReply {
reserved_cycles_limit: Some(candid::Nat::from(10_u32)),
wasm_memory_limit: Some(candid::Nat::from(11_u32)),
log_visibility: Some(LogVisibility::Controllers),
wasm_memory_threshold: Some(candid::Nat::from(6_u32)),
},
cycles: candid::Nat::from(42_u32),
idle_cycles_burned_per_day: Some(candid::Nat::from(43_u32)),
Expand Down
13 changes: 12 additions & 1 deletion rs/nns/integration_tests/src/update_canister_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ fn test_update_canister_settings_proposal(
let target_memory_allocation = 1u64 << 33;
let target_compute_allocation = 10u64;
let target_freezing_threshold = 100_000u64;
let target_wasm_memory_limit = 1u64 << 32;
// `target_wasm_memory_limit` needs to be larger than `target_wasm_memory_threshold`.
let target_wasm_memory_limit = 1u64 << 36;
let target_wasm_memory_threshold = 1u64 << 34;
let target_log_visibility = Some(LogVisibility::Public);
let canister_settings = || -> DefiniteCanisterSettings {
get_canister_status(
Expand Down Expand Up @@ -66,6 +68,10 @@ fn test_update_canister_settings_proposal(
original_settings.wasm_memory_limit,
Some(Nat::from(target_wasm_memory_limit))
);
assert_ne!(
original_settings.wasm_memory_threshold,
Some(Nat::from(target_wasm_memory_threshold))
);
assert_ne!(original_settings.log_visibility, target_log_visibility);

// Step 3: Make a proposal to update settings of the registry canister and make sure the
Expand All @@ -88,6 +94,7 @@ fn test_update_canister_settings_proposal(
freezing_threshold: Some(target_freezing_threshold),
wasm_memory_limit: Some(target_wasm_memory_limit),
log_visibility: Some(GovernanceLogVisibility::Public as i32),
wasm_memory_threshold: Some(target_wasm_memory_threshold),
}),
},
)),
Expand Down Expand Up @@ -119,6 +126,10 @@ fn test_update_canister_settings_proposal(
updated_settings.wasm_memory_limit,
Some(Nat::from(target_wasm_memory_limit))
);
assert_eq!(
updated_settings.wasm_memory_threshold,
Some(Nat::from(target_wasm_memory_threshold))
);
assert_eq!(updated_settings.log_visibility, target_log_visibility);
}

Expand Down
7 changes: 6 additions & 1 deletion rs/registry/admin/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1198,11 +1198,14 @@ struct ProposeToUpdateCanisterSettingsCmd {
/// If set, it will update the canister's freezing threshold to this value.
freezing_threshold: Option<u64>,
#[clap(long)]
/// If set, it will update the canister's log wasm memory limit to this value.
/// If set, it will update the canister's wasm memory limit to this value.
wasm_memory_limit: Option<u64>,
#[clap(long)]
/// If set, it will update the canister's log visibility to this value.
log_visibility: Option<LogVisibility>,
#[clap(long)]
/// If set, it will update the canister's wasm memory threshold to this value.
wasm_memory_threshold: Option<u64>,
}

impl ProposalTitle for ProposeToUpdateCanisterSettingsCmd {
Expand Down Expand Up @@ -1232,6 +1235,7 @@ impl ProposalAction for ProposeToUpdateCanisterSettingsCmd {
let memory_allocation = self.memory_allocation;
let freezing_threshold = self.freezing_threshold;
let wasm_memory_limit = self.wasm_memory_limit;
let wasm_memory_threshold = self.wasm_memory_threshold;
let log_visibility = match self.log_visibility {
Some(LogVisibility::Controllers) => Some(GovernanceLogVisibility::Controllers as i32),
Some(LogVisibility::Public) => Some(GovernanceLogVisibility::Public as i32),
Expand All @@ -1247,6 +1251,7 @@ impl ProposalAction for ProposeToUpdateCanisterSettingsCmd {
freezing_threshold,
wasm_memory_limit,
log_visibility,
wasm_memory_threshold,
}),
};

Expand Down
2 changes: 2 additions & 0 deletions rs/sns/governance/api/src/ic_sns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,8 @@ pub struct ManageDappCanisterSettings {
pub log_visibility: ::core::option::Option<i32>,
#[prost(uint64, optional, tag = "7")]
pub wasm_memory_limit: ::core::option::Option<u64>,
#[prost(uint64, optional, tag = "8")]
pub wasm_memory_threshold: ::core::option::Option<u64>,
}
/// Unlike `Governance.Version`, this message has optional fields and is the recommended one
/// to use in APIs that can evolve. For example, the SNS Governance could eventually support
Expand Down
Loading

0 comments on commit 4c775db

Please sign in to comment.