Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: (ic-utils) support canister upgrade option wasm_memory_persistence #502

Merged
merged 9 commits into from
Jul 15, 2024
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* Removed the Bitcoin query methods from `ManagementCanister`. Users should use `BitcoinCanister` for that.
* Added `BitcoinCanister` to `ic-utils`.
* Upgraded MSRV to 1.75.0.
* Changed `ic_utils::interfaces::management_canister::builders::InstallMode::Upgrade` variant to be `Option<CanisterUpgradeOptions>`:
* `CanisterUpgradeOptions` is a new struct which covers the new upgrade option: `wasm_memory_persistence: Option<WasmMemoryPersistence>`.
* `WasmMemoryPersistence` is a new enum which controls Wasm main memory retention on upgrades which has two variants: `Keep` and `Replace`.

## [0.36.0] - 2024-06-04

Expand Down
2 changes: 1 addition & 1 deletion ic-agent/src/agent/http_transport/route_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ mod tests {
fn test_routes_rotation() {
let provider = RoundRobinRouteProvider::new(vec!["https://url1.com", "https://url2.com"])
.expect("failed to create a route provider");
let url_strings = vec![
let url_strings = [
"https://url1.com/api/v2/",
"https://url2.com/api/v2/",
"https://url1.com/api/v2/",
Expand Down
4 changes: 2 additions & 2 deletions ic-identity-hsm/src/hsm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ fn get_ec_point(

let blocks =
from_der(der_encoded_ec_point.as_slice()).map_err(HardwareIdentityError::ASN1Decode)?;
let block = blocks.get(0).ok_or(HardwareIdentityError::EcPointEmpty)?;
let block = blocks.first().ok_or(HardwareIdentityError::EcPointEmpty)?;
if let OctetString(_size, data) = block {
Ok(data.clone())
} else {
Expand All @@ -302,7 +302,7 @@ fn get_attribute_length(
ctx.get_attribute_value(session_handle, object_handle, &mut attributes)?;

let first = attributes
.get(0)
.first()
.ok_or(HardwareIdentityError::AttributeNotFound(attribute_type))?;
Ok(first.ulValueLen as usize)
}
Expand Down
162 changes: 81 additions & 81 deletions ic-transport-types/src/request_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,87 @@ impl SerializeTupleVariant for TupleVariantSerializer {
}
}

// can't use serde_bytes on by-value arrays
// these impls are effectively #[serde(with = "serde_bytes")]
impl Serialize for RequestId {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
if serializer.is_human_readable() {
let mut text = [0u8; 64];
hex::encode_to_slice(self.0, &mut text).unwrap();
serializer.serialize_str(std::str::from_utf8(&text).unwrap())
} else {
serializer.serialize_bytes(&self.0)
}
}
}

impl<'de> Deserialize<'de> for RequestId {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
if deserializer.is_human_readable() {
deserializer.deserialize_str(RequestIdVisitor)
} else {
deserializer.deserialize_bytes(RequestIdVisitor)
}
}
}

struct RequestIdVisitor;

impl<'de> Visitor<'de> for RequestIdVisitor {
type Value = RequestId;
fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str("a sha256 hash")
}

fn visit_bytes<E>(self, v: &[u8]) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(RequestId::new(v.try_into().map_err(|_| {
E::custom(format_args!("must be 32 bytes long, was {}", v.len()))
})?))
}

fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
where
A: de::SeqAccess<'de>,
{
let mut arr = Sha256Hash::default();
for (i, byte) in arr.iter_mut().enumerate() {
*byte = seq.next_element()?.ok_or(A::Error::custom(format_args!(
"must be 32 bytes long, was {}",
i - 1
)))?;
}
if seq.next_element::<u8>()?.is_some() {
Err(A::Error::custom("must be 32 bytes long, was more"))
} else {
Ok(RequestId(arr))
}
}

fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
where
E: de::Error,
{
if v.len() != 64 {
return Err(E::custom(format_args!(
"must be 32 bytes long, was {}",
v.len() / 2
)));
}
let mut arr = Sha256Hash::default();
hex::decode_to_slice(v, &mut arr).map_err(E::custom)?;
Ok(RequestId(arr))
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -877,84 +958,3 @@ mod tests {
);
}
}

// can't use serde_bytes on by-value arrays
// these impls are effectively #[serde(with = "serde_bytes")]
impl Serialize for RequestId {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
if serializer.is_human_readable() {
let mut text = [0u8; 64];
hex::encode_to_slice(self.0, &mut text).unwrap();
serializer.serialize_str(std::str::from_utf8(&text).unwrap())
} else {
serializer.serialize_bytes(&self.0)
}
}
}

impl<'de> Deserialize<'de> for RequestId {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
if deserializer.is_human_readable() {
deserializer.deserialize_str(RequestIdVisitor)
} else {
deserializer.deserialize_bytes(RequestIdVisitor)
}
}
}

struct RequestIdVisitor;

impl<'de> Visitor<'de> for RequestIdVisitor {
type Value = RequestId;
fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str("a sha256 hash")
}

fn visit_bytes<E>(self, v: &[u8]) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(RequestId::new(v.try_into().map_err(|_| {
E::custom(format_args!("must be 32 bytes long, was {}", v.len()))
})?))
}

fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
where
A: de::SeqAccess<'de>,
{
let mut arr = Sha256Hash::default();
for (i, byte) in arr.iter_mut().enumerate() {
*byte = seq.next_element()?.ok_or(A::Error::custom(format_args!(
"must be 32 bytes long, was {}",
i - 1
)))?;
}
if seq.next_element::<u8>()?.is_some() {
Err(A::Error::custom("must be 32 bytes long, was more"))
} else {
Ok(RequestId(arr))
}
}

fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
where
E: de::Error,
{
if v.len() != 64 {
return Err(E::custom(format_args!(
"must be 32 bytes long, was {}",
v.len() / 2
)));
}
let mut arr = Sha256Hash::default();
hex::decode_to_slice(v, &mut arr).map_err(E::custom)?;
Ok(RequestId(arr))
}
}
32 changes: 24 additions & 8 deletions ic-utils/src/interfaces/management_canister/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,27 @@ impl<'agent, 'canister: 'agent> IntoFuture for CreateCanisterBuilder<'agent, 'ca
}
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, CandidType, Copy)]
/// Wasm main memory retention on upgrades.
/// Currently used to specify the persistence of Wasm main memory.
pub enum WasmMemoryPersistence {
/// Retain the main memory across upgrades.
/// Used for enhanced orthogonal persistence, as implemented in Motoko
Keep,
/// Reinitialize the main memory on upgrade.
/// Default behavior without enhanced orthogonal persistence.
Replace,
}

#[derive(Debug, Copy, Clone, CandidType, Deserialize, Eq, PartialEq)]
/// Upgrade options.
pub struct CanisterUpgradeOptions {
/// Skip pre-upgrade hook. Only for exceptional cases, see the IC documentation. Not useful for Motoko.
pub skip_pre_upgrade: Option<bool>,
/// Support for enhanced orthogonal persistence: Retain the main memory on upgrade.
pub wasm_memory_persistence: Option<WasmMemoryPersistence>,
}

/// The install mode of the canister to install. If a canister is already installed,
/// using [InstallMode::Install] will be an error. [InstallMode::Reinstall] overwrites
/// the module, and [InstallMode::Upgrade] performs an Upgrade step.
Expand All @@ -484,12 +505,9 @@ pub enum InstallMode {
/// Overwrite the canister with this module.
#[serde(rename = "reinstall")]
Reinstall,
/// Upgrade the canister with this module.
/// Upgrade the canister with this module and some options.
#[serde(rename = "upgrade")]
Upgrade {
/// If true, skip a canister's `#[pre_upgrade]` function.
skip_pre_upgrade: Option<bool>,
},
Upgrade(Option<CanisterUpgradeOptions>),
}

/// A prepared call to `install_code`.
Expand All @@ -514,9 +532,7 @@ impl FromStr for InstallMode {
match s {
"install" => Ok(InstallMode::Install),
"reinstall" => Ok(InstallMode::Reinstall),
"upgrade" => Ok(InstallMode::Upgrade {
skip_pre_upgrade: Some(false),
}),
"upgrade" => Ok(InstallMode::Upgrade(None)),
&_ => Err(format!("Invalid install mode: {}", s)),
}
}
Expand Down
25 changes: 15 additions & 10 deletions ref-tests/tests/ic-ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ mod management_canister {
call::AsyncCall,
interfaces::{
management_canister::{
builders::{CanisterSettings, InstallMode},
builders::{
CanisterSettings, CanisterUpgradeOptions, InstallMode, WasmMemoryPersistence,
},
CanisterStatus, StatusCallResult,
},
wallet::CreateResult,
Expand Down Expand Up @@ -161,18 +163,20 @@ mod management_canister {

// Upgrade should succeed.
ic00.install_code(&canister_id, &canister_wasm)
.with_mode(InstallMode::Upgrade {
skip_pre_upgrade: None,
})
.with_mode(InstallMode::Upgrade(Some(CanisterUpgradeOptions {
skip_pre_upgrade: Some(true),
wasm_memory_persistence: None,
})))
.call_and_wait()
.await?;

// Upgrade with another agent should fail.
let result = other_ic00
.install_code(&canister_id, &canister_wasm)
.with_mode(InstallMode::Upgrade {
.with_mode(InstallMode::Upgrade(Some(CanisterUpgradeOptions {
skip_pre_upgrade: None,
})
wasm_memory_persistence: Some(WasmMemoryPersistence::Keep),
})))
.call_and_wait()
.await;
assert!(matches!(result, Err(AgentError::UncertifiedReject(..))));
Expand Down Expand Up @@ -302,7 +306,7 @@ mod management_canister {
.iter()
.cloned()
.collect::<HashSet<_>>();
let expected = vec![agent_principal, other_agent_principal]
let expected = [agent_principal, other_agent_principal]
.iter()
.cloned()
.collect::<HashSet<_>>();
Expand All @@ -320,7 +324,7 @@ mod management_canister {
.iter()
.cloned()
.collect::<HashSet<_>>();
let expected = vec![agent_principal, other_agent_principal]
let expected = [agent_principal, other_agent_principal]
.iter()
.cloned()
.collect::<HashSet<_>>();
Expand Down Expand Up @@ -485,9 +489,10 @@ mod management_canister {

// Upgrade should succeed
ic00.install_code(&canister_id, &canister_wasm)
.with_mode(InstallMode::Upgrade {
.with_mode(InstallMode::Upgrade(Some(CanisterUpgradeOptions {
skip_pre_upgrade: None,
})
wasm_memory_persistence: Some(WasmMemoryPersistence::Replace),
})))
.call_and_wait()
.await?;

Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
# MSRV
# Avoid updating this field unless we use new Rust features
# Sync rust-version in workspace Cargo.toml
channel = "1.70.0"
channel = "1.75.0"
components = ["rustfmt", "clippy"]
targets = ["wasm32-unknown-unknown"]