galacticcouncil / galacticcouncil/hydration-node

Improve circuit breaker with a global deposit (ingress) limit

Open
#1,502 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
208
Forks
109
Avg merge
6d 3h
Merged PRs (30d)
3

Description

### Detailed Description

Add a **global deposit/ingress limit** to `pallet-circuit-breaker`.

Hydration currently has two related protections:

- a per-asset issuance-increase fuse (`IssuanceIncreaseFuse`) backed by each asset's `xcm_rate_limit`; and
- a global withdrawal/egress limit, which values participating assets in HDX and maintains a decaying accumulator.

There is no chain-wide cap on aggregate inbound issuance. An invalid or compromised bridge/minter can therefore spread deposits across several assets, or use an asset whose per-asset rate limit is missing/misconfigured, without hitting a single aggregate exposure budget.

The new limit should cap aggregate cross-chain ingress, expressed in HDX reference units, across every participating asset and ingress path. It should be an additional layer: it must not replace the per-asset issuance fuse.

#### Required semantics

- Maintain an **independent gross-ingress accumulator** with its own governance-configurable `limit` and `window`.
- A participating deposit succeeds only when the decayed accumulator plus the new ingress remains below the configured limit, matching the boundary convention used by the current global withdrawal limit.
- The accumulator decays using the same time source and decay semantics as the existing withdrawal accumulator.
- `None` (and, for compatibility with the withdrawal limiter, a zero window) disables the limit.
- Opposite-direction egress must **not** reduce the deposit accumulator. Otherwise an attacker could repeatedly deposit one asset, exchange it, withdraw another asset, and recreate deposit capacity.
- The current behavior in which genuine ingress can credit the withdrawal accumulator may remain for backward compatibility, but the deposit accumulator itself must measure gross ingress.
- Internal balance movements, fee withdrawals/refunds, and other explicitly ignored system operations must not consume or create limit capacity.
- A participating asset that cannot be converted to HDX must not silently bypass the deposit limit. The operation must fail closed before the deposited balance becomes spendable, or the funds must be placed in a non-spendable quarantine/trap flow.

#### What counts as ingress

Use the existing `GlobalAssetCategory` and overrides as the source of participation:

- `External`: issuance/deposit representing assets entering Hydration, including XCM and authorized NTT minting.
- `Local`: assets returning from an egress account/boundary.
- `None`: ordinary internal assets and balance movements are not counted.

Only a real boundary crossing should be counted. A transfer between normal Hydration accounts is not ingress.

#### Atomicity and failure behavior

The current `OnDepositHook` and `OnTransferHook` are invoked after the underlying currency mutation in several `pallet-currencies` paths. The new limiter must not simply return `GlobalDepositLimitExceeded` from a post-mutation hook and assume every caller is transactional.

Before implementation, define and enforce one of these safe contracts for every covered path:

1. preflight the projected ingress before mutating balances/issuance; or
2. execute the mutation and accounting in a storage transaction that is guaranteed to roll back on an accounting error.

On rejection:

- no newly deposited/minted balance may remain spendable;
- the deposit accumulator must remain unchanged;
- unrelated withdrawal state must remain unchanged; and
- XCM processing must have an explicit trap/quarantine outcome rather than leaving partial accounting.

This must be coordinated with #1150, which tracks the existing operational difficulty of recovering XCM assets trapped when a deposit limit rejects an intermediate deposit.

#### XCM accounting

Extend/replace `XcmEgressBuffer`; adding a second total to the current HDX-wide tuple is not sufficient.

Within one XCM execution, temporary withdrawals and refunds should not be treated as new cross-chain flow. Net **per asset** first, then aggregate the positive sides in HDX:

```text
asset_net_ingress = max(asset_deposited - asset_withdrawn, 0)
asset_net_egress = max(asset_withdrawn - asset_deposited, 0)

message_ingress = Σ convert_to_hdx(asset_net_ingress)
message_egress = Σ convert_to_hdx(asset_net_egress)
```

Do not net all assets into one signed HDX total. Cross-asset netting would allow an ingress of asset A and an egress of asset B to cancel even though both risk budgets should be consumed.

The XCM path must:

- include already buffered ingress when prechecking subsequent deposits in the same message;
- use checked arithmetic rather than saturating away overflow;
- account successful balance changes even when a later XCM instruction fails (partial execution);
- avoid double-counting `deposit_asset` plus the currency mutation hook;
- cover queued XCMP/DMP processing and locally executed XCM; and
- prove that an over-limit deposit does not leave a spendable minted balance.

### Context

The per-asset issuance fuse and a global ingress limit protect against different failure modes:

