diff --git a/README.md b/README.md index 97d886b..3739d29 100644 --- a/README.md +++ b/README.md @@ -137,13 +137,18 @@ The wrapper exposes a small set of generic proxy methods: | `call0` | Proxy a call that takes **no arguments** | | `call_blob` | Proxy a call with a **Candid-encoded argument blob** | -Every proxy method follows the same two-step internal logic: +Every proxy method follows the same internal logic: -1. **Charge the fee** – the payment guard deducts the requested amount from the caller (or from a designated payer) using any supported payment type (attached cycles, ICRC-2 approve, patron pays, …). -2. **Forward the call** – once the fee is settled, the wrapper performs a raw inter-canister call to the target canister and method, passing the arguments and any optional cycles through transparently. +1. **Look up the price** – the wrapper reads the operator-configured `MethodConfig` for the `(target, method)` pair. The fee and the cycles to forward are set by the wrapper operator, **not** by the caller. If no configuration exists the call is rejected. +2. **Charge the fee** – the payment guard deducts the configured fee from the caller (or from a designated payer) using the supported payment type the caller selects (attached cycles, ICRC-2 approve, patron pays, …). +3. **Forward the call** – once the fee is settled, the wrapper performs a raw inter-canister call to the target canister and method, attaching the configured number of cycles. + +The caller only chooses **which payment method** to use; it can never set the fee or the forwarded-cycle amount. This is what prevents a caller from paying a trivial fee while forwarding a large amount (which would drain the wrapper's cycle balance). When a method forwards cycles, the operator configuration must denominate the fee in cycles and set it to at least the forwarded amount; the wrapper enforces this at configuration time. If the fee deduction fails the call is rejected immediately and the target canister is never reached. Self-calls (calling the wrapper itself) are blocked. +Configuration is controller-only, via `set_method_config` / `remove_method_config`, and is inspectable via the `get_method_config` / `list_method_configs` queries. It is persisted across canister upgrades. + ### Flow diagram ```mermaid @@ -157,8 +162,9 @@ sequenceDiagram Note over Payer,Ledger: Optional – only needed for patron-pays or ICRC-2 flows Payer->>Ledger: icrc2_approve(spender=Wrapper, amount=fee) - Caller->>Wrapper: call_blob / call0(target, method, args, fee, payment?) + Caller->>Wrapper: call_blob / call0(target, method, args, payment?) activate Wrapper + Note over Wrapper: Look up operator-configured fee & forward cycles Wrapper->>Ledger: icrc2_transfer_from (or deduct attached cycles) Ledger-->>Wrapper: ok Wrapper->>Target: raw canister call (method, args) @@ -175,18 +181,33 @@ sequenceDiagram You can deploy the wrapper yourself from `src/wrapper`, or use the shared instance already published to the IC mainnet (see `canister_ids.json`). -#### 2. Call a payable endpoint +#### 2. Configure the price (operator / controller only) + +Before a `(target, method)` can be proxied, a controller of the wrapper registers its price. `forward_cycles` is optional; when set, the fee must be denominated in cycles and cover it. + +```bash +dfx canister call "$WRAPPER_ID" set_method_config '( + record { target = principal "'$TARGET_CANISTER_ID'"; method = "my_method" }, + record { + fee = record { amount = 1_000_000 : nat; denom = variant { Cycles } }; + supported = vec { variant { AttachedCycles }; variant { CallerPaysIcrc2Cycles } }; + forward_cycles = opt (1_000_000 : nat); + } +)' +``` + +#### 3. Call a payable endpoint + +The caller supplies only the `(target, method)`, the arguments, and which payment type to use — never a fee or cycle amount. **Paying with attached cycles (simplest):** ```bash dfx canister call "$WRAPPER_ID" call0 \ '(record { - target = principal "'$TARGET_CANISTER_ID'"; - method = "my_method"; - fee_amount = 1_000_000 : nat; - payment = null; # defaults to AttachedCycles - cycles_to_forward = null; + target = principal "'$TARGET_CANISTER_ID'"; + method = "my_method"; + payment = null; # defaults to AttachedCycles })' \ --with-cycles 1000000 ``` @@ -202,12 +223,10 @@ dfx canister call $LEDGER icrc2_approve '(record { # 2. Call through the wrapper dfx canister call "$WRAPPER_ID" call_blob '(record { - target = principal "'$TARGET_CANISTER_ID'"; - method = "store_data"; - args_blob = blob "\44\49\44\4c\00\00"; - fee_amount = 5_000_000 : nat; - payment = opt variant { CallerPaysIcrc2Cycles }; - cycles_to_forward = null; + target = principal "'$TARGET_CANISTER_ID'"; + method = "store_data"; + args_blob = blob "\44\49\44\4c\00\00"; + payment = opt variant { CallerPaysIcrc2Tokens = record { ledger = principal "'$LEDGER'" } }; })' ``` @@ -223,28 +242,36 @@ dfx canister call $LEDGER icrc2_approve '(record { # Caller references the payer when calling through the wrapper dfx canister call "$WRAPPER_ID" call0 '(record { - target = principal "'$TARGET_CANISTER_ID'"; - method = "my_method"; - fee_amount = 1_000_000 : nat; - payment = opt variant { + target = principal "'$TARGET_CANISTER_ID'"; + method = "my_method"; + payment = opt variant { PatronPaysIcrc2Cycles = record { owner = principal "'$PAYER_ID'" } }; - cycles_to_forward = null; })' ``` #### Key parameters -| Parameter | Type | Description | -| ------------------- | ----------------- | ---------------------------------------------------------- | -| `target` | `Principal` | The canister to forward the call to | -| `method` | `Text` | The method name on the target canister | -| `fee_amount` | `Nat` | Fee charged by the wrapper before forwarding | -| `payment` | `opt PaymentType` | Payment mechanism; defaults to `AttachedCycles` if omitted | -| `args_blob` | `Blob` | Candid-encoded arguments (for `call_blob`) | -| `cycles_to_forward` | `opt Nat` | Additional cycles to pass to the target alongside the call | +Call parameters (per proxy call): + +| Parameter | Type | Description | +| ----------- | ----------------- | ---------------------------------------------------------- | +| `target` | `Principal` | The canister to forward the call to | +| `method` | `Text` | The method name on the target canister | +| `payment` | `opt PaymentType` | Payment mechanism; defaults to `AttachedCycles` if omitted | +| `args_blob` | `Blob` | Candid-encoded arguments (for `call_blob`) | + +Configuration parameters (per `(target, method)`, set by the operator via `set_method_config`): + +| Field | Type | Description | +| ---------------- | -------------------------- | --------------------------------------------------------------- | +| `fee` | `record { amount; denom }` | The fee charged before forwarding, and its denomination | +| `supported` | `vec VendorPaymentConfig` | Informational only for now — see note below | +| `forward_cycles` | `opt Nat` | Cycles attached to the forwarded call; must be covered by `fee` | + +> **Note on `supported`:** this field is not yet enforced per method. `bridge_call` currently validates the caller's payment type against the wrapper's global `PAYMENT_GUARD`, not against `MethodConfig.supported`. Until per-method enforcement is wired in, any payment type the global guard accepts is accepted for every configured method, regardless of what `supported` lists. -The response is returned as `Result`: the raw Candid-encoded response bytes on success, or an error string describing what went wrong (guard failure or target rejection). +The response is returned as `Result`: the raw Candid-encoded response bytes on success, or an error string describing what went wrong (method not configured, guard failure, or target rejection). --- diff --git a/src/wrapper/ic_papi_wrapper.did b/src/wrapper/ic_papi_wrapper.did index 2acd170..6b4d72e 100644 --- a/src/wrapper/ic_papi_wrapper.did +++ b/src/wrapper/ic_papi_wrapper.did @@ -1,14 +1,14 @@ type Account = record { owner : principal; subaccount : opt blob }; // Arguments for the `call0` function. +// +// Note: the fee and the cycles to forward are **not** caller-supplied; they are +// looked up from the operator-configured [`MethodConfig`] for `(target, method)`. +// The caller only chooses which supported payment type to pay with. type Call0Args = record { // The name of the method to call. method : text; - // Optional cycles to forward to the target canister. - cycles_to_forward : opt nat; // The principal of the canister to call. target : principal; - // The amount of fee to charge. - fee_amount : nat; // Optional payment configuration (defaults to `AttachedCycles`). payment : opt PaymentType; }; @@ -18,12 +18,8 @@ type CallBlobArgs = record { method : text; // The Candid-encoded arguments as a byte buffer. args_blob : blob; - // Optional cycles to forward to the target canister. - cycles_to_forward : opt nat; // The principal of the canister to call. target : principal; - // The amount of fee to charge. - fee_amount : nat; // Optional payment configuration (defaults to `AttachedCycles`). payment : opt PaymentType; }; @@ -33,16 +29,20 @@ type CallTextArgs = record { method : text; // The Candid text representation of the arguments. args_text : text; - // Optional cycles to forward to the target canister. - cycles_to_forward : opt nat; // The principal of the canister to call. target : principal; - // The amount of fee to charge. - fee_amount : nat; // Optional payment configuration (defaults to `AttachedCycles`). payment : opt PaymentType; }; type CallerPaysIcrc2Tokens = record { ledger : principal }; +type FeeDenom = variant { Icrc2 : CallerPaysIcrc2Tokens; Cycles }; +type FeeSpec = record { amount : nat; denom : FeeDenom }; +type MethodConfig = record { + fee : FeeSpec; + forward_cycles : opt nat; + supported : vec VendorPaymentConfig; +}; +type MethodKey = record { method : text; target : principal }; type PatronPaysIcrc2Tokens = record { ledger : principal; patron : Account }; // How a caller states that they will pay. type PaymentType = variant { @@ -62,6 +62,22 @@ type PaymentType = variant { PatronPaysIcrc2Cycles : Account; }; type Result = variant { Ok : blob; Err : text }; +type Result_1 = variant { Ok : opt MethodConfig; Err : text }; +type Result_2 = variant { Ok; Err : text }; +// Vendor payment configuration, including details that may not necessarily be shared with the customer. +type VendorPaymentConfig = variant { + // A patron pays tokens to a subaccount belonging to the vendor on the chosen ledger. + // - The vendor needs to move the tokens to their main account. + PatronPaysIcrc2Tokens : CallerPaysIcrc2Tokens; + // Cycles are received by the vendor canister. + AttachedCycles; + // Cycles are received by the vendor canister. + CallerPaysIcrc2Cycles; + // The caller pays tokens to the vendor's main account on the chosen ledger. + CallerPaysIcrc2Tokens : CallerPaysIcrc2Tokens; + // Cycles are received by the vendor canister. + PatronPaysIcrc2Cycles; +}; service : { // Proxies a call to a target method that takes **no arguments**. call0 : (Call0Args) -> (Result); @@ -69,4 +85,12 @@ service : { call_blob : (CallBlobArgs) -> (Result); // Proxies a call using **Candid text** (currently disabled). call_text : (CallTextArgs) -> (Result); + // Read the price configured for a `(target, method)` pair. + get_method_config : (MethodKey) -> (opt MethodConfig) query; + // List every configured `(target, method)` price. + list_method_configs : () -> (vec record { MethodKey; MethodConfig }) query; + // Remove the price for a `(target, method)` pair, returning any prior value. + remove_method_config : (MethodKey) -> (Result_1); + // Register or replace the price for a `(target, method)` pair. + set_method_config : (MethodKey, MethodConfig) -> (Result_2); } diff --git a/src/wrapper/src/api/call.rs b/src/wrapper/src/api/call.rs index e2b368d..caf2cc4 100644 --- a/src/wrapper/src/api/call.rs +++ b/src/wrapper/src/api/call.rs @@ -2,8 +2,9 @@ use candid::Principal; use ic_papi_api::PaymentType; use crate::domain::errors::BridgeError; -use crate::domain::types::BridgeCallArgs; +use crate::domain::types::{BridgeCallArgs, MethodKey}; use crate::payments::guard_config::PAYMENT_GUARD; +use crate::state; use crate::util::cycles::forward_raw; /// The IC management canister principal (`aaaaa-aa`). @@ -27,7 +28,13 @@ fn map_guard_err(e: E) -> String { BridgeError::GuardError(format!("{e:?}")).to_string() } -/// Internal helper to unify the bridge logic: charge fee -> forward call. +/// Internal helper to unify the bridge logic: look up price -> charge fee -> forward call. +/// +/// The fee and the number of cycles to forward are taken from the +/// operator-configured [`crate::domain::types::MethodConfig`] for the +/// `(target, method)` pair — never from the caller. This prevents a caller from +/// naming a trivial fee while forwarding a large amount, which would drain the +/// wrapper's own cycle balance. // TODO: The caller may have to provide more type information than they are used to. Normally dfx will use the target canister's candid file to convert to the correct types; without that information it will guess more simply and won't always get this conversion right. pub async fn bridge_call(args: BridgeCallArgs) -> Result, String> { if args.target == ic_cdk::api::canister_self() { @@ -43,16 +50,50 @@ pub async fn bridge_call(args: BridgeCallArgs) -> Result, String> { .to_string()); } - // 1) Charge fee using the payment guard + // Look up the operator-configured price for this `(target, method)`. + let key = MethodKey { + target: args.target, + method: args.method.clone(), + }; + let config = state::get_config(&key).ok_or_else(|| { + BridgeError::MethodNotConfigured { + target: args.target.to_string(), + method: args.method.clone(), + } + .to_string() + })?; + let p = args.payment.unwrap_or(PaymentType::AttachedCycles); + let cycles = config.forward_cycles.unwrap_or(0); + + // If this method forwards cycles, the caller must pay in cycles so that the + // fee actually credits the wrapper's cycle balance. `set_method_config` + // already guarantees `fee.denom == Cycles` and `fee.amount >= forward_cycles` + // for such methods; here we additionally ensure the *caller's chosen* payment + // type is cycle-denominated (token payments credit a token account, not cycles). + if cycles > 0 && !is_cycle_payment(&p) { + return Err(BridgeError::ForwardRequiresCyclePayment.to_string()); + } + + // 1) Charge the operator-set fee. PAYMENT_GUARD - .deduct(p, args.fee_amount) + .deduct(p, config.fee.amount) .await .map_err(map_guard_err)?; - // 2) Forward the call to target - let cycles = args.cycles_to_forward.unwrap_or(0); + // 2) Forward the call with the operator-set cycles. forward_raw(args.target, &args.method, args.args, cycles) .await .map_err(|e| BridgeError::TargetRejected(e).to_string()) } + +/// Whether a payment type credits this canister's *cycle* balance (as opposed to +/// a token ledger account), and can therefore fund forwarded cycles. +fn is_cycle_payment(payment: &PaymentType) -> bool { + matches!( + payment, + PaymentType::AttachedCycles + | PaymentType::CallerPaysIcrc2Cycles + | PaymentType::PatronPaysIcrc2Cycles(_) + ) +} diff --git a/src/wrapper/src/domain/errors.rs b/src/wrapper/src/domain/errors.rs index 782dccd..e972a42 100644 --- a/src/wrapper/src/domain/errors.rs +++ b/src/wrapper/src/domain/errors.rs @@ -1,6 +1,4 @@ /// Errors returned by the bridge canister. -/// -/// Kept small because the canister is stateless; pricing/governance are pushed to the caller. use std::fmt; #[derive(Debug)] @@ -11,6 +9,12 @@ pub enum BridgeError { TargetRejected(String), /// Fee deduction failed (insufficient cycles/allowance/etc.). GuardError(String), + /// No operator-configured price exists for the requested `(target, method)`. + MethodNotConfigured { target: String, method: String }, + /// The configured method forwards cycles, but the chosen payment type is not + /// cycle-denominated, so the forwarded cycles would come out of the wrapper's + /// own balance rather than being funded by the payment. + ForwardRequiresCyclePayment, /// The requested target canister may not be reached through the bridge. ForbiddenTarget(String), } @@ -21,6 +25,17 @@ impl fmt::Display for BridgeError { BridgeError::Candid(e) => write!(f, "Candid error: {e}"), BridgeError::TargetRejected(e) => write!(f, "Target canister rejected call: {e}"), BridgeError::GuardError(e) => write!(f, "Payment guard error: {e}"), + BridgeError::MethodNotConfigured { target, method } => write!( + f, + "No price is configured for method `{method}` on canister `{target}`. \ + The wrapper operator must register it first." + ), + BridgeError::ForwardRequiresCyclePayment => write!( + f, + "This method forwards cycles, which requires a cycle-denominated payment type \ + (AttachedCycles, CallerPaysIcrc2Cycles or PatronPaysIcrc2Cycles); \ + token payments do not credit the wrapper's cycle balance." + ), BridgeError::ForbiddenTarget(e) => write!(f, "Forbidden target: {e}"), } } diff --git a/src/wrapper/src/domain/types.rs b/src/wrapper/src/domain/types.rs index 00796f6..58475d0 100644 --- a/src/wrapper/src/domain/types.rs +++ b/src/wrapper/src/domain/types.rs @@ -24,25 +24,25 @@ pub struct MethodConfig { pub forward_cycles: Option, } -#[derive(Debug, CandidType, Deserialize, Clone, Eq, PartialEq)] +#[derive(Debug, CandidType, Deserialize, Clone, Eq, PartialEq, Hash)] pub struct MethodKey { pub target: Principal, pub method: String, } /// Arguments for the `call0` function. +/// +/// Note: the fee and the cycles to forward are **not** caller-supplied; they are +/// looked up from the operator-configured [`MethodConfig`] for `(target, method)`. +/// The caller only chooses which supported payment type to pay with. #[derive(Debug, CandidType, Deserialize, Clone, Eq, PartialEq)] pub struct Call0Args { /// The principal of the canister to call. pub target: Principal, /// The name of the method to call. pub method: String, - /// The amount of fee to charge. - pub fee_amount: u128, /// Optional payment configuration (defaults to `AttachedCycles`). pub payment: Option, - /// Optional cycles to forward to the target canister. - pub cycles_to_forward: Option, } /// Arguments for the `call_blob` function. @@ -54,12 +54,8 @@ pub struct CallBlobArgs { pub method: String, /// The Candid-encoded arguments as a byte buffer. pub args_blob: ByteBuf, - /// The amount of fee to charge. - pub fee_amount: u128, /// Optional payment configuration (defaults to `AttachedCycles`). pub payment: Option, - /// Optional cycles to forward to the target canister. - pub cycles_to_forward: Option, } /// Arguments for the `call_text` function. @@ -71,12 +67,8 @@ pub struct CallTextArgs { pub method: String, /// The Candid text representation of the arguments. pub args_text: String, - /// The amount of fee to charge. - pub fee_amount: u128, /// Optional payment configuration (defaults to `AttachedCycles`). pub payment: Option, - /// Optional cycles to forward to the target canister. - pub cycles_to_forward: Option, } /// Internal arguments for the bridge call logic. @@ -85,9 +77,7 @@ pub struct BridgeCallArgs { pub target: Principal, pub method: String, pub args: Vec, - pub fee_amount: u128, pub payment: Option, - pub cycles_to_forward: Option, } impl From for BridgeCallArgs { @@ -96,9 +86,7 @@ impl From for BridgeCallArgs { target: args.target, method: args.method, args: Encode!(&()).unwrap(), - fee_amount: args.fee_amount, payment: args.payment, - cycles_to_forward: args.cycles_to_forward, } } } @@ -109,9 +97,7 @@ impl From for BridgeCallArgs { target: args.target, method: args.method, args: args.args_blob.into_vec(), - fee_amount: args.fee_amount, payment: args.payment, - cycles_to_forward: args.cycles_to_forward, } } } @@ -123,9 +109,7 @@ impl From for BridgeCallArgs { method: args.method, // Note: args_text is not used yet as call_text is disabled args: vec![], - fee_amount: args.fee_amount, payment: args.payment, - cycles_to_forward: args.cycles_to_forward, } } } @@ -140,16 +124,12 @@ mod tests { let args = Call0Args { target: Principal::anonymous(), method: "test".to_string(), - fee_amount: 100, payment: Some(PaymentType::AttachedCycles), - cycles_to_forward: Some(50), }; let bridge_args: BridgeCallArgs = args.clone().into(); assert_eq!(bridge_args.target, args.target); assert_eq!(bridge_args.method, args.method); - assert_eq!(bridge_args.fee_amount, args.fee_amount); assert_eq!(bridge_args.payment, args.payment); - assert_eq!(bridge_args.cycles_to_forward, args.cycles_to_forward); // call0 should encode unit () assert_eq!(bridge_args.args, Encode!(&()).unwrap()); } @@ -161,16 +141,12 @@ mod tests { target: Principal::anonymous(), method: "test_blob".to_string(), args_blob: ByteBuf::from(blob.clone()), - fee_amount: 200, payment: None, - cycles_to_forward: None, }; let bridge_args: BridgeCallArgs = args.clone().into(); assert_eq!(bridge_args.target, args.target); assert_eq!(bridge_args.method, args.method); - assert_eq!(bridge_args.fee_amount, 200); assert_eq!(bridge_args.payment, None); - assert_eq!(bridge_args.cycles_to_forward, None); assert_eq!(bridge_args.args, blob); } @@ -180,16 +156,12 @@ mod tests { target: Principal::anonymous(), method: "test_text".to_string(), args_text: "(record { x = 42 })".to_string(), - fee_amount: 300, payment: None, - cycles_to_forward: None, }; let bridge_args: BridgeCallArgs = args.clone().into(); assert_eq!(bridge_args.target, args.target); assert_eq!(bridge_args.method, args.method); - assert_eq!(bridge_args.fee_amount, args.fee_amount); assert_eq!(bridge_args.payment, args.payment); - assert_eq!(bridge_args.cycles_to_forward, args.cycles_to_forward); // call_text currently does not use args_text, so args should be empty assert_eq!(bridge_args.args, Vec::::new()); } diff --git a/src/wrapper/src/lib.rs b/src/wrapper/src/lib.rs index 31015d7..5600572 100644 --- a/src/wrapper/src/lib.rs +++ b/src/wrapper/src/lib.rs @@ -1,13 +1,17 @@ +use ic_cdk::api::{is_controller, msg_caller}; use ic_cdk::export_candid; -use ic_cdk::update; +use ic_cdk::{post_upgrade, pre_upgrade, query, update}; pub mod api; pub mod domain; pub mod payments; +pub mod state; pub mod util; use crate::api::call::bridge_call; -use crate::domain::types::{BridgeCallArgs, Call0Args, CallBlobArgs, CallTextArgs}; +use crate::domain::types::{ + BridgeCallArgs, Call0Args, CallBlobArgs, CallTextArgs, FeeDenom, MethodConfig, MethodKey, +}; /// Proxies a call to a target method that takes **no arguments**. #[update] @@ -30,4 +34,150 @@ pub fn call_text(args: CallTextArgs) -> Result, String> { Err("call_text is currently disabled due to a workspace dependency conflict with the Candid parser. Please use call_blob instead.".to_string()) } +// -------------------------------------------------------------------------- +// Operator configuration (controller-only) +// +// Pricing is server-side: the operator registers, per `(target, method)`, the +// fee to charge and the cycles to forward. Callers can never set these, so they +// cannot make the wrapper forward more cycles than it is paid. +// -------------------------------------------------------------------------- + +fn ensure_controller() -> Result<(), String> { + if is_controller(&msg_caller()) { + Ok(()) + } else { + Err("Only a canister controller may change the wrapper configuration.".to_string()) + } +} + +/// Reject configurations that would let the wrapper forward more cycles than the +/// fee funds. When a method forwards cycles, the fee must be denominated in +/// cycles and be at least the forwarded amount. +fn validate_config(config: &MethodConfig) -> Result<(), String> { + if let Some(forward) = config.forward_cycles { + if forward > 0 { + if config.fee.denom != FeeDenom::Cycles { + return Err( + "A method that forwards cycles must charge its fee in cycles (fee.denom = Cycles)." + .to_string(), + ); + } + if config.fee.amount < forward { + return Err(format!( + "The fee ({}) must cover the cycles to forward ({forward}).", + config.fee.amount + )); + } + } + } + Ok(()) +} + +/// Register or replace the price for a `(target, method)` pair. +#[update] +pub fn set_method_config(key: MethodKey, config: MethodConfig) -> Result<(), String> { + ensure_controller()?; + validate_config(&config)?; + state::set_config(key, config); + Ok(()) +} + +/// Remove the price for a `(target, method)` pair, returning any prior value. +#[update] +#[allow(clippy::needless_pass_by_value)] +pub fn remove_method_config(key: MethodKey) -> Result, String> { + ensure_controller()?; + Ok(state::remove_config(&key)) +} + +/// Read the price configured for a `(target, method)` pair. +#[query] +#[must_use] +#[allow(clippy::needless_pass_by_value)] +pub fn get_method_config(key: MethodKey) -> Option { + state::get_config(&key) +} + +/// List every configured `(target, method)` price. +#[query] +#[must_use] +pub fn list_method_configs() -> Vec<(MethodKey, MethodConfig)> { + state::list_configs() +} + +// -------------------------------------------------------------------------- +// Upgrade persistence +// -------------------------------------------------------------------------- + +#[pre_upgrade] +fn pre_upgrade() { + let configs = state::list_configs(); + ic_cdk::storage::stable_save((configs,)).expect("Failed to persist method configs on upgrade"); +} + +#[post_upgrade] +fn post_upgrade() { + match ic_cdk::storage::stable_restore::<(Vec<(MethodKey, MethodConfig)>,)>() { + Ok((configs,)) => state::replace_all(configs), + // Do not trap: trapping in `post_upgrade` would make the canister + // permanently un-upgradable. But a silent failure would bring the + // wrapper up with an empty registry, causing every call to fail with + // "No price is configured" and no explanation. Log loudly so operators + // can diagnose the lost configuration. + Err(err) => { + ic_cdk::println!( + "post_upgrade: failed to restore method configs from stable memory, \ + starting with an EMPTY registry. All calls will fail until reconfigured. Error: {err}" + ); + } + } +} + export_candid!(); + +#[cfg(test)] +mod tests { + use super::*; + use crate::domain::types::{FeeSpec, MethodConfig}; + + fn config(fee_amount: u128, denom: FeeDenom, forward: Option) -> MethodConfig { + MethodConfig { + fee: FeeSpec { + amount: fee_amount, + denom, + }, + supported: vec![], + forward_cycles: forward, + } + } + + #[test] + fn accepts_config_without_forwarding() { + // Any fee denomination is fine when no cycles are forwarded. + assert!(validate_config(&config(0, FeeDenom::Cycles, None)).is_ok()); + assert!(validate_config(&config(5, FeeDenom::Cycles, Some(0))).is_ok()); + let ledger = candid::Principal::anonymous(); + assert!(validate_config(&config(5, FeeDenom::Icrc2 { ledger }, None)).is_ok()); + } + + #[test] + fn accepts_forwarding_covered_by_cycle_fee() { + assert!(validate_config(&config(1000, FeeDenom::Cycles, Some(1000))).is_ok()); + assert!(validate_config(&config(2000, FeeDenom::Cycles, Some(1000))).is_ok()); + } + + #[test] + fn rejects_forwarding_with_token_fee() { + let ledger = candid::Principal::anonymous(); + let err = validate_config(&config(1000, FeeDenom::Icrc2 { ledger }, Some(1000))) + .expect_err("token-denominated fee cannot fund forwarded cycles"); + assert!(err.contains("cycles"), "unexpected error: {err}"); + } + + #[test] + fn rejects_forwarding_exceeding_fee() { + let err = validate_config(&config(999, FeeDenom::Cycles, Some(1000))) + .expect_err("fee below forwarded amount must be rejected"); + assert!(err.contains("cover"), "unexpected error: {err}"); + } +} diff --git a/src/wrapper/src/state.rs b/src/wrapper/src/state.rs new file mode 100644 index 0000000..373c110 --- /dev/null +++ b/src/wrapper/src/state.rs @@ -0,0 +1,57 @@ +//! Operator-controlled configuration state for the wrapper. +//! +//! The wrapper prices each proxied `(target, method)` server-side via a +//! [`MethodConfig`]. Only the wrapper's controllers may change this +//! configuration; callers can never set their own fee or forwarded-cycle amount. +//! +//! State is held on the heap and persisted across upgrades in stable memory via +//! the `pre_upgrade`/`post_upgrade` hooks in `lib.rs`. + +use crate::domain::types::{MethodConfig, MethodKey}; +use std::cell::RefCell; +use std::collections::HashMap; + +thread_local! { + static CONFIGS: RefCell> = RefCell::new(HashMap::new()); +} + +/// Look up the operator configuration for a `(target, method)` pair. +#[must_use] +pub fn get_config(key: &MethodKey) -> Option { + CONFIGS.with(|c| c.borrow().get(key).cloned()) +} + +/// Insert or replace the configuration for a `(target, method)` pair. +pub fn set_config(key: MethodKey, config: MethodConfig) { + CONFIGS.with(|c| { + c.borrow_mut().insert(key, config); + }); +} + +/// Remove the configuration for a `(target, method)` pair, returning any prior value. +#[must_use] +pub fn remove_config(key: &MethodKey) -> Option { + CONFIGS.with(|c| c.borrow_mut().remove(key)) +} + +/// Snapshot of all configured `(target, method)` prices. +#[must_use] +pub fn list_configs() -> Vec<(MethodKey, MethodConfig)> { + CONFIGS.with(|c| { + c.borrow() + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect() + }) +} + +/// Replace the whole registry (used when restoring after an upgrade). +pub fn replace_all(items: Vec<(MethodKey, MethodConfig)>) { + CONFIGS.with(|c| { + let mut map = c.borrow_mut(); + map.clear(); + for (k, v) in items { + map.insert(k, v); + } + }); +} diff --git a/src/wrapper/tests/it/bridge_tests.rs b/src/wrapper/tests/it/bridge_tests.rs index a4709cc..ee7165f 100644 --- a/src/wrapper/tests/it/bridge_tests.rs +++ b/src/wrapper/tests/it/bridge_tests.rs @@ -1,8 +1,8 @@ use crate::util::pic_canister::PicCanisterTrait; use crate::util::test_environment::TestSetup; -use candid::Principal; +use candid::{decode_one, encode_args, Principal}; use ic_papi_api::PaymentType; -use ic_papi_wrapper::domain::types::Call0Args; +use ic_papi_wrapper::domain::types::{Call0Args, FeeDenom, FeeSpec, MethodConfig, MethodKey}; #[test] fn bridge_call_fails_if_target_is_self() { @@ -10,9 +10,7 @@ fn bridge_call_fails_if_target_is_self() { let args = Call0Args { target: setup.wrapper.canister_id(), method: "any".to_string(), - fee_amount: 0, payment: Some(PaymentType::AttachedCycles), - cycles_to_forward: None, }; let result: Result, String>, String> = @@ -24,13 +22,14 @@ fn bridge_call_fails_if_target_is_self() { #[test] fn bridge_call_fails_if_target_is_management_canister() { + // The management canister is a forbidden target: proxying to it would run + // with the bridge's own principal as caller. This is rejected before any + // pricing lookup, so no configuration is needed for the test. let setup = TestSetup::default(); let args = Call0Args { target: Principal::management_canister(), method: "update_settings".to_string(), - fee_amount: 0, payment: Some(PaymentType::AttachedCycles), - cycles_to_forward: None, }; let result: Result, String>, String> = @@ -41,20 +40,55 @@ fn bridge_call_fails_if_target_is_management_canister() { } #[test] -fn bridge_call_fails_if_insufficient_cycles() { +fn bridge_call_fails_if_method_not_configured() { + // With operator-controlled pricing, a call to a `(target, method)` that the + // operator has not registered must be rejected outright -- there is no + // caller-supplied fee to fall back on. let setup = TestSetup::default(); let args = Call0Args { target: setup.target.canister_id(), - method: "any_method".to_string(), - fee_amount: 1000, + method: "unconfigured_method".to_string(), payment: Some(PaymentType::AttachedCycles), - cycles_to_forward: None, }; let result: Result, String>, String> = setup.wrapper.update(setup.user, "call0", args); let inner_result = result.expect("Failed to reach canister"); - let err = inner_result.expect_err("Should have failed due to insufficient cycles"); - // Since we didn't attach cycles, it should fail with Payment guard error: Insufficient funds... - assert!(err.contains("Payment guard error")); + let err = inner_result.expect_err("Should have failed: method not configured"); + assert!( + err.contains("No price is configured"), + "unexpected error: {err}" + ); +} + +#[test] +fn set_method_config_requires_controller() { + // Pricing is operator-only: a non-controller caller must not be able to + // register or change a method's price (and thus its forwarded cycles). + let setup = TestSetup::default(); + let key = MethodKey { + target: setup.target.canister_id(), + method: "any".to_string(), + }; + let config = MethodConfig { + fee: FeeSpec { + amount: 0, + denom: FeeDenom::Cycles, + }, + supported: vec![], + forward_cycles: None, + }; + + let bytes = setup + .pic + .update_call( + setup.wrapper.canister_id(), + setup.user, // not a controller of the wrapper + "set_method_config", + encode_args((key, config)).unwrap(), + ) + .expect("Failed to reach canister"); + let res: Result<(), String> = decode_one(&bytes).unwrap(); + let err = res.expect_err("A non-controller must not be able to set config"); + assert!(err.contains("controller"), "unexpected error: {err}"); }