From 121636415bf79442fea2215e2ccc172b91de773d Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Wed, 31 Jul 2024 16:56:42 +0000 Subject: [PATCH 01/42] Fix test issue --- tests/integration/src/dfx_orbit/assets.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/integration/src/dfx_orbit/assets.rs b/tests/integration/src/dfx_orbit/assets.rs index 93647abb1..785e10122 100644 --- a/tests/integration/src/dfx_orbit/assets.rs +++ b/tests/integration/src/dfx_orbit/assets.rs @@ -8,7 +8,7 @@ use station_api::{ RequestSpecifierDTO, ValidationMethodResourceTargetDTO, }; use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use tempfile::tempdir; +use tempfile::Builder; use crate::{ dfx_orbit::{ @@ -66,7 +66,10 @@ fn assets_update() { // Setup a tmpdir, and store two assets in it // We generate the assets dyniamically, since we want to make sure we are not // fetching old assets - let asset_dir = tempdir().unwrap(); + // NOTE: Currently, the local asset computation skips hidden files while the + // remote version does not. This creates an issue if we just used tempdir(), as that + // uses `.` prefix. + let asset_dir = Builder::new().prefix("asset").tempdir().unwrap(); let asset_a = format!( "This is the current time: {}", SystemTime::now() From d6ee0b41ba1693f1337759848a873daef5c347af Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 12:45:20 +0000 Subject: [PATCH 02/42] Update dependencies --- Cargo.lock | 70 +++++++++++++++++++++++++++++++++++++++++++----------- Cargo.toml | 10 ++++---- 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bbf69a21c..8f831ad05 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -85,6 +85,21 @@ dependencies = [ "memchr", ] +[[package]] +name = "alloc-no-stdlib" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc7bb162ec39d46ab1ca8c77bf72e890535becd1751bb45f64c597edb4c8c6b3" + +[[package]] +name = "alloc-stdlib" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94fb8275041c72129eb51b7d0322c29b8387a0386127718b096429201a5d6ece" +dependencies = [ + "alloc-no-stdlib", +] + [[package]] name = "allocator-api2" version = "0.2.18" @@ -443,6 +458,27 @@ dependencies = [ "subtle", ] +[[package]] +name = "brotli" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74f7971dbd9326d58187408ab83117d8ac1bb9c17b085fdacd1cf2f598719b6b" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", + "brotli-decompressor", +] + +[[package]] +name = "brotli-decompressor" +version = "4.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a45bd2e4095a8b518033b128020dd4a55aab1c0a381ba4404a472630f4bc362" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", +] + [[package]] name = "bs58" version = "0.4.0" @@ -1041,10 +1077,11 @@ checksum = "339544cc9e2c4dc3fc7149fd630c5f22263a4fdf18a98afd0075784968b5cf00" [[package]] name = "dfx-core" version = "0.0.1" -source = "git+https://github.com/dfinity/sdk.git?tag=0.20.2-beta.0#06eaa58f9c52d27a015cd8f754ecd3d0d0ba555b" +source = "git+https://github.com/dfinity/sdk.git?tag=0.22.0#d0c8be188d1a19c1908f148e3bc5331b62de780a" dependencies = [ "aes-gcm", "argon2", + "backoff", "bip32", "byte-unit", "bytes", @@ -1070,6 +1107,7 @@ dependencies = [ "semver", "serde", "serde_json", + "sha2 0.10.8", "slog", "tar", "tempfile", @@ -1082,10 +1120,11 @@ dependencies = [ [[package]] name = "dfx-core" version = "0.0.1" -source = "git+https://github.com/dfinity/sdk.git?rev=ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e#ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e" +source = "git+https://github.com/dfinity/sdk.git?branch=leon/upload_and_propose#8e1ca2dbb5e146400d73c19f9cbbc876d20643c2" dependencies = [ "aes-gcm", "argon2", + "backoff", "bip32", "byte-unit", "bytes", @@ -1111,6 +1150,7 @@ dependencies = [ "semver", "serde", "serde_json", + "sha2 0.10.8", "slog", "tar", "tempfile", @@ -1129,7 +1169,7 @@ dependencies = [ "candid_parser", "cap-std", "clap", - "dfx-core 0.0.1 (git+https://github.com/dfinity/sdk.git?tag=0.20.2-beta.0)", + "dfx-core 0.0.1 (git+https://github.com/dfinity/sdk.git?tag=0.22.0)", "ic-agent", "ic-asset", "ic-certified-assets", @@ -1934,8 +1974,8 @@ dependencies = [ [[package]] name = "ic-agent" -version = "0.35.0" -source = "git+https://github.com/dfinity/agent-rs.git?rev=8273d321e9a09fd8373bd4e38b0676ec6ad9c260#8273d321e9a09fd8373bd4e38b0676ec6ad9c260" +version = "0.36.0" +source = "git+https://github.com/dfinity/agent-rs.git?rev=be929fd7967249c879f48f2f494cbfc5805a7d98#be929fd7967249c879f48f2f494cbfc5805a7d98" dependencies = [ "async-lock 3.4.0", "backoff", @@ -1975,12 +2015,13 @@ dependencies = [ [[package]] name = "ic-asset" version = "0.20.0" -source = "git+https://github.com/dfinity/sdk.git?rev=ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e#ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e" +source = "git+https://github.com/dfinity/sdk.git?branch=leon/upload_and_propose#8e1ca2dbb5e146400d73c19f9cbbc876d20643c2" dependencies = [ "backoff", + "brotli", "candid", "derivative", - "dfx-core 0.0.1 (git+https://github.com/dfinity/sdk.git?rev=ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e)", + "dfx-core 0.0.1 (git+https://github.com/dfinity/sdk.git?branch=leon/upload_and_propose)", "flate2", "futures", "futures-intrusive", @@ -2131,7 +2172,7 @@ dependencies = [ [[package]] name = "ic-certified-assets" version = "0.2.5" -source = "git+https://github.com/dfinity/sdk.git?rev=ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e#ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e" +source = "git+https://github.com/dfinity/sdk.git?branch=leon/upload_and_propose#8e1ca2dbb5e146400d73c19f9cbbc876d20643c2" dependencies = [ "base64 0.13.1", "candid", @@ -2142,6 +2183,7 @@ dependencies = [ "ic-response-verification", "itertools 0.10.5", "num-traits", + "percent-encoding", "serde", "serde_bytes", "serde_cbor", @@ -2165,8 +2207,8 @@ dependencies = [ [[package]] name = "ic-identity-hsm" -version = "0.35.0" -source = "git+https://github.com/dfinity/agent-rs.git?rev=8273d321e9a09fd8373bd4e38b0676ec6ad9c260#8273d321e9a09fd8373bd4e38b0676ec6ad9c260" +version = "0.36.0" +source = "git+https://github.com/dfinity/agent-rs.git?rev=be929fd7967249c879f48f2f494cbfc5805a7d98#be929fd7967249c879f48f2f494cbfc5805a7d98" dependencies = [ "hex", "ic-agent", @@ -2236,8 +2278,8 @@ dependencies = [ [[package]] name = "ic-transport-types" -version = "0.35.0" -source = "git+https://github.com/dfinity/agent-rs.git?rev=8273d321e9a09fd8373bd4e38b0676ec6ad9c260#8273d321e9a09fd8373bd4e38b0676ec6ad9c260" +version = "0.36.0" +source = "git+https://github.com/dfinity/agent-rs.git?rev=be929fd7967249c879f48f2f494cbfc5805a7d98#be929fd7967249c879f48f2f494cbfc5805a7d98" dependencies = [ "candid", "hex", @@ -2252,8 +2294,8 @@ dependencies = [ [[package]] name = "ic-utils" -version = "0.35.0" -source = "git+https://github.com/dfinity/agent-rs.git?rev=8273d321e9a09fd8373bd4e38b0676ec6ad9c260#8273d321e9a09fd8373bd4e38b0676ec6ad9c260" +version = "0.36.0" +source = "git+https://github.com/dfinity/agent-rs.git?rev=be929fd7967249c879f48f2f494cbfc5805a7d98#be929fd7967249c879f48f2f494cbfc5805a7d98" dependencies = [ "async-trait", "candid", diff --git a/Cargo.toml b/Cargo.toml index 3050eb0b6..4b52a5f3d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,21 +37,21 @@ candid = "0.10.3" candid_parser = "0.1.3" cap-std = "3.1.0" clap = { version = "4.5.7", features = ["derive"] } -dfx-core = { git = "https://github.com/dfinity/sdk.git", tag = "0.20.2-beta.0" } +dfx-core = { git = "https://github.com/dfinity/sdk.git", tag = "0.22.0" } convert_case = "0.6" futures = "0.3" getrandom = { version = "0.2", features = ["custom"] } hex = "0.4" # The ic-agent matches the one sed by bthe -ic-agent = { git = "https://github.com/dfinity/agent-rs.git", rev = "8273d321e9a09fd8373bd4e38b0676ec6ad9c260" } -ic-asset = { git = "https://github.com/dfinity/sdk.git", rev = "ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e" } -ic-certified-assets = { git = "https://github.com/dfinity/sdk.git", rev = "ae10a96b381cfce3d8ac5a6cb940d19224ea6d2e" } +ic-agent = { git = "https://github.com/dfinity/agent-rs.git", rev = "be929fd7967249c879f48f2f494cbfc5805a7d98" } +ic-asset = { git = "https://github.com/dfinity/sdk.git", branch = "leon/upload_and_propose" } +ic-certified-assets = { git = "https://github.com/dfinity/sdk.git", branch = "leon/upload_and_propose" } ic-cdk = "0.13.2" ic-cdk-macros = "0.9" ic-cdk-timers = "0.7.0" ic-ledger-types = "0.10.0" ic-stable-structures = "0.6.4" -ic-utils = { git = "https://github.com/dfinity/agent-rs.git", rev = "8273d321e9a09fd8373bd4e38b0676ec6ad9c260" } +ic-utils = { git = "https://github.com/dfinity/agent-rs.git", rev = "be929fd7967249c879f48f2f494cbfc5805a7d98" } lazy_static = "1.4.0" mockall = "0.12.1" num-bigint = "0.4" From 6dbd40e40166d6dcbc6c32838baac652f588d7f5 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 13:16:58 +0000 Subject: [PATCH 03/42] Move logger up a layer --- tools/dfx-orbit/src/cli/asset.rs | 2 +- tools/dfx-orbit/src/lib.rs | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 08a59ea39..67c09a1be 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -42,7 +42,7 @@ impl DfxOrbit { .collect(); let canister_id = self.canister_id(&canister)?; - let logger = self.dfx.logger().clone(); + let logger = self.logger.clone(); // Upload assets: let canister_agent = CanisterBuilder::new() diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index 00ab7706f..68805a6a9 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -13,6 +13,8 @@ pub mod station_agent; use candid::Principal; use dfx_core::{config::model::canister_id_store::CanisterIdStore, DfxInterface}; use dfx_extension_api::OrbitExtensionAgent; +use ic_utils::{canister::CanisterBuilder, Canister}; +use slog::{o, Drain, Logger}; pub use station_agent::StationAgent; pub struct DfxOrbit { @@ -22,6 +24,8 @@ pub struct DfxOrbit { pub dfx: OrbitExtensionAgent, // The dfx interface pub interface: DfxInterface, + /// A logger; some public `sdk` repository methods require a specific type of logger so this is a compatible logger. + logger: Logger, } impl DfxOrbit { @@ -32,17 +36,23 @@ impl DfxOrbit { .ok_or_else(|| anyhow::format_err!("No default station specified"))?; let interface = agent.dfx_interface().await?; + let decorator = slog_term::TermDecorator::new().build(); + let drain = slog_term::FullFormat::new(decorator).build().fuse(); + let drain = slog_async::Async::new(drain).build().fuse(); + let logger = slog::Logger::root(drain, o!()); + Ok(Self { station: StationAgent::new(interface.agent().clone(), config), dfx: agent, interface, + logger, }) } /// Gets the ID of a given canister name. If the name is already an ID, it is returned as is. pub fn canister_id(&self, canister_name: &str) -> anyhow::Result { let canister_id_store = CanisterIdStore::new( - self.dfx.logger(), + &self.logger, self.interface.network_descriptor(), self.interface.config(), )?; @@ -52,4 +62,11 @@ impl DfxOrbit { Ok(canister_id) } + + pub fn canister_agent(&self, canister_id: Principal) -> anyhow::Result { + Ok(CanisterBuilder::new() + .with_agent(self.interface.agent()) + .with_canister_id(canister_id) + .build()?) + } } From 52b9415fb30c3992029f26e03c75454fe064b690 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 13:26:41 +0000 Subject: [PATCH 04/42] WIP refactoring asset upload into multiple smaller methods --- tools/dfx-orbit/src/cli/asset.rs | 27 ++++------------ tools/dfx-orbit/src/cli/asset/evidence.rs | 39 +++++++++++++++++++++++ tools/dfx-orbit/src/cli/asset/upload.rs | 0 tools/dfx-orbit/src/dfx_extension_api.rs | 21 ++---------- 4 files changed, 47 insertions(+), 40 deletions(-) create mode 100644 tools/dfx-orbit/src/cli/asset/evidence.rs create mode 100644 tools/dfx-orbit/src/cli/asset/upload.rs diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 67c09a1be..6843d97fc 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -1,12 +1,12 @@ //! Implements the `dfx-orbit canister upload-http-assets` CLI command. +mod evidence; +mod upload; mod util; use crate::DfxOrbit; use candid::{Nat, Principal}; -use ic_asset::canister_api::{ - methods::batch::compute_evidence, types::batch_upload::common::ComputeEvidenceArguments, -}; + use ic_utils::canister::CanisterBuilder; use serde::{Deserialize, Serialize}; use serde_bytes::ByteBuf; @@ -53,26 +53,11 @@ impl DfxOrbit { let batch_id = ic_asset::upload_and_propose(&canister_agent, assets, &logger).await?; println!("Proposed batch_id: {}", batch_id); // Compute evidence locally: - let local_evidence = { - let local_evidence = - ic_asset::compute_evidence(&canister_agent, source_paths.as_ref(), &logger).await?; - escape_hex_string(&local_evidence) - }; + let local_evidence = self.compute_evidence(canister_id, &source_paths).await?; + let local_evidence = escape_hex_string(&local_evidence); // Wait for the canister to compute evidence: - // This part is stolen from ic_asset::sync::prepare_sync_for_proposal. Unfortunately the relevant functions are private. - // The docs explicitly include waiting for the evidence so this should really be made easier! See: https://github.com/dfinity/sdk/blob/2509e81e11e71dce4045c679686c952809525470/docs/design/asset-canister-interface.md?plain=1#L85 - let compute_evidence_arg = ComputeEvidenceArguments { - batch_id: batch_id.clone(), - max_iterations: Some(97), // 75% of max(130) = 97.5 - }; - info!(logger, "Computing evidence."); - let canister_evidence_bytes = loop { - if let Some(evidence) = compute_evidence(&canister_agent, &compute_evidence_arg).await? - { - break evidence; - } - }; + let canister_evidence_bytes = self.request_evidence(canister_id, batch_id.clone()).await?; let canister_evidence = blob_from_bytes(&canister_evidence_bytes); // TODO: Move this out of the agent into the tool diff --git a/tools/dfx-orbit/src/cli/asset/evidence.rs b/tools/dfx-orbit/src/cli/asset/evidence.rs new file mode 100644 index 000000000..c40e8d236 --- /dev/null +++ b/tools/dfx-orbit/src/cli/asset/evidence.rs @@ -0,0 +1,39 @@ +use crate::DfxOrbit; +use candid::{Nat, Principal}; +use ic_asset::canister_api::{ + methods::batch::compute_evidence, types::batch_upload::common::ComputeEvidenceArguments, +}; +use serde_bytes::ByteBuf; +use std::path::Path; + +impl DfxOrbit { + pub async fn request_evidence( + &self, + canister_id: Principal, + batch_id: Nat, + ) -> anyhow::Result { + let canister_agent = self.canister_agent(canister_id)?; + // This part is stolen from ic_asset::sync::prepare_sync_for_proposal. Unfortunately the relevant functions are private. + // The docs explicitly include waiting for the evidence so this should really be made easier! See: https://github.com/dfinity/sdk/blob/2509e81e11e71dce4045c679686c952809525470/docs/design/asset-canister-interface.md?plain=1#L85 + let compute_evidence_arg = ComputeEvidenceArguments { + batch_id: batch_id.clone(), + max_iterations: Some(97), // 75% of max(130) = 97.5 + }; + let canister_evidence_bytes = loop { + if let Some(evidence) = compute_evidence(&canister_agent, &compute_evidence_arg).await? + { + break evidence; + } + }; + Ok(canister_evidence_bytes) + } + + pub async fn compute_evidence( + &self, + canister_id: Principal, + sources: &[&Path], + ) -> anyhow::Result { + let canister_agent = self.canister_agent(canister_id)?; + Ok(ic_asset::compute_evidence(&canister_agent, sources, &self.logger).await?) + } +} diff --git a/tools/dfx-orbit/src/cli/asset/upload.rs b/tools/dfx-orbit/src/cli/asset/upload.rs new file mode 100644 index 000000000..e69de29bb diff --git a/tools/dfx-orbit/src/dfx_extension_api.rs b/tools/dfx-orbit/src/dfx_extension_api.rs index 1f186bdca..6d3978d77 100644 --- a/tools/dfx-orbit/src/dfx_extension_api.rs +++ b/tools/dfx-orbit/src/dfx_extension_api.rs @@ -1,7 +1,7 @@ //! Placeholders for the proposed dfx extension API methods. use anyhow::Context; use dfx_core::interface::dfx::DfxInterface; -use slog::{o, Drain, Logger}; + use std::path::PathBuf; /// The name of the Orbit dfx extension. @@ -11,8 +11,6 @@ const ORBIT_EXTENSION_NAME: &str = "orbit"; pub struct OrbitExtensionAgent { /// The directory where all extension configuration files are stored, including those of other extensions. extensions_dir: cap_std::fs::Dir, - /// A logger; some public `sdk` repository methods require a specific type of logger so this is a compatible logger. - logger: Logger, } impl OrbitExtensionAgent { @@ -24,22 +22,7 @@ impl OrbitExtensionAgent { } fn new_from_dir(extensions_dir: cap_std::fs::Dir) -> Self { - let logger = { - let decorator = slog_term::TermDecorator::new().build(); - let drain = slog_term::FullFormat::new(decorator).build().fuse(); - let drain = slog_async::Async::new(drain).build().fuse(); - - slog::Logger::root(drain, o!()) - }; - Self { - extensions_dir, - logger, - } - } - - /// A logger; some public `sdk` repository methods require a specific type of logger so this is a compatible logger. - pub fn logger(&self) -> &Logger { - &self.logger + Self { extensions_dir } } /// Gets the extensions directory, typically at `~/.config/dfx/extensions` From 9926c6b7a0a72bfa92b20b05a1d09490aa4ff7d1 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 13:37:16 +0000 Subject: [PATCH 05/42] WIP factoring out asset functionality into it's own methods --- tools/dfx-orbit/src/cli/asset.rs | 61 ++--------------------- tools/dfx-orbit/src/cli/asset/evidence.rs | 2 + tools/dfx-orbit/src/cli/asset/upload.rs | 60 ++++++++++++++++++++++ 3 files changed, 67 insertions(+), 56 deletions(-) diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 6843d97fc..c2b9741c3 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -6,16 +6,10 @@ mod util; use crate::DfxOrbit; use candid::{Nat, Principal}; - -use ic_utils::canister::CanisterBuilder; use serde::{Deserialize, Serialize}; use serde_bytes::ByteBuf; use slog::{info, warn}; -use std::{ - collections::HashMap, - path::{Path, PathBuf}, -}; -use walkdir::WalkDir; +use std::path::{Path, PathBuf}; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct AssetUploadRequest { @@ -45,13 +39,10 @@ impl DfxOrbit { let logger = self.logger.clone(); // Upload assets: - let canister_agent = CanisterBuilder::new() - .with_agent(self.interface.agent()) - .with_canister_id(canister_id) - .build()?; - let assets = assets_as_hash_map(&files); - let batch_id = ic_asset::upload_and_propose(&canister_agent, assets, &logger).await?; - println!("Proposed batch_id: {}", batch_id); + let batch_id = self + .upload_assets_actual(canister_id, &source_paths) + .await?; + info!(self.logger, "Proposed batch_id: {}", batch_id); // Compute evidence locally: let local_evidence = self.compute_evidence(canister_id, &source_paths).await?; let local_evidence = escape_hex_string(&local_evidence); @@ -83,48 +74,6 @@ impl DfxOrbit { } } -// TODO: Implement request_upload_commit - -// TODO: Move all these funtions to util -/// Lists all the files at the given path. -/// -/// - Links are followed. -/// - Only files are returned. -/// - The files are sorted by name. -/// - Any files that cannot be read are ignored. -/// - The path includes the prefix. -fn list_assets(path: &str) -> Vec { - WalkDir::new(path) - .sort_by_file_name() - .follow_links(true) - .into_iter() - .filter_map(|e| e.ok()) - .filter(|entry| entry.file_type().is_file()) - .map(|entry| entry.into_path()) - .collect() -} - -/// A hash map of all assets. -/// -/// Note: Given that ordering in a HashMap is not deterministic, is this really the best API? -fn assets_as_hash_map(asset_dirs: &[String]) -> HashMap { - asset_dirs - .iter() - .flat_map(|asset_dir| { - list_assets(asset_dir).into_iter().map(move |asset_path| { - let relative_path = asset_path.strip_prefix(asset_dir).expect( - "Internal error: list_assets should have returned only files in the asset_dir", - ); - let http_path = format!( - "/{relative_path}", - relative_path = relative_path.to_string_lossy() - ); - (http_path, asset_path) - }) - }) - .collect() -} - /// Converts a hex string into one escaped as in a candid blob. fn escape_hex_string(s: &str) -> String { let mut ans = String::with_capacity(s.len() + s.len() / 2); diff --git a/tools/dfx-orbit/src/cli/asset/evidence.rs b/tools/dfx-orbit/src/cli/asset/evidence.rs index c40e8d236..d1291facd 100644 --- a/tools/dfx-orbit/src/cli/asset/evidence.rs +++ b/tools/dfx-orbit/src/cli/asset/evidence.rs @@ -36,4 +36,6 @@ impl DfxOrbit { let canister_agent = self.canister_agent(canister_id)?; Ok(ic_asset::compute_evidence(&canister_agent, sources, &self.logger).await?) } + + // TODO: check_evidence function } diff --git a/tools/dfx-orbit/src/cli/asset/upload.rs b/tools/dfx-orbit/src/cli/asset/upload.rs index e69de29bb..311489fee 100644 --- a/tools/dfx-orbit/src/cli/asset/upload.rs +++ b/tools/dfx-orbit/src/cli/asset/upload.rs @@ -0,0 +1,60 @@ +use std::{ + collections::HashMap, + path::{Path, PathBuf}, +}; + +use candid::{Nat, Principal}; +use walkdir::WalkDir; + +use crate::DfxOrbit; + +impl DfxOrbit { + pub async fn upload_assets_actual( + &self, + canister_id: Principal, + sources: &[&Path], + ) -> anyhow::Result { + let canister_agent = self.canister_agent(canister_id)?; + let assets = assets_as_hash_map(sources); + Ok(ic_asset::upload_and_propose(&canister_agent, assets, &self.logger).await?) + } + + // TODO: Implement request_upload_commit +} + +/// A hash map of all assets. +fn assets_as_hash_map(asset_dirs: &[&Path]) -> HashMap { + asset_dirs + .iter() + .flat_map(|asset_dir| { + list_assets(asset_dir).into_iter().map(move |asset_path| { + let relative_path = asset_path.strip_prefix(asset_dir).expect( + "Internal error: list_assets should have returned only files in the asset_dir", + ); + let http_path = format!( + "/{relative_path}", + relative_path = relative_path.to_string_lossy() + ); + (http_path, asset_path) + }) + }) + .collect() +} + +/// Lists all the files at the given path. +/// +/// - Links are followed. +/// - Only files are returned. +/// - The files are sorted by name. +/// - Any files that cannot be read are ignored. +/// - The path includes the prefix. +fn list_assets(path: &Path) -> Vec { + WalkDir::new(path) + .sort_by_file_name() + .follow_links(true) + .into_iter() + .filter_map(|e| e.ok()) + .filter(|entry| entry.file_type().is_file()) + .map(|entry| entry.into_path()) + .collect() +} From 2c9629d9ea4c0353277e0eeaa58ed82faa6838b2 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 14:05:15 +0000 Subject: [PATCH 06/42] WIP refactoring asset upload --- tools/dfx-orbit/src/cli/asset.rs | 26 +++++++++++++++------- tools/dfx-orbit/src/cli/asset/evidence.rs | 27 ++++++++--------------- tools/dfx-orbit/src/cli/asset/upload.rs | 23 +++++++++---------- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index c2b9741c3..931fc9bfb 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -6,11 +6,17 @@ mod util; use crate::DfxOrbit; use candid::{Nat, Principal}; +use ic_utils::Canister; use serde::{Deserialize, Serialize}; use serde_bytes::ByteBuf; -use slog::{info, warn}; +use slog::{info, warn, Logger}; use std::path::{Path, PathBuf}; +pub struct AssetAgent<'agent> { + canister_agent: Canister<'agent>, + logger: Logger, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct AssetUploadRequest { station_principal: Principal, @@ -19,7 +25,6 @@ pub struct AssetUploadRequest { evidence: ByteBuf, } -// TODO: Use StationAgentResult instead of anyhow result impl DfxOrbit { /// The main entry point for the `dfx orbit canister upload-http-assets` CLI. pub async fn upload_assets( @@ -36,22 +41,20 @@ impl DfxOrbit { .collect(); let canister_id = self.canister_id(&canister)?; + let asset_agent = self.asset_agent(canister_id)?; let logger = self.logger.clone(); // Upload assets: - let batch_id = self - .upload_assets_actual(canister_id, &source_paths) - .await?; + let batch_id = asset_agent.upload_assets(&source_paths).await?; info!(self.logger, "Proposed batch_id: {}", batch_id); // Compute evidence locally: - let local_evidence = self.compute_evidence(canister_id, &source_paths).await?; + let local_evidence = asset_agent.compute_evidence(&source_paths).await?; let local_evidence = escape_hex_string(&local_evidence); // Wait for the canister to compute evidence: - let canister_evidence_bytes = self.request_evidence(canister_id, batch_id.clone()).await?; + let canister_evidence_bytes = asset_agent.request_evidence(batch_id.clone()).await?; let canister_evidence = blob_from_bytes(&canister_evidence_bytes); - // TODO: Move this out of the agent into the tool println!(r#"Proposed batch_id: {batch_id}"#); if local_evidence == canister_evidence { info!(logger, "Local evidence matches canister evidence."); @@ -72,6 +75,13 @@ impl DfxOrbit { Ok(upload_request) } + + pub fn asset_agent(&self, canister_id: Principal) -> anyhow::Result { + Ok(AssetAgent { + canister_agent: self.canister_agent(canister_id)?, + logger: self.logger.clone(), + }) + } } /// Converts a hex string into one escaped as in a candid blob. diff --git a/tools/dfx-orbit/src/cli/asset/evidence.rs b/tools/dfx-orbit/src/cli/asset/evidence.rs index d1291facd..53a32b908 100644 --- a/tools/dfx-orbit/src/cli/asset/evidence.rs +++ b/tools/dfx-orbit/src/cli/asset/evidence.rs @@ -1,18 +1,13 @@ -use crate::DfxOrbit; -use candid::{Nat, Principal}; +use super::AssetAgent; +use candid::Nat; use ic_asset::canister_api::{ methods::batch::compute_evidence, types::batch_upload::common::ComputeEvidenceArguments, }; use serde_bytes::ByteBuf; use std::path::Path; -impl DfxOrbit { - pub async fn request_evidence( - &self, - canister_id: Principal, - batch_id: Nat, - ) -> anyhow::Result { - let canister_agent = self.canister_agent(canister_id)?; +impl AssetAgent<'_> { + pub async fn request_evidence(&self, batch_id: Nat) -> anyhow::Result { // This part is stolen from ic_asset::sync::prepare_sync_for_proposal. Unfortunately the relevant functions are private. // The docs explicitly include waiting for the evidence so this should really be made easier! See: https://github.com/dfinity/sdk/blob/2509e81e11e71dce4045c679686c952809525470/docs/design/asset-canister-interface.md?plain=1#L85 let compute_evidence_arg = ComputeEvidenceArguments { @@ -20,7 +15,8 @@ impl DfxOrbit { max_iterations: Some(97), // 75% of max(130) = 97.5 }; let canister_evidence_bytes = loop { - if let Some(evidence) = compute_evidence(&canister_agent, &compute_evidence_arg).await? + if let Some(evidence) = + compute_evidence(&self.canister_agent, &compute_evidence_arg).await? { break evidence; } @@ -28,14 +24,9 @@ impl DfxOrbit { Ok(canister_evidence_bytes) } - pub async fn compute_evidence( - &self, - canister_id: Principal, - sources: &[&Path], - ) -> anyhow::Result { - let canister_agent = self.canister_agent(canister_id)?; - Ok(ic_asset::compute_evidence(&canister_agent, sources, &self.logger).await?) + pub async fn compute_evidence(&self, sources: &[&Path]) -> anyhow::Result { + Ok(ic_asset::compute_evidence(&self.canister_agent, sources, &self.logger).await?) } - // TODO: check_evidence function + // TODO: check_evidence method } diff --git a/tools/dfx-orbit/src/cli/asset/upload.rs b/tools/dfx-orbit/src/cli/asset/upload.rs index 311489fee..579aeb22c 100644 --- a/tools/dfx-orbit/src/cli/asset/upload.rs +++ b/tools/dfx-orbit/src/cli/asset/upload.rs @@ -1,25 +1,22 @@ +use super::AssetAgent; +use crate::DfxOrbit; +use candid::Nat; use std::{ collections::HashMap, path::{Path, PathBuf}, }; - -use candid::{Nat, Principal}; use walkdir::WalkDir; -use crate::DfxOrbit; - impl DfxOrbit { - pub async fn upload_assets_actual( - &self, - canister_id: Principal, - sources: &[&Path], - ) -> anyhow::Result { - let canister_agent = self.canister_agent(canister_id)?; + // TODO: Move the entire upload asset function here + // TODO: Implement request_upload_commit +} + +impl AssetAgent<'_> { + pub async fn upload_assets(&self, sources: &[&Path]) -> anyhow::Result { let assets = assets_as_hash_map(sources); - Ok(ic_asset::upload_and_propose(&canister_agent, assets, &self.logger).await?) + Ok(ic_asset::upload_and_propose(&self.canister_agent, assets, &self.logger).await?) } - - // TODO: Implement request_upload_commit } /// A hash map of all assets. From 8eb7f747f0f6712efd9b031ff42405a6161a7dce Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 14:47:44 +0000 Subject: [PATCH 07/42] Remove unused mut --- .../src/dfx_orbit/canister_call.rs | 2 +- tests/integration/src/dfx_orbit/me.rs | 2 +- tests/integration/src/dfx_orbit/review.rs | 2 +- tools/dfx-orbit/src/cli/asset.rs | 1 - tools/dfx-orbit/src/station_agent.rs | 19 ++++++++----------- 5 files changed, 11 insertions(+), 15 deletions(-) diff --git a/tests/integration/src/dfx_orbit/canister_call.rs b/tests/integration/src/dfx_orbit/canister_call.rs index 9365a856c..4322c859c 100644 --- a/tests/integration/src/dfx_orbit/canister_call.rs +++ b/tests/integration/src/dfx_orbit/canister_call.rs @@ -55,7 +55,7 @@ fn canister_call() { let request = dfx_orbit_test(&mut env, async { // Setup the station agent - let mut dfx_orbit = setup_dfx_orbit(canister_ids.station).await; + let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; // Call the counter canister let request = dfx_orbit diff --git a/tests/integration/src/dfx_orbit/me.rs b/tests/integration/src/dfx_orbit/me.rs index a337a806e..b8aefd3c4 100644 --- a/tests/integration/src/dfx_orbit/me.rs +++ b/tests/integration/src/dfx_orbit/me.rs @@ -17,7 +17,7 @@ fn me() { let response = dfx_orbit_test(&mut env, async { // Setup the station agent - let mut dfx_orbit = setup_dfx_orbit(canister_ids.station).await; + let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; // Call the counter canister dfx_orbit.station.me().await.unwrap() diff --git a/tests/integration/src/dfx_orbit/review.rs b/tests/integration/src/dfx_orbit/review.rs index 14b9e2853..06b940f45 100644 --- a/tests/integration/src/dfx_orbit/review.rs +++ b/tests/integration/src/dfx_orbit/review.rs @@ -73,7 +73,7 @@ fn review() { dfx_orbit_test(&mut env, async { // Setup the station agent - let mut dfx_orbit = setup_dfx_orbit(canister_ids.station).await; + let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; let list_request_response = dfx_orbit .station diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 931fc9bfb..5ea5a9329 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -26,7 +26,6 @@ pub struct AssetUploadRequest { } impl DfxOrbit { - /// The main entry point for the `dfx orbit canister upload-http-assets` CLI. pub async fn upload_assets( &mut self, canister: String, diff --git a/tools/dfx-orbit/src/station_agent.rs b/tools/dfx-orbit/src/station_agent.rs index 4b13381ce..b3404e043 100644 --- a/tools/dfx-orbit/src/station_agent.rs +++ b/tools/dfx-orbit/src/station_agent.rs @@ -25,47 +25,44 @@ impl StationAgent { } pub async fn request( - &mut self, + &self, input: CreateRequestInput, ) -> StationAgentResult { self.update_orbit_typed("create_request", input).await } pub async fn submit( - &mut self, + &self, args: SubmitRequestApprovalInput, ) -> StationAgentResult { self.update_orbit_typed("submit_request_approval", args) .await } - pub async fn me(&mut self) -> StationAgentResult { + pub async fn me(&self) -> StationAgentResult { self.update_orbit_typed("me", ()).await } - pub async fn review_id( - &mut self, - args: GetRequestInput, - ) -> StationAgentResult { + pub async fn review_id(&self, args: GetRequestInput) -> StationAgentResult { self.update_orbit_typed("get_request", args).await } pub async fn review_list( - &mut self, + &self, args: ListRequestsInput, ) -> StationAgentResult { self.update_orbit_typed("list_requests", args).await } pub async fn review_next( - &mut self, + &self, args: GetNextApprovableRequestInput, ) -> StationAgentResult { self.update_orbit_typed("get_next_approvable_request", args) .await } - async fn update_orbit(&mut self, method_name: &str) -> UpdateBuilder { + async fn update_orbit(&self, method_name: &str) -> UpdateBuilder { self.agent.update(&self.config.station_id, method_name) } @@ -73,7 +70,7 @@ impl StationAgent { /// /// This version integrates candid encoding / decoding async fn update_orbit_typed( - &mut self, + &self, method_name: &str, request: Req, ) -> StationAgentResult From 8a953afb54069a363d2cf322a383447cd5780076 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 14:48:47 +0000 Subject: [PATCH 08/42] Implement check evidence method --- tools/dfx-orbit/src/cli/asset/evidence.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tools/dfx-orbit/src/cli/asset/evidence.rs b/tools/dfx-orbit/src/cli/asset/evidence.rs index 53a32b908..12900f324 100644 --- a/tools/dfx-orbit/src/cli/asset/evidence.rs +++ b/tools/dfx-orbit/src/cli/asset/evidence.rs @@ -4,6 +4,7 @@ use ic_asset::canister_api::{ methods::batch::compute_evidence, types::batch_upload::common::ComputeEvidenceArguments, }; use serde_bytes::ByteBuf; +use slog::{info, warn}; use std::path::Path; impl AssetAgent<'_> { @@ -28,5 +29,21 @@ impl AssetAgent<'_> { Ok(ic_asset::compute_evidence(&self.canister_agent, sources, &self.logger).await?) } - // TODO: check_evidence method + pub async fn check_evidence(&self, batch_id: Nat, sources: &[&Path]) -> anyhow::Result { + let local_evidence = self.compute_evidence(sources).await?; + let remote_evidence = hex::encode(&self.request_evidence(batch_id).await?); + + if local_evidence != remote_evidence { + warn!( + self.logger, + "Local evidence does not match remotely calculated evidence" + ); + warn!(self.logger, "Local: {local_evidence}"); + warn!(self.logger, "Remote: {remote_evidence}"); + Ok(false) + } else { + info!(self.logger, "Local and remote evidence match"); + Ok(true) + } + } } From 84ea59e30aea370109aa879837fa7407bb18d2b4 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 14:49:08 +0000 Subject: [PATCH 09/42] Implement request_commit_batch method --- Cargo.lock | 1 + tests/integration/src/dfx_orbit/assets.rs | 6 +-- tools/dfx-orbit/Cargo.toml | 1 + tools/dfx-orbit/src/cli.rs | 4 +- tools/dfx-orbit/src/cli/asset/upload.rs | 45 +++++++++++++++++++++-- tools/dfx-orbit/src/cli/asset/util.rs | 8 ++-- tools/dfx-orbit/src/lib.rs | 3 +- 7 files changed, 54 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8f831ad05..db9c22ced 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1170,6 +1170,7 @@ dependencies = [ "cap-std", "clap", "dfx-core 0.0.1 (git+https://github.com/dfinity/sdk.git?tag=0.22.0)", + "hex", "ic-agent", "ic-asset", "ic-certified-assets", diff --git a/tests/integration/src/dfx_orbit/assets.rs b/tests/integration/src/dfx_orbit/assets.rs index 785e10122..2c75c1417 100644 --- a/tests/integration/src/dfx_orbit/assets.rs +++ b/tests/integration/src/dfx_orbit/assets.rs @@ -1,4 +1,4 @@ -use dfx_orbit::StationAgent; +use dfx_orbit::AssetAgent; use pocket_ic::PocketIc; use rand::{thread_rng, Rng}; use station_api::{ @@ -52,7 +52,7 @@ fn assets_update() { canister_id: asset_canister, method_name: String::from("grant_permission"), }, - arg: Some(StationAgent::request_prepare_permission_payload(dfx_principal).unwrap()), + arg: Some(AssetAgent::request_prepare_permission_payload(dfx_principal).unwrap()), execution_method_cycles: None, }, ), @@ -112,7 +112,7 @@ fn assets_update() { method_name: String::from("commit_proposed_batch"), }, arg: Some( - StationAgent::commit_proposed_batch_payload(upload_request).unwrap(), + AssetAgent::commit_proposed_batch_payload(upload_request).unwrap(), ), execution_method_cycles: None, }, diff --git a/tools/dfx-orbit/Cargo.toml b/tools/dfx-orbit/Cargo.toml index 4fdf8d150..dfd480895 100644 --- a/tools/dfx-orbit/Cargo.toml +++ b/tools/dfx-orbit/Cargo.toml @@ -20,6 +20,7 @@ serde_bytes.workspace = true serde_json.workspace = true cap-std.workspace = true dfx-core.workspace = true +hex.workspace = true ic-agent.workspace = true ic-asset.workspace = true ic-certified-assets.workspace = true diff --git a/tools/dfx-orbit/src/cli.rs b/tools/dfx-orbit/src/cli.rs index e771b3987..e92666a9a 100644 --- a/tools/dfx-orbit/src/cli.rs +++ b/tools/dfx-orbit/src/cli.rs @@ -1,6 +1,6 @@ //! Implementation of the `dfx-orbit` commands. -mod asset; -mod station; +pub(crate) mod asset; +pub(crate) mod station; pub use crate::cli::asset::AssetUploadRequest; use crate::{ diff --git a/tools/dfx-orbit/src/cli/asset/upload.rs b/tools/dfx-orbit/src/cli/asset/upload.rs index 579aeb22c..6a5d734e8 100644 --- a/tools/dfx-orbit/src/cli/asset/upload.rs +++ b/tools/dfx-orbit/src/cli/asset/upload.rs @@ -1,6 +1,12 @@ use super::AssetAgent; use crate::DfxOrbit; -use candid::Nat; +use candid::{Nat, Principal}; +use ic_certified_assets::types::CommitProposedBatchArguments; +use orbit_station_api::{ + CallExternalCanisterOperationInput, CanisterMethodDTO, CreateRequestInput, + CreateRequestResponse, RequestOperationInput, +}; +use serde_bytes::ByteBuf; use std::{ collections::HashMap, path::{Path, PathBuf}, @@ -8,8 +14,41 @@ use std::{ use walkdir::WalkDir; impl DfxOrbit { - // TODO: Move the entire upload asset function here - // TODO: Implement request_upload_commit + pub async fn upload(&self) -> anyhow::Result<()> { + todo!() + } + + pub async fn request_commit_batch( + &self, + canister_id: Principal, + batch_id: Nat, + evidence: ByteBuf, + ) -> anyhow::Result { + let args = CommitProposedBatchArguments { batch_id, evidence }; + let arg = candid::encode_one(args)?; + + let response = self + .station + .request(CreateRequestInput { + operation: RequestOperationInput::CallExternalCanister( + CallExternalCanisterOperationInput { + validation_method: None, + execution_method: CanisterMethodDTO { + canister_id, + method_name: String::from("commit_proposed_batch"), + }, + arg: Some(arg), + execution_method_cycles: None, + }, + ), + title: None, + summary: None, + execution_plan: None, + }) + .await?; + + Ok(response) + } } impl AssetAgent<'_> { diff --git a/tools/dfx-orbit/src/cli/asset/util.rs b/tools/dfx-orbit/src/cli/asset/util.rs index 749f6b2da..f4f6aeab0 100644 --- a/tools/dfx-orbit/src/cli/asset/util.rs +++ b/tools/dfx-orbit/src/cli/asset/util.rs @@ -1,13 +1,10 @@ +use super::{AssetAgent, AssetUploadRequest}; use candid::Principal; use ic_certified_assets::types::{ CommitProposedBatchArguments, GrantPermissionArguments, Permission, }; -use crate::StationAgent; - -use super::AssetUploadRequest; - -impl StationAgent { +impl AssetAgent<'_> { pub fn request_prepare_permission_payload( canister: Principal, ) -> Result, candid::Error> { @@ -19,6 +16,7 @@ impl StationAgent { candid::encode_one(args) } + // TODO: Remove pub fn commit_proposed_batch_payload( upload_request: AssetUploadRequest, ) -> Result, candid::Error> { diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index 68805a6a9..38d53c9d8 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -15,8 +15,9 @@ use dfx_core::{config::model::canister_id_store::CanisterIdStore, DfxInterface}; use dfx_extension_api::OrbitExtensionAgent; use ic_utils::{canister::CanisterBuilder, Canister}; use slog::{o, Drain, Logger}; -pub use station_agent::StationAgent; +pub use cli::asset::AssetAgent; +pub use station_agent::StationAgent; pub struct DfxOrbit { // The station agent that handles communication with the station pub station: StationAgent, From c1710c7898072eb8f23dac26db8e7cb335084bb4 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 16:20:53 +0000 Subject: [PATCH 10/42] WIP improve UX around asset --- Cargo.lock | 9 +- Cargo.toml | 4 +- tests/integration/src/dfx_orbit/assets.rs | 39 ++----- tools/dfx-orbit/Cargo.toml | 1 - tools/dfx-orbit/src/args/asset.rs | 39 +++++++ tools/dfx-orbit/src/cli.rs | 13 +-- tools/dfx-orbit/src/cli/asset.rs | 120 ++++++++-------------- tools/dfx-orbit/src/cli/asset/evidence.rs | 51 ++------- tools/dfx-orbit/src/cli/asset/upload.rs | 82 +++++++-------- tools/dfx-orbit/src/cli/asset/util.rs | 19 +--- 10 files changed, 145 insertions(+), 232 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index db9c22ced..e80da4517 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1120,7 +1120,7 @@ dependencies = [ [[package]] name = "dfx-core" version = "0.0.1" -source = "git+https://github.com/dfinity/sdk.git?branch=leon/upload_and_propose#8e1ca2dbb5e146400d73c19f9cbbc876d20643c2" +source = "git+https://github.com/dfinity/sdk.git?rev=75c080ebae22a70578c06ddf1eda0b18ef091845#75c080ebae22a70578c06ddf1eda0b18ef091845" dependencies = [ "aes-gcm", "argon2", @@ -1184,7 +1184,6 @@ dependencies = [ "station-api", "thiserror", "tokio", - "walkdir", ] [[package]] @@ -2016,13 +2015,13 @@ dependencies = [ [[package]] name = "ic-asset" version = "0.20.0" -source = "git+https://github.com/dfinity/sdk.git?branch=leon/upload_and_propose#8e1ca2dbb5e146400d73c19f9cbbc876d20643c2" +source = "git+https://github.com/dfinity/sdk.git?rev=75c080ebae22a70578c06ddf1eda0b18ef091845#75c080ebae22a70578c06ddf1eda0b18ef091845" dependencies = [ "backoff", "brotli", "candid", "derivative", - "dfx-core 0.0.1 (git+https://github.com/dfinity/sdk.git?branch=leon/upload_and_propose)", + "dfx-core 0.0.1 (git+https://github.com/dfinity/sdk.git?rev=75c080ebae22a70578c06ddf1eda0b18ef091845)", "flate2", "futures", "futures-intrusive", @@ -2173,7 +2172,7 @@ dependencies = [ [[package]] name = "ic-certified-assets" version = "0.2.5" -source = "git+https://github.com/dfinity/sdk.git?branch=leon/upload_and_propose#8e1ca2dbb5e146400d73c19f9cbbc876d20643c2" +source = "git+https://github.com/dfinity/sdk.git?rev=75c080ebae22a70578c06ddf1eda0b18ef091845#75c080ebae22a70578c06ddf1eda0b18ef091845" dependencies = [ "base64 0.13.1", "candid", diff --git a/Cargo.toml b/Cargo.toml index 4b52a5f3d..28618af50 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,8 +44,8 @@ getrandom = { version = "0.2", features = ["custom"] } hex = "0.4" # The ic-agent matches the one sed by bthe ic-agent = { git = "https://github.com/dfinity/agent-rs.git", rev = "be929fd7967249c879f48f2f494cbfc5805a7d98" } -ic-asset = { git = "https://github.com/dfinity/sdk.git", branch = "leon/upload_and_propose" } -ic-certified-assets = { git = "https://github.com/dfinity/sdk.git", branch = "leon/upload_and_propose" } +ic-asset = { git = "https://github.com/dfinity/sdk.git", rev = "75c080ebae22a70578c06ddf1eda0b18ef091845" } +ic-certified-assets = { git = "https://github.com/dfinity/sdk.git", rev = "75c080ebae22a70578c06ddf1eda0b18ef091845" } ic-cdk = "0.13.2" ic-cdk-macros = "0.9" ic-cdk-timers = "0.7.0" diff --git a/tests/integration/src/dfx_orbit/assets.rs b/tests/integration/src/dfx_orbit/assets.rs index 2c75c1417..a61c528de 100644 --- a/tests/integration/src/dfx_orbit/assets.rs +++ b/tests/integration/src/dfx_orbit/assets.rs @@ -3,9 +3,9 @@ use pocket_ic::PocketIc; use rand::{thread_rng, Rng}; use station_api::{ AddRequestPolicyOperationInput, CallExternalCanisterOperationInput, - CallExternalCanisterResourceTargetDTO, CanisterMethodDTO, CreateRequestInput, - ExecutionMethodResourceTargetDTO, RequestOperationInput, RequestPolicyRuleDTO, - RequestSpecifierDTO, ValidationMethodResourceTargetDTO, + CallExternalCanisterResourceTargetDTO, CanisterMethodDTO, ExecutionMethodResourceTargetDTO, + RequestOperationInput, RequestPolicyRuleDTO, RequestSpecifierDTO, + ValidationMethodResourceTargetDTO, }; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tempfile::Builder; @@ -89,38 +89,15 @@ fn assets_update() { dfx_orbit_test(&mut env, async { // Setup the station agent - let mut dfx_orbit = setup_dfx_orbit(canister_ids.station).await; + let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; // As dfx user: Request to upload new files to the asset canister - let upload_request = dfx_orbit - .upload_assets( - asset_canister.to_string(), - vec![asset_dir.path().to_str().unwrap().to_string()], - ) + let (batch_id, evidence) = dfx_orbit + .upload(asset_canister, &[asset_dir.path()], false) .await .unwrap(); - - // As dfx user: Request commitment of the batch - let _result = dfx_orbit - .station - .request(CreateRequestInput { - operation: station_api::RequestOperationInput::CallExternalCanister( - CallExternalCanisterOperationInput { - validation_method: None, - execution_method: CanisterMethodDTO { - canister_id: asset_canister, - method_name: String::from("commit_proposed_batch"), - }, - arg: Some( - AssetAgent::commit_proposed_batch_payload(upload_request).unwrap(), - ), - execution_method_cycles: None, - }, - ), - title: None, - summary: None, - execution_plan: None, - }) + dfx_orbit + .request_commit_batch(asset_canister, batch_id, evidence) .await .unwrap(); diff --git a/tools/dfx-orbit/Cargo.toml b/tools/dfx-orbit/Cargo.toml index dfd480895..60c7aedf6 100644 --- a/tools/dfx-orbit/Cargo.toml +++ b/tools/dfx-orbit/Cargo.toml @@ -32,7 +32,6 @@ thiserror.workspace = true tokio = { workspace = true, features = ['rt'] } orbit-station-api = { path = "../../core/station/api", package = "station-api" } -walkdir = "2.5.0" [lib] doctest = false diff --git a/tools/dfx-orbit/src/args/asset.rs b/tools/dfx-orbit/src/args/asset.rs index a826140a7..aac5c796c 100644 --- a/tools/dfx-orbit/src/args/asset.rs +++ b/tools/dfx-orbit/src/args/asset.rs @@ -12,6 +12,12 @@ pub struct AssetArgs { pub enum AssetArgsAction { /// Upload assets to an asset canister Upload(AssetUploadArgs), + /// Compute local evidence + ComputeEvidence(AssetComputerEvidenceArgs), + /// Commit assets previously uploaded to an assed canister + Commit(AssetCommitArgs), + /// Check an asset upload request + Check(AssetCheckArgs), } #[derive(Debug, Clone, Parser)] @@ -19,7 +25,40 @@ pub struct AssetUploadArgs { /// The name of the asset canister targeted by this action pub(crate) canister: String, + /// Do not abort the upload, if the evidence does not match between local and remote calculation + #[clap(long)] + pub(crate) ignore_evidence: bool, + + /// Do not submit a request to commit the batch ID to the orbit station + #[clap(long)] + pub(crate) skip_commit: bool, + /// The source directories to upload (multiple values possible) #[clap(num_args = 1..)] pub(crate) files: Vec, } + +#[derive(Debug, Clone, Parser)] +pub struct AssetComputerEvidenceArgs { + /// The source directories to compute evidence from (multiple values possible) + #[clap(num_args = 1..)] + pub(crate) files: Vec, +} + +#[derive(Debug, Clone, Parser)] +pub struct AssetCommitArgs { + // TODO: +} +#[derive(Debug, Clone, Parser)] +pub struct AssetCheckArgs { + /// The ID of the request to commit the assets + pub(crate) request_id: String, + + /// The source directories of the asset upload (multiple values possible) + #[clap(num_args = 1..)] + pub(crate) files: Vec, + + /// Automatically approve the request, if the request evidence matches the local evidence + #[clap(long)] + auto_approve: bool, +} diff --git a/tools/dfx-orbit/src/cli.rs b/tools/dfx-orbit/src/cli.rs index e92666a9a..2d6fdf7e1 100644 --- a/tools/dfx-orbit/src/cli.rs +++ b/tools/dfx-orbit/src/cli.rs @@ -2,9 +2,8 @@ pub(crate) mod asset; pub(crate) mod station; -pub use crate::cli::asset::AssetUploadRequest; use crate::{ - args::{asset::AssetArgsAction, review::ReviewArgs, DfxOrbitArgs, DfxOrbitSubcommands}, + args::{review::ReviewArgs, DfxOrbitArgs, DfxOrbitSubcommands}, dfx_extension_api::OrbitExtensionAgent, DfxOrbit, }; @@ -77,16 +76,8 @@ pub async fn exec(args: DfxOrbitArgs) -> anyhow::Result<()> { } }, DfxOrbitSubcommands::Asset(asset_args) => { - match asset_args.action { - AssetArgsAction::Upload(upload_args) => { - dfx_orbit - .upload_assets(upload_args.canister, upload_args.files) - .await?; - } - } - + dfx_orbit.exec_asset(asset_args).await?; Ok(()) - // } _ => unreachable!(), } diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 5ea5a9329..de6b74202 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -4,75 +4,59 @@ mod evidence; mod upload; mod util; -use crate::DfxOrbit; -use candid::{Nat, Principal}; -use ic_utils::Canister; -use serde::{Deserialize, Serialize}; -use serde_bytes::ByteBuf; -use slog::{info, warn, Logger}; use std::path::{Path, PathBuf}; +use crate::{ + args::asset::{AssetArgs, AssetArgsAction}, + DfxOrbit, +}; +use candid::Principal; +use ic_utils::Canister; +use slog::Logger; + pub struct AssetAgent<'agent> { canister_agent: Canister<'agent>, logger: Logger, } -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct AssetUploadRequest { - station_principal: Principal, - asset_canister_principal: Principal, - batch_id: Nat, - evidence: ByteBuf, -} - impl DfxOrbit { - pub async fn upload_assets( - &mut self, - canister: String, - files: Vec, - ) -> anyhow::Result { - // The path is needed in various forms. - let source_pathbufs: Vec = - files.iter().map(|source| PathBuf::from(&source)).collect(); - let source_paths: Vec<&Path> = source_pathbufs - .iter() - .map(|pathbuf| pathbuf.as_path()) - .collect(); - - let canister_id = self.canister_id(&canister)?; - let asset_agent = self.asset_agent(canister_id)?; - let logger = self.logger.clone(); + pub async fn exec_asset(&mut self, args: AssetArgs) -> anyhow::Result<()> { + match args.action { + AssetArgsAction::Upload(upload_args) => { + let source_pathbufs: Vec = upload_args + .files + .iter() + .map(|source| PathBuf::from(&source)) + .collect(); + let source_paths: Vec<&Path> = source_pathbufs + .iter() + .map(|pathbuf| pathbuf.as_path()) + .collect(); - // Upload assets: - let batch_id = asset_agent.upload_assets(&source_paths).await?; - info!(self.logger, "Proposed batch_id: {}", batch_id); - // Compute evidence locally: - let local_evidence = asset_agent.compute_evidence(&source_paths).await?; - let local_evidence = escape_hex_string(&local_evidence); - // Wait for the canister to compute evidence: + let canister_id = self.canister_id(&upload_args.canister)?; + let (batch_id, evidence) = self + .upload(canister_id, &source_paths, upload_args.ignore_evidence) + .await?; - let canister_evidence_bytes = asset_agent.request_evidence(batch_id.clone()).await?; - let canister_evidence = blob_from_bytes(&canister_evidence_bytes); + if !upload_args.skip_commit { + let result = self + .request_commit_batch(canister_id, batch_id.clone(), evidence) + .await?; + let request_id = result.request.id; - println!(r#"Proposed batch_id: {batch_id}"#); - if local_evidence == canister_evidence { - info!(logger, "Local evidence matches canister evidence."); - } else { - warn!(logger, "Local evidence does not match canister evidence:\n local: {local_evidence}\n canister:{canister_evidence}"); + println!("Created request to commit batches. To verify the batch against local files, run:"); + println!("dfx-orbit asset check {request_id}"); + } else { + let evidence = hex::encode(&evidence); + println!("Prepared the batches. To commit, run:"); + println!("dfx-orbit asset commit {canister_id} {batch_id} {evidence}"); + } + Ok(()) + } + AssetArgsAction::Commit(_) => todo!(), + AssetArgsAction::ComputeEvidence(_) => todo!(), + AssetArgsAction::Check(_) => todo!(), } - println!(r#"Assets have been uploaded. For the changes to take effect, run:"#); - println!( - r#"dfx-orbit request canister call {canister} commit_proposed_batch '(record {{ batch_id = {batch_id} : nat; evidence = blob "{canister_evidence}" }})'"# - ); - - let upload_request = AssetUploadRequest { - station_principal: self.station.config.station_id, - asset_canister_principal: canister_id, - batch_id, - evidence: canister_evidence_bytes, - }; - - Ok(upload_request) } pub fn asset_agent(&self, canister_id: Principal) -> anyhow::Result { @@ -82,25 +66,3 @@ impl DfxOrbit { }) } } - -/// Converts a hex string into one escaped as in a candid blob. -fn escape_hex_string(s: &str) -> String { - let mut ans = String::with_capacity(s.len() + s.len() / 2); - for chunk in s.chars().collect::>()[..].chunks(2) { - ans.push('\\'); - for char in chunk { - ans.push(*char); - } - } - ans -} - -/// Converts a byte array into one escaped as a candid blob -fn blob_from_bytes(bytes: &[u8]) -> String { - let mut ans = String::with_capacity(bytes.len() + bytes.len() / 2); - for byte in bytes { - ans.push('\\'); - ans.push_str(&format!("{:02x}", byte)); - } - ans -} diff --git a/tools/dfx-orbit/src/cli/asset/evidence.rs b/tools/dfx-orbit/src/cli/asset/evidence.rs index 12900f324..abc26be82 100644 --- a/tools/dfx-orbit/src/cli/asset/evidence.rs +++ b/tools/dfx-orbit/src/cli/asset/evidence.rs @@ -1,49 +1,20 @@ +use crate::DfxOrbit; + use super::AssetAgent; -use candid::Nat; -use ic_asset::canister_api::{ - methods::batch::compute_evidence, types::batch_upload::common::ComputeEvidenceArguments, -}; -use serde_bytes::ByteBuf; -use slog::{info, warn}; use std::path::Path; -impl AssetAgent<'_> { - pub async fn request_evidence(&self, batch_id: Nat) -> anyhow::Result { - // This part is stolen from ic_asset::sync::prepare_sync_for_proposal. Unfortunately the relevant functions are private. - // The docs explicitly include waiting for the evidence so this should really be made easier! See: https://github.com/dfinity/sdk/blob/2509e81e11e71dce4045c679686c952809525470/docs/design/asset-canister-interface.md?plain=1#L85 - let compute_evidence_arg = ComputeEvidenceArguments { - batch_id: batch_id.clone(), - max_iterations: Some(97), // 75% of max(130) = 97.5 - }; - let canister_evidence_bytes = loop { - if let Some(evidence) = - compute_evidence(&self.canister_agent, &compute_evidence_arg).await? - { - break evidence; - } - }; - Ok(canister_evidence_bytes) +impl DfxOrbit { + pub async fn check_evidence( + &self, + _request_id: String, + _sources: &[&Path], + ) -> anyhow::Result { + todo!() } +} +impl AssetAgent<'_> { pub async fn compute_evidence(&self, sources: &[&Path]) -> anyhow::Result { Ok(ic_asset::compute_evidence(&self.canister_agent, sources, &self.logger).await?) } - - pub async fn check_evidence(&self, batch_id: Nat, sources: &[&Path]) -> anyhow::Result { - let local_evidence = self.compute_evidence(sources).await?; - let remote_evidence = hex::encode(&self.request_evidence(batch_id).await?); - - if local_evidence != remote_evidence { - warn!( - self.logger, - "Local evidence does not match remotely calculated evidence" - ); - warn!(self.logger, "Local: {local_evidence}"); - warn!(self.logger, "Remote: {remote_evidence}"); - Ok(false) - } else { - info!(self.logger, "Local and remote evidence match"); - Ok(true) - } - } } diff --git a/tools/dfx-orbit/src/cli/asset/upload.rs b/tools/dfx-orbit/src/cli/asset/upload.rs index 6a5d734e8..7dbfed3c8 100644 --- a/tools/dfx-orbit/src/cli/asset/upload.rs +++ b/tools/dfx-orbit/src/cli/asset/upload.rs @@ -1,5 +1,6 @@ use super::AssetAgent; use crate::DfxOrbit; +use anyhow::bail; use candid::{Nat, Principal}; use ic_certified_assets::types::CommitProposedBatchArguments; use orbit_station_api::{ @@ -7,15 +8,37 @@ use orbit_station_api::{ CreateRequestResponse, RequestOperationInput, }; use serde_bytes::ByteBuf; -use std::{ - collections::HashMap, - path::{Path, PathBuf}, -}; -use walkdir::WalkDir; +use slog::{info, warn}; +use std::path::Path; impl DfxOrbit { - pub async fn upload(&self) -> anyhow::Result<()> { - todo!() + pub async fn upload( + &self, + canister_id: Principal, + sources: &[&Path], + ignore_evidence: bool, + ) -> anyhow::Result<(Nat, ByteBuf)> { + let asset_agent = self.asset_agent(canister_id)?; + let (batch_id, evidence) = asset_agent.upload_assets(sources).await?; + + let remote_evidence = hex::encode(&evidence); + let local_evidence = asset_agent.compute_evidence(sources).await?; + + if !ignore_evidence { + if local_evidence != remote_evidence { + warn!( + self.logger, + "Local evidence does not match remotely calculated evidence" + ); + warn!(self.logger, "Local: {local_evidence}"); + warn!(self.logger, "Remote: {remote_evidence}"); + bail!("Evidence did not match!"); + } else { + info!(self.logger, "Local and remote evidence match!"); + } + } + + Ok((batch_id, evidence)) } pub async fn request_commit_batch( @@ -52,45 +75,10 @@ impl DfxOrbit { } impl AssetAgent<'_> { - pub async fn upload_assets(&self, sources: &[&Path]) -> anyhow::Result { - let assets = assets_as_hash_map(sources); - Ok(ic_asset::upload_and_propose(&self.canister_agent, assets, &self.logger).await?) + pub async fn upload_assets(&self, sources: &[&Path]) -> anyhow::Result<(Nat, ByteBuf)> { + Ok( + ic_asset::prepare_sync_for_proposal(&self.canister_agent, sources, &self.logger) + .await?, + ) } } - -/// A hash map of all assets. -fn assets_as_hash_map(asset_dirs: &[&Path]) -> HashMap { - asset_dirs - .iter() - .flat_map(|asset_dir| { - list_assets(asset_dir).into_iter().map(move |asset_path| { - let relative_path = asset_path.strip_prefix(asset_dir).expect( - "Internal error: list_assets should have returned only files in the asset_dir", - ); - let http_path = format!( - "/{relative_path}", - relative_path = relative_path.to_string_lossy() - ); - (http_path, asset_path) - }) - }) - .collect() -} - -/// Lists all the files at the given path. -/// -/// - Links are followed. -/// - Only files are returned. -/// - The files are sorted by name. -/// - Any files that cannot be read are ignored. -/// - The path includes the prefix. -fn list_assets(path: &Path) -> Vec { - WalkDir::new(path) - .sort_by_file_name() - .follow_links(true) - .into_iter() - .filter_map(|e| e.ok()) - .filter(|entry| entry.file_type().is_file()) - .map(|entry| entry.into_path()) - .collect() -} diff --git a/tools/dfx-orbit/src/cli/asset/util.rs b/tools/dfx-orbit/src/cli/asset/util.rs index f4f6aeab0..33246ba4e 100644 --- a/tools/dfx-orbit/src/cli/asset/util.rs +++ b/tools/dfx-orbit/src/cli/asset/util.rs @@ -1,10 +1,9 @@ -use super::{AssetAgent, AssetUploadRequest}; +use super::AssetAgent; use candid::Principal; -use ic_certified_assets::types::{ - CommitProposedBatchArguments, GrantPermissionArguments, Permission, -}; +use ic_certified_assets::types::{GrantPermissionArguments, Permission}; impl AssetAgent<'_> { + // TODO: Turn into a functionality pub fn request_prepare_permission_payload( canister: Principal, ) -> Result, candid::Error> { @@ -15,16 +14,4 @@ impl AssetAgent<'_> { candid::encode_one(args) } - - // TODO: Remove - pub fn commit_proposed_batch_payload( - upload_request: AssetUploadRequest, - ) -> Result, candid::Error> { - let args = CommitProposedBatchArguments { - batch_id: upload_request.batch_id, - evidence: upload_request.evidence, - }; - - candid::encode_one(args) - } } From c2a773709d18310f1b2df1dcf0805731be21d703 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 2 Aug 2024 16:53:04 +0000 Subject: [PATCH 11/42] WIP improving UX around assets --- tools/dfx-orbit/src/args/asset.rs | 21 +++++++++++++--- tools/dfx-orbit/src/cli/asset.rs | 41 ++++++++++++++++++++----------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/tools/dfx-orbit/src/args/asset.rs b/tools/dfx-orbit/src/args/asset.rs index aac5c796c..3432f0ff7 100644 --- a/tools/dfx-orbit/src/args/asset.rs +++ b/tools/dfx-orbit/src/args/asset.rs @@ -1,3 +1,4 @@ +use candid::Nat; use clap::{Parser, Subcommand}; /// Station management commands. @@ -13,7 +14,7 @@ pub enum AssetArgsAction { /// Upload assets to an asset canister Upload(AssetUploadArgs), /// Compute local evidence - ComputeEvidence(AssetComputerEvidenceArgs), + ComputeEvidence(AssetComputeEvidenceArgs), /// Commit assets previously uploaded to an assed canister Commit(AssetCommitArgs), /// Check an asset upload request @@ -39,7 +40,9 @@ pub struct AssetUploadArgs { } #[derive(Debug, Clone, Parser)] -pub struct AssetComputerEvidenceArgs { +pub struct AssetComputeEvidenceArgs { + /// The name of the asset canister targeted by this action + pub(crate) canister: String, /// The source directories to compute evidence from (multiple values possible) #[clap(num_args = 1..)] pub(crate) files: Vec, @@ -47,10 +50,20 @@ pub struct AssetComputerEvidenceArgs { #[derive(Debug, Clone, Parser)] pub struct AssetCommitArgs { - // TODO: + /// The name of the asset canister targeted by this action + pub(crate) canister: String, + + /// The batch ID to commit to + pub(crate) batch_id: Nat, + + /// The evidence (as hex string) to commit to + pub(crate) evidence: String, } #[derive(Debug, Clone, Parser)] pub struct AssetCheckArgs { + /// The name of the asset canister targeted by this action + pub(crate) canister: String, + /// The ID of the request to commit the assets pub(crate) request_id: String, @@ -60,5 +73,5 @@ pub struct AssetCheckArgs { /// Automatically approve the request, if the request evidence matches the local evidence #[clap(long)] - auto_approve: bool, + try_approve: bool, } diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index de6b74202..5f801d9b8 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -4,8 +4,6 @@ mod evidence; mod upload; mod util; -use std::path::{Path, PathBuf}; - use crate::{ args::asset::{AssetArgs, AssetArgsAction}, DfxOrbit, @@ -13,6 +11,7 @@ use crate::{ use candid::Principal; use ic_utils::Canister; use slog::Logger; +use std::path::{Path, PathBuf}; pub struct AssetAgent<'agent> { canister_agent: Canister<'agent>, @@ -23,19 +22,13 @@ impl DfxOrbit { pub async fn exec_asset(&mut self, args: AssetArgs) -> anyhow::Result<()> { match args.action { AssetArgsAction::Upload(upload_args) => { - let source_pathbufs: Vec = upload_args - .files - .iter() - .map(|source| PathBuf::from(&source)) - .collect(); - let source_paths: Vec<&Path> = source_pathbufs - .iter() - .map(|pathbuf| pathbuf.as_path()) - .collect(); + let pathbufs = as_path_bufs(upload_args.files); + let paths = as_paths(&pathbufs); - let canister_id = self.canister_id(&upload_args.canister)?; + let canister_name = upload_args.canister; + let canister_id = self.canister_id(&canister_name)?; let (batch_id, evidence) = self - .upload(canister_id, &source_paths, upload_args.ignore_evidence) + .upload(canister_id, &paths, upload_args.ignore_evidence) .await?; if !upload_args.skip_commit { @@ -45,7 +38,7 @@ impl DfxOrbit { let request_id = result.request.id; println!("Created request to commit batches. To verify the batch against local files, run:"); - println!("dfx-orbit asset check {request_id}"); + println!("dfx-orbit asset check {canister_name} {request_id} [FILES]"); } else { let evidence = hex::encode(&evidence); println!("Prepared the batches. To commit, run:"); @@ -54,7 +47,17 @@ impl DfxOrbit { Ok(()) } AssetArgsAction::Commit(_) => todo!(), - AssetArgsAction::ComputeEvidence(_) => todo!(), + AssetArgsAction::ComputeEvidence(compute_args) => { + let pathbufs = as_path_bufs(compute_args.files); + let paths = as_paths(&pathbufs); + + let canister_id = self.canister_id(&compute_args.canister)?; + let asset_agent = self.asset_agent(canister_id)?; + + let evidence = asset_agent.compute_evidence(&paths).await?; + println!("{evidence}"); + Ok(()) + } AssetArgsAction::Check(_) => todo!(), } } @@ -66,3 +69,11 @@ impl DfxOrbit { }) } } + +fn as_path_bufs(paths: Vec) -> Vec { + paths.iter().map(|source| PathBuf::from(&source)).collect() +} + +fn as_paths(paths: &Vec) -> Vec<&Path> { + paths.iter().map(|pathbuf| pathbuf.as_path()).collect() +} From 0d54940564ddf77a396ed4842d0486a133cc774c Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Mon, 5 Aug 2024 11:41:32 +0000 Subject: [PATCH 12/42] WIP UX in assets improvements --- tests/integration/src/dfx_orbit/assets.rs | 4 ++-- tools/dfx-orbit/src/args/asset.rs | 26 +++++++++-------------- tools/dfx-orbit/src/cli/asset.rs | 26 +++++++++++------------ tools/dfx-orbit/src/cli/asset/upload.rs | 6 ++++-- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/tests/integration/src/dfx_orbit/assets.rs b/tests/integration/src/dfx_orbit/assets.rs index a61c528de..feb1d8674 100644 --- a/tests/integration/src/dfx_orbit/assets.rs +++ b/tests/integration/src/dfx_orbit/assets.rs @@ -21,7 +21,7 @@ use crate::{ }; #[test] -fn assets_update() { +fn assets_upload() { let TestEnv { mut env, canister_ids, @@ -97,7 +97,7 @@ fn assets_update() { .await .unwrap(); dfx_orbit - .request_commit_batch(asset_canister, batch_id, evidence) + .request_commit_batch(asset_canister, batch_id, evidence, None, None) .await .unwrap(); diff --git a/tools/dfx-orbit/src/args/asset.rs b/tools/dfx-orbit/src/args/asset.rs index 3432f0ff7..9c0dfcbe0 100644 --- a/tools/dfx-orbit/src/args/asset.rs +++ b/tools/dfx-orbit/src/args/asset.rs @@ -15,8 +15,6 @@ pub enum AssetArgsAction { Upload(AssetUploadArgs), /// Compute local evidence ComputeEvidence(AssetComputeEvidenceArgs), - /// Commit assets previously uploaded to an assed canister - Commit(AssetCommitArgs), /// Check an asset upload request Check(AssetCheckArgs), } @@ -30,9 +28,13 @@ pub struct AssetUploadArgs { #[clap(long)] pub(crate) ignore_evidence: bool, - /// Do not submit a request to commit the batch ID to the orbit station + /// The title of the request to commit the batch #[clap(long)] - pub(crate) skip_commit: bool, + pub(crate) title: Option, + + /// The summary of the request to commit the batch + #[clap(long)] + pub(crate) summary: Option, /// The source directories to upload (multiple values possible) #[clap(num_args = 1..)] @@ -48,17 +50,6 @@ pub struct AssetComputeEvidenceArgs { pub(crate) files: Vec, } -#[derive(Debug, Clone, Parser)] -pub struct AssetCommitArgs { - /// The name of the asset canister targeted by this action - pub(crate) canister: String, - - /// The batch ID to commit to - pub(crate) batch_id: Nat, - - /// The evidence (as hex string) to commit to - pub(crate) evidence: String, -} #[derive(Debug, Clone, Parser)] pub struct AssetCheckArgs { /// The name of the asset canister targeted by this action @@ -67,11 +58,14 @@ pub struct AssetCheckArgs { /// The ID of the request to commit the assets pub(crate) request_id: String, + /// The batch ID to commit to + pub(crate) batch_id: Nat, + /// The source directories of the asset upload (multiple values possible) #[clap(num_args = 1..)] pub(crate) files: Vec, /// Automatically approve the request, if the request evidence matches the local evidence #[clap(long)] - try_approve: bool, + then_approve: bool, } diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 5f801d9b8..6dcb60db1 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -31,22 +31,22 @@ impl DfxOrbit { .upload(canister_id, &paths, upload_args.ignore_evidence) .await?; - if !upload_args.skip_commit { - let result = self - .request_commit_batch(canister_id, batch_id.clone(), evidence) - .await?; - let request_id = result.request.id; + let result = self + .request_commit_batch( + canister_id, + batch_id.clone(), + evidence, + upload_args.title, + upload_args.summary, + ) + .await?; + let request_id = result.request.id; + + println!("Created request to commit batches. To verify the batch against local files, run:"); + println!("dfx-orbit asset check {canister_name} {request_id} [FILES]"); - println!("Created request to commit batches. To verify the batch against local files, run:"); - println!("dfx-orbit asset check {canister_name} {request_id} [FILES]"); - } else { - let evidence = hex::encode(&evidence); - println!("Prepared the batches. To commit, run:"); - println!("dfx-orbit asset commit {canister_id} {batch_id} {evidence}"); - } Ok(()) } - AssetArgsAction::Commit(_) => todo!(), AssetArgsAction::ComputeEvidence(compute_args) => { let pathbufs = as_path_bufs(compute_args.files); let paths = as_paths(&pathbufs); diff --git a/tools/dfx-orbit/src/cli/asset/upload.rs b/tools/dfx-orbit/src/cli/asset/upload.rs index 7dbfed3c8..e953cdb6a 100644 --- a/tools/dfx-orbit/src/cli/asset/upload.rs +++ b/tools/dfx-orbit/src/cli/asset/upload.rs @@ -46,6 +46,8 @@ impl DfxOrbit { canister_id: Principal, batch_id: Nat, evidence: ByteBuf, + title: Option, + summary: Option, ) -> anyhow::Result { let args = CommitProposedBatchArguments { batch_id, evidence }; let arg = candid::encode_one(args)?; @@ -64,8 +66,8 @@ impl DfxOrbit { execution_method_cycles: None, }, ), - title: None, - summary: None, + title, + summary, execution_plan: None, }) .await?; From ea13a3dc6c0c2cf562278ee705aee7c160c9dd07 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Mon, 5 Aug 2024 12:43:52 +0000 Subject: [PATCH 13/42] WIP implement asset check --- Cargo.lock | 1 + tools/dfx-orbit/Cargo.toml | 1 + tools/dfx-orbit/src/cli/asset.rs | 23 ++++++-- tools/dfx-orbit/src/cli/asset/evidence.rs | 70 +++++++++++++++++++++-- 4 files changed, 85 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e80da4517..c7b69b0ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1178,6 +1178,7 @@ dependencies = [ "serde", "serde_bytes", "serde_json", + "sha2 0.10.8", "slog", "slog-async", "slog-term", diff --git a/tools/dfx-orbit/Cargo.toml b/tools/dfx-orbit/Cargo.toml index 60c7aedf6..19fa8834a 100644 --- a/tools/dfx-orbit/Cargo.toml +++ b/tools/dfx-orbit/Cargo.toml @@ -25,6 +25,7 @@ ic-agent.workspace = true ic-asset.workspace = true ic-certified-assets.workspace = true ic-utils.workspace = true +sha2.workspace = true slog.workspace = true slog-term.workspace = true slog-async.workspace = true diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 6dcb60db1..8956b1aa1 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -47,18 +47,33 @@ impl DfxOrbit { Ok(()) } - AssetArgsAction::ComputeEvidence(compute_args) => { - let pathbufs = as_path_bufs(compute_args.files); + AssetArgsAction::ComputeEvidence(args) => { + let pathbufs = as_path_bufs(args.files); let paths = as_paths(&pathbufs); - let canister_id = self.canister_id(&compute_args.canister)?; + let canister_id = self.canister_id(&args.canister)?; let asset_agent = self.asset_agent(canister_id)?; let evidence = asset_agent.compute_evidence(&paths).await?; println!("{evidence}"); Ok(()) } - AssetArgsAction::Check(_) => todo!(), + AssetArgsAction::Check(args) => { + let pathbufs = as_path_bufs(args.files); + let paths = as_paths(&pathbufs); + + let canister_id = self.canister_id(&args.canister)?; + let asset_agent = self.asset_agent(canister_id)?; + + let evidence = asset_agent.compute_evidence(&paths).await?; + self.check_evidence(canister_id, args.request_id, args.batch_id, evidence) + .await?; + + println!("Local evidence matches expected arguments"); + + // TODO: Handle then approve + Ok(()) + } } } diff --git a/tools/dfx-orbit/src/cli/asset/evidence.rs b/tools/dfx-orbit/src/cli/asset/evidence.rs index abc26be82..0f3e4f75c 100644 --- a/tools/dfx-orbit/src/cli/asset/evidence.rs +++ b/tools/dfx-orbit/src/cli/asset/evidence.rs @@ -1,15 +1,73 @@ -use crate::DfxOrbit; - use super::AssetAgent; +use crate::DfxOrbit; +use anyhow::bail; +use candid::{Nat, Principal}; +use ic_certified_assets::types::CommitProposedBatchArguments; +use orbit_station_api::{CanisterMethodDTO, GetRequestInput, RequestOperationDTO}; +use serde_bytes::ByteBuf; +use sha2::{Digest, Sha256}; use std::path::Path; impl DfxOrbit { + /// Check that the locally computed evidence will lead to the correcst sha256 checksum + /// of the args of the request pub async fn check_evidence( &self, - _request_id: String, - _sources: &[&Path], - ) -> anyhow::Result { - todo!() + canister_id: Principal, + request_id: String, + batch_id: Nat, + evidence: String, + ) -> anyhow::Result<()> { + let request = self + .station + .review_id(GetRequestInput { + request_id: request_id.clone(), + }) + .await?; + + // Check: + // - Request is actually a CallExternalCanister + // - Target is the canister we are expecting + // - Method is `propose_commit_batch` + // - `arg_checksum` exists + let RequestOperationDTO::CallExternalCanister(request) = request.request.operation else { + bail!("{} is not an external canister request. Are you sure you have the correct request id?", {request_id}); + }; + let CanisterMethodDTO { + canister_id: request_canister_id, + method_name, + } = request.execution_method; + if request_canister_id != canister_id { + bail!( + "Canister id of the request {} does not match canister id of asset canister {}", + request_canister_id, + canister_id + ); + } + if &method_name != "propose_commit_batch" { + bail!( + "Method name if the request is not \"request_commit_batch\", but instead \"{}\"", + method_name + ); + } + let Some(remote_checksum) = request.arg_checksum else { + bail!("The request has no arguments. This likely means that is is malformed."); + }; + + // Now we check that the argument that we construct locally matches the hash of the argument + let evidence = hex::decode(evidence)?; + let args = CommitProposedBatchArguments { + batch_id, + evidence: ByteBuf::from(evidence), + }; + let arg = candid::encode_one(args)?; + let local_checksum = hex::encode(Sha256::digest(arg)); + + if local_checksum != remote_checksum { + bail!("Local evidence does not match expected arguments"); + } + + Ok(()) } } From 1df44ee34e86efe863683749d6e3b91bacf0cc91 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Mon, 5 Aug 2024 12:53:00 +0000 Subject: [PATCH 14/42] Test asset check --- tests/integration/src/dfx_orbit/assets.rs | 19 ++++++++++++++++++- tools/dfx-orbit/src/cli/asset/evidence.rs | 4 ++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/tests/integration/src/dfx_orbit/assets.rs b/tests/integration/src/dfx_orbit/assets.rs index feb1d8674..4e2536ea4 100644 --- a/tests/integration/src/dfx_orbit/assets.rs +++ b/tests/integration/src/dfx_orbit/assets.rs @@ -96,8 +96,25 @@ fn assets_upload() { .upload(asset_canister, &[asset_dir.path()], false) .await .unwrap(); + let response = dfx_orbit + .request_commit_batch( + asset_canister, + batch_id.clone(), + evidence.clone(), + None, + None, + ) + .await + .unwrap(); + + // Check whether the request passes the asset check dfx_orbit - .request_commit_batch(asset_canister, batch_id, evidence, None, None) + .check_evidence( + asset_canister, + response.request.id, + batch_id, + hex::encode(evidence), + ) .await .unwrap(); diff --git a/tools/dfx-orbit/src/cli/asset/evidence.rs b/tools/dfx-orbit/src/cli/asset/evidence.rs index 0f3e4f75c..669cd5ceb 100644 --- a/tools/dfx-orbit/src/cli/asset/evidence.rs +++ b/tools/dfx-orbit/src/cli/asset/evidence.rs @@ -44,9 +44,9 @@ impl DfxOrbit { canister_id ); } - if &method_name != "propose_commit_batch" { + if &method_name != "commit_proposed_batch" { bail!( - "Method name if the request is not \"request_commit_batch\", but instead \"{}\"", + "Method name if the request is not \"commit_proposed_batch\", but instead \"{}\"", method_name ); } From 552da4125402d911982d4d383980c94e438b009d Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Mon, 5 Aug 2024 13:18:35 +0000 Subject: [PATCH 15/42] Update README --- tools/dfx-orbit/README.md | 24 ++++++++++++++++++------ tools/dfx-orbit/src/cli/asset.rs | 21 +++++++++++---------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/tools/dfx-orbit/README.md b/tools/dfx-orbit/README.md index 293ecd3e5..1aaf2ff0a 100644 --- a/tools/dfx-orbit/README.md +++ b/tools/dfx-orbit/README.md @@ -224,14 +224,26 @@ A developer may upload one or more directories of HTTP assets with: dfx-orbit asset upload CANISTER_NAME SOME_DIR/ OTHER_DIR/ ``` -The developer may now request that the assets be published. The command for this is printed at the end of the upload command. Example: +This will upload the assets to the asset canister and then request the orbit station to publish +the assets. + +#### Verifying an asset update + +After the request has been made, the reviewers can locally verify the request: ``` -... -Jul 03 09:36:42.148 INFO Computing evidence. -Proposed batch_id: 5 -Assets have been uploaded. For the changes to take effect, run: -dfx-orbit request canister call frontend commit_proposed_batch '(record { batch_id = 5 : nat; evidence = blob "\e3\b0\c4\42\98\fc\1c\14\9a\fb\f4\c8\99\6f\b9\24\27\ae\41\e4\64\9b\93\4c\a4\95\99\1b\78\52\b8\55" })' +dfx-orbit asset check --then-approve CANISTER REQUEST_ID BATCH_ID SOME_DIR/ OTHER_DIR/ ``` +The exact command is printed in the output of `dfx-orbit asset upload` and must be distributed +from the proposer to the verifiers. + +> The verifiers needs to have the same set of data as was used in the request. +> How the verifier accomplishes this is outside the scope of this document. +> +> - The verifier might either download a tarball from the requester and manually verify the content +> - The verifier might check out a git revision and check that the content matches +> - If there are build scripts used while generating the assets, care must be taken to make +> the build step deterministic, such that verifiers can recreate the exact assets + Once the request has been approved, the changes will take effect. diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 8956b1aa1..ffe8f68f0 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -21,14 +21,14 @@ pub struct AssetAgent<'agent> { impl DfxOrbit { pub async fn exec_asset(&mut self, args: AssetArgs) -> anyhow::Result<()> { match args.action { - AssetArgsAction::Upload(upload_args) => { - let pathbufs = as_path_bufs(upload_args.files); + AssetArgsAction::Upload(args) => { + let pathbufs = as_path_bufs(&args.files); let paths = as_paths(&pathbufs); - let canister_name = upload_args.canister; + let canister_name = args.canister; let canister_id = self.canister_id(&canister_name)?; let (batch_id, evidence) = self - .upload(canister_id, &paths, upload_args.ignore_evidence) + .upload(canister_id, &paths, args.ignore_evidence) .await?; let result = self @@ -36,19 +36,20 @@ impl DfxOrbit { canister_id, batch_id.clone(), evidence, - upload_args.title, - upload_args.summary, + args.title, + args.summary, ) .await?; let request_id = result.request.id; + let files = args.files.join(" "); println!("Created request to commit batches. To verify the batch against local files, run:"); - println!("dfx-orbit asset check {canister_name} {request_id} [FILES]"); + println!("dfx-orbit asset check {canister_name} {request_id} {batch_id} {files}"); Ok(()) } AssetArgsAction::ComputeEvidence(args) => { - let pathbufs = as_path_bufs(args.files); + let pathbufs = as_path_bufs(&args.files); let paths = as_paths(&pathbufs); let canister_id = self.canister_id(&args.canister)?; @@ -59,7 +60,7 @@ impl DfxOrbit { Ok(()) } AssetArgsAction::Check(args) => { - let pathbufs = as_path_bufs(args.files); + let pathbufs = as_path_bufs(&args.files); let paths = as_paths(&pathbufs); let canister_id = self.canister_id(&args.canister)?; @@ -85,7 +86,7 @@ impl DfxOrbit { } } -fn as_path_bufs(paths: Vec) -> Vec { +fn as_path_bufs(paths: &Vec) -> Vec { paths.iter().map(|source| PathBuf::from(&source)).collect() } From fcd6211ee35356df617671fc7108728ed814e127 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Mon, 5 Aug 2024 13:57:51 +0000 Subject: [PATCH 16/42] Update version and readme --- Cargo.lock | 2 +- tools/dfx-orbit/Cargo.toml | 2 +- tools/dfx-orbit/README.md | 33 ++++++++++++++++++--------------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c7b69b0ad..19b257294 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1162,7 +1162,7 @@ dependencies = [ [[package]] name = "dfx-orbit" -version = "0.1.0" +version = "0.2.0" dependencies = [ "anyhow", "candid", diff --git a/tools/dfx-orbit/Cargo.toml b/tools/dfx-orbit/Cargo.toml index 19fa8834a..2c349461b 100644 --- a/tools/dfx-orbit/Cargo.toml +++ b/tools/dfx-orbit/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "dfx-orbit" -version = "0.1.0" +version = "0.2.0" description = "Command line tool for interacting with the Orbit digital asset manager on the ICP blockchain." authors.workspace = true edition.workspace = true diff --git a/tools/dfx-orbit/README.md b/tools/dfx-orbit/README.md index 1aaf2ff0a..93d6b73fe 100644 --- a/tools/dfx-orbit/README.md +++ b/tools/dfx-orbit/README.md @@ -5,9 +5,15 @@ It is designed to work alongside `dfx` to allow a `dfx`-like workflow to manage ## Getting started -### Installation +### Prequisites + +This guide assumes, that the user has setup and is acqainted with the following tools: -Build the tool: +- A fairly recent rust toolchain. This tool is known to work on linux using rust `1.79.0`. +- A working `dfx` development setup. +- An internet identity and an Orbit account with the correct permissions. + +### Installation Currently, there are two ways of installing `dfx-orbit`: @@ -19,19 +25,22 @@ To get the most recent version of `dfx-orbit` without manually cloning the entir cargo install -f --git https://github.com/dfinity/orbit.git --bin dfx-orbit ``` -#### Install from the repository +#### Clone and install from the repository + +This version is potentially more useful, if you want to make patches or use a specific branch. ``` -$ cargo build -p dfx-orbit +git clone https://github.com/dfinity/orbit.git +cargo install -f --path tools/dfx-orbit/ ``` Verify that the tool works: ``` -$ ./target/debug/dfx-orbit --version +$ dfx-orbit --version dfx-orbit 0.1.0 -$ ./target/debug/dfx-orbit --help +$ dfx-orbit --help Command line tool for interacting with the Orbit digital asset manager on the ICP blockchain. Usage: dfx-orbit @@ -41,13 +50,8 @@ Commands: ... ``` -Add `dfx-orbit` to your `PATH`. - ### Connect to Orbit -> **NOTE**: This assumes that you already have a `dfx` setup working. -> If you need to set up a new identity, have a look at `dfx identity new`. - Connect your local dfx identity to your Orbit identity: - Log in to Orbit. @@ -84,8 +88,6 @@ Tell the command line tool where to find the orbit station: dfx-orbit me ``` -TODO: The Oisy canister ID is also called the wallet ID and the station ID. Consistent nomenclature that doesn't conflict with established terminology would be nice. - ### Grant permission to make requests You can check which permissions you have with: @@ -165,12 +167,13 @@ This will create an Orbit request. Once approved you will be able to propose can Suppose that you have built a new Wasm and put a copy at `./MY-CANISTER.wasm.gz`. To upgrade your canister to the new Wasm: ``` -dfx-orbit request canister install --mode upgrade --wasm ./MY-CANISTER.wasm.gz MY_CANISTER +dfx-orbit request canister install --mode upgrade MY_CANISTER --wasm ./MY-CANISTER.wasm.gz ``` ### Upload assets to a canister -We will assume that Orbit is a controller of the asset canister. If not, please adapt the following commands by using `dfx canister call` instead of `dfx-orbit request canister call`. +We will assume that Orbit is a controller of the asset canister. +If not, please transfer the control of the canister to the orbit station. #### Authorize the developer to upload assets From 929d4afc791d1a7df2d2503ead189cd0f5b2b8e8 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Mon, 5 Aug 2024 13:58:01 +0000 Subject: [PATCH 17/42] Update approval UX --- tools/dfx-orbit/src/args/asset.rs | 4 ++-- tools/dfx-orbit/src/cli.rs | 12 ++++++++++-- tools/dfx-orbit/src/cli/asset.rs | 20 +++++++++++++++++--- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/tools/dfx-orbit/src/args/asset.rs b/tools/dfx-orbit/src/args/asset.rs index 9c0dfcbe0..68a71a075 100644 --- a/tools/dfx-orbit/src/args/asset.rs +++ b/tools/dfx-orbit/src/args/asset.rs @@ -65,7 +65,7 @@ pub struct AssetCheckArgs { #[clap(num_args = 1..)] pub(crate) files: Vec, - /// Automatically approve the request, if the request evidence matches the local evidence + /// Automatically approve the request, if the request's evidence matches the local evidence #[clap(long)] - then_approve: bool, + pub(crate) then_approve: bool, } diff --git a/tools/dfx-orbit/src/cli.rs b/tools/dfx-orbit/src/cli.rs index 2d6fdf7e1..3d5eef1de 100644 --- a/tools/dfx-orbit/src/cli.rs +++ b/tools/dfx-orbit/src/cli.rs @@ -2,6 +2,8 @@ pub(crate) mod asset; pub(crate) mod station; +use orbit_station_api::{RequestApprovalStatusDTO, SubmitRequestApprovalInput}; + use crate::{ args::{review::ReviewArgs, DfxOrbitArgs, DfxOrbitSubcommands}, dfx_extension_api::OrbitExtensionAgent, @@ -67,8 +69,14 @@ pub async fn exec(args: DfxOrbitArgs) -> anyhow::Result<()> { )? ); - // TODO: Reaffirm user consent before progressing with submitting - if let Ok(submit) = args.try_into() { + if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { + let action = match submit.decision { + RequestApprovalStatusDTO::Approved => "approve", + RequestApprovalStatusDTO::Rejected => "reject", + }; + dfx_core::cli::ask_for_consent(&format!( + "Would you like to {action} this request?" + ))?; dfx_orbit.station.submit(submit).await?; }; diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index ffe8f68f0..60439fccb 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -10,6 +10,7 @@ use crate::{ }; use candid::Principal; use ic_utils::Canister; +use orbit_station_api::{RequestApprovalStatusDTO, SubmitRequestApprovalInput}; use slog::Logger; use std::path::{Path, PathBuf}; @@ -67,12 +68,25 @@ impl DfxOrbit { let asset_agent = self.asset_agent(canister_id)?; let evidence = asset_agent.compute_evidence(&paths).await?; - self.check_evidence(canister_id, args.request_id, args.batch_id, evidence) - .await?; + self.check_evidence( + canister_id, + args.request_id.clone(), + args.batch_id, + evidence, + ) + .await?; println!("Local evidence matches expected arguments"); - // TODO: Handle then approve + if args.then_approve { + dfx_core::cli::ask_for_consent("Do you want to approve the request?")?; + let args = SubmitRequestApprovalInput { + decision: RequestApprovalStatusDTO::Approved, + request_id: args.request_id, + reason: None, + }; + self.station.submit(args).await?; + } Ok(()) } } From 32a84232b283059d4713e4da02c354beb89aed15 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Mon, 5 Aug 2024 14:16:16 +0000 Subject: [PATCH 18/42] Implement a request_prepare_permission method --- tests/integration/src/dfx_orbit/assets.rs | 35 +++++------------ tools/dfx-orbit/src/cli/asset.rs | 4 +- tools/dfx-orbit/src/cli/asset/util.rs | 46 +++++++++++++++++++++++ tools/dfx-orbit/src/lib.rs | 8 ++++ 4 files changed, 66 insertions(+), 27 deletions(-) diff --git a/tests/integration/src/dfx_orbit/assets.rs b/tests/integration/src/dfx_orbit/assets.rs index 4e2536ea4..223372839 100644 --- a/tests/integration/src/dfx_orbit/assets.rs +++ b/tests/integration/src/dfx_orbit/assets.rs @@ -1,11 +1,9 @@ -use dfx_orbit::AssetAgent; use pocket_ic::PocketIc; use rand::{thread_rng, Rng}; use station_api::{ - AddRequestPolicyOperationInput, CallExternalCanisterOperationInput, - CallExternalCanisterResourceTargetDTO, CanisterMethodDTO, ExecutionMethodResourceTargetDTO, - RequestOperationInput, RequestPolicyRuleDTO, RequestSpecifierDTO, - ValidationMethodResourceTargetDTO, + AddRequestPolicyOperationInput, CallExternalCanisterResourceTargetDTO, + ExecutionMethodResourceTargetDTO, RequestOperationInput, RequestPolicyRuleDTO, + RequestSpecifierDTO, ValidationMethodResourceTargetDTO, }; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tempfile::Builder; @@ -28,7 +26,7 @@ fn assets_upload() { .. } = setup_new_env(); - let (dfx_principal, _dfx_user) = setup_dfx_user(&env, &canister_ids); + let (_dfx_principal, _dfx_user) = setup_dfx_user(&env, &canister_ids); // Install the assets canister under orbit control let asset_canister = create_canister(&mut env, canister_ids.station); @@ -40,25 +38,6 @@ fn assets_upload() { Some(canister_ids.station), ); - // As admin: Setup the prepare permission in the asset canister for the dfx user - execute_request( - &env, - WALLET_ADMIN_USER, - canister_ids.station, - station_api::RequestOperationInput::CallExternalCanister( - CallExternalCanisterOperationInput { - validation_method: None, - execution_method: CanisterMethodDTO { - canister_id: asset_canister, - method_name: String::from("grant_permission"), - }, - arg: Some(AssetAgent::request_prepare_permission_payload(dfx_principal).unwrap()), - execution_method_cycles: None, - }, - ), - ) - .unwrap(); - // As admin: Grant the user the call permission, set auto-approval for external calls permit_call_operation(&env, &canister_ids); set_auto_approve(&env, &canister_ids); @@ -91,6 +70,12 @@ fn assets_upload() { // Setup the station agent let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; + // As dfx user: Request to have Prepare permission for asset_canister + let _response = dfx_orbit + .request_prepare_permission(asset_canister, None, None) + .await + .unwrap(); + // As dfx user: Request to upload new files to the asset canister let (batch_id, evidence) = dfx_orbit .upload(asset_canister, &[asset_dir.path()], false) diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 60439fccb..c7ad9cd5b 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -100,10 +100,10 @@ impl DfxOrbit { } } -fn as_path_bufs(paths: &Vec) -> Vec { +fn as_path_bufs(paths: &[String]) -> Vec { paths.iter().map(|source| PathBuf::from(&source)).collect() } -fn as_paths(paths: &Vec) -> Vec<&Path> { +fn as_paths(paths: &[PathBuf]) -> Vec<&Path> { paths.iter().map(|pathbuf| pathbuf.as_path()).collect() } diff --git a/tools/dfx-orbit/src/cli/asset/util.rs b/tools/dfx-orbit/src/cli/asset/util.rs index 33246ba4e..bb8c6e75d 100644 --- a/tools/dfx-orbit/src/cli/asset/util.rs +++ b/tools/dfx-orbit/src/cli/asset/util.rs @@ -1,6 +1,52 @@ +use crate::DfxOrbit; + use super::AssetAgent; use candid::Principal; use ic_certified_assets::types::{GrantPermissionArguments, Permission}; +use orbit_station_api::{ + CallExternalCanisterOperationInput, CanisterMethodDTO, CreateRequestInput, + CreateRequestResponse, RequestOperationInput, +}; + +impl DfxOrbit { + /// Request from the station to grant the `Prepare` permission for the asset canister + pub async fn request_prepare_permission( + &self, + canister_id: Principal, + title: Option, + summary: Option, + ) -> anyhow::Result { + let me = self.own_principal()?; + + let args = GrantPermissionArguments { + to_principal: me, + permission: Permission::Prepare, + }; + let arg = candid::encode_one(args)?; + + let response = self + .station + .request(CreateRequestInput { + operation: RequestOperationInput::CallExternalCanister( + CallExternalCanisterOperationInput { + validation_method: None, + execution_method: CanisterMethodDTO { + canister_id, + method_name: String::from("grant_permission"), + }, + arg: Some(arg), + execution_method_cycles: None, + }, + ), + title, + summary, + execution_plan: None, + }) + .await?; + + Ok(response) + } +} impl AssetAgent<'_> { // TODO: Turn into a functionality diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index 38d53c9d8..4ffae6a91 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -64,6 +64,14 @@ impl DfxOrbit { Ok(canister_id) } + pub fn own_principal(&self) -> anyhow::Result { + self + .interface + .identity() + .sender() + .map_err(anyhow::Error::msg) + } + pub fn canister_agent(&self, canister_id: Principal) -> anyhow::Result { Ok(CanisterBuilder::new() .with_agent(self.interface.agent()) From cbf9848b9a5c8a476fb87921e4dc7d90f87fed3d Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Mon, 5 Aug 2024 14:33:45 +0000 Subject: [PATCH 19/42] Functionality to request prepare permission --- tools/dfx-orbit/README.md | 34 ++++++------------------------- tools/dfx-orbit/src/args/asset.rs | 16 +++++++++++++++ tools/dfx-orbit/src/cli.rs | 9 +++----- tools/dfx-orbit/src/cli/asset.rs | 9 ++++++++ tools/dfx-orbit/src/lib.rs | 12 +++++++++-- 5 files changed, 44 insertions(+), 36 deletions(-) diff --git a/tools/dfx-orbit/README.md b/tools/dfx-orbit/README.md index 93d6b73fe..ba65cac8a 100644 --- a/tools/dfx-orbit/README.md +++ b/tools/dfx-orbit/README.md @@ -180,43 +180,21 @@ If not, please transfer the control of the canister to the orbit station. Note: Uploaded assets are not published. They are only prepared for release. ``` -developer_principal="$(dfx identity get-principal)" -dfx-orbit request canister call frontend grant_permission " -( - record { - permission = variant { Prepare }; - to_principal = principal \"$developer_principal\"; - }, -) -" +dfx-orbit asset request-prepare-permission frontend ``` -When the request has been approved, check the list of principals permitted to prepare assets: +In case you want to verify, whether you have the `Prepare` permission on the asset canister, +run: ``` dfx canister call frontend list_permitted '(record { permission = variant { Prepare } })' ``` -#### Authorize the orbit station to commit assets - -Note: Committing uploaded assets causes them to be published on the asset canister web site. - -``` -station_principal="$(dfx-orbit station show | jq -r .station_id)" -dfx-orbit request canister call frontend grant_permission " -( - record { - permission = variant { Commit }; - to_principal = principal \"$station_principal\"; - }, -) -" -``` - -When the request has been approved, check the list of principals permitted to commit assets: +and check whether your principal is among the ones listed. +You can optain your own principal via: ``` -dfx canister call frontend list_permitted '(record { permission = variant { Commit } })' +dfx identity get-principal ``` #### Request an asset update diff --git a/tools/dfx-orbit/src/args/asset.rs b/tools/dfx-orbit/src/args/asset.rs index 68a71a075..b12894aa9 100644 --- a/tools/dfx-orbit/src/args/asset.rs +++ b/tools/dfx-orbit/src/args/asset.rs @@ -11,6 +11,8 @@ pub struct AssetArgs { #[derive(Debug, Clone, Subcommand)] pub enum AssetArgsAction { + /// Request to grant this user Prepare permission for the asset canister + RequestPreparePermission(AssetReqeustPreparePermissionArgs), /// Upload assets to an asset canister Upload(AssetUploadArgs), /// Compute local evidence @@ -19,6 +21,20 @@ pub enum AssetArgsAction { Check(AssetCheckArgs), } +#[derive(Debug, Clone, Parser)] +pub struct AssetReqeustPreparePermissionArgs { + /// The name of the asset canister targeted by this action + pub(crate) canister: String, + + /// The title of the request to grant Prepare permission + #[clap(long)] + pub(crate) title: Option, + + /// The summary of the request to grant Prepare permission + #[clap(long)] + pub(crate) summary: Option, +} + #[derive(Debug, Clone, Parser)] pub struct AssetUploadArgs { /// The name of the asset canister targeted by this action diff --git a/tools/dfx-orbit/src/cli.rs b/tools/dfx-orbit/src/cli.rs index 3d5eef1de..8ac0a63f2 100644 --- a/tools/dfx-orbit/src/cli.rs +++ b/tools/dfx-orbit/src/cli.rs @@ -29,15 +29,12 @@ pub async fn exec(args: DfxOrbitArgs) -> anyhow::Result<()> { Ok(()) } DfxOrbitSubcommands::Request(request_args) => { - let response = dfx_orbit + let request = dfx_orbit .station .request(request_args.into_create_request_input(&dfx_orbit)?) .await?; - let request_id = &response.request.id; - let request_url = dfx_orbit.station.request_url(request_id); - println!("Created request: {request_id}"); - println!("Request URL: {request_url}"); - println!("To view the request, run: dfx-orbit review id {request_id}"); + dfx_orbit.print_create_request_info(&request); + Ok(()) } DfxOrbitSubcommands::Review(review_args) => match review_args { diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index c7ad9cd5b..3f94fd816 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -22,6 +22,15 @@ pub struct AssetAgent<'agent> { impl DfxOrbit { pub async fn exec_asset(&mut self, args: AssetArgs) -> anyhow::Result<()> { match args.action { + AssetArgsAction::RequestPreparePermission(args) => { + let canister_id = self.canister_id(&args.canister)?; + let request = self + .request_prepare_permission(canister_id, args.title, args.summary) + .await?; + self.print_create_request_info(&request); + + Ok(()) + } AssetArgsAction::Upload(args) => { let pathbufs = as_path_bufs(&args.files); let paths = as_paths(&pathbufs); diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index 4ffae6a91..694469923 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -14,6 +14,7 @@ use candid::Principal; use dfx_core::{config::model::canister_id_store::CanisterIdStore, DfxInterface}; use dfx_extension_api::OrbitExtensionAgent; use ic_utils::{canister::CanisterBuilder, Canister}; +use orbit_station_api::CreateRequestResponse; use slog::{o, Drain, Logger}; pub use cli::asset::AssetAgent; @@ -65,8 +66,7 @@ impl DfxOrbit { } pub fn own_principal(&self) -> anyhow::Result { - self - .interface + self.interface .identity() .sender() .map_err(anyhow::Error::msg) @@ -78,4 +78,12 @@ impl DfxOrbit { .with_canister_id(canister_id) .build()?) } + + pub fn print_create_request_info(&self, response: &CreateRequestResponse) { + let request_id = &response.request.id; + let request_url = self.station.request_url(request_id); + println!("Created request: {request_id}"); + println!("Request URL: {request_url}"); + println!("To view the request, run: dfx-orbit review id {request_id}"); + } } From 977d4dc5a2dc2cba542462647705a89d6cc277d8 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Tue, 6 Aug 2024 12:39:02 +0000 Subject: [PATCH 20/42] WIP Implement lookup for sources --- tools/dfx-orbit/src/args/asset.rs | 3 --- tools/dfx-orbit/src/cli/asset.rs | 21 +++++----------- tools/dfx-orbit/src/cli/asset/util.rs | 36 +++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/tools/dfx-orbit/src/args/asset.rs b/tools/dfx-orbit/src/args/asset.rs index b12894aa9..42221c065 100644 --- a/tools/dfx-orbit/src/args/asset.rs +++ b/tools/dfx-orbit/src/args/asset.rs @@ -53,7 +53,6 @@ pub struct AssetUploadArgs { pub(crate) summary: Option, /// The source directories to upload (multiple values possible) - #[clap(num_args = 1..)] pub(crate) files: Vec, } @@ -62,7 +61,6 @@ pub struct AssetComputeEvidenceArgs { /// The name of the asset canister targeted by this action pub(crate) canister: String, /// The source directories to compute evidence from (multiple values possible) - #[clap(num_args = 1..)] pub(crate) files: Vec, } @@ -78,7 +76,6 @@ pub struct AssetCheckArgs { pub(crate) batch_id: Nat, /// The source directories of the asset upload (multiple values possible) - #[clap(num_args = 1..)] pub(crate) files: Vec, /// Automatically approve the request, if the request's evidence matches the local evidence diff --git a/tools/dfx-orbit/src/cli/asset.rs b/tools/dfx-orbit/src/cli/asset.rs index 3f94fd816..5cd8731c3 100644 --- a/tools/dfx-orbit/src/cli/asset.rs +++ b/tools/dfx-orbit/src/cli/asset.rs @@ -12,7 +12,6 @@ use candid::Principal; use ic_utils::Canister; use orbit_station_api::{RequestApprovalStatusDTO, SubmitRequestApprovalInput}; use slog::Logger; -use std::path::{Path, PathBuf}; pub struct AssetAgent<'agent> { canister_agent: Canister<'agent>, @@ -32,8 +31,8 @@ impl DfxOrbit { Ok(()) } AssetArgsAction::Upload(args) => { - let pathbufs = as_path_bufs(&args.files); - let paths = as_paths(&pathbufs); + let pathbufs = self.as_path_bufs(&args.canister, &args.files)?; + let paths = Self::as_paths(&pathbufs); let canister_name = args.canister; let canister_id = self.canister_id(&canister_name)?; @@ -59,8 +58,8 @@ impl DfxOrbit { Ok(()) } AssetArgsAction::ComputeEvidence(args) => { - let pathbufs = as_path_bufs(&args.files); - let paths = as_paths(&pathbufs); + let pathbufs = self.as_path_bufs(&args.canister, &args.files)?; + let paths = Self::as_paths(&pathbufs); let canister_id = self.canister_id(&args.canister)?; let asset_agent = self.asset_agent(canister_id)?; @@ -70,8 +69,8 @@ impl DfxOrbit { Ok(()) } AssetArgsAction::Check(args) => { - let pathbufs = as_path_bufs(&args.files); - let paths = as_paths(&pathbufs); + let pathbufs = self.as_path_bufs(&args.canister, &args.files)?; + let paths = Self::as_paths(&pathbufs); let canister_id = self.canister_id(&args.canister)?; let asset_agent = self.asset_agent(canister_id)?; @@ -108,11 +107,3 @@ impl DfxOrbit { }) } } - -fn as_path_bufs(paths: &[String]) -> Vec { - paths.iter().map(|source| PathBuf::from(&source)).collect() -} - -fn as_paths(paths: &[PathBuf]) -> Vec<&Path> { - paths.iter().map(|pathbuf| pathbuf.as_path()).collect() -} diff --git a/tools/dfx-orbit/src/cli/asset/util.rs b/tools/dfx-orbit/src/cli/asset/util.rs index bb8c6e75d..44acff5b5 100644 --- a/tools/dfx-orbit/src/cli/asset/util.rs +++ b/tools/dfx-orbit/src/cli/asset/util.rs @@ -1,7 +1,11 @@ +use std::path::{Path, PathBuf}; + use crate::DfxOrbit; use super::AssetAgent; +use anyhow::{anyhow, bail}; use candid::Principal; +use dfx_core::config::model::dfinity::CanisterTypeProperties; use ic_certified_assets::types::{GrantPermissionArguments, Permission}; use orbit_station_api::{ CallExternalCanisterOperationInput, CanisterMethodDTO, CreateRequestInput, @@ -46,6 +50,38 @@ impl DfxOrbit { Ok(response) } + + pub(crate) fn as_path_bufs( + &self, + canister: &str, + paths: &[String], + ) -> anyhow::Result> { + if paths.is_empty() { + let config = self.interface.config().ok_or_else(|| { + anyhow!("Could not read \"dfx.json\". Are you in the correct directory?") + })?; + + let canister_config = config + .get_config() + .canisters + .as_ref() + .ok_or_else(|| anyhow!("No canisters defined in this \"dfx.json\""))? + .get(canister) + .ok_or_else(|| anyhow!("Could not find {canister} in \"dfx.json\""))?; + + let CanisterTypeProperties::Assets { source, .. } = &canister_config.type_specific + else { + bail!("Canister {canister} is not an asset canister"); + }; + Ok(source.clone()) + } else { + Ok(paths.iter().map(|source| PathBuf::from(&source)).collect()) + } + } + + pub(crate) fn as_paths(paths: &[PathBuf]) -> Vec<&Path> { + paths.iter().map(|pathbuf| pathbuf.as_path()).collect() + } } impl AssetAgent<'_> { From 834c07ccbe16a42a3a4f57c931935f292565ee92 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Tue, 6 Aug 2024 13:32:05 +0000 Subject: [PATCH 21/42] Test the new functionality --- Cargo.lock | 10 ++++ Cargo.toml | 1 + tests/integration/Cargo.toml | 1 + tests/integration/src/dfx_orbit.rs | 52 ++++++++++++++++--- tests/integration/src/dfx_orbit/assets.rs | 43 ++++++++++----- .../src/dfx_orbit/canister_call.rs | 6 ++- tests/integration/src/dfx_orbit/me.rs | 4 +- tests/integration/src/dfx_orbit/review.rs | 5 +- tools/dfx-orbit/src/cli/asset/util.rs | 6 +-- 9 files changed, 98 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 19b257294..cb460d72e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2401,6 +2401,7 @@ dependencies = [ "hex", "ic-cdk 0.13.2", "ic-ledger-types", + "itertools 0.13.0", "lazy_static", "num-bigint 0.4.5", "orbit-essentials", @@ -2486,6 +2487,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.11" diff --git a/Cargo.toml b/Cargo.toml index 28618af50..c397e8818 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,7 @@ ic-cdk-timers = "0.7.0" ic-ledger-types = "0.10.0" ic-stable-structures = "0.6.4" ic-utils = { git = "https://github.com/dfinity/agent-rs.git", rev = "be929fd7967249c879f48f2f494cbfc5805a7d98" } +itertools = "0.13.0" lazy_static = "1.4.0" mockall = "0.12.1" num-bigint = "0.4" diff --git a/tests/integration/Cargo.toml b/tests/integration/Cargo.toml index 743928c68..d5248d50f 100644 --- a/tests/integration/Cargo.toml +++ b/tests/integration/Cargo.toml @@ -12,6 +12,7 @@ hex = { workspace = true } orbit-essentials = { path = '../../libs/orbit-essentials', version = '0.0.2-alpha.3' } ic-cdk = { workspace = true } ic-ledger-types = { workspace = true } +itertools = { workspace = true } lazy_static = { workspace = true } num-bigint = { workspace = true } pocket-ic = { workspace = true } diff --git a/tests/integration/src/dfx_orbit.rs b/tests/integration/src/dfx_orbit.rs index 3e749aeb6..8badb9e8f 100644 --- a/tests/integration/src/dfx_orbit.rs +++ b/tests/integration/src/dfx_orbit.rs @@ -5,12 +5,14 @@ use crate::{ }; use candid::Principal; use dfx_orbit::{dfx_extension_api::OrbitExtensionAgent, station_agent::StationConfig, DfxOrbit}; +use itertools::Itertools; use pocket_ic::PocketIc; use rand::Rng; use rand_chacha::{rand_core::SeedableRng, ChaCha8Rng}; use station_api::UserDTO; use std::{ cell::RefCell, + collections::BTreeMap, future::Future, hash::{DefaultHasher, Hash, Hasher}, path::Path, @@ -43,7 +45,17 @@ const IDENTITY_JSON: &str = " \"default\": \"default\" }"; -fn dfx_orbit_test(env: &mut PocketIc, test_func: F) -> F::Output +/// The test setup needs to be configurable +/// +/// This struct allows to gradually introduce configurations into the `dfx_orbit` tests +/// to allow testing more fine grained controls +#[derive(Debug, Clone, Default)] +struct DfxOrbitTestConfig { + /// Sets the asset canisters to be defined in the dfx.json, maps name tp list of paths + asset_canisters: BTreeMap>, +} + +fn dfx_orbit_test(env: &mut PocketIc, config: DfxOrbitTestConfig, test_func: F) -> F::Output where F: Future, { @@ -90,7 +102,7 @@ where *port.borrow() }); - setup_test_dfx_json(tmp_dir.path()); + setup_test_dfx_json(tmp_dir.path(), config); setup_identity(tmp_dir.path()); // Start the live environment @@ -123,15 +135,38 @@ fn setup_identity(dfx_root: &Path) { std::fs::write(default_id_path.join("identity.pem"), TEST_KEY).unwrap(); } -fn setup_test_dfx_json(dfx_root: &Path) { +/// Sets up a custom `dfx.json` from the provided `config` +fn setup_test_dfx_json(dfx_root: &Path, config: DfxOrbitTestConfig) { let port = PORT.with(|port| *port.borrow()); - let dfx_json = test_dfx_json_from_template(port); + let dfx_json = test_dfx_json_from_template(config, port); + println!("{}", &dfx_json); std::fs::write(dfx_root.join("dfx.json"), dfx_json).unwrap(); } -fn test_dfx_json_from_template(port: u16) -> String { +/// Generate a custom `dfx.json` from the provided `config` +fn test_dfx_json_from_template(config: DfxOrbitTestConfig, port: u16) -> String { + let asset_canisters = config + .asset_canisters + .iter() + .map(|(name, sources)| { + ( + name, + sources + .iter() + .map(|source| format!("\"{source}\"")) + .join(","), + ) + }) + .map(|(name, sources)| { + format!("\"{name}\": {{ \"source\": [{sources}], \"type\": \"assets\"}}") + }) + .join(","); + format!( "{{ + \"canisters\": {{ + {asset_canisters} + }}, \"networks\": {{ \"test\": {{ \"providers\": [ @@ -175,6 +210,7 @@ fn setup_dfx_user(env: &PocketIc, canister_ids: &CanisterIds) -> (Principal, Use (dfx_principal, dfx_user) } +/// Install the counter cansiter under given `canister_id` into the running IC fn setup_counter_canister(env: &mut PocketIc, canister_ids: &CanisterIds) -> Principal { // create and install the counter canister let canister_id = create_canister(env, canister_ids.station); @@ -192,6 +228,10 @@ fn setup_counter_canister(env: &mut PocketIc, canister_ids: &CanisterIds) -> Pri canister_id } +/// Fetches an asset from the local host and port +/// +/// This is a bit tricky, as the boundary node uses the `Referer` header to determine the +/// resource being fetched. async fn fetch_asset(canister_id: Principal, path: &str) -> Vec { let port = PORT.with(|port| *port.borrow()); let local_url = format!("http://localhost:{}{}", port, path); @@ -208,5 +248,3 @@ async fn fetch_asset(canister_id: Principal, path: &str) -> Vec { .unwrap() .into() } - -// TODO: Test canister update diff --git a/tests/integration/src/dfx_orbit/assets.rs b/tests/integration/src/dfx_orbit/assets.rs index 223372839..32fd2ff28 100644 --- a/tests/integration/src/dfx_orbit/assets.rs +++ b/tests/integration/src/dfx_orbit/assets.rs @@ -1,13 +1,4 @@ -use pocket_ic::PocketIc; -use rand::{thread_rng, Rng}; -use station_api::{ - AddRequestPolicyOperationInput, CallExternalCanisterResourceTargetDTO, - ExecutionMethodResourceTargetDTO, RequestOperationInput, RequestPolicyRuleDTO, - RequestSpecifierDTO, ValidationMethodResourceTargetDTO, -}; -use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use tempfile::Builder; - +use super::DfxOrbitTestConfig; use crate::{ dfx_orbit::{ canister_call::permit_call_operation, dfx_orbit_test, fetch_asset, setup_dfx_orbit, @@ -17,6 +8,19 @@ use crate::{ utils::execute_request, CanisterIds, TestEnv, }; +use pocket_ic::PocketIc; +use rand::{thread_rng, Rng}; +use station_api::{ + AddRequestPolicyOperationInput, CallExternalCanisterResourceTargetDTO, + ExecutionMethodResourceTargetDTO, RequestOperationInput, RequestPolicyRuleDTO, + RequestSpecifierDTO, ValidationMethodResourceTargetDTO, +}; +use std::{ + collections::BTreeMap, + path::Path, + time::{Duration, SystemTime, UNIX_EPOCH}, +}; +use tempfile::Builder; #[test] fn assets_upload() { @@ -66,7 +70,14 @@ fn assets_upload() { ) .unwrap(); - dfx_orbit_test(&mut env, async { + let mut asset_canisters = BTreeMap::new(); + asset_canisters.insert( + String::from("test_asset_upload"), + vec![asset_dir.path().to_str().unwrap().to_string()], + ); + let config = DfxOrbitTestConfig { asset_canisters }; + + dfx_orbit_test(&mut env, config, async { // Setup the station agent let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; @@ -75,10 +86,18 @@ fn assets_upload() { .request_prepare_permission(asset_canister, None, None) .await .unwrap(); + tokio::time::sleep(Duration::from_secs(1)).await; + + // Test that we can retreive the sources from `dfx.json` + let sources = dfx_orbit.as_path_bufs("test_asset_upload", &[]).unwrap(); + let sources_path = sources + .iter() + .map(|pathbuf| pathbuf.as_path()) + .collect::>(); // As dfx user: Request to upload new files to the asset canister let (batch_id, evidence) = dfx_orbit - .upload(asset_canister, &[asset_dir.path()], false) + .upload(asset_canister, &sources_path, false) .await .unwrap(); let response = dfx_orbit diff --git a/tests/integration/src/dfx_orbit/canister_call.rs b/tests/integration/src/dfx_orbit/canister_call.rs index 4322c859c..ff914315d 100644 --- a/tests/integration/src/dfx_orbit/canister_call.rs +++ b/tests/integration/src/dfx_orbit/canister_call.rs @@ -1,5 +1,7 @@ use crate::{ - dfx_orbit::{dfx_orbit_test, setup_counter_canister, setup_dfx_orbit, setup_dfx_user}, + dfx_orbit::{ + dfx_orbit_test, setup_counter_canister, setup_dfx_orbit, setup_dfx_user, DfxOrbitTestConfig, + }, setup::{setup_new_env, WALLET_ADMIN_USER}, utils::{ add_user, execute_request, submit_request_approval, update_raw, user_test_id, @@ -53,7 +55,7 @@ fn canister_call() { execution_plan: None, }; - let request = dfx_orbit_test(&mut env, async { + let request = dfx_orbit_test(&mut env, DfxOrbitTestConfig::default(), async { // Setup the station agent let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; diff --git a/tests/integration/src/dfx_orbit/me.rs b/tests/integration/src/dfx_orbit/me.rs index b8aefd3c4..ed3a127e3 100644 --- a/tests/integration/src/dfx_orbit/me.rs +++ b/tests/integration/src/dfx_orbit/me.rs @@ -1,5 +1,5 @@ use crate::{ - dfx_orbit::{dfx_orbit_test, setup_dfx_orbit, setup_dfx_user}, + dfx_orbit::{dfx_orbit_test, setup_dfx_orbit, setup_dfx_user, DfxOrbitTestConfig}, setup::setup_new_env, TestEnv, }; @@ -15,7 +15,7 @@ fn me() { let (_, dfx_user) = setup_dfx_user(&env, &canister_ids); - let response = dfx_orbit_test(&mut env, async { + let response = dfx_orbit_test(&mut env, DfxOrbitTestConfig::default(), async { // Setup the station agent let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; diff --git a/tests/integration/src/dfx_orbit/review.rs b/tests/integration/src/dfx_orbit/review.rs index 06b940f45..a658aea0d 100644 --- a/tests/integration/src/dfx_orbit/review.rs +++ b/tests/integration/src/dfx_orbit/review.rs @@ -10,7 +10,8 @@ use station_api::{ use crate::{ dfx_orbit::{ canister_call::{permit_call_operation, set_four_eyes_on_call}, - dfx_orbit_test, setup_counter_canister, setup_dfx_orbit, TEST_PRINCIPAL, + dfx_orbit_test, setup_counter_canister, setup_dfx_orbit, DfxOrbitTestConfig, + TEST_PRINCIPAL, }, setup::{setup_new_env, WALLET_ADMIN_USER}, utils::{ @@ -71,7 +72,7 @@ fn review() { let ctr = update_raw(&env, canister_id, Principal::anonymous(), "read", vec![]).unwrap(); assert_eq!(ctr, 0_u32.to_le_bytes()); - dfx_orbit_test(&mut env, async { + dfx_orbit_test(&mut env, DfxOrbitTestConfig::default(), async { // Setup the station agent let dfx_orbit = setup_dfx_orbit(canister_ids.station).await; diff --git a/tools/dfx-orbit/src/cli/asset/util.rs b/tools/dfx-orbit/src/cli/asset/util.rs index 44acff5b5..edfc8a099 100644 --- a/tools/dfx-orbit/src/cli/asset/util.rs +++ b/tools/dfx-orbit/src/cli/asset/util.rs @@ -51,11 +51,7 @@ impl DfxOrbit { Ok(response) } - pub(crate) fn as_path_bufs( - &self, - canister: &str, - paths: &[String], - ) -> anyhow::Result> { + pub fn as_path_bufs(&self, canister: &str, paths: &[String]) -> anyhow::Result> { if paths.is_empty() { let config = self.interface.config().ok_or_else(|| { anyhow!("Could not read \"dfx.json\". Are you in the correct directory?") From 4d5259ddad9df6e7c9f90aa228daf473d38219d8 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Wed, 7 Aug 2024 14:58:04 +0000 Subject: [PATCH 22/42] Settable verbosity levels --- Cargo.lock | 3 +++ tests/integration/Cargo.toml | 3 +++ tests/integration/src/dfx_orbit.rs | 12 +++++++++++- tools/dfx-orbit/src/args.rs | 25 ++++++++++++++++++------- tools/dfx-orbit/src/cli.rs | 27 ++++++++++++++++++++++++++- tools/dfx-orbit/src/lib.rs | 9 ++------- tools/dfx-orbit/src/main.rs | 1 - 7 files changed, 63 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cb460d72e..e5d1bb512 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2411,6 +2411,9 @@ dependencies = [ "reqwest", "serde", "sha2 0.10.8", + "slog", + "slog-async", + "slog-term", "station-api", "tempfile", "tokio", diff --git a/tests/integration/Cargo.toml b/tests/integration/Cargo.toml index d5248d50f..a8903d42d 100644 --- a/tests/integration/Cargo.toml +++ b/tests/integration/Cargo.toml @@ -21,6 +21,9 @@ rand_chacha = { workspace = true } reqwest = { workspace = true } sha2 = { workspace = true } serde = { workspace = true, features = ['derive'] } +slog = { workspace = true } +slog-async = { workspace = true } +slog-term = { workspace = true } tempfile = { workspace = true } tokio.workspace = true uuid = { workspace = true } diff --git a/tests/integration/src/dfx_orbit.rs b/tests/integration/src/dfx_orbit.rs index 8badb9e8f..78f8df24a 100644 --- a/tests/integration/src/dfx_orbit.rs +++ b/tests/integration/src/dfx_orbit.rs @@ -181,6 +181,16 @@ fn test_dfx_json_from_template(config: DfxOrbitTestConfig, port: u16) -> String /// Setup the station agent for the test async fn setup_dfx_orbit(station_id: Principal) -> DfxOrbit { + // Setup a logger with highest log level. Capture logging by test harness + use slog::Drain; + let decorator = slog_term::PlainDecorator::new(slog_term::TestStdoutWriter); + let drain = slog_term::FullFormat::new(decorator) + .build() + .filter_level(slog::Level::Trace) + .fuse(); + let drain = slog_async::Async::new(drain).build().fuse(); + let logger = slog::Logger::root(drain, slog::o!()); + let port = PORT.with(|port| *port.borrow()); let orbit_agent = OrbitExtensionAgent::new().unwrap(); @@ -193,7 +203,7 @@ async fn setup_dfx_orbit(station_id: Principal) -> DfxOrbit { }) .unwrap(); - DfxOrbit::new(orbit_agent).await.unwrap() + DfxOrbit::new(orbit_agent, logger).await.unwrap() } /// Create the dfx user's identities and add them to the station diff --git a/tools/dfx-orbit/src/args.rs b/tools/dfx-orbit/src/args.rs index 1c3b25813..cd2f860e9 100644 --- a/tools/dfx-orbit/src/args.rs +++ b/tools/dfx-orbit/src/args.rs @@ -13,26 +13,37 @@ use station::StationArgs; /// Manages Orbit on the Internet Computer. // TODO: Specify --station to not use the default station // TODO: Better version information -// TODO: -v flag #[derive(Parser, Debug)] -#[command(version, about, long_about = None)] +#[clap(version, about, long_about = None)] pub struct DfxOrbitArgs { + /// Increase verbosity level + #[clap(short, long, action = clap::ArgAction::Count, conflicts_with = "quiet")] + pub(crate) verbose: u8, + + /// Reduce verbosity level + #[clap(short, long, action = clap::ArgAction::Count, conflicts_with = "verbose")] + pub(crate) quiet: u8, + + /// Name of the station to execute the command on. (Uses default station if unspecified) + #[clap(short, long)] + pub(crate) station: Option, + /// Manage Orbit stations. - #[command(subcommand)] - pub command: DfxOrbitSubcommands, + #[clap(subcommand)] + pub(crate) command: DfxOrbitSubcommands, } /// CLI commands for managing Orbit on the Internet Computer. #[derive(Debug, Subcommand)] -#[command(version, about, long_about = None)] +#[clap(version, about, long_about = None)] pub enum DfxOrbitSubcommands { /// Manage Orbit stations. - #[command(subcommand)] + #[clap(subcommand)] Station(StationArgs), /// Make requests to Orbit Request(RequestArgs), /// View and decide on requests. - #[command(subcommand)] + #[clap(subcommand)] Review(ReviewArgs), /// Manage assets stored in an asset canister through Orbit Asset(AssetArgs), diff --git a/tools/dfx-orbit/src/cli.rs b/tools/dfx-orbit/src/cli.rs index 8ac0a63f2..0ff07dfd9 100644 --- a/tools/dfx-orbit/src/cli.rs +++ b/tools/dfx-orbit/src/cli.rs @@ -3,6 +3,7 @@ pub(crate) mod asset; pub(crate) mod station; use orbit_station_api::{RequestApprovalStatusDTO, SubmitRequestApprovalInput}; +use slog::trace; use crate::{ args::{review::ReviewArgs, DfxOrbitArgs, DfxOrbitSubcommands}, @@ -12,6 +13,9 @@ use crate::{ /// A command line tool for interacting with Orbit on the Internet Computer. pub async fn exec(args: DfxOrbitArgs) -> anyhow::Result<()> { + let logger = init_logger(args.verbose, args.quiet); + trace!(logger, "Calling tool with arguments:\n{:#?}", args); + let orbit_agent = OrbitExtensionAgent::new()?; // We don't need to instanciate a StationAgent to execute this command directly on the orbit agent @@ -20,7 +24,7 @@ pub async fn exec(args: DfxOrbitArgs) -> anyhow::Result<()> { return Ok(()); }; - let mut dfx_orbit = DfxOrbit::new(orbit_agent).await?; + let mut dfx_orbit = DfxOrbit::new(orbit_agent, logger).await?; match args.command { DfxOrbitSubcommands::Me => { @@ -87,3 +91,24 @@ pub async fn exec(args: DfxOrbitArgs) -> anyhow::Result<()> { _ => unreachable!(), } } + +/// Initalize the logger +/// +/// Default log level is WARN, can be turned up to TRCE by adding -v flags +/// and down to CRIT by adding -q flags +fn init_logger(verbose: u8, quiet: u8) -> slog::Logger { + use slog::Drain; + + let verbose = verbose.clamp(0, 3); + let quiet = quiet.clamp(0, 2); + let level = 3 + verbose - quiet; + let level = slog::Level::from_usize(level as usize).unwrap(); + + let decorator = slog_term::TermDecorator::new().build(); + let drain = slog_term::FullFormat::new(decorator) + .build() + .filter_level(level) + .fuse(); + let drain = slog_async::Async::new(drain).build().fuse(); + slog::Logger::root(drain, slog::o!()) +} diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index 694469923..585b6b304 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -15,9 +15,9 @@ use dfx_core::{config::model::canister_id_store::CanisterIdStore, DfxInterface}; use dfx_extension_api::OrbitExtensionAgent; use ic_utils::{canister::CanisterBuilder, Canister}; use orbit_station_api::CreateRequestResponse; -use slog::{o, Drain, Logger}; pub use cli::asset::AssetAgent; +use slog::Logger; pub use station_agent::StationAgent; pub struct DfxOrbit { // The station agent that handles communication with the station @@ -32,17 +32,12 @@ pub struct DfxOrbit { impl DfxOrbit { /// Creates a new agent for communicating with the default station. - pub async fn new(mut agent: OrbitExtensionAgent) -> anyhow::Result { + pub async fn new(mut agent: OrbitExtensionAgent, logger: Logger) -> anyhow::Result { let config = agent .default_station()? .ok_or_else(|| anyhow::format_err!("No default station specified"))?; let interface = agent.dfx_interface().await?; - let decorator = slog_term::TermDecorator::new().build(); - let drain = slog_term::FullFormat::new(decorator).build().fuse(); - let drain = slog_async::Async::new(drain).build().fuse(); - let logger = slog::Logger::root(drain, o!()); - Ok(Self { station: StationAgent::new(interface.agent().clone(), config), dfx: agent, diff --git a/tools/dfx-orbit/src/main.rs b/tools/dfx-orbit/src/main.rs index d42130464..f74ff8f34 100644 --- a/tools/dfx-orbit/src/main.rs +++ b/tools/dfx-orbit/src/main.rs @@ -8,7 +8,6 @@ use tokio::runtime::Builder; fn main() { let args = DfxOrbitArgs::parse(); - dbg!(&args); let runtime = Builder::new_current_thread() .enable_all() .build() From 9a3cea2128f2faa38ff5dc9614b71130c600241a Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Thu, 8 Aug 2024 14:42:03 +0000 Subject: [PATCH 23/42] Settable --station --- tests/integration/src/dfx_orbit.rs | 17 +++++++---------- tools/dfx-orbit/src/cli.rs | 9 ++++++++- tools/dfx-orbit/src/dfx_extension_api.rs | 11 +++++------ tools/dfx-orbit/src/lib.rs | 12 +++++++----- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/tests/integration/src/dfx_orbit.rs b/tests/integration/src/dfx_orbit.rs index f532c4c05..16255a88c 100644 --- a/tests/integration/src/dfx_orbit.rs +++ b/tests/integration/src/dfx_orbit.rs @@ -193,16 +193,13 @@ async fn setup_dfx_orbit(station_id: Principal) -> DfxOrbit { let port = PORT.with(|port| *port.borrow()); let orbit_agent = OrbitExtensionAgent::new().unwrap(); - orbit_agent - .add_station(StationConfig { - name: String::from("Test"), - station_id, - network: String::from("test"), - url: format!("http://localhost:{}", port), - }) - .unwrap(); - - DfxOrbit::new(orbit_agent, logger).await.unwrap() + let config = StationConfig { + name: String::from("Test"), + station_id, + network: String::from("test"), + url: format!("http://localhost:{}", port), + }; + DfxOrbit::new(orbit_agent, config, logger).await.unwrap() } /// Create the dfx user's identities and add them to the station diff --git a/tools/dfx-orbit/src/cli.rs b/tools/dfx-orbit/src/cli.rs index 9d18388d3..255c5d91c 100644 --- a/tools/dfx-orbit/src/cli.rs +++ b/tools/dfx-orbit/src/cli.rs @@ -23,7 +23,14 @@ pub async fn exec(args: DfxOrbitArgs) -> anyhow::Result<()> { return Ok(()); }; - let mut dfx_orbit = DfxOrbit::new(orbit_agent, logger).await?; + let config = match args.station { + Some(station_name) => orbit_agent.station(&station_name)?, + None => orbit_agent + .default_station()? + .ok_or_else(|| anyhow::format_err!("No default station specified"))?, + }; + + let mut dfx_orbit = DfxOrbit::new(orbit_agent, config, logger).await?; match args.command { DfxOrbitSubcommands::Me => { diff --git a/tools/dfx-orbit/src/dfx_extension_api.rs b/tools/dfx-orbit/src/dfx_extension_api.rs index 6d3978d77..5cd1dc082 100644 --- a/tools/dfx-orbit/src/dfx_extension_api.rs +++ b/tools/dfx-orbit/src/dfx_extension_api.rs @@ -94,12 +94,11 @@ impl OrbitExtensionAgent { } /// Gets the dfx_core interface - pub async fn dfx_interface(&mut self) -> anyhow::Result { - let network_name = self - .station_or_default(None) - .with_context(|| "Failed to get station")? - .network; - let interface_builder = DfxInterface::builder().with_network_named(&network_name); + pub(crate) async fn dfx_interface( + &mut self, + network_name: &str, + ) -> anyhow::Result { + let interface_builder = DfxInterface::builder().with_network_named(network_name); let interface = interface_builder.build().await?; if !interface.network_descriptor().is_ic { interface.agent().fetch_root_key().await?; diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index a60d4e443..ac14fe3d7 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -18,6 +18,7 @@ use ic_utils::{canister::CanisterBuilder, Canister}; use orbit_station_api::CreateRequestResponse; use slog::Logger; pub use station_agent::StationAgent; +use station_agent::StationConfig; pub struct DfxOrbit { // The station agent that handles communication with the station @@ -32,11 +33,12 @@ pub struct DfxOrbit { impl DfxOrbit { /// Creates a new agent for communicating with the default station. - pub async fn new(mut agent: OrbitExtensionAgent, logger: Logger) -> anyhow::Result { - let config = agent - .default_station()? - .ok_or_else(|| anyhow::format_err!("No default station specified"))?; - let interface = agent.dfx_interface().await?; + pub async fn new( + mut agent: OrbitExtensionAgent, + config: StationConfig, + logger: Logger, + ) -> anyhow::Result { + let interface = agent.dfx_interface(&config.network).await?; Ok(Self { station: StationAgent::new(interface.agent().clone(), config), From 301f362f4838a4611057e59e44b3ffb7d55b4d52 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 9 Aug 2024 13:32:18 +0000 Subject: [PATCH 24/42] WIP improve review UX --- Cargo.lock | 143 +++++++++++++++++++++++++++- Cargo.toml | 2 + tools/dfx-orbit/Cargo.toml | 2 + tools/dfx-orbit/src/args.rs | 1 - tools/dfx-orbit/src/args/request.rs | 11 +-- tools/dfx-orbit/src/args/review.rs | 11 ++- tools/dfx-orbit/src/cli.rs | 48 +--------- tools/dfx-orbit/src/cli/review.rs | 49 ++++++++++ 8 files changed, 206 insertions(+), 61 deletions(-) create mode 100644 tools/dfx-orbit/src/cli/review.rs diff --git a/Cargo.lock b/Cargo.lock index e5d1bb512..62f0294af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -112,6 +112,21 @@ version = "0.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" +[[package]] +name = "android-tzdata" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0" + +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anstream" version = "0.6.14" @@ -514,6 +529,12 @@ dependencies = [ "utf8-width", ] +[[package]] +name = "bytecount" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ce89b21cab1437276d2650d57e971f9d548a2d9037cc231abdc0562b97498ce" + [[package]] name = "byteorder" version = "1.5.0" @@ -705,6 +726,20 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "chrono" +version = "0.4.38" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a21f936df1771bf62b77f047b726c4625ff2e8aa607c01ec06e5a05bd8463401" +dependencies = [ + "android-tzdata", + "iana-time-zone", + "js-sys", + "num-traits", + "wasm-bindgen", + "windows-targets 0.52.6", +] + [[package]] name = "cipher" version = "0.3.0" @@ -1026,6 +1061,18 @@ version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e8566979429cf69b49a5c740c60791108e86440e8be149bbea4fe54d2c32d6e2" +[[package]] +name = "dateparser" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2ef451feee09ae5ecd8a02e738bd9adee9266b8fa9b44e22d3ce968d8694238" +dependencies = [ + "anyhow", + "chrono", + "lazy_static", + "regex", +] + [[package]] name = "der" version = "0.6.1" @@ -1169,6 +1216,7 @@ dependencies = [ "candid_parser", "cap-std", "clap", + "dateparser", "dfx-core 0.0.1 (git+https://github.com/dfinity/sdk.git?tag=0.22.0)", "hex", "ic-agent", @@ -1183,6 +1231,7 @@ dependencies = [ "slog-async", "slog-term", "station-api", + "tabled", "thiserror", "tokio", ] @@ -1973,6 +2022,29 @@ dependencies = [ "tracing", ] +[[package]] +name = "iana-time-zone" +version = "0.1.60" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7ffbb5a1b541ea2561f8c41c087286cc091e21e556a4f09a8f6cbf17b69b141" +dependencies = [ + "android_system_properties", + "core-foundation-sys", + "iana-time-zone-haiku", + "js-sys", + "wasm-bindgen", + "windows-core", +] + +[[package]] +name = "iana-time-zone-haiku" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" +dependencies = [ + "cc", +] + [[package]] name = "ic-agent" version = "0.36.0" @@ -3057,6 +3129,17 @@ dependencies = [ "group 0.12.1", ] +[[package]] +name = "papergrid" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7419ad52a7de9b60d33e11085a0fe3df1fbd5926aa3f93d3dd53afbc9e86725" +dependencies = [ + "bytecount", + "fnv", + "unicode-width", +] + [[package]] name = "parking" version = "2.2.0" @@ -3436,6 +3519,30 @@ dependencies = [ "toml_edit", ] +[[package]] +name = "proc-macro-error" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" +dependencies = [ + "proc-macro-error-attr", + "proc-macro2", + "quote", + "syn 1.0.109", + "version_check", +] + +[[package]] +name = "proc-macro-error-attr" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" +dependencies = [ + "proc-macro2", + "quote", + "version_check", +] + [[package]] name = "proc-macro2" version = "1.0.86" @@ -4497,6 +4604,29 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a7065abeca94b6a8a577f9bd45aa0867a2238b74e8eb67cf10d492bc39351394" +[[package]] +name = "tabled" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77c9303ee60b9bedf722012ea29ae3711ba13a67c9b9ae28993838b63057cb1b" +dependencies = [ + "papergrid", + "tabled_derive", +] + +[[package]] +name = "tabled_derive" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf0fb8bfdc709786c154e24a66777493fb63ae97e3036d914c8666774c477069" +dependencies = [ + "heck 0.4.1", + "proc-macro-error", + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "take_mut" version = "0.2.2" @@ -4922,9 +5052,9 @@ checksum = "d4c87d22b6e3f4a18d4d40ef354e97c90fcb14dd91d7dc0aa9d8a1172ebf7202" [[package]] name = "unicode-width" -version = "0.1.13" +version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0336d538f7abc86d282a4189614dfaa90810dfc2c6f6427eaf88e16311dd225d" +checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85" [[package]] name = "unicode-xid" @@ -5233,6 +5363,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-core" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9" +dependencies = [ + "windows-targets 0.52.6", +] + [[package]] name = "windows-sys" version = "0.48.0" diff --git a/Cargo.toml b/Cargo.toml index c397e8818..2aa945245 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ candid = "0.10.3" candid_parser = "0.1.3" cap-std = "3.1.0" clap = { version = "4.5.7", features = ["derive"] } +dateparser = "0.2" dfx-core = { git = "https://github.com/dfinity/sdk.git", tag = "0.22.0" } convert_case = "0.6" futures = "0.3" @@ -74,6 +75,7 @@ slog = "2.5.2" slog-async = "2.4.0" slog-term = "2.9.0" syn = { version = "2.0", features = ["extra-traits", "full"] } +tabled = "0.16" tempfile = "3.10" thiserror = "1.0.48" time = { version = "0.3", features = ["formatting", "parsing"] } diff --git a/tools/dfx-orbit/Cargo.toml b/tools/dfx-orbit/Cargo.toml index 2c349461b..6eb48954f 100644 --- a/tools/dfx-orbit/Cargo.toml +++ b/tools/dfx-orbit/Cargo.toml @@ -19,6 +19,7 @@ serde.workspace = true serde_bytes.workspace = true serde_json.workspace = true cap-std.workspace = true +dateparser.workspace = true dfx-core.workspace = true hex.workspace = true ic-agent.workspace = true @@ -29,6 +30,7 @@ sha2.workspace = true slog.workspace = true slog-term.workspace = true slog-async.workspace = true +tabled.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ['rt'] } diff --git a/tools/dfx-orbit/src/args.rs b/tools/dfx-orbit/src/args.rs index cd2f860e9..d8bee6008 100644 --- a/tools/dfx-orbit/src/args.rs +++ b/tools/dfx-orbit/src/args.rs @@ -43,7 +43,6 @@ pub enum DfxOrbitSubcommands { /// Make requests to Orbit Request(RequestArgs), /// View and decide on requests. - #[clap(subcommand)] Review(ReviewArgs), /// Manage assets stored in an asset canister through Orbit Asset(AssetArgs), diff --git a/tools/dfx-orbit/src/args/request.rs b/tools/dfx-orbit/src/args/request.rs index 6abafca68..2526f6051 100644 --- a/tools/dfx-orbit/src/args/request.rs +++ b/tools/dfx-orbit/src/args/request.rs @@ -2,7 +2,7 @@ pub mod canister; pub mod permission; -use crate::{DfxOrbit, StationAgent}; +use crate::DfxOrbit; use canister::RequestCanisterArgs; use clap::{Parser, Subcommand}; use orbit_station_api::CreateRequestInput; @@ -36,15 +36,6 @@ pub enum RequestArgsActions { Permission(RequestPermissionArgs), } -/// Converts the CLI arg type into the equivalent Orbit API type. -pub trait CreateRequestArgs { - /// Converts the CLI arg type into the equivalent Orbit API type. - fn into_create_request_input( - self, - station_agent: &StationAgent, - ) -> anyhow::Result; -} - impl RequestArgs { pub(crate) fn into_create_request_input( self, diff --git a/tools/dfx-orbit/src/args/review.rs b/tools/dfx-orbit/src/args/review.rs index 68eb44a93..6fed97855 100644 --- a/tools/dfx-orbit/src/args/review.rs +++ b/tools/dfx-orbit/src/args/review.rs @@ -3,15 +3,20 @@ pub mod id; pub mod list; pub mod next; -use clap::Subcommand; +use clap::{Parser, Subcommand}; use id::ReviewIdArgs; use list::ReviewListArgs; use next::ReviewNextArgs; /// Station management commands. +#[derive(Debug, Parser)] +pub struct ReviewArgs { + #[clap(subcommand)] + pub(crate) action: ReviewActionArgs, +} + #[derive(Debug, Subcommand)] -#[command(version, about, long_about = None)] -pub enum ReviewArgs { +pub enum ReviewActionArgs { /// List requests List(ReviewListArgs), /// Review the next request. diff --git a/tools/dfx-orbit/src/cli.rs b/tools/dfx-orbit/src/cli.rs index 255c5d91c..0732da138 100644 --- a/tools/dfx-orbit/src/cli.rs +++ b/tools/dfx-orbit/src/cli.rs @@ -1,13 +1,13 @@ //! Implementation of the `dfx-orbit` commands. pub(crate) mod asset; +pub(crate) mod review; pub(crate) mod station; use crate::{ - args::{review::ReviewArgs, DfxOrbitArgs, DfxOrbitSubcommands}, + args::{DfxOrbitArgs, DfxOrbitSubcommands}, dfx_extension_api::OrbitExtensionAgent, DfxOrbit, }; -use orbit_station_api::{RequestApprovalStatusDTO, SubmitRequestApprovalInput}; use slog::trace; /// A command line tool for interacting with Orbit on the Internet Computer. @@ -47,49 +47,7 @@ pub async fn exec(args: DfxOrbitArgs) -> anyhow::Result<()> { Ok(()) } - DfxOrbitSubcommands::Review(review_args) => match review_args { - ReviewArgs::List(args) => { - println!( - "{}", - serde_json::to_string_pretty( - &dfx_orbit.station.review_list(args.into()).await? - )? - ); - - Ok(()) - } - ReviewArgs::Next(args) => { - println!( - "{}", - serde_json::to_string_pretty( - &dfx_orbit.station.review_next(args.into()).await? - )? - ); - - Ok(()) - } - ReviewArgs::Id(args) => { - println!( - "{}", - serde_json::to_string_pretty( - &dfx_orbit.station.review_id(args.clone().into()).await? - )? - ); - - if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { - let action = match submit.decision { - RequestApprovalStatusDTO::Approved => "approve", - RequestApprovalStatusDTO::Rejected => "reject", - }; - dfx_core::cli::ask_for_consent(&format!( - "Would you like to {action} this request?" - ))?; - dfx_orbit.station.submit(submit).await?; - }; - - Ok(()) - } - }, + DfxOrbitSubcommands::Review(review_args) => dfx_orbit.exec_review(review_args).await, DfxOrbitSubcommands::Asset(asset_args) => { dfx_orbit.exec_asset(asset_args).await?; Ok(()) diff --git a/tools/dfx-orbit/src/cli/review.rs b/tools/dfx-orbit/src/cli/review.rs new file mode 100644 index 000000000..e4b73c696 --- /dev/null +++ b/tools/dfx-orbit/src/cli/review.rs @@ -0,0 +1,49 @@ +use crate::{ + args::review::{ReviewActionArgs, ReviewArgs}, + DfxOrbit, +}; +use orbit_station_api::{RequestApprovalStatusDTO, SubmitRequestApprovalInput}; + +impl DfxOrbit { + pub(crate) async fn exec_review(&self, args: ReviewArgs) -> anyhow::Result<()> { + match args.action { + ReviewActionArgs::List(args) => { + println!( + "{}", + serde_json::to_string_pretty(&self.station.review_list(args.into()).await?)? + ); + + Ok(()) + } + ReviewActionArgs::Next(args) => { + println!( + "{}", + serde_json::to_string_pretty(&self.station.review_next(args.into()).await?)? + ); + + Ok(()) + } + ReviewActionArgs::Id(args) => { + println!( + "{}", + serde_json::to_string_pretty( + &self.station.review_id(args.clone().into()).await? + )? + ); + + if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { + let action = match submit.decision { + RequestApprovalStatusDTO::Approved => "approve", + RequestApprovalStatusDTO::Rejected => "reject", + }; + dfx_core::cli::ask_for_consent(&format!( + "Would you like to {action} this request?" + ))?; + self.station.submit(submit).await?; + }; + + Ok(()) + } + } + } +} From acb49eb6d554471357fbbfce61ce9272e0a827e1 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 9 Aug 2024 13:50:37 +0000 Subject: [PATCH 25/42] Some renaming --- tools/dfx-orbit/src/args/asset.rs | 4 ++-- tools/dfx-orbit/src/args/request.rs | 8 ++++---- tools/dfx-orbit/src/args/request/canister.rs | 4 ++-- tools/dfx-orbit/src/args/request/permission.rs | 2 +- tools/dfx-orbit/src/args/station.rs | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/dfx-orbit/src/args/asset.rs b/tools/dfx-orbit/src/args/asset.rs index 42221c065..b47407267 100644 --- a/tools/dfx-orbit/src/args/asset.rs +++ b/tools/dfx-orbit/src/args/asset.rs @@ -3,9 +3,9 @@ use clap::{Parser, Subcommand}; /// Station management commands. #[derive(Debug, Clone, Parser)] -#[command(version, about, long_about = None)] +#[clap(version, about, long_about = None)] pub struct AssetArgs { - #[command(subcommand)] + #[clap(subcommand)] pub(crate) action: AssetArgsAction, } diff --git a/tools/dfx-orbit/src/args/request.rs b/tools/dfx-orbit/src/args/request.rs index 2526f6051..28ab50f6c 100644 --- a/tools/dfx-orbit/src/args/request.rs +++ b/tools/dfx-orbit/src/args/request.rs @@ -10,7 +10,7 @@ use permission::RequestPermissionArgs; /// Request canister changes. #[derive(Debug, Clone, Parser)] -#[command(version, about, long_about = None)] +#[clap(version, about, long_about = None)] pub struct RequestArgs { /// Title of the request #[clap(long)] @@ -22,17 +22,17 @@ pub struct RequestArgs { // TODO: Summary file as an alternative to summary // TODO: Execution plan - #[command(subcommand)] + #[clap(subcommand)] action: RequestArgsActions, } #[derive(Debug, Clone, Subcommand)] -#[command(version, about, long_about = None)] +#[clap(version, about, long_about = None)] pub enum RequestArgsActions { /// Request canister operations through Orbit Canister(RequestCanisterArgs), /// Request permissions - #[command(subcommand)] + #[clap(subcommand)] Permission(RequestPermissionArgs), } diff --git a/tools/dfx-orbit/src/args/request/canister.rs b/tools/dfx-orbit/src/args/request/canister.rs index 1773992c4..11862e09b 100644 --- a/tools/dfx-orbit/src/args/request/canister.rs +++ b/tools/dfx-orbit/src/args/request/canister.rs @@ -13,12 +13,12 @@ use orbit_station_api::RequestOperationInput; #[derive(Debug, Clone, Parser)] pub struct RequestCanisterArgs { /// The operation to request - #[command(subcommand)] + #[clap(subcommand)] action: RequestCanisterActionArgs, } #[derive(Debug, Clone, Subcommand)] -#[command(version, about, long_about = None)] +#[clap(version, about, long_about = None)] pub enum RequestCanisterActionArgs { /// Request to upgrade the canister wasm Install(RequestCanisterInstallArgs), diff --git a/tools/dfx-orbit/src/args/request/permission.rs b/tools/dfx-orbit/src/args/request/permission.rs index 995376f55..5df709e99 100644 --- a/tools/dfx-orbit/src/args/request/permission.rs +++ b/tools/dfx-orbit/src/args/request/permission.rs @@ -11,7 +11,7 @@ use permission::{RequestPermissionReadPermissionsArgs, RequestPermissionUpdatePe /// Request permission. #[derive(Debug, Clone, Subcommand)] -#[command(version, about, long_about = None)] +#[clap(version, about, long_about = None)] pub enum RequestPermissionArgs { /// Request permission to read permission(s) ReadPermissions(RequestPermissionReadPermissionsArgs), diff --git a/tools/dfx-orbit/src/args/station.rs b/tools/dfx-orbit/src/args/station.rs index 7846d668f..389ab6f04 100644 --- a/tools/dfx-orbit/src/args/station.rs +++ b/tools/dfx-orbit/src/args/station.rs @@ -6,7 +6,7 @@ use std::fmt::{self, Display, Formatter}; /// Station management commands. #[derive(Debug, Subcommand)] -#[command(version, about, long_about = None)] +#[clap(version, about, long_about = None)] pub enum StationArgs { /// Adds an Orbit station to the local dfx configuration. Add(Add), From 8b1aef06be5e2c25041aae7b2fe538cfe93d9074 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Fri, 9 Aug 2024 14:55:20 +0000 Subject: [PATCH 26/42] WIP implement display for review list --- tools/dfx-orbit/src/args/review.rs | 4 +++ tools/dfx-orbit/src/cli/review.rs | 36 +++++++++++++---------- tools/dfx-orbit/src/cli/review/display.rs | 36 +++++++++++++++++++++++ 3 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 tools/dfx-orbit/src/cli/review/display.rs diff --git a/tools/dfx-orbit/src/args/review.rs b/tools/dfx-orbit/src/args/review.rs index 6fed97855..7168d1260 100644 --- a/tools/dfx-orbit/src/args/review.rs +++ b/tools/dfx-orbit/src/args/review.rs @@ -11,6 +11,10 @@ use next::ReviewNextArgs; /// Station management commands. #[derive(Debug, Parser)] pub struct ReviewArgs { + /// Return output as JSON + #[clap(short, long)] + pub(crate) json: bool, + #[clap(subcommand)] pub(crate) action: ReviewActionArgs, } diff --git a/tools/dfx-orbit/src/cli/review.rs b/tools/dfx-orbit/src/cli/review.rs index e4b73c696..69db79f8c 100644 --- a/tools/dfx-orbit/src/cli/review.rs +++ b/tools/dfx-orbit/src/cli/review.rs @@ -1,35 +1,34 @@ +mod display; + use crate::{ args::review::{ReviewActionArgs, ReviewArgs}, DfxOrbit, }; +use display::display_list; use orbit_station_api::{RequestApprovalStatusDTO, SubmitRequestApprovalInput}; +use serde::Serialize; impl DfxOrbit { pub(crate) async fn exec_review(&self, args: ReviewArgs) -> anyhow::Result<()> { + let as_json = args.json; + match args.action { ReviewActionArgs::List(args) => { - println!( - "{}", - serde_json::to_string_pretty(&self.station.review_list(args.into()).await?)? - ); + let response = self.station.review_list(args.into()).await?; + if as_json { + display_as_json(&response); + } else { + display_list(&response); + } Ok(()) } ReviewActionArgs::Next(args) => { - println!( - "{}", - serde_json::to_string_pretty(&self.station.review_next(args.into()).await?)? - ); - + display_as_json(&self.station.review_next(args.into()).await?); Ok(()) } ReviewActionArgs::Id(args) => { - println!( - "{}", - serde_json::to_string_pretty( - &self.station.review_id(args.clone().into()).await? - )? - ); + display_as_json(&self.station.review_id(args.clone().into()).await?); if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { let action = match submit.decision { @@ -47,3 +46,10 @@ impl DfxOrbit { } } } + +fn display_as_json(data: D) +where + D: Serialize, +{ + println!("{}", serde_json::to_string_pretty(&data).unwrap()); +} diff --git a/tools/dfx-orbit/src/cli/review/display.rs b/tools/dfx-orbit/src/cli/review/display.rs new file mode 100644 index 000000000..531b773c6 --- /dev/null +++ b/tools/dfx-orbit/src/cli/review/display.rs @@ -0,0 +1,36 @@ +use orbit_station_api::{ListRequestsResponse, RequestStatusDTO}; +use tabled::{ + settings::{Settings, Style}, + Table, +}; + +pub(crate) fn display_list(data: &ListRequestsResponse) { + let data_iter = data.requests.iter().map(|request| { + [ + request.id.clone(), + request.title.clone(), + request.summary.clone().unwrap_or(String::from("-")), + match request.status { + RequestStatusDTO::Created => String::from("Created"), + RequestStatusDTO::Approved => String::from("Approved"), + RequestStatusDTO::Rejected => String::from("Rejected"), + RequestStatusDTO::Cancelled { .. } => String::from("Cancelled"), + RequestStatusDTO::Scheduled { .. } => String::from("Scheduled"), + RequestStatusDTO::Processing { .. } => String::from("Processing"), + RequestStatusDTO::Completed { .. } => String::from("Completed"), + RequestStatusDTO::Failed { .. } => String::from("Failed"), + }, + ] + }); + let titled_iter = std::iter::once([ + String::from("ID"), + String::from("Title"), + String::from("Summary"), + String::from("Status"), + ]) + .chain(data_iter); + + let table_config = Settings::default().with(Style::psql()); + let table = Table::from_iter(titled_iter).with(table_config).to_string(); + println!("{}", table); +} From ff10355bcc2351d595867f581ae5db49a8799df8 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Tue, 13 Aug 2024 14:13:57 +0000 Subject: [PATCH 27/42] Small improvements to the list arg parsing --- tools/dfx-orbit/src/args/review/list.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/dfx-orbit/src/args/review/list.rs b/tools/dfx-orbit/src/args/review/list.rs index 3316d6a71..cd3bea935 100644 --- a/tools/dfx-orbit/src/args/review/list.rs +++ b/tools/dfx-orbit/src/args/review/list.rs @@ -1,7 +1,13 @@ //! CLI arguments for `dfx-orbit review list`. use clap::Parser; -use orbit_station_api::ListRequestsInput; +use orbit_station_api::{ListRequestsInput, SortDirection}; + +// TODO: Ideas what we could filter by: +// - Filter by status: -> Only the ones which are in Creates +// - Filter by times -> There are four times that could be set +// - Filter by request ids +// - Filter by default only for external canister calls -> --all for all /// Reviews the next request. #[derive(Debug, Parser)] @@ -13,7 +19,6 @@ pub struct ReviewListArgs { impl From for ListRequestsInput { fn from(args: ReviewListArgs) -> Self { - let ReviewListArgs { only_approvable } = args; Self { requester_ids: None, approver_ids: None, @@ -24,8 +29,10 @@ impl From for ListRequestsInput { created_from_dt: None, created_to_dt: None, paginate: None, - sort_by: None, - only_approvable, + sort_by: Some(orbit_station_api::ListRequestsSortBy::CreatedAt( + SortDirection::Asc, + )), + only_approvable: args.only_approvable, with_evaluation_results: true, } } From f5f03220c32227d36d5ed1ff677a6c63545d74aa Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Tue, 13 Aug 2024 14:14:38 +0000 Subject: [PATCH 28/42] Improve review list output --- tools/dfx-orbit/src/cli/review.rs | 33 +++++++------ tools/dfx-orbit/src/cli/review/display.rs | 58 +++++++++++++++++++++-- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/tools/dfx-orbit/src/cli/review.rs b/tools/dfx-orbit/src/cli/review.rs index 69db79f8c..af1adab4b 100644 --- a/tools/dfx-orbit/src/cli/review.rs +++ b/tools/dfx-orbit/src/cli/review.rs @@ -5,7 +5,7 @@ use crate::{ DfxOrbit, }; use display::display_list; -use orbit_station_api::{RequestApprovalStatusDTO, SubmitRequestApprovalInput}; +use orbit_station_api::{RequestApprovalStatusDTO, RequestStatusDTO, SubmitRequestApprovalInput}; use serde::Serialize; impl DfxOrbit { @@ -19,7 +19,7 @@ impl DfxOrbit { if as_json { display_as_json(&response); } else { - display_list(&response); + display_list(response); } Ok(()) } @@ -28,19 +28,24 @@ impl DfxOrbit { Ok(()) } ReviewActionArgs::Id(args) => { - display_as_json(&self.station.review_id(args.clone().into()).await?); - - if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { - let action = match submit.decision { - RequestApprovalStatusDTO::Approved => "approve", - RequestApprovalStatusDTO::Rejected => "reject", - }; - dfx_core::cli::ask_for_consent(&format!( - "Would you like to {action} this request?" - ))?; - self.station.submit(submit).await?; - }; + let request = &self.station.review_id(args.clone().into()).await?; + display_as_json(request); + match request.request.status { + RequestStatusDTO::Created => { + if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { + let action = match submit.decision { + RequestApprovalStatusDTO::Approved => "approve", + RequestApprovalStatusDTO::Rejected => "reject", + }; + dfx_core::cli::ask_for_consent(&format!( + "Would you like to {action} this request?" + ))?; + self.station.submit(submit).await?; + }; + } + _ => (), + } Ok(()) } } diff --git a/tools/dfx-orbit/src/cli/review/display.rs b/tools/dfx-orbit/src/cli/review/display.rs index 531b773c6..6a24e1302 100644 --- a/tools/dfx-orbit/src/cli/review/display.rs +++ b/tools/dfx-orbit/src/cli/review/display.rs @@ -1,15 +1,62 @@ -use orbit_station_api::{ListRequestsResponse, RequestStatusDTO}; +use orbit_station_api::{ListRequestsResponse, RequestOperationDTO, RequestStatusDTO}; +use std::collections::HashMap; use tabled::{ settings::{Settings, Style}, Table, }; -pub(crate) fn display_list(data: &ListRequestsResponse) { +pub(crate) fn display_list(data: ListRequestsResponse) { + let add_info = data + .additional_info + .into_iter() + .map(|info| (info.id.clone(), info)) + .collect::>(); + let data_iter = data.requests.iter().map(|request| { + let add_info = add_info.get(&request.id); + [ request.id.clone(), + add_info + .map(|add_info| add_info.requester_name.clone()) + .unwrap_or(String::from("-")), request.title.clone(), - request.summary.clone().unwrap_or(String::from("-")), + match request.operation { + RequestOperationDTO::Transfer(_) => String::from("Transfer"), + RequestOperationDTO::AddAccount(_) => String::from("AddAccount"), + RequestOperationDTO::EditAccount(_) => String::from("EditAccount"), + RequestOperationDTO::AddAddressBookEntry(_) => String::from("AddAddressBookEntry"), + RequestOperationDTO::EditAddressBookEntry(_) => { + String::from("EditAddressBookEntry") + } + RequestOperationDTO::RemoveAddressBookEntry(_) => { + String::from("RemoveAddressBookEntry") + } + RequestOperationDTO::AddUser(_) => String::from("AddUser"), + RequestOperationDTO::EditUser(_) => String::from("EditUser"), + RequestOperationDTO::AddUserGroup(_) => String::from("AddUserGroup"), + RequestOperationDTO::EditUserGroup(_) => String::from("EditUserGroup"), + RequestOperationDTO::RemoveUserGroup(_) => String::from("RemoveUserGroup"), + RequestOperationDTO::ChangeCanister(_) => String::from("ChangeCanister"), + RequestOperationDTO::SetDisasterRecovery(_) => String::from("SetDisasterRecovery"), + RequestOperationDTO::ChangeExternalCanister(_) => { + String::from("ChangeExternalCanister") + } + RequestOperationDTO::CreateExternalCanister(_) => { + String::from("CreateExternalCanister") + } + RequestOperationDTO::ConfigureExternalCanister(_) => { + String::from("ConfigureExternalCanister") + } + RequestOperationDTO::CallExternalCanister(_) => { + String::from("CallExternalCanister") + } + RequestOperationDTO::EditPermission(_) => String::from("EditPermission"), + RequestOperationDTO::AddRequestPolicy(_) => String::from("AddRequestPolicy"), + RequestOperationDTO::EditRequestPolicy(_) => String::from("EditRequestPolicy"), + RequestOperationDTO::RemoveRequestPolicy(_) => String::from("RemoveRequestPolicy"), + RequestOperationDTO::ManageSystemInfo(_) => String::from("ManageSystemInfo"), + }, match request.status { RequestStatusDTO::Created => String::from("Created"), RequestStatusDTO::Approved => String::from("Approved"), @@ -24,9 +71,10 @@ pub(crate) fn display_list(data: &ListRequestsResponse) { }); let titled_iter = std::iter::once([ String::from("ID"), + String::from("Requested by"), String::from("Title"), - String::from("Summary"), - String::from("Status"), + String::from("Operation"), + String::from("Execution Status"), ]) .chain(data_iter); From 6dce4a2693e64359da81696c5693721fe2babae7 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Tue, 13 Aug 2024 14:41:18 +0000 Subject: [PATCH 29/42] Updating TODOs --- tools/dfx-orbit/src/args/request/canister.rs | 5 +++++ tools/dfx-orbit/src/args/request/canister/install.rs | 3 +-- tools/dfx-orbit/src/args/review/next.rs | 2 ++ tools/dfx-orbit/src/cli/review.rs | 10 +++++----- tools/dfx-orbit/src/cli/review/display.rs | 8 ++++++-- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/tools/dfx-orbit/src/args/request/canister.rs b/tools/dfx-orbit/src/args/request/canister.rs index 11862e09b..0c641b35d 100644 --- a/tools/dfx-orbit/src/args/request/canister.rs +++ b/tools/dfx-orbit/src/args/request/canister.rs @@ -9,6 +9,11 @@ use clap::{Parser, Subcommand}; use install::RequestCanisterInstallArgs; use orbit_station_api::RequestOperationInput; +// TODO: Move the canister subsection away from request +// TODO: Support Canister create + integration test +// TODO: Support Canister install check +// TODO: Canister get response functionality + /// Request canister operations through Orbit #[derive(Debug, Clone, Parser)] pub struct RequestCanisterArgs { diff --git a/tools/dfx-orbit/src/args/request/canister/install.rs b/tools/dfx-orbit/src/args/request/canister/install.rs index 5a711cf19..7c49f9817 100644 --- a/tools/dfx-orbit/src/args/request/canister/install.rs +++ b/tools/dfx-orbit/src/args/request/canister/install.rs @@ -1,12 +1,11 @@ //! Arguments for `dfx orbit canister change wasm`. +use crate::DfxOrbit; use clap::{Parser, ValueEnum}; use orbit_station_api::{ CanisterInstallMode, ChangeExternalCanisterOperationInput, RequestOperationInput, }; -use crate::DfxOrbit; - /// Requests that a canister be installed or updated. Equivalent to `orbit_station_api::CanisterInstallMode`. #[derive(Debug, Clone, Parser)] pub struct RequestCanisterInstallArgs { diff --git a/tools/dfx-orbit/src/args/review/next.rs b/tools/dfx-orbit/src/args/review/next.rs index 3a961e23e..545b410f8 100644 --- a/tools/dfx-orbit/src/args/review/next.rs +++ b/tools/dfx-orbit/src/args/review/next.rs @@ -3,6 +3,8 @@ use clap::Parser; use orbit_station_api::GetNextApprovableRequestInput; +// TODO: Only show review types that are relevant to dfx-orbbit -> can deactivate with --all + /// Reviews the next request. #[derive(Debug, Parser)] pub struct ReviewNextArgs {} diff --git a/tools/dfx-orbit/src/cli/review.rs b/tools/dfx-orbit/src/cli/review.rs index af1adab4b..e449a0bf2 100644 --- a/tools/dfx-orbit/src/cli/review.rs +++ b/tools/dfx-orbit/src/cli/review.rs @@ -17,19 +17,19 @@ impl DfxOrbit { let response = self.station.review_list(args.into()).await?; if as_json { - display_as_json(&response); + print_as_json(&response); } else { - display_list(response); + println!("{}", display_list(response)); } Ok(()) } ReviewActionArgs::Next(args) => { - display_as_json(&self.station.review_next(args.into()).await?); + print_as_json(&self.station.review_next(args.into()).await?); Ok(()) } ReviewActionArgs::Id(args) => { let request = &self.station.review_id(args.clone().into()).await?; - display_as_json(request); + print_as_json(request); match request.request.status { RequestStatusDTO::Created => { @@ -52,7 +52,7 @@ impl DfxOrbit { } } -fn display_as_json(data: D) +fn print_as_json(data: D) where D: Serialize, { diff --git a/tools/dfx-orbit/src/cli/review/display.rs b/tools/dfx-orbit/src/cli/review/display.rs index 6a24e1302..c5e0f6a22 100644 --- a/tools/dfx-orbit/src/cli/review/display.rs +++ b/tools/dfx-orbit/src/cli/review/display.rs @@ -5,7 +5,7 @@ use tabled::{ Table, }; -pub(crate) fn display_list(data: ListRequestsResponse) { +pub(crate) fn display_list(data: ListRequestsResponse) -> String { let add_info = data .additional_info .into_iter() @@ -80,5 +80,9 @@ pub(crate) fn display_list(data: ListRequestsResponse) { let table_config = Settings::default().with(Style::psql()); let table = Table::from_iter(titled_iter).with(table_config).to_string(); - println!("{}", table); + + table } + +// TODO: Display request for review id and review next +// TODO: ^ This needs canister id to name reverse backup From ed5eaff9bf025fd85793865ba0082eef82640461 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Tue, 13 Aug 2024 14:49:46 +0000 Subject: [PATCH 30/42] Bump version --- Cargo.lock | 2 +- tools/dfx-orbit/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62f0294af..41f2b1b97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1209,7 +1209,7 @@ dependencies = [ [[package]] name = "dfx-orbit" -version = "0.2.0" +version = "0.3.0" dependencies = [ "anyhow", "candid", diff --git a/tools/dfx-orbit/Cargo.toml b/tools/dfx-orbit/Cargo.toml index 6eb48954f..0b5cd5981 100644 --- a/tools/dfx-orbit/Cargo.toml +++ b/tools/dfx-orbit/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "dfx-orbit" -version = "0.2.0" +version = "0.3.0" description = "Command line tool for interacting with the Orbit digital asset manager on the ICP blockchain." authors.workspace = true edition.workspace = true From 4525d06c93f2c4f02a96a89a52fdb816b8e2ccd8 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Wed, 14 Aug 2024 09:57:23 +0000 Subject: [PATCH 31/42] Update TODOs, remove unused code --- tools/dfx-orbit/src/args.rs | 1 - tools/dfx-orbit/src/args/request.rs | 1 - tools/dfx-orbit/src/args/request/canister.rs | 1 - .../src/args/request/canister/call.rs | 3 +-- .../src/args/request/canister/install.rs | 1 - tools/dfx-orbit/src/cli/asset/util.rs | 19 +------------------ 6 files changed, 2 insertions(+), 24 deletions(-) diff --git a/tools/dfx-orbit/src/args.rs b/tools/dfx-orbit/src/args.rs index d8bee6008..8654b3236 100644 --- a/tools/dfx-orbit/src/args.rs +++ b/tools/dfx-orbit/src/args.rs @@ -11,7 +11,6 @@ use review::ReviewArgs; use station::StationArgs; /// Manages Orbit on the Internet Computer. -// TODO: Specify --station to not use the default station // TODO: Better version information #[derive(Parser, Debug)] #[clap(version, about, long_about = None)] diff --git a/tools/dfx-orbit/src/args/request.rs b/tools/dfx-orbit/src/args/request.rs index 28ab50f6c..ccbb18681 100644 --- a/tools/dfx-orbit/src/args/request.rs +++ b/tools/dfx-orbit/src/args/request.rs @@ -21,7 +21,6 @@ pub struct RequestArgs { summary: Option, // TODO: Summary file as an alternative to summary - // TODO: Execution plan #[clap(subcommand)] action: RequestArgsActions, } diff --git a/tools/dfx-orbit/src/args/request/canister.rs b/tools/dfx-orbit/src/args/request/canister.rs index 0c641b35d..4750e6189 100644 --- a/tools/dfx-orbit/src/args/request/canister.rs +++ b/tools/dfx-orbit/src/args/request/canister.rs @@ -9,7 +9,6 @@ use clap::{Parser, Subcommand}; use install::RequestCanisterInstallArgs; use orbit_station_api::RequestOperationInput; -// TODO: Move the canister subsection away from request // TODO: Support Canister create + integration test // TODO: Support Canister install check // TODO: Canister get response functionality diff --git a/tools/dfx-orbit/src/args/request/canister/call.rs b/tools/dfx-orbit/src/args/request/canister/call.rs index e6355bfc2..e1844c573 100644 --- a/tools/dfx-orbit/src/args/request/canister/call.rs +++ b/tools/dfx-orbit/src/args/request/canister/call.rs @@ -16,8 +16,7 @@ pub struct RequestCanisterCallArgs { method_name: String, /// The argument to pass to the method. argument: Option, - // TODO: - // /// The format of the argument. + // TODO: The format of the argument. // #[clap(short, long)] // r#type: Option, // TODO: Read argument from a file diff --git a/tools/dfx-orbit/src/args/request/canister/install.rs b/tools/dfx-orbit/src/args/request/canister/install.rs index 7c49f9817..156b994d5 100644 --- a/tools/dfx-orbit/src/args/request/canister/install.rs +++ b/tools/dfx-orbit/src/args/request/canister/install.rs @@ -9,7 +9,6 @@ use orbit_station_api::{ /// Requests that a canister be installed or updated. Equivalent to `orbit_station_api::CanisterInstallMode`. #[derive(Debug, Clone, Parser)] pub struct RequestCanisterInstallArgs { - // TODO: Poll, waiting for the request to be accepted. /// The canister name or ID. canister: String, /// The installation mode. diff --git a/tools/dfx-orbit/src/cli/asset/util.rs b/tools/dfx-orbit/src/cli/asset/util.rs index edfc8a099..6cf3d3db0 100644 --- a/tools/dfx-orbit/src/cli/asset/util.rs +++ b/tools/dfx-orbit/src/cli/asset/util.rs @@ -1,8 +1,4 @@ -use std::path::{Path, PathBuf}; - use crate::DfxOrbit; - -use super::AssetAgent; use anyhow::{anyhow, bail}; use candid::Principal; use dfx_core::config::model::dfinity::CanisterTypeProperties; @@ -11,6 +7,7 @@ use orbit_station_api::{ CallExternalCanisterOperationInput, CanisterMethodDTO, CreateRequestInput, CreateRequestResponse, RequestOperationInput, }; +use std::path::{Path, PathBuf}; impl DfxOrbit { /// Request from the station to grant the `Prepare` permission for the asset canister @@ -79,17 +76,3 @@ impl DfxOrbit { paths.iter().map(|pathbuf| pathbuf.as_path()).collect() } } - -impl AssetAgent<'_> { - // TODO: Turn into a functionality - pub fn request_prepare_permission_payload( - canister: Principal, - ) -> Result, candid::Error> { - let args = GrantPermissionArguments { - to_principal: canister, - permission: Permission::Prepare, - }; - - candid::encode_one(args) - } -} From f6fe73883da7cefaaa4abe2e27742f733a9daf95 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Wed, 14 Aug 2024 09:57:34 +0000 Subject: [PATCH 32/42] Refactor candid loading --- tools/dfx-orbit/src/args/request/canister.rs | 22 ++++++++++++++ .../src/args/request/canister/call.rs | 29 +++++-------------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/tools/dfx-orbit/src/args/request/canister.rs b/tools/dfx-orbit/src/args/request/canister.rs index 4750e6189..5181fb5bd 100644 --- a/tools/dfx-orbit/src/args/request/canister.rs +++ b/tools/dfx-orbit/src/args/request/canister.rs @@ -4,6 +4,7 @@ pub mod call; pub mod install; use crate::DfxOrbit; +use anyhow::Context; use call::RequestCanisterCallArgs; use clap::{Parser, Subcommand}; use install::RequestCanisterInstallArgs; @@ -56,3 +57,24 @@ impl RequestCanisterActionArgs { } } } + +fn candid_from_string_or_file( + arg_string: &Option, + arg_path: &Option, +) -> anyhow::Result>> { + // TODO: It would be really nice to be able to use `blob_from_arguments(..)` here, as in dfx, to geta ll the nice things such as help composing the argument. + // First try to read the argument file, if it was provided + Ok(arg_path + .as_ref() + .map(|path| std::fs::read_to_string(path)) + .transpose()? + // Otherwise use the argument from the command line + .or_else(|| arg_string.clone()) + // Parse the candid + .map(|arg_string| { + candid_parser::parse_idl_args(&arg_string) + .with_context(|| "Invalid Candid values".to_string())? + .to_bytes() + }) + .transpose()?) +} diff --git a/tools/dfx-orbit/src/args/request/canister/call.rs b/tools/dfx-orbit/src/args/request/canister/call.rs index e1844c573..944717bc9 100644 --- a/tools/dfx-orbit/src/args/request/canister/call.rs +++ b/tools/dfx-orbit/src/args/request/canister/call.rs @@ -1,12 +1,13 @@ //! CLI arguments for `dfx-orbit canister call`. use crate::DfxOrbit; -use anyhow::Context; use clap::Parser; use orbit_station_api::{ CallExternalCanisterOperationInput, CanisterMethodDTO, RequestOperationInput, }; +use super::candid_from_string_or_file; + /// Requests that a call be made to a canister. #[derive(Debug, Clone, Parser)] pub struct RequestCanisterCallArgs { @@ -19,7 +20,8 @@ pub struct RequestCanisterCallArgs { // TODO: The format of the argument. // #[clap(short, long)] // r#type: Option, - // TODO: Read argument from a file + #[clap(short = 'f', long, conflicts_with = "argument")] + arg_file: Option, /// Specifies the amount of cycles to send on the call. #[clap(short, long)] with_cycles: Option, @@ -31,33 +33,18 @@ impl RequestCanisterCallArgs { self, dfx_orbit: &DfxOrbit, ) -> anyhow::Result { - let RequestCanisterCallArgs { - canister, - method_name, - with_cycles, - argument, - } = self; - let canister_id = dfx_orbit.canister_id(&canister)?; + let canister_id = dfx_orbit.canister_id(&self.canister)?; + let arg = candid_from_string_or_file(&self.argument, &self.arg_file)?; - // TODO: It would be really nice to be able to use `blob_from_arguments(..)` here, as in dfx, to geta ll the nice things such as help composing the argument. - let arg = if let Some(argument) = argument { - Some( - candid_parser::parse_idl_args(&argument) - .with_context(|| "Invalid Candid values".to_string())? - .to_bytes()?, - ) - } else { - None - }; Ok(RequestOperationInput::CallExternalCanister( CallExternalCanisterOperationInput { validation_method: None, execution_method: CanisterMethodDTO { canister_id, - method_name, + method_name: self.method_name, }, arg, - execution_method_cycles: with_cycles, + execution_method_cycles: self.with_cycles, }, )) } From e9c37894edbf0e39e5c844de68a0d35083b3e298 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Wed, 14 Aug 2024 10:00:06 +0000 Subject: [PATCH 33/42] Clippy --- tools/dfx-orbit/src/args/request/canister.rs | 2 +- tools/dfx-orbit/src/cli/review.rs | 24 +++++++++----------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/tools/dfx-orbit/src/args/request/canister.rs b/tools/dfx-orbit/src/args/request/canister.rs index 5181fb5bd..f93b24a97 100644 --- a/tools/dfx-orbit/src/args/request/canister.rs +++ b/tools/dfx-orbit/src/args/request/canister.rs @@ -66,7 +66,7 @@ fn candid_from_string_or_file( // First try to read the argument file, if it was provided Ok(arg_path .as_ref() - .map(|path| std::fs::read_to_string(path)) + .map(std::fs::read_to_string) .transpose()? // Otherwise use the argument from the command line .or_else(|| arg_string.clone()) diff --git a/tools/dfx-orbit/src/cli/review.rs b/tools/dfx-orbit/src/cli/review.rs index e449a0bf2..e1bded5a1 100644 --- a/tools/dfx-orbit/src/cli/review.rs +++ b/tools/dfx-orbit/src/cli/review.rs @@ -31,21 +31,19 @@ impl DfxOrbit { let request = &self.station.review_id(args.clone().into()).await?; print_as_json(request); - match request.request.status { - RequestStatusDTO::Created => { - if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { - let action = match submit.decision { - RequestApprovalStatusDTO::Approved => "approve", - RequestApprovalStatusDTO::Rejected => "reject", - }; - dfx_core::cli::ask_for_consent(&format!( - "Would you like to {action} this request?" - ))?; - self.station.submit(submit).await?; + if let RequestStatusDTO::Created = request.request.status { + if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { + let action = match submit.decision { + RequestApprovalStatusDTO::Approved => "approve", + RequestApprovalStatusDTO::Rejected => "reject", }; - } - _ => (), + dfx_core::cli::ask_for_consent(&format!( + "Would you like to {action} this request?" + ))?; + self.station.submit(submit).await?; + }; } + Ok(()) } } From 40b74715bd490d8eb24b8c2324e3192d644222f5 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Wed, 14 Aug 2024 11:19:49 +0000 Subject: [PATCH 34/42] Add canister lookup functionality --- tools/dfx-orbit/src/args/review/list.rs | 2 +- tools/dfx-orbit/src/lib.rs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tools/dfx-orbit/src/args/review/list.rs b/tools/dfx-orbit/src/args/review/list.rs index cd3bea935..187cb2a37 100644 --- a/tools/dfx-orbit/src/args/review/list.rs +++ b/tools/dfx-orbit/src/args/review/list.rs @@ -4,7 +4,7 @@ use clap::Parser; use orbit_station_api::{ListRequestsInput, SortDirection}; // TODO: Ideas what we could filter by: -// - Filter by status: -> Only the ones which are in Creates +// - Filter by status: -> Only the ones which are in Created // - Filter by times -> There are four times that could be set // - Filter by request ids // - Filter by default only for external canister calls -> --all for all diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index ac14fe3d7..9456f6b1c 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -10,6 +10,7 @@ pub mod dfx_extension_api; pub mod local_config; pub mod station_agent; +use anyhow::anyhow; use candid::Principal; pub use cli::asset::AssetAgent; use dfx_core::{config::model::canister_id_store::CanisterIdStore, DfxInterface}; @@ -62,6 +63,23 @@ impl DfxOrbit { Ok(canister_id) } + /// Gets the name of the canister given it's id + pub fn canister_name(&self, canister_id: &Principal) -> anyhow::Result { + let canister_id_store = CanisterIdStore::new( + &self.logger, + self.interface.network_descriptor(), + self.interface.config(), + )?; + + let canister_id = canister_id.to_string(); + let canister_name = canister_id_store + .get_name(&canister_id) + .cloned() + .ok_or(anyhow!("Failed to find canister name"))?; + + Ok(canister_name) + } + pub fn own_principal(&self) -> anyhow::Result { self.interface .identity() From d35f441c76a5b6ca7ad7bb39760be947ea40d6de Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Wed, 14 Aug 2024 12:27:50 +0000 Subject: [PATCH 35/42] Better display functionality --- Cargo.lock | 1 + tools/dfx-orbit/Cargo.toml | 1 + tools/dfx-orbit/src/cli/review.rs | 22 +++- tools/dfx-orbit/src/cli/review/display.rs | 141 ++++++++++++++-------- 4 files changed, 111 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 41f2b1b97..a5450de56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1223,6 +1223,7 @@ dependencies = [ "ic-asset", "ic-certified-assets", "ic-utils", + "itertools 0.13.0", "serde", "serde_bytes", "serde_json", diff --git a/tools/dfx-orbit/Cargo.toml b/tools/dfx-orbit/Cargo.toml index 0b5cd5981..7b4428748 100644 --- a/tools/dfx-orbit/Cargo.toml +++ b/tools/dfx-orbit/Cargo.toml @@ -26,6 +26,7 @@ ic-agent.workspace = true ic-asset.workspace = true ic-certified-assets.workspace = true ic-utils.workspace = true +itertools.workspace = true sha2.workspace = true slog.workspace = true slog-term.workspace = true diff --git a/tools/dfx-orbit/src/cli/review.rs b/tools/dfx-orbit/src/cli/review.rs index e1bded5a1..b288e7d69 100644 --- a/tools/dfx-orbit/src/cli/review.rs +++ b/tools/dfx-orbit/src/cli/review.rs @@ -4,7 +4,7 @@ use crate::{ args::review::{ReviewActionArgs, ReviewArgs}, DfxOrbit, }; -use display::display_list; +use display::{display_get_request_response, display_list}; use orbit_station_api::{RequestApprovalStatusDTO, RequestStatusDTO, SubmitRequestApprovalInput}; use serde::Serialize; @@ -24,12 +24,26 @@ impl DfxOrbit { Ok(()) } ReviewActionArgs::Next(args) => { - print_as_json(&self.station.review_next(args.into()).await?); + let request = self.station.review_next(args.into()).await?; + + let Some(request) = request else { + return Ok(()); + }; + if as_json { + print_as_json(&request); + } else { + println!("{}", display_get_request_response(request)) + } + Ok(()) } ReviewActionArgs::Id(args) => { - let request = &self.station.review_id(args.clone().into()).await?; - print_as_json(request); + let request = self.station.review_id(args.clone().into()).await?; + if as_json { + print_as_json(&request); + } else { + println!("{}", display_get_request_response(request.clone())) + } if let RequestStatusDTO::Created = request.request.status { if let Ok(submit) = SubmitRequestApprovalInput::try_from(args) { diff --git a/tools/dfx-orbit/src/cli/review/display.rs b/tools/dfx-orbit/src/cli/review/display.rs index c5e0f6a22..65fbf6a62 100644 --- a/tools/dfx-orbit/src/cli/review/display.rs +++ b/tools/dfx-orbit/src/cli/review/display.rs @@ -1,5 +1,8 @@ -use orbit_station_api::{ListRequestsResponse, RequestOperationDTO, RequestStatusDTO}; -use std::collections::HashMap; +use itertools::Itertools; +use orbit_station_api::{ + GetRequestResponse, ListRequestsResponse, RequestOperationDTO, RequestStatusDTO, +}; +use std::{collections::HashMap, fmt::Write}; use tabled::{ settings::{Settings, Style}, Table, @@ -21,52 +24,8 @@ pub(crate) fn display_list(data: ListRequestsResponse) -> String { .map(|add_info| add_info.requester_name.clone()) .unwrap_or(String::from("-")), request.title.clone(), - match request.operation { - RequestOperationDTO::Transfer(_) => String::from("Transfer"), - RequestOperationDTO::AddAccount(_) => String::from("AddAccount"), - RequestOperationDTO::EditAccount(_) => String::from("EditAccount"), - RequestOperationDTO::AddAddressBookEntry(_) => String::from("AddAddressBookEntry"), - RequestOperationDTO::EditAddressBookEntry(_) => { - String::from("EditAddressBookEntry") - } - RequestOperationDTO::RemoveAddressBookEntry(_) => { - String::from("RemoveAddressBookEntry") - } - RequestOperationDTO::AddUser(_) => String::from("AddUser"), - RequestOperationDTO::EditUser(_) => String::from("EditUser"), - RequestOperationDTO::AddUserGroup(_) => String::from("AddUserGroup"), - RequestOperationDTO::EditUserGroup(_) => String::from("EditUserGroup"), - RequestOperationDTO::RemoveUserGroup(_) => String::from("RemoveUserGroup"), - RequestOperationDTO::ChangeCanister(_) => String::from("ChangeCanister"), - RequestOperationDTO::SetDisasterRecovery(_) => String::from("SetDisasterRecovery"), - RequestOperationDTO::ChangeExternalCanister(_) => { - String::from("ChangeExternalCanister") - } - RequestOperationDTO::CreateExternalCanister(_) => { - String::from("CreateExternalCanister") - } - RequestOperationDTO::ConfigureExternalCanister(_) => { - String::from("ConfigureExternalCanister") - } - RequestOperationDTO::CallExternalCanister(_) => { - String::from("CallExternalCanister") - } - RequestOperationDTO::EditPermission(_) => String::from("EditPermission"), - RequestOperationDTO::AddRequestPolicy(_) => String::from("AddRequestPolicy"), - RequestOperationDTO::EditRequestPolicy(_) => String::from("EditRequestPolicy"), - RequestOperationDTO::RemoveRequestPolicy(_) => String::from("RemoveRequestPolicy"), - RequestOperationDTO::ManageSystemInfo(_) => String::from("ManageSystemInfo"), - }, - match request.status { - RequestStatusDTO::Created => String::from("Created"), - RequestStatusDTO::Approved => String::from("Approved"), - RequestStatusDTO::Rejected => String::from("Rejected"), - RequestStatusDTO::Cancelled { .. } => String::from("Cancelled"), - RequestStatusDTO::Scheduled { .. } => String::from("Scheduled"), - RequestStatusDTO::Processing { .. } => String::from("Processing"), - RequestStatusDTO::Completed { .. } => String::from("Completed"), - RequestStatusDTO::Failed { .. } => String::from("Failed"), - }, + display_request_operation(&request.operation).to_string(), + display_request_status(&request.status).to_string(), ] }); let titled_iter = std::iter::once([ @@ -84,5 +43,87 @@ pub(crate) fn display_list(data: ListRequestsResponse) -> String { table } -// TODO: Display request for review id and review next -// TODO: ^ This needs canister id to name reverse backup +pub(crate) fn display_get_request_response(request: GetRequestResponse) -> String { + let base_info = request.request; + let add_info = request.additional_info; + + let mut output = String::new(); + + // General request information + writeln!(output, "===REQUEST===").unwrap(); + writeln!(output, "ID: {}", base_info.id).unwrap(); + writeln!( + output, + "Operation: {}", + display_request_operation(&base_info.operation) + ) + .unwrap(); + writeln!( + output, + "Status: {}", + display_request_status(&base_info.status) + ) + .unwrap(); + writeln!(output, "Title: {}", base_info.title).unwrap(); + if let Some(summary) = base_info.summary { + writeln!(output, "Summary: {}", summary).unwrap() + } + writeln!(output, "Requested by: {}", add_info.requester_name).unwrap(); + writeln!( + output, + "Approved by: {}", + add_info + .approvers + .into_iter() + .map(|approver| approver.name) + .join(", ") + ) + .unwrap(); + // write!(output, "ID: {}\n", base_info.id).unwrap(); + // approved by (comma sepatated) + + // TODO: Display operation + // TODO: Per operation additional information + + output +} + +fn display_request_operation(op: &RequestOperationDTO) -> &'static str { + match op { + RequestOperationDTO::Transfer(_) => "Transfer", + RequestOperationDTO::AddAccount(_) => "AddAccount", + RequestOperationDTO::EditAccount(_) => "EditAccount", + RequestOperationDTO::AddAddressBookEntry(_) => "AddAddressBookEntry", + RequestOperationDTO::EditAddressBookEntry(_) => "EditAddressBookEntry", + RequestOperationDTO::RemoveAddressBookEntry(_) => "RemoveAddressBookEntry", + RequestOperationDTO::AddUser(_) => "AddUser", + RequestOperationDTO::EditUser(_) => "EditUser", + RequestOperationDTO::AddUserGroup(_) => "AddUserGroup", + RequestOperationDTO::EditUserGroup(_) => "EditUserGroup", + RequestOperationDTO::RemoveUserGroup(_) => "RemoveUserGroup", + RequestOperationDTO::ChangeCanister(_) => "ChangeCanister", + RequestOperationDTO::SetDisasterRecovery(_) => "SetDisasterRecovery", + RequestOperationDTO::ChangeExternalCanister(_) => "ChangeExternalCanister", + RequestOperationDTO::CreateExternalCanister(_) => "CreateExternalCanister", + RequestOperationDTO::ConfigureExternalCanister(_) => "ConfigureExternalCanister", + RequestOperationDTO::CallExternalCanister(_) => "CallExternalCanister", + RequestOperationDTO::EditPermission(_) => "EditPermission", + RequestOperationDTO::AddRequestPolicy(_) => "AddRequestPolicy", + RequestOperationDTO::EditRequestPolicy(_) => "EditRequestPolicy", + RequestOperationDTO::RemoveRequestPolicy(_) => "RemoveRequestPolicy", + RequestOperationDTO::ManageSystemInfo(_) => "ManageSystemInfo", + } +} + +fn display_request_status(status: &RequestStatusDTO) -> &'static str { + match status { + RequestStatusDTO::Created => "Created", + RequestStatusDTO::Approved => "Approved", + RequestStatusDTO::Rejected => "Rejected", + RequestStatusDTO::Cancelled { .. } => "Cancelled", + RequestStatusDTO::Scheduled { .. } => "Scheduled", + RequestStatusDTO::Processing { .. } => "Processing", + RequestStatusDTO::Completed { .. } => "Completed", + RequestStatusDTO::Failed { .. } => "Failed", + } +} From 589b288a7f6eec6249b54a38fe38302457abd900 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Wed, 14 Aug 2024 12:45:02 +0000 Subject: [PATCH 36/42] Implement detailed view for change canister --- tools/dfx-orbit/src/cli/review.rs | 7 +- tools/dfx-orbit/src/cli/review/display.rs | 256 +++++++++++++--------- 2 files changed, 151 insertions(+), 112 deletions(-) diff --git a/tools/dfx-orbit/src/cli/review.rs b/tools/dfx-orbit/src/cli/review.rs index b288e7d69..0599234b5 100644 --- a/tools/dfx-orbit/src/cli/review.rs +++ b/tools/dfx-orbit/src/cli/review.rs @@ -4,7 +4,6 @@ use crate::{ args::review::{ReviewActionArgs, ReviewArgs}, DfxOrbit, }; -use display::{display_get_request_response, display_list}; use orbit_station_api::{RequestApprovalStatusDTO, RequestStatusDTO, SubmitRequestApprovalInput}; use serde::Serialize; @@ -19,7 +18,7 @@ impl DfxOrbit { if as_json { print_as_json(&response); } else { - println!("{}", display_list(response)); + println!("{}", self.display_list(response)); } Ok(()) } @@ -32,7 +31,7 @@ impl DfxOrbit { if as_json { print_as_json(&request); } else { - println!("{}", display_get_request_response(request)) + println!("{}", self.display_get_request_response(request)) } Ok(()) @@ -42,7 +41,7 @@ impl DfxOrbit { if as_json { print_as_json(&request); } else { - println!("{}", display_get_request_response(request.clone())) + println!("{}", self.display_get_request_response(request.clone())) } if let RequestStatusDTO::Created = request.request.status { diff --git a/tools/dfx-orbit/src/cli/review/display.rs b/tools/dfx-orbit/src/cli/review/display.rs index 65fbf6a62..738aa3b03 100644 --- a/tools/dfx-orbit/src/cli/review/display.rs +++ b/tools/dfx-orbit/src/cli/review/display.rs @@ -1,6 +1,7 @@ use itertools::Itertools; use orbit_station_api::{ - GetRequestResponse, ListRequestsResponse, RequestOperationDTO, RequestStatusDTO, + CanisterInstallMode, ChangeExternalCanisterOperationDTO, GetRequestResponse, + ListRequestsResponse, RequestOperationDTO, RequestStatusDTO, }; use std::{collections::HashMap, fmt::Write}; use tabled::{ @@ -8,122 +9,161 @@ use tabled::{ Table, }; -pub(crate) fn display_list(data: ListRequestsResponse) -> String { - let add_info = data - .additional_info - .into_iter() - .map(|info| (info.id.clone(), info)) - .collect::>(); +use crate::DfxOrbit; - let data_iter = data.requests.iter().map(|request| { - let add_info = add_info.get(&request.id); +impl DfxOrbit { + pub(crate) fn display_list(&self, data: ListRequestsResponse) -> String { + let add_info = data + .additional_info + .into_iter() + .map(|info| (info.id.clone(), info)) + .collect::>(); + + let data_iter = data.requests.iter().map(|request| { + let add_info = add_info.get(&request.id); + + [ + request.id.clone(), + add_info + .map(|add_info| add_info.requester_name.clone()) + .unwrap_or(String::from("-")), + request.title.clone(), + self.display_request_operation(&request.operation) + .to_string(), + self.display_request_status(&request.status).to_string(), + ] + }); + let titled_iter = std::iter::once([ + String::from("ID"), + String::from("Requested by"), + String::from("Title"), + String::from("Operation"), + String::from("Execution Status"), + ]) + .chain(data_iter); + + let table_config = Settings::default().with(Style::psql()); + let table = Table::from_iter(titled_iter).with(table_config).to_string(); + + table + } + + pub(crate) fn display_get_request_response(&self, request: GetRequestResponse) -> String { + let base_info = request.request; + let add_info = request.additional_info; + + let mut output = String::new(); - [ - request.id.clone(), + // General request information + writeln!(output, "===REQUEST===").unwrap(); + writeln!(output, "ID: {}", base_info.id).unwrap(); + writeln!( + output, + "Operation: {}", + self.display_request_operation(&base_info.operation) + ) + .unwrap(); + writeln!( + output, + "Status: {}", + self.display_request_status(&base_info.status) + ) + .unwrap(); + writeln!(output, "Title: {}", base_info.title).unwrap(); + if let Some(summary) = base_info.summary { + writeln!(output, "Summary: {}", summary).unwrap() + } + writeln!(output, "Requested by: {}", add_info.requester_name).unwrap(); + writeln!( + output, + "Approved by: {}", add_info - .map(|add_info| add_info.requester_name.clone()) - .unwrap_or(String::from("-")), - request.title.clone(), - display_request_operation(&request.operation).to_string(), - display_request_status(&request.status).to_string(), - ] - }); - let titled_iter = std::iter::once([ - String::from("ID"), - String::from("Requested by"), - String::from("Title"), - String::from("Operation"), - String::from("Execution Status"), - ]) - .chain(data_iter); - - let table_config = Settings::default().with(Style::psql()); - let table = Table::from_iter(titled_iter).with(table_config).to_string(); - - table -} + .approvers + .into_iter() + .map(|approver| approver.name) + .join(", ") + ) + .unwrap(); + + match base_info.operation { + RequestOperationDTO::ChangeExternalCanister(op) => { + self.display_change_canister_operation(&mut output, op.as_ref()) + } + _ => (), + }; + // write!(output, "ID: {}\n", base_info.id).unwrap(); + // approved by (comma sepatated) -pub(crate) fn display_get_request_response(request: GetRequestResponse) -> String { - let base_info = request.request; - let add_info = request.additional_info; - - let mut output = String::new(); - - // General request information - writeln!(output, "===REQUEST===").unwrap(); - writeln!(output, "ID: {}", base_info.id).unwrap(); - writeln!( - output, - "Operation: {}", - display_request_operation(&base_info.operation) - ) - .unwrap(); - writeln!( - output, - "Status: {}", - display_request_status(&base_info.status) - ) - .unwrap(); - writeln!(output, "Title: {}", base_info.title).unwrap(); - if let Some(summary) = base_info.summary { - writeln!(output, "Summary: {}", summary).unwrap() + // TODO: Display operation + // TODO: Per operation additional information + + output } - writeln!(output, "Requested by: {}", add_info.requester_name).unwrap(); - writeln!( - output, - "Approved by: {}", - add_info - .approvers - .into_iter() - .map(|approver| approver.name) - .join(", ") - ) - .unwrap(); - // write!(output, "ID: {}\n", base_info.id).unwrap(); - // approved by (comma sepatated) - // TODO: Display operation - // TODO: Per operation additional information + fn display_change_canister_operation( + &self, + output: &mut String, + op: &ChangeExternalCanisterOperationDTO, + ) { + writeln!(output, "=== Change External Canister ===").unwrap(); + match self.canister_name(&op.canister_id).ok() { + Some(canister_name) => { + writeln!(output, "Target: {} ({})", canister_name, &op.canister_id) + } + None => writeln!(output, "Target: {}", &op.canister_id), + } + .unwrap(); - output -} + let mode = match op.mode { + CanisterInstallMode::Install => "Install", + CanisterInstallMode::Reinstall => "Reinstall", + CanisterInstallMode::Upgrade => "Upgrade", + }; + writeln!(output, "Mode {}", mode).unwrap(); -fn display_request_operation(op: &RequestOperationDTO) -> &'static str { - match op { - RequestOperationDTO::Transfer(_) => "Transfer", - RequestOperationDTO::AddAccount(_) => "AddAccount", - RequestOperationDTO::EditAccount(_) => "EditAccount", - RequestOperationDTO::AddAddressBookEntry(_) => "AddAddressBookEntry", - RequestOperationDTO::EditAddressBookEntry(_) => "EditAddressBookEntry", - RequestOperationDTO::RemoveAddressBookEntry(_) => "RemoveAddressBookEntry", - RequestOperationDTO::AddUser(_) => "AddUser", - RequestOperationDTO::EditUser(_) => "EditUser", - RequestOperationDTO::AddUserGroup(_) => "AddUserGroup", - RequestOperationDTO::EditUserGroup(_) => "EditUserGroup", - RequestOperationDTO::RemoveUserGroup(_) => "RemoveUserGroup", - RequestOperationDTO::ChangeCanister(_) => "ChangeCanister", - RequestOperationDTO::SetDisasterRecovery(_) => "SetDisasterRecovery", - RequestOperationDTO::ChangeExternalCanister(_) => "ChangeExternalCanister", - RequestOperationDTO::CreateExternalCanister(_) => "CreateExternalCanister", - RequestOperationDTO::ConfigureExternalCanister(_) => "ConfigureExternalCanister", - RequestOperationDTO::CallExternalCanister(_) => "CallExternalCanister", - RequestOperationDTO::EditPermission(_) => "EditPermission", - RequestOperationDTO::AddRequestPolicy(_) => "AddRequestPolicy", - RequestOperationDTO::EditRequestPolicy(_) => "EditRequestPolicy", - RequestOperationDTO::RemoveRequestPolicy(_) => "RemoveRequestPolicy", - RequestOperationDTO::ManageSystemInfo(_) => "ManageSystemInfo", + writeln!(output, "Module checksum: 0x{}", &op.module_checksum).unwrap(); + op.arg_checksum + .as_ref() + .map(|arg_checksum| writeln!(output, "Arg checksum: 0x{}", arg_checksum).unwrap()); + } + + fn display_request_operation(&self, op: &RequestOperationDTO) -> &'static str { + match op { + RequestOperationDTO::Transfer(_) => "Transfer", + RequestOperationDTO::AddAccount(_) => "AddAccount", + RequestOperationDTO::EditAccount(_) => "EditAccount", + RequestOperationDTO::AddAddressBookEntry(_) => "AddAddressBookEntry", + RequestOperationDTO::EditAddressBookEntry(_) => "EditAddressBookEntry", + RequestOperationDTO::RemoveAddressBookEntry(_) => "RemoveAddressBookEntry", + RequestOperationDTO::AddUser(_) => "AddUser", + RequestOperationDTO::EditUser(_) => "EditUser", + RequestOperationDTO::AddUserGroup(_) => "AddUserGroup", + RequestOperationDTO::EditUserGroup(_) => "EditUserGroup", + RequestOperationDTO::RemoveUserGroup(_) => "RemoveUserGroup", + RequestOperationDTO::ChangeCanister(_) => "ChangeCanister", + RequestOperationDTO::SetDisasterRecovery(_) => "SetDisasterRecovery", + RequestOperationDTO::ChangeExternalCanister(_) => "ChangeExternalCanister", + RequestOperationDTO::CreateExternalCanister(_) => "CreateExternalCanister", + RequestOperationDTO::ConfigureExternalCanister(_) => "ConfigureExternalCanister", + RequestOperationDTO::CallExternalCanister(_) => "CallExternalCanister", + RequestOperationDTO::EditPermission(_) => "EditPermission", + RequestOperationDTO::AddRequestPolicy(_) => "AddRequestPolicy", + RequestOperationDTO::EditRequestPolicy(_) => "EditRequestPolicy", + RequestOperationDTO::RemoveRequestPolicy(_) => "RemoveRequestPolicy", + RequestOperationDTO::ManageSystemInfo(_) => "ManageSystemInfo", + } } -} -fn display_request_status(status: &RequestStatusDTO) -> &'static str { - match status { - RequestStatusDTO::Created => "Created", - RequestStatusDTO::Approved => "Approved", - RequestStatusDTO::Rejected => "Rejected", - RequestStatusDTO::Cancelled { .. } => "Cancelled", - RequestStatusDTO::Scheduled { .. } => "Scheduled", - RequestStatusDTO::Processing { .. } => "Processing", - RequestStatusDTO::Completed { .. } => "Completed", - RequestStatusDTO::Failed { .. } => "Failed", + fn display_request_status(&self, status: &RequestStatusDTO) -> &'static str { + match status { + RequestStatusDTO::Created => "Created", + RequestStatusDTO::Approved => "Approved", + RequestStatusDTO::Rejected => "Rejected", + RequestStatusDTO::Cancelled { .. } => "Cancelled", + RequestStatusDTO::Scheduled { .. } => "Scheduled", + RequestStatusDTO::Processing { .. } => "Processing", + RequestStatusDTO::Completed { .. } => "Completed", + RequestStatusDTO::Failed { .. } => "Failed", + } } } From 09dae7ed1511fae407b5fa704ff0cd3116bd0d2d Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Thu, 15 Aug 2024 12:28:09 +0000 Subject: [PATCH 37/42] Filter by external canister by default --- tools/dfx-orbit/src/args/review.rs | 10 ++++++++++ tools/dfx-orbit/src/args/review/list.rs | 9 +++++++-- tools/dfx-orbit/src/args/review/next.rs | 13 ++++++++----- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/tools/dfx-orbit/src/args/review.rs b/tools/dfx-orbit/src/args/review.rs index 7168d1260..b74a7c762 100644 --- a/tools/dfx-orbit/src/args/review.rs +++ b/tools/dfx-orbit/src/args/review.rs @@ -7,6 +7,7 @@ use clap::{Parser, Subcommand}; use id::ReviewIdArgs; use list::ReviewListArgs; use next::ReviewNextArgs; +use orbit_station_api::ListRequestsOperationTypeDTO; /// Station management commands. #[derive(Debug, Parser)] @@ -28,3 +29,12 @@ pub enum ReviewActionArgs { /// Review a specific request. Id(ReviewIdArgs), } + +fn external_canister_operations() -> Vec { + vec![ + ListRequestsOperationTypeDTO::ChangeExternalCanister(None), + ListRequestsOperationTypeDTO::CreateExternalCanister, + ListRequestsOperationTypeDTO::CallExternalCanister(None), + ListRequestsOperationTypeDTO::ConfigureExternalCanister(None), + ] +} diff --git a/tools/dfx-orbit/src/args/review/list.rs b/tools/dfx-orbit/src/args/review/list.rs index 187cb2a37..5cb78cf70 100644 --- a/tools/dfx-orbit/src/args/review/list.rs +++ b/tools/dfx-orbit/src/args/review/list.rs @@ -3,15 +3,20 @@ use clap::Parser; use orbit_station_api::{ListRequestsInput, SortDirection}; +use super::external_canister_operations; + // TODO: Ideas what we could filter by: // - Filter by status: -> Only the ones which are in Created // - Filter by times -> There are four times that could be set // - Filter by request ids -// - Filter by default only for external canister calls -> --all for all /// Reviews the next request. #[derive(Debug, Parser)] pub struct ReviewListArgs { + /// Show all request types, not only the ones related to canister management + #[clap(short, long)] + pub all: bool, + /// Show only approvable requests. #[clap(short, long)] pub only_approvable: bool, @@ -23,7 +28,7 @@ impl From for ListRequestsInput { requester_ids: None, approver_ids: None, statuses: None, - operation_types: None, + operation_types: (!args.all).then(external_canister_operations), expiration_from_dt: None, expiration_to_dt: None, created_from_dt: None, diff --git a/tools/dfx-orbit/src/args/review/next.rs b/tools/dfx-orbit/src/args/review/next.rs index 545b410f8..07f6b4994 100644 --- a/tools/dfx-orbit/src/args/review/next.rs +++ b/tools/dfx-orbit/src/args/review/next.rs @@ -1,19 +1,22 @@ //! CLI arguments for `dfx-orbit review next`. +use super::external_canister_operations; use clap::Parser; use orbit_station_api::GetNextApprovableRequestInput; -// TODO: Only show review types that are relevant to dfx-orbbit -> can deactivate with --all - /// Reviews the next request. #[derive(Debug, Parser)] -pub struct ReviewNextArgs {} +pub struct ReviewNextArgs { + /// Show any request type, not only the ones related to canister management + #[clap(short, long)] + any: bool, +} impl From for GetNextApprovableRequestInput { - fn from(_: ReviewNextArgs) -> Self { + fn from(args: ReviewNextArgs) -> Self { Self { excluded_request_ids: vec![], - operation_types: None, + operation_types: (!args.any).then(external_canister_operations), } } } From 3bf0ab9e3ac1e877e64af4b94e90da5aadac2f4f Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Thu, 15 Aug 2024 12:46:14 +0000 Subject: [PATCH 38/42] WIP display for call canister --- tools/dfx-orbit/src/cli/review/display.rs | 67 +++++++++++++++++++---- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/tools/dfx-orbit/src/cli/review/display.rs b/tools/dfx-orbit/src/cli/review/display.rs index 738aa3b03..9afd0ae92 100644 --- a/tools/dfx-orbit/src/cli/review/display.rs +++ b/tools/dfx-orbit/src/cli/review/display.rs @@ -1,7 +1,8 @@ +use candid::Principal; use itertools::Itertools; use orbit_station_api::{ - CanisterInstallMode, ChangeExternalCanisterOperationDTO, GetRequestResponse, - ListRequestsResponse, RequestOperationDTO, RequestStatusDTO, + CallExternalCanisterOperationDTO, CanisterInstallMode, ChangeExternalCanisterOperationDTO, + GetRequestResponse, ListRequestsResponse, RequestOperationDTO, RequestStatusDTO, }; use std::{collections::HashMap, fmt::Write}; use tabled::{ @@ -89,12 +90,13 @@ impl DfxOrbit { RequestOperationDTO::ChangeExternalCanister(op) => { self.display_change_canister_operation(&mut output, op.as_ref()) } + RequestOperationDTO::CallExternalCanister(op) => { + self.display_call_canister_operation(&mut output, op.as_ref()) + } _ => (), }; // write!(output, "ID: {}\n", base_info.id).unwrap(); - // approved by (comma sepatated) - // TODO: Display operation // TODO: Per operation additional information output @@ -106,12 +108,11 @@ impl DfxOrbit { op: &ChangeExternalCanisterOperationDTO, ) { writeln!(output, "=== Change External Canister ===").unwrap(); - match self.canister_name(&op.canister_id).ok() { - Some(canister_name) => { - writeln!(output, "Target: {} ({})", canister_name, &op.canister_id) - } - None => writeln!(output, "Target: {}", &op.canister_id), - } + writeln!( + output, + "Target: {}", + self.try_reverse_lookup(&op.canister_id) + ) .unwrap(); let mode = match op.mode { @@ -124,7 +125,51 @@ impl DfxOrbit { writeln!(output, "Module checksum: 0x{}", &op.module_checksum).unwrap(); op.arg_checksum .as_ref() - .map(|arg_checksum| writeln!(output, "Arg checksum: 0x{}", arg_checksum).unwrap()); + .map(|arg_checksum| writeln!(output, "Argument checksum: 0x{}", arg_checksum).unwrap()); + } + + fn display_call_canister_operation( + &self, + output: &mut String, + op: &CallExternalCanisterOperationDTO, + ) { + writeln!(output, "=== Call External Canister ===").unwrap(); + writeln!( + output, + "Execution method: \"{}\" of canister {}", + op.execution_method.method_name, + self.try_reverse_lookup(&op.execution_method.canister_id) + ) + .unwrap(); + op.validation_method.as_ref().map(|validation_method| { + writeln!( + output, + "Validation method: \"{}\" of canister {}", + validation_method.method_name, + self.try_reverse_lookup(&validation_method.canister_id) + ) + .unwrap() + }); + op.arg_checksum + .as_ref() + .map(|checksum| writeln!(output, "Argument checksum: 0x{}", checksum).unwrap()); + op.arg_rendering + .as_ref() + .map(|args| writeln!(output, "Argument: {}", args).unwrap()); + op.execution_method_cycles + .as_ref() + .map(|cycles| writeln!(output, "Execution method cycles: {}", cycles)); + + todo!() + } + + fn try_reverse_lookup(&self, canister_id: &Principal) -> String { + match self.canister_name(canister_id).ok() { + Some(canister_name) => { + format!("{} ({})", canister_name, canister_id) + } + None => format!("{}", canister_id), + } } fn display_request_operation(&self, op: &RequestOperationDTO) -> &'static str { From 18e49fd2bd65ad3dfc44984e94ec9830ce159441 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Thu, 15 Aug 2024 13:00:02 +0000 Subject: [PATCH 39/42] Display call external cansiter w/o types --- tools/dfx-orbit/src/cli/review/display.rs | 38 +++++++++++++---------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/tools/dfx-orbit/src/cli/review/display.rs b/tools/dfx-orbit/src/cli/review/display.rs index 9afd0ae92..e5a744f19 100644 --- a/tools/dfx-orbit/src/cli/review/display.rs +++ b/tools/dfx-orbit/src/cli/review/display.rs @@ -123,9 +123,9 @@ impl DfxOrbit { writeln!(output, "Mode {}", mode).unwrap(); writeln!(output, "Module checksum: 0x{}", &op.module_checksum).unwrap(); - op.arg_checksum - .as_ref() - .map(|arg_checksum| writeln!(output, "Argument checksum: 0x{}", arg_checksum).unwrap()); + if let Some(arg_checksum) = &op.arg_checksum { + writeln!(output, "Argument checksum: 0x{}", arg_checksum).unwrap() + } } fn display_call_canister_operation( @@ -141,7 +141,7 @@ impl DfxOrbit { self.try_reverse_lookup(&op.execution_method.canister_id) ) .unwrap(); - op.validation_method.as_ref().map(|validation_method| { + if let Some(validation_method) = &op.validation_method { writeln!( output, "Validation method: \"{}\" of canister {}", @@ -149,18 +149,24 @@ impl DfxOrbit { self.try_reverse_lookup(&validation_method.canister_id) ) .unwrap() - }); - op.arg_checksum - .as_ref() - .map(|checksum| writeln!(output, "Argument checksum: 0x{}", checksum).unwrap()); - op.arg_rendering - .as_ref() - .map(|args| writeln!(output, "Argument: {}", args).unwrap()); - op.execution_method_cycles - .as_ref() - .map(|cycles| writeln!(output, "Execution method cycles: {}", cycles)); - - todo!() + } + if let Some(checksum) = &op.arg_checksum { + writeln!(output, "Argument checksum: 0x{}", checksum).unwrap() + } + if let Some(args) = &op.arg_rendering { + writeln!(output, "Argument: {}", args).unwrap() + } + if let Some(cycles) = &op.execution_method_cycles { + writeln!(output, "Execution method cycles: {}", cycles).unwrap() + } + if let Some(reply) = &op.execution_method_reply { + match candid_parser::IDLArgs::from_bytes(&reply) { + // TODO: Check if we can get the type information from somewhere to annotate this with types + Ok(response) => writeln!(output, "Execution response: {}", response), + Err(_) => writeln!(output, "FAILED TO PARSE EXECUTION RESPONSE"), + } + .unwrap(); + } } fn try_reverse_lookup(&self, canister_id: &Principal) -> String { From 58d319966a7d1da992912fad90760deb97444643 Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Thu, 15 Aug 2024 15:02:26 +0000 Subject: [PATCH 40/42] More UX improvements for the CanisterCall functionality --- tools/dfx-orbit/src/args/review.rs | 1 + tools/dfx-orbit/src/cli/review/display.rs | 44 +++++++++++++++-------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/tools/dfx-orbit/src/args/review.rs b/tools/dfx-orbit/src/args/review.rs index b74a7c762..2e3d49f93 100644 --- a/tools/dfx-orbit/src/args/review.rs +++ b/tools/dfx-orbit/src/args/review.rs @@ -30,6 +30,7 @@ pub enum ReviewActionArgs { Id(ReviewIdArgs), } +// FIXME: Using this list doesn't seem to work. fn external_canister_operations() -> Vec { vec![ ListRequestsOperationTypeDTO::ChangeExternalCanister(None), diff --git a/tools/dfx-orbit/src/cli/review/display.rs b/tools/dfx-orbit/src/cli/review/display.rs index e5a744f19..348d442d2 100644 --- a/tools/dfx-orbit/src/cli/review/display.rs +++ b/tools/dfx-orbit/src/cli/review/display.rs @@ -1,3 +1,4 @@ +use crate::DfxOrbit; use candid::Principal; use itertools::Itertools; use orbit_station_api::{ @@ -10,8 +11,6 @@ use tabled::{ Table, }; -use crate::DfxOrbit; - impl DfxOrbit { pub(crate) fn display_list(&self, data: ListRequestsResponse) -> String { let add_info = data @@ -56,7 +55,7 @@ impl DfxOrbit { let mut output = String::new(); // General request information - writeln!(output, "===REQUEST===").unwrap(); + writeln!(output, "=== REQUEST ===").unwrap(); writeln!(output, "ID: {}", base_info.id).unwrap(); writeln!( output, @@ -64,12 +63,6 @@ impl DfxOrbit { self.display_request_operation(&base_info.operation) ) .unwrap(); - writeln!( - output, - "Status: {}", - self.display_request_status(&base_info.status) - ) - .unwrap(); writeln!(output, "Title: {}", base_info.title).unwrap(); if let Some(summary) = base_info.summary { writeln!(output, "Summary: {}", summary).unwrap() @@ -85,6 +78,15 @@ impl DfxOrbit { .join(", ") ) .unwrap(); + writeln!( + output, + "Status: {}", + self.display_request_status(&base_info.status) + ) + .unwrap(); + if let Some(additional_status) = self.display_additional_stats_info(&base_info.status) { + writeln!(output, "{}", additional_status).unwrap(); + } match base_info.operation { RequestOperationDTO::ChangeExternalCanister(op) => { @@ -95,9 +97,9 @@ impl DfxOrbit { } _ => (), }; - // write!(output, "ID: {}\n", base_info.id).unwrap(); - // TODO: Per operation additional information + // TODO: CreateCanister Additional information + // TODO: Configure Canist Additional information output } @@ -120,7 +122,7 @@ impl DfxOrbit { CanisterInstallMode::Reinstall => "Reinstall", CanisterInstallMode::Upgrade => "Upgrade", }; - writeln!(output, "Mode {}", mode).unwrap(); + writeln!(output, "Mode: {}", mode).unwrap(); writeln!(output, "Module checksum: 0x{}", &op.module_checksum).unwrap(); if let Some(arg_checksum) = &op.arg_checksum { @@ -136,7 +138,7 @@ impl DfxOrbit { writeln!(output, "=== Call External Canister ===").unwrap(); writeln!( output, - "Execution method: \"{}\" of canister {}", + "Execution method: \"{}\" of {}", op.execution_method.method_name, self.try_reverse_lookup(&op.execution_method.canister_id) ) @@ -144,7 +146,7 @@ impl DfxOrbit { if let Some(validation_method) = &op.validation_method { writeln!( output, - "Validation method: \"{}\" of canister {}", + "Validation method: \"{}\" of {}", validation_method.method_name, self.try_reverse_lookup(&validation_method.canister_id) ) @@ -160,7 +162,7 @@ impl DfxOrbit { writeln!(output, "Execution method cycles: {}", cycles).unwrap() } if let Some(reply) = &op.execution_method_reply { - match candid_parser::IDLArgs::from_bytes(&reply) { + match candid_parser::IDLArgs::from_bytes(reply) { // TODO: Check if we can get the type information from somewhere to annotate this with types Ok(response) => writeln!(output, "Execution response: {}", response), Err(_) => writeln!(output, "FAILED TO PARSE EXECUTION RESPONSE"), @@ -217,4 +219,16 @@ impl DfxOrbit { RequestStatusDTO::Failed { .. } => "Failed", } } + + fn display_additional_stats_info(&self, status: &RequestStatusDTO) -> Option { + match status { + RequestStatusDTO::Cancelled { reason } => { + reason.clone().map(|reason| format!("Reason: {}", reason)) + } + RequestStatusDTO::Failed { reason } => { + reason.clone().map(|reason| format!("Reason: {}", reason)) + } + _ => None, + } + } } From bbbaf8323e7e2a38af2781fa325fb324f44fed0a Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Thu, 15 Aug 2024 16:12:10 +0000 Subject: [PATCH 41/42] Refactor --- tools/dfx-orbit/src/cli/asset/util.rs | 21 +++++++------------- tools/dfx-orbit/src/lib.rs | 28 ++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/tools/dfx-orbit/src/cli/asset/util.rs b/tools/dfx-orbit/src/cli/asset/util.rs index 6cf3d3db0..897d87693 100644 --- a/tools/dfx-orbit/src/cli/asset/util.rs +++ b/tools/dfx-orbit/src/cli/asset/util.rs @@ -1,5 +1,5 @@ use crate::DfxOrbit; -use anyhow::{anyhow, bail}; +use anyhow::bail; use candid::Principal; use dfx_core::config::model::dfinity::CanisterTypeProperties; use ic_certified_assets::types::{GrantPermissionArguments, Permission}; @@ -48,20 +48,13 @@ impl DfxOrbit { Ok(response) } - pub fn as_path_bufs(&self, canister: &str, paths: &[String]) -> anyhow::Result> { + pub(crate) fn as_path_bufs( + &self, + canister: &str, + paths: &[String], + ) -> anyhow::Result> { if paths.is_empty() { - let config = self.interface.config().ok_or_else(|| { - anyhow!("Could not read \"dfx.json\". Are you in the correct directory?") - })?; - - let canister_config = config - .get_config() - .canisters - .as_ref() - .ok_or_else(|| anyhow!("No canisters defined in this \"dfx.json\""))? - .get(canister) - .ok_or_else(|| anyhow!("Could not find {canister} in \"dfx.json\""))?; - + let canister_config = self.get_canister_config(canister)?; let CanisterTypeProperties::Assets { source, .. } = &canister_config.type_specific else { bail!("Canister {canister} is not an asset canister"); diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index 9456f6b1c..021fa5c7a 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -13,13 +13,20 @@ pub mod station_agent; use anyhow::anyhow; use candid::Principal; pub use cli::asset::AssetAgent; -use dfx_core::{config::model::canister_id_store::CanisterIdStore, DfxInterface}; +use dfx_core::{ + config::model::{ + canister_id_store::CanisterIdStore, + dfinity::{Config, ConfigCanistersCanister}, + }, + DfxInterface, +}; use dfx_extension_api::OrbitExtensionAgent; use ic_utils::{canister::CanisterBuilder, Canister}; use orbit_station_api::CreateRequestResponse; use slog::Logger; pub use station_agent::StationAgent; use station_agent::StationConfig; +use std::sync::Arc; pub struct DfxOrbit { // The station agent that handles communication with the station @@ -101,4 +108,23 @@ impl DfxOrbit { println!("Request URL: {request_url}"); println!("To view the request, run: dfx-orbit review id {request_id}"); } + + pub fn get_config(&self) -> anyhow::Result> { + Ok(self.interface.config().ok_or_else(|| { + anyhow!("Could not read \"dfx.json\". Are you in the correct directory?") + })?) + } + + pub fn get_canister_config(&self, canister: &str) -> anyhow::Result { + let config = self.get_config()?; + let canister_config = config + .get_config() + .canisters + .as_ref() + .ok_or_else(|| anyhow!("No canisters defined in this \"dfx.json\""))? + .get(canister) + .ok_or_else(|| anyhow!("Could not find {canister} in \"dfx.json\""))?; + + Ok(canister_config.clone()) + } } From 878310ff7f559fabf5f07e20379502543ef11e7d Mon Sep 17 00:00:00 2001 From: Leon Tan Date: Thu, 15 Aug 2024 16:26:16 +0000 Subject: [PATCH 42/42] Small refactors --- tools/dfx-orbit/src/args/asset.rs | 2 +- .../dfx-orbit/src/args/request/canister/call.rs | 1 + .../src/args/request/canister/install.rs | 17 +++++------------ tools/dfx-orbit/src/cli/asset/util.rs | 6 +----- tools/dfx-orbit/src/lib.rs | 4 ++-- 5 files changed, 10 insertions(+), 20 deletions(-) diff --git a/tools/dfx-orbit/src/args/asset.rs b/tools/dfx-orbit/src/args/asset.rs index b47407267..eeb05b642 100644 --- a/tools/dfx-orbit/src/args/asset.rs +++ b/tools/dfx-orbit/src/args/asset.rs @@ -79,6 +79,6 @@ pub struct AssetCheckArgs { pub(crate) files: Vec, /// Automatically approve the request, if the request's evidence matches the local evidence - #[clap(long)] + #[clap(short = 'a', long)] pub(crate) then_approve: bool, } diff --git a/tools/dfx-orbit/src/args/request/canister/call.rs b/tools/dfx-orbit/src/args/request/canister/call.rs index 944717bc9..507f0b222 100644 --- a/tools/dfx-orbit/src/args/request/canister/call.rs +++ b/tools/dfx-orbit/src/args/request/canister/call.rs @@ -20,6 +20,7 @@ pub struct RequestCanisterCallArgs { // TODO: The format of the argument. // #[clap(short, long)] // r#type: Option, + /// Pass the argument as a file. #[clap(short = 'f', long, conflicts_with = "argument")] arg_file: Option, /// Specifies the amount of cycles to send on the call. diff --git a/tools/dfx-orbit/src/args/request/canister/install.rs b/tools/dfx-orbit/src/args/request/canister/install.rs index 156b994d5..423aaeab2 100644 --- a/tools/dfx-orbit/src/args/request/canister/install.rs +++ b/tools/dfx-orbit/src/args/request/canister/install.rs @@ -31,29 +31,22 @@ impl RequestCanisterInstallArgs { self, dfx_orbit: &DfxOrbit, ) -> anyhow::Result { - let RequestCanisterInstallArgs { - canister, - mode, - wasm, - arg, - arg_file, - } = self; - let canister_id = dfx_orbit.canister_id(&canister)?; + let canister_id = dfx_orbit.canister_id(&self.canister)?; let operation = { - let module = std::fs::read(wasm) + let module = std::fs::read(self.wasm) .expect("Could not read Wasm file") .to_vec(); - let arg = if let Some(file) = arg_file { + let arg = if let Some(file) = self.arg_file { Some( std::fs::read(file) .expect("Could not read argument file") .to_vec(), ) } else { - arg.map(|arg| arg.as_bytes().to_vec()) + self.arg.map(|arg| arg.as_bytes().to_vec()) }; - let mode = mode.into(); + let mode = self.mode.into(); ChangeExternalCanisterOperationInput { canister_id, mode, diff --git a/tools/dfx-orbit/src/cli/asset/util.rs b/tools/dfx-orbit/src/cli/asset/util.rs index 897d87693..61b71b9b5 100644 --- a/tools/dfx-orbit/src/cli/asset/util.rs +++ b/tools/dfx-orbit/src/cli/asset/util.rs @@ -48,11 +48,7 @@ impl DfxOrbit { Ok(response) } - pub(crate) fn as_path_bufs( - &self, - canister: &str, - paths: &[String], - ) -> anyhow::Result> { + pub fn as_path_bufs(&self, canister: &str, paths: &[String]) -> anyhow::Result> { if paths.is_empty() { let canister_config = self.get_canister_config(canister)?; let CanisterTypeProperties::Assets { source, .. } = &canister_config.type_specific diff --git a/tools/dfx-orbit/src/lib.rs b/tools/dfx-orbit/src/lib.rs index 021fa5c7a..1e10e02b7 100644 --- a/tools/dfx-orbit/src/lib.rs +++ b/tools/dfx-orbit/src/lib.rs @@ -110,9 +110,9 @@ impl DfxOrbit { } pub fn get_config(&self) -> anyhow::Result> { - Ok(self.interface.config().ok_or_else(|| { + self.interface.config().ok_or_else(|| { anyhow!("Could not read \"dfx.json\". Are you in the correct directory?") - })?) + }) } pub fn get_canister_config(&self, canister: &str) -> anyhow::Result {