chore: improve validation message and error message

Author: zensh

Cargo.lock

@@ -14,7 +14,7 @@ strip = true
 opt-level = 's'
 
 [workspace.package]
-version = "0.8.9"
+version = "0.8.10"
 edition = "2021"
 repository = "https://github.com/ldclabs/ic-cose"
 keywords = ["config", "cbor", "canister", "icp", "encryption"]

Cargo.toml

@@ -16,7 +16,7 @@ crate-type = ["cdylib"]
 
 [dependencies]
 ic_cose_types = { path = "../ic_cose_types", version = "0.8" }
-candid = { workspace = true }
+candid = { workspace = true, features = ["value", "printer"] }
 ciborium = { workspace = true }
 hex = { workspace = true }
 ic-cdk = { workspace = true }

src/ic_cose_canister/Cargo.toml

@@ -1,4 +1,4 @@
-use candid::Principal;
+use candid::{pretty::candid::value::pp_value, CandidType, IDLValue, Principal};
 use ic_cose_types::validate_principals;
 use ic_cose_types::{
     types::namespace::{CreateNamespaceInput, NamespaceInfo},
@@ -98,7 +98,7 @@ fn validate_admin_add_managers(args: BTreeSet<Principal>) -> Result<(), String>
 #[ic_cdk::update]
 fn validate2_admin_add_managers(args: BTreeSet<Principal>) -> Result<String, String> {
     validate_principals(&args)?;
-    Ok("ok".to_string())
+    pretty_format(&args)
 }
 
 #[ic_cdk::update]
@@ -110,7 +110,7 @@ fn validate_admin_remove_managers(args: BTreeSet<Principal>) -> Result<(), Strin
 #[ic_cdk::update]
 fn validate2_admin_remove_managers(args: BTreeSet<Principal>) -> Result<String, String> {
     validate_principals(&args)?;
-    Ok("ok".to_string())
+    pretty_format(&args)
 }
 
 #[ic_cdk::update]
@@ -122,7 +122,7 @@ fn validate_admin_add_auditors(args: BTreeSet<Principal>) -> Result<(), String>
 #[ic_cdk::update]
 fn validate2_admin_add_auditors(args: BTreeSet<Principal>) -> Result<String, String> {
     validate_principals(&args)?;
-    Ok("ok".to_string())
+    pretty_format(&args)
 }
 
 #[ic_cdk::update]
@@ -134,7 +134,7 @@ fn validate_admin_remove_auditors(args: BTreeSet<Principal>) -> Result<(), Strin
 #[ic_cdk::update]
 fn validate2_admin_remove_auditors(args: BTreeSet<Principal>) -> Result<String, String> {
     validate_principals(&args)?;
-    Ok("ok".to_string())
+    pretty_format(&args)
 }
 
 #[ic_cdk::update]
@@ -143,8 +143,8 @@ fn validate_admin_add_allowed_apis(_args: BTreeSet<String>) -> Result<(), String
 }
 
 #[ic_cdk::update]
-fn validate2_admin_add_allowed_apis(_args: BTreeSet<String>) -> Result<String, String> {
-    Ok("ok".to_string())
+fn validate2_admin_add_allowed_apis(args: BTreeSet<String>) -> Result<String, String> {
+    pretty_format(&args)
 }
 
 #[ic_cdk::update]
@@ -153,6 +153,16 @@ fn validate_admin_remove_allowed_apis(_args: BTreeSet<String>) -> Result<(), Str
 }
 
 #[ic_cdk::update]
-fn validate2_admin_remove_allowed_apis(_args: BTreeSet<String>) -> Result<String, String> {
-    Ok("ok".to_string())
+fn validate2_admin_remove_allowed_apis(args: BTreeSet<String>) -> Result<String, String> {
+    pretty_format(&args)
+}
+
+fn pretty_format<T>(data: &T) -> Result<String, String>
+where
+    T: CandidType,
+{
+    let val = IDLValue::try_from_candid_type(data).map_err(|err| format!("{err:?}"))?;
+    let doc = pp_value(7, &val);
+
+    Ok(format!("{}", doc.pretty(120)))
 }

src/ic_cose_canister/src/api_admin.rs

@@ -32,7 +32,7 @@ fn namespace_get_delegators(
         }
 
         ns.fixed_id_names.get(&name).map_or_else(
-            || Err("name not found".to_string()),
+            || Err("NotFound: name not found".to_string()),
             |delegators| Ok(delegators.clone()),
         )
     })