- the per-asset fuse bounds one asset and can reserve/lock excess issuance;
- the global limit bounds aggregate HDX-equivalent exposure across all participating assets;
- the global limit provides defense in depth when several assets are attacked together or one asset lacks a correct `xcm_rate_limit`.

Relevant current code:

- `pallets/circuit-breaker/src/lib.rs`
- `GlobalWithdrawLimitConfig`
- `WithdrawLimitAccumulator`
- `note_egress`
- `note_deposit`
- `ensure_xcm_withdraw_can_proceed`
- `XcmEgressBuffer`
- `pallets/circuit-breaker/src/fuses/issuance.rs`
- `runtime/hydradx/src/circuit_breaker.rs`
- asset categorization and HDX conversion
- withdraw/deposit/transfer hooks
- `runtime/hydradx/src/xcm.rs`
- `ProcessXcmWithBreaker`
- `LocalAssetTransactor`
- `pallets/currencies/src/lib.rs` and `pallets/currencies/src/fungibles.rs`
- `runtime/hydradx/src/evm/precompiles/multicurrency.rs` (NTT mint path)

Related issues/changes:

- #1255 / #1331 introduced the global withdrawal limit.
- #1150 covers recovery of assets trapped when the existing deposit fuse rejects an XCM intermediate-account deposit.

### Possible Implementation

Prefer an additive implementation that preserves the existing withdrawal storage:

- generalize `GlobalWithdrawLimitParameters` to a reusable `GlobalLimitParameters`, or introduce an equivalent deposit parameter type;
- add `GlobalDepositLimitConfig`;
- add `DepositLimitAccumulator: (Balance, Moment)`;
- add `DepositLockdownUntil` only if manual ingress lockdown is required for operational parity;
- add `note_ingress`/`ensure_ingress_can_proceed` helpers;
- update the flow buffer to retain asset identity with an explicit bound;
- append new call indices for configuration/reset/lockdown calls without renumbering existing extrinsics;
- add deposit-specific events and errors (`DepositLimitConfigUpdated`, `GlobalDepositLimitExceeded`, reset/lockdown events as applicable); and
- rename `AssetWithdrawHandler`, `WithdrawFuseControl`, and `IgnoreWithdrawLimit` to direction-neutral equivalents if they now govern both ingress and egress.

The ignore mechanism must cover both directions and restore its prior value on every success/error path. A scoped/transaction-safe guard is preferable to manually writing a global boolean before and after fallible fee operations.

Governance operations should support:

- setting/updating the deposit limit and window;
- resetting only the deposit accumulator;
- disabling the limiter without corrupting accumulated state; and
- optionally applying/lifting a manual deposit lockdown.

Use the existing `AuthorityOrigin` unless there is a reason to separate deposit-risk governance.

#### Acceptance criteria

- [ ] Aggregate ingress across two or more participating assets is limited in HDX reference units.
- [ ] An asset with no configured per-asset issuance limit is still covered when its global category participates.
- [ ] Egress does not replenish the deposit accumulator.
- [ ] Same-asset XCM staging/refunds are netted once, while cross-asset ingress and egress consume their respective limits.
- [ ] Over-limit direct deposits/mints/transfers are atomic: balances, issuance, events, and both accumulators are unchanged.
- [ ] Over-limit XCM ingress leaves no spendable balance and has a tested trap/quarantine/recovery outcome.
- [ ] NTT `mint` reverts cleanly when the global deposit limit is reached and can be retried after capacity becomes available.
- [ ] Unpriced participating ingress cannot bypass accounting.
- [ ] Fee charging/refunds and explicitly ignored internal operations affect neither accumulator, including when a fallible operation returns early.
- [ ] Disabled configuration, zero window, exact-limit boundary, decay, overflow, reset, and optional lockdown behavior are covered by unit tests.
- [ ] Queued XCM, `polkadot_xcm::execute`, `pallet-currencies` `MultiCurrency`, fungibles `Mutate`, and NTT precompile paths have integration coverage.
- [ ] Existing global withdrawal-limit behavior and tests remain green.
- [ ] New dispatchables are benchmarked; generated runtime weights, crate/runtime versions, and any required storage migration are included.

Contributor guide

Open the contributing guide

Research direction

Start by reading the existing global withdrawal implementation in pallets/circuit-breaker/src/lib.rs, then trace the hooks and XCM paths in runtime/hydradx/src/circuit_breaker.rs, runtime/hydradx/src/xcm.rs, and pallets/currencies. Run the existing circuit-breaker and XCM tests before changing behavior. Done means the acceptance criteria are covered across direct, NTT, queued XCM, and local XCM paths without regressing withdrawal-limit tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend-api-design, blockchain, security, testing
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.