feat(wrapper): operator-controlled pricing via MethodConfig registry#232
Merged
Conversation
Move fee and forwarded-cycle amounts from caller-supplied arguments to a server-side, controller-managed registry keyed by (target, method). Callers now choose only the payment type; they can no longer pay a trivial fee while forwarding a large amount to drain the wrapper's cycle balance. - Registry in state.rs with controller-gated set/remove and query reads, persisted across upgrades. - set_method_config validates that a cycle-forwarding method charges its fee in cycles and covers the forwarded amount. - bridge_call looks up the config, rejects unconfigured methods, and requires a cycle-denominated payment when forwarding cycles. - Drops fee_amount/cycles_to_forward from Call0Args/CallBlobArgs/CallTextArgs; regenerated .did and updated README.
|
✅ No security or compliance issues detected. Reviewed everything up to 41ea101. Security Overview
Detected Code Changes
|
There was a problem hiding this comment.
Pull request overview
This PR addresses the wrapper “cycle-drain” vulnerability by moving fee and forwarded-cycles selection from caller-controlled inputs to an operator-controlled, persisted (target, method) -> MethodConfig registry, and updating the wrapper’s public API, Candid interface, documentation, and tests accordingly.
Changes:
- Introduces a heap-backed
MethodConfigregistry persisted across upgrades (pre_upgrade/post_upgrade) and exposes controller-gated admin/query APIs to manage it. - Updates
bridge_callto require a configured(target, method), charge the operator-set fee, and forward operator-set cycles (with a guard requiring cycle-denominated payment when forwarding). - Removes caller-supplied
fee_amount/cycles_to_forwardfrom the publiccall*APIs and regenerates the.didplus README updates and integration tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/wrapper/tests/it/bridge_tests.rs | Updates integration tests for operator-priced flow (unconfigured method + controller-gated config). |
| src/wrapper/src/state.rs | Adds in-canister registry storage for method pricing configs. |
| src/wrapper/src/lib.rs | Adds controller-only config endpoints, config validation, and upgrade persistence hooks. |
| src/wrapper/src/domain/types.rs | Updates argument types to remove caller-priced fields; adds hashing for MethodKey. |
| src/wrapper/src/domain/errors.rs | Adds errors for “method not configured” and “forward requires cycle payment”. |
| src/wrapper/src/api/call.rs | Switches bridge_call to registry-based pricing and configured forwarded cycles. |
| src/wrapper/ic_papi_wrapper.did | Regenerates the public Candid interface for the breaking API + new admin endpoints. |
| README.md | Updates usage/docs to reflect operator-controlled pricing and new configuration flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…re, fix README - Add #[must_use] to state query fns and controller query endpoints - Allow needless_pass_by_value on candid endpoints taking owned MethodKey - post_upgrade: log (not swallow) stable-restore failures so an empty registry after a bad upgrade is diagnosable - README: use CallerPaysIcrc2Tokens (not a cycles variant) in the ckUSDC example; note that MethodConfig.supported is not yet enforced
AntonioVentilii
enabled auto-merge (squash)
July 15, 2026 11:22
DenysKarmazynDFINITY
approved these changes
Jul 15, 2026
…fig-pricing # Conflicts: # src/wrapper/src/api/call.rs # src/wrapper/src/domain/errors.rs # src/wrapper/tests/it/bridge_tests.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves fee and forwarded-cycle amounts out of caller arguments and into an operator-controlled, per-
(target, method)registry. Callers choose only which payment type to use; they can no longer set what the wrapper charges or forwards.Problem
bridge_callcharged a caller-suppliedfee_amountand, independently, forwarded a caller-suppliedcycles_to_forwardfrom the wrapper's own cycle balance. The two were unrelated, so a caller could pay a trivial (or zero) fee while forwarding a large amount and drain the wrapper. Ingress callers can't attach cycles at all, so they paid nothing.That drain is the sharp edge of a deeper mismatch: PAPI is a payment gateway where the API operator sets the price. Caller-supplied
fee_amount/cycles_to_forwardlet the customer name their own price, which is wrong regardless of the drain.Behavior
state.rs):(target, method) → MethodConfig { fee, supported, forward_cycles }, persisted across upgrades viapre_upgrade/post_upgrade.is_controllerguard):set_method_config/remove_method_config; plusget_method_config/list_method_configsqueries.validate_config): a method that forwards cycles must denominate its fee in cycles and set it>=the forwarded amount. A draining config cannot be stored.bridge_call: looks up the config, rejects unconfigured(target, method), takes the fee and forward amount from the config, and requires a cycle-denominated payment type when forwarding (token payments credit a token account, not the cycle balance).The caller can no longer influence what the wrapper pays out, so the drain is closed structurally rather than by a numeric check. This also completes the existing
MethodConfig { fee, forward_cycles }scaffolding, which was defined but never wired in.Breaking change
Callers:
fee_amountandcycles_to_forwardare removed fromCall0Args/CallBlobArgs/CallTextArgs. Callers sendtarget,method, args, andpaymentonly.(target, method)that hasn't been registered is rejected withNo price is configured.Operators:
(target, method)must be registered viaset_method_configbefore it is callable.ic_papi_wrapper.didis regenerated and the README updated to match.Tests
validate_config): accepts no-forward and covered-forward configs; rejects a token-denominated fee with forwarding; rejects a fee below the forwarded amount.set_method_configrejected.Known gaps
MethodConfig.supportedis stored but not yet enforced per-method inbridge_call(payment-type support still comes from the globalPAYMENT_GUARD).stable_save/stable_restore;ic-stable-structureswould scale better if config counts grow large.