@@ -99,7 +99,7 @@ fn namespace_sign_delegation(input: SignDelegationInput) -> Result<SignInRespons
             }
             return Err(format!("caller {} is not a delegator", caller));
         }
-        Err("name not found".to_string())
+        Err("NotFound: name not found".to_string())
     })?;
     if session_expires_in_ms == 0 {
         return Err("delegation is disabled".to_string());

src/ic_cose_canister/src/api_identity.rs

@@ -684,7 +684,7 @@ pub mod ns {
         NAMESPACES_STORE.with_borrow(|r| {
             r.get(namespace)
                 .map(f)
-                .unwrap_or_else(|| Err(format!("namespace {} not found", namespace)))
+                .unwrap_or_else(|| Err(format!("NotFound: namespace {} not found", namespace)))
         })
     }
 
@@ -700,7 +700,7 @@ pub mod ns {
                 }
                 Err(err) => Err(err),
             },
-            None => Err(format!("namespace {} not found", namespace)),
+            None => Err(format!("NotFound: namespace {} not found", namespace)),
         })
     }
 
@@ -1055,7 +1055,7 @@ pub mod ns {
                 r.remove(&namespace);
                 Ok(())
             }
-            None => Err(format!("namespace {} not found", namespace)),
+            None => Err(format!("NotFound: namespace {} not found", namespace)),
         })
     }
 
@@ -1076,14 +1076,14 @@ pub mod ns {
 
     pub fn get_setting_info(caller: Principal, spk: SettingPathKey) -> Result<SettingInfo, String> {
         let setting = try_get_setting(&caller, &spk)
-            .ok_or_else(|| format!("setting {} not found or no permission", spk))?;
+            .ok_or_else(|| format!("NotFound: setting {} not found or no permission", spk))?;
 
         Ok(setting.into_info(spk.2, spk.3, false))
     }
 
     pub fn get_setting(caller: Principal, spk: SettingPathKey) -> Result<SettingInfo, String> {
         let setting = try_get_setting(&caller, &spk)
-            .ok_or_else(|| format!("setting {} not found or no permission", &spk))?;
+            .ok_or_else(|| format!("NotFound: setting {} not found or no permission", &spk))?;
 
         if spk.4 != 0 && spk.4 != setting.version {
             Err("version mismatch".to_string())?;
@@ -1097,15 +1097,15 @@ pub mod ns {
         spk: SettingPathKey,
     ) -> Result<SettingArchivedPayload, String> {
         let setting = try_get_setting(&caller, &spk)
-            .ok_or_else(|| format!("setting {} not found or no permission", &spk))?;
+            .ok_or_else(|| format!("NotFound: setting {} not found or no permission", &spk))?;
 
         if spk.4 == 0 || spk.4 >= setting.version {
             Err("version mismatch".to_string())?;
         };
 
         let payload = PAYLOADS_STORE.with_borrow(|r| {
             r.get(&spk)
-                .ok_or_else(|| format!("setting {} payload not found", &spk))
+                .ok_or_else(|| format!("NotFound: setting {} payload not found", &spk))
         })?;
 
         Ok(SettingArchivedPayload {
@@ -1220,7 +1220,7 @@ pub mod ns {
                         Err(err) => Err(err),
                     }
                 }
-                None => Err(format!("setting {} not found", &spk)),
+                None => Err(format!("NotFound: setting {} not found", &spk)),
             })
         })
     }
@@ -1254,7 +1254,7 @@ pub mod ns {
 
                     Ok(())
                 }
-                None => Err(format!("setting {} not found", &spk)),
+                None => Err(format!("NotFound: setting {} not found", &spk)),
             })
         })
     }
@@ -1332,7 +1332,7 @@ pub mod ns {
                         version: setting.version,
                     })
                 }
-                None => Err(format!("setting {} not found", &spk)),
+                None => Err(format!("NotFound: setting {} not found", &spk)),
             })?;
 
             ns.payload_bytes_total = ns.payload_bytes_total.saturating_add(size as u64);

src/ic_cose_canister/src/store.rs

@@ -16,7 +16,7 @@ crate-type = ["cdylib"]
 
 [dependencies]
 ic_cose_types = { path = "../ic_cose_types", version = "0.8" }
-candid = { workspace = true }
+candid = { workspace = true, features = ["value", "printer"] }
 futures = { workspace = true }
 ciborium = { workspace = true }
 hex = { workspace = true }

src/ic_wasm_canister/Cargo.toml

@@ -25,7 +25,7 @@ fn get_wasm(hash: ByteArray<32>) -> Result<WasmInfo, String> {
             wasm: w.wasm,
             hash,
         })
-        .ok_or_else(|| "wasm not found".to_string())
+        .ok_or_else(|| "NotFound: wasm not found".to_string())
 }
 
 #[ic_cdk::query]
@@ -47,7 +47,7 @@ async fn get_canister_status(
     if canister != self_id {
         store::state::with(|s| {
             if !s.deployed_list.contains_key(&canister) {
-                return Err("canister not found".to_string());
+                return Err("NotFound: canister not found".to_string());
             }
             Ok(())
         })?;

src/ic_wasm_canister/src/api.rs

@@ -1,4 +1,4 @@
-use candid::Principal;
+use candid::{pretty::candid::value::pp_value, CandidType, IDLArgs, IDLValue, Principal};
 use ic_cdk::management_canister as mgt;
 use ic_cose_types::{
     format_error,
@@ -55,25 +55,25 @@ fn admin_remove_committers(args: BTreeSet<Principal>) -> Result<(), String> {
 #[ic_cdk::update]
 fn validate_admin_add_managers(args: BTreeSet<Principal>) -> Result<String, String> {
     validate_principals(&args)?;
-    Ok("ok".to_string())
+    pretty_format(&args)
 }
 
 #[ic_cdk::update]
 fn validate_admin_remove_managers(args: BTreeSet<Principal>) -> Result<String, String> {
     validate_principals(&args)?;
-    Ok("ok".to_string())
+    pretty_format(&args)
 }
 
 #[ic_cdk::update]
 fn validate_admin_add_committers(args: BTreeSet<Principal>) -> Result<String, String> {
     validate_principals(&args)?;
-    Ok("ok".to_string())
+    pretty_format(&args)
 }
 
 #[ic_cdk::update]
 fn validate_admin_remove_committers(args: BTreeSet<Principal>) -> Result<String, String> {
     validate_principals(&args)?;
-    Ok("ok".to_string())
+    pretty_format(&args)
 }
 
 #[ic_cdk::update(guard = "is_controller_or_manager_or_committer")]
@@ -95,14 +95,16 @@ async fn validate_admin_add_wasm(
     args: AddWasmInput,
     force_prev_hash: Option<ByteArray<32>>,
 ) -> Result<String, String> {
+    let rt = pretty_format(&(&args.name, &args.description, &force_prev_hash))?;
     store::wasm::add_wasm(
         ic_cdk::api::msg_caller(),
         ic_cdk::api::time() / MILLISECONDS,
         args,
         force_prev_hash,
         true,
     )?;
-    Ok("ok".to_string())
+
+    Ok(rt)
 }
 
 #[ic_cdk::update(guard = "is_controller")]
@@ -206,22 +208,26 @@ async fn admin_create_on(
 #[ic_cdk::update]
 fn validate_admin_create_canister(
     wasm_name: String,
-    _settings: Option<mgt::CanisterSettings>,
-    _args: Option<ByteBuf>,
+    settings: Option<mgt::CanisterSettings>,
+    args: Option<ByteBuf>,
 ) -> Result<String, String> {
     let _ = store::wasm::get_latest(&wasm_name)?;
-    Ok("ok".to_string())
+    let args = IDLArgs::from_bytes(&args.unwrap_or_else(|| ByteBuf::from(EMPTY_CANDID_ARGS)))
+        .map_err(|err| format!("Invalid args: {err}"))?;
+    pretty_format(&(&wasm_name, &settings, &args.to_string()))
 }
 
 #[ic_cdk::update]
 fn validate_admin_create_on(
-    _subnet: Principal,
+    subnet: Principal,
     wasm_name: String,
-    _settings: Option<mgt::CanisterSettings>,
-    _args: Option<ByteBuf>,
+    settings: Option<mgt::CanisterSettings>,
+    args: Option<ByteBuf>,
 ) -> Result<String, String> {
     let _ = store::wasm::get_latest(&wasm_name)?;
-    Ok("ok".to_string())
+    let args = IDLArgs::from_bytes(&args.unwrap_or_else(|| ByteBuf::from(EMPTY_CANDID_ARGS)))
+        .map_err(|err| format!("Invalid args: {err}"))?;
+    pretty_format(&(&subnet, &wasm_name, &settings, &args.to_string()))
 }
 
 #[ic_cdk::update(guard = "is_controller")]
@@ -304,6 +310,18 @@ async fn validate_admin_deploy(
     args: DeployWasmInput,
     ignore_prev_hash: Option<ByteArray<32>>,
 ) -> Result<String, String> {
+    let args_ = IDLArgs::from_bytes(
+        &args
+            .args
+            .unwrap_or_else(|| ByteBuf::from(EMPTY_CANDID_ARGS)),
+    )
+    .map_err(|err| format!("Invalid args: {err}"))?;
+    let rt = pretty_format(&(
+        &args.name,
+        &args.canister,
+        &args_.to_string(),
+        &ignore_prev_hash,
+    ))?;
     let info = mgt::canister_info(&mgt::CanisterInfoArgs {
         canister_id: args.canister,
         num_requested_changes: None,
@@ -340,12 +358,12 @@ async fn validate_admin_deploy(
                 .unwrap_or_default()
         });
         let _ = store::wasm::get_wasm(&hash)
-            .ok_or_else(|| format!("wasm not found: {}", hex::encode(hash.as_ref())))?;
+            .ok_or_else(|| format!("NotFound: wasm not found: {}", hex::encode(hash.as_ref())))?;
     } else {
         store::wasm::next_version(prev_hash)?;
     }
 
-    Ok("ok".to_string())
+    Ok(rt)
 }
 
 #[ic_cdk::update(guard = "is_controller_or_manager")]
@@ -429,7 +447,7 @@ async fn admin_batch_topup() -> Result<u128, String> {
 async fn admin_update_canister_settings(args: mgt::UpdateSettingsArgs) -> Result<(), String> {
     store::state::with(|s| {
         if !s.deployed_list.contains_key(&args.canister_id) {
-            return Err("canister not found".to_string());
+            return Err("NotFound: canister not found".to_string());
         }
         Ok(())
     })?;
@@ -439,11 +457,13 @@ async fn admin_update_canister_settings(args: mgt::UpdateSettingsArgs) -> Result
 
 #[ic_cdk::update]
 async fn validate_admin_batch_call(
-    _canisters: BTreeSet<Principal>,
-    _method: String,
-    _args: Option<ByteBuf>,
+    canisters: BTreeSet<Principal>,
+    method: String,
+    args: Option<ByteBuf>,
 ) -> Result<String, String> {
-    Ok("ok".to_string())
+    let args = IDLArgs::from_bytes(&args.unwrap_or_else(|| ByteBuf::from(EMPTY_CANDID_ARGS)))
+        .map_err(|err| format!("Invalid args: {err}"))?;
+    pretty_format(&(&canisters, &method, &args.to_string()))
 }
 
 #[ic_cdk::update]
@@ -457,9 +477,19 @@ async fn validate_admin_update_canister_settings(
 ) -> Result<String, String> {
     store::state::with(|s| {
         if !s.deployed_list.contains_key(&args.canister_id) {
-            return Err("canister not found".to_string());
+            return Err("NotFound: canister not found".to_string());
         }
         Ok(())
     })?;
-    Ok("ok".to_string())
+    pretty_format(&args)
+}
+
+fn pretty_format<T>(data: &T) -> Result<String, String>
+where
+    T: CandidType,
+{
+    let val = IDLValue::try_from_candid_type(data).map_err(|err| format!("{err:?}"))?;
+    let doc = pp_value(7, &val);
+
+    Ok(format!("{}", doc.pretty(120)))
 }