galacticcouncil / galacticcouncil/hydration-node
Add support for multiple AAVE Money Market instances to Runtime API
- Dominant language
- Rust
- Stars
- 208
- Forks
- 109
- Avg merge
- 6d 3h
- Merged PRs (30d)
- 3
Description
## Summary
The `AaveTradeExecutor` runtime API currently reports pools from a single Aave Money Market instance (`pallet_liquidation::BorrowingContract`). A second instance has been introduced (`GigaHdxPoolContract`), and more are expected. This spec extends `pairs()`, `pool()`, `pools()`, and `liquidity_depth()` to cover every active instance, plus extends internal supply/withdraw/routing to target the correct instance for each asset.
**Key invariant (per product plan):** each reserve asset lives in exactly one Aave MM instance. The main `BorrowingContract` hosts multiple reserves; each additional instance (`GigaHdxPoolContract`, future ones) hosts exactly one reserve. Consequence: `(reserve, atoken)` → `pool_address` is a function — no disambiguator is needed in the public API.
## Motivation
- `runtime/hydradx/src/lib.rs:1158-1191` hard-codes `BorrowingContract::get()` as the only pool in the Aave runtime API.
- `GigaHdxPoolContract` is already used internally by `AaveMoneyMarket` (`runtime/hydradx/src/gigahdx.rs:45,64`) and by `liquidate_gigahdx` (`pallets/liquidation/src/lib.rs:499`), and it holds its own reserve/aToken — invisible to consumers of the runtime API today.
- Without this change, every new Aave instance requires additional out-of-band wiring to be visible to clients and is unreachable through the trade-executor path.
**Users / actors:** off-chain indexers, swap/routing UIs, analytics dashboards, integrators consuming the `AaveTradeExecutor` runtime API.
## Existing building blocks (already in-tree)
Most per-pool plumbing already exists — only the *aggregation* layer is missing:
| Primitive | Location | Pool-parametric? |
|---|---|---|
| `AaveTradeExecutor::get_reserves_list(pool)` | `aave_trade_executor.rs:157` | ✅ |
| `AaveTradeExecutor::get_reserve_data(pool, asset)` | `aave_trade_executor.rs:196` | ✅ |
| `AaveTradeExecutor::do_supply_on_behalf_of(pool, ...)` | `aave_trade_executor.rs:306` | ✅ |
| `AaveTradeExecutor::do_withdraw(pool, ...)` | `aave_trade_executor.rs:346` | ✅ |
| `AaveTradeExecutor::do_set_use_reserve_as_collateral(pool, ...)` | `aave_trade_executor.rs:330` | ✅ |
**Still hardcoded to `BorrowingContract::::get()`** (the actual work):
- `AaveTradeExecutor::supply()` — `aave_trade_executor.rs:303`
- `AaveTradeExecutor::withdraw()` — `aave_trade_executor.rs:367`
- `AaveTradeExecutor::do_withdraw_all_to()` — `aave_trade_executor.rs:374`
- `TradeExecution::get_liquidity_depth` — `aave_trade_executor.rs:566`
- Runtime API resolver `pairs()` / `pool()` / `pools()` / `liquidity_depth()` — `lib.rs:1158-1191`
- `precompiles/flash-loan/src/lib.rs:129`
**Out of scope:** `node/src/liquidation_worker.rs` runs against a single `PAP_CONTRACT` (line 38) and a single `MoneyMarketData` (`liquidation-worker-support/src/lib.rs:946`). Multi-instance worker support is a separate follow-up.
## Design
Because of the one-reserve-per-non-main-instance invariant, **the public runtime API shape does not change**. `pairs()` / `pool(reserve, atoken)` / `pools()` / `liquidity_depth(asset_in, asset_out)` keep their current signatures and `PoolData` keeps its current fields. Only the resolver and the trade-executor internals change.
Two options differ only in *where* the list of pool contracts lives.
---
### Option A — Light (no storage change)
**Idea:** A runtime-level helper returns the list of known pool contracts. Resolver and trade-executor internals iterate it.
**Changes:**
1. Add a runtime helper in `runtime/hydradx/src/lib.rs` near the API impl:
```rust
fn aave_pool_contracts() -> Vec {
let mut pools = vec![pallet_liquidation::BorrowingContract::::get()];
let giga = pallet_liquidation::GigaHdxPoolContract::::get();
if giga != pools[0] { // dedupe — see edge cases
pools.push(giga);
}
pools
}
```
2. Rewrite `pairs()` (`lib.rs:1159-1173`) to flat-map over `aave_pool_contracts()`.
3. Rewrite `pool(reserve, atoken)` and `liquidity_depth(asset_in, asset_out)` to find the hosting pool via an internal helper (see point 4) before calling through to `AaveTradeExecutor` logic.
4. Add an internal helper on `AaveTradeExecutor`:
```rust
fn find_pool_for_asset(asset: EvmAddress) -> Option
```
Iterates `aave_pool_contracts()`, calls `get_reserves_list(pool)`, returns the first pool whose reserves include `asset`. This is the `(reserve) → pool_address` lookup that the invariant makes unambiguous.
5. Update the remaining hardcoded callers to use `find_pool_for_asset` instead of `BorrowingContract::::get()`:
- `supply()`, `withdraw()`, `do_withdraw_all_to()`
- `TradeExecution::get_liquidity_depth`
- (`precompiles/flash-loan/src/lib.rs:129` — either route through the helper or keep pinned to `BorrowingContract` if flash-loans are main-pool-only; call out in review.)
6. In `do_sell` / `execute_sell` / `execute_buy`, the asset address on the supply or withdraw side determines the pool via `find_pool_for_asset`.
**Pros**
- No storage migration, no new extrinsic, no `spec_version` bump beyond a possible minor runtime-API version bump.
- Touches only the runtime crate plus `AaveTradeExecutor` internals.
- Public runtime API unchanged — no indexer/UI breakage.
**Cons**
- Adding another instance requires a runtime release.
- Knowledge of "which Aave instances exist" is split between `pallet_liquidation` storage and a runtime-level list.
- `find_pool_for_asset` is O(instances × reserves) per call; acceptable given small bounds but worth caching if hot.
**Migration:** none. API shape unchanged.
---
### Option B — Heavier (storage-backed registry)
**Idea:** Make `pallet_liquidation` the single source of truth for Aave pool contracts.
**Changes:**
1. In `pallets/liquidation/src/lib.rs`, add:
```rust
#[pallet::storage]
pub type AavePoolContracts =
StorageValue<_, BoundedVec, ValueQuery>;
```
plus a `MaxAavePoolContracts` config constant (start at e.g. 8).
2. Add `AuthorityOrigin`-gated extrinsics `add_aave_pool_contract(EvmAddress)` / `remove_aave_pool_contract(EvmAddress)` with events `AavePoolContractAdded` / `AavePoolContractRemoved`.
3. Storage migration: on upgrade, seed `AavePoolContracts` with the distinct values of `[BorrowingContract::get(), GigaHdxPoolContract::get()]`.
4. Deprecate `BorrowingContract` / `GigaHdxPoolContract` as primary storage — either keep them as read-through aliases over the new vec, or migrate all call sites and remove them.
5. Resolver + trade-executor changes are identical to Option A points 2–6, except `aave_pool_contracts()` reads from `AavePoolContracts::get()`.
**Pros**
- Adding a new instance is a single extrinsic — no runtime upgrade.
- Single, on-chain, governance-controlled registry.
- Eliminates the split of truth between pallet and runtime.
**Cons**
- Storage migration + `spec_version` bump.
- More review surface: extrinsic, origin check, bounded bounds, migration correctness.
- All existing consumers of `BorrowingContract` / `GigaHdxPoolContract` must be migrated or shimmed.
**Migration:** one-shot `OnRuntimeUpgrade` seeds the new `BoundedVec`. Existing storage either becomes a view or is removed in a follow-up.
**API compatibility:** runtime API shape unchanged in both options.
---
### Interactions
- `pallet_liquidation` — storage source in both options (read-only in A, new writes in B).
- `runtime/hydradx/src/gigahdx.rs` (`AaveMoneyMarket`) — continues reading `GigaHdxPoolContract` for its fixed role; unaffected in A, rerouted in B if aliases are removed.
- `precompiles/flash-loan/src/lib.rs:129` — still reads `BorrowingContract`; decision in review whether to route through `find_pool_for_asset` or keep pinned.
- `HydraErc20Mapping::address_to_asset` — unchanged.
- `node/src/liquidation_worker.rs` and `MoneyMarketData` — out of scope.
## Constraints
- **Default-address collision.** `DefaultBorrowingContract` and `DefaultGigaHdxPoolContract` both default to `0x1b02E051683b5cfaC5929C25E84adb26ECf87B38` (`pallets/liquidation/src/lib.rs:162,172`). On any chain where the GigaHdx contract hasn't been explicitly set, the registry / helper must **dedupe by pool address** or `pairs()` double-counts. Same rule applies to Option B's migration seeding.
- **Security (Option B).** `add_aave_pool_contract` / `remove_aave_pool_contract` must be restricted to a governance or technical-committee origin.
- **Weight.** `pools()` and `find_pool_for_asset` are O(instances × reserves). Today ≤ 2 instances × handful of reserves — trivial. Keep `MaxAavePoolContracts` ≤ 8.
- **Governance.** Option B's setters are governable; Option A hard-codes the list.
## Edge Cases
- **Invariant breach — asset appears on two instances.** Product invariant says it won't happen, but if it does (governance misconfiguration), `find_pool_for_asset` must behave deterministically: return the first match (iteration order: `BorrowingContract` first, then additional instances) and log a warning. Do not panic, do not return multiple pools.
- **Two instances configured to the same contract address.** Dedupe by pool address before iterating (see default-address collision). Otherwise `pairs()` returns every pair twice on a fresh chain.
- **Empty / zero pool contract.** Skip zero-address entries. Current `pairs()` already swallows `get_reserves_list` errors to `vec![]` — preserve that per-instance.
- **Partial failures.** A failing `get_reserves_list` or `get_reserve_data` on one instance must not poison results from the others.
- **Flash-loan caller.** `precompiles/flash-loan/src/lib.rs:129` is pinned to `BorrowingContract`. Confirm in review whether flash loans are main-pool-only (leave as-is) or should resolve per asset (route through helper).
## Acceptance Criteria
- [ ] `pairs()` returns reserves/aTokens from both `BorrowingContract` and `GigaHdxPoolContract` instances (and any future registered instance in Option B).
- [ ] `pools()` returns one `PoolData` per `(reserve, atoken)` across all instances, with no duplicates.
- [ ] `pool(reserve, atoken)` resolves correctly regardless of which instance hosts the pair.
- [ ] `liquidity_depth(asset_in, asset_out)` returns correct depth regardless of hosting instance.
- [ ] Trade-executor `supply` / `withdraw` / `do_withdraw_all_to` route to the correct Aave pool for the given asset.
- [ ] `execute_sell` / `execute_buy` through `PoolType::Aave` succeed for pairs hosted on a non-main instance (e.g., HDX ↔ GIGAHDX).
- [ ] Given both `BorrowingContract` and `GigaHdxPoolContract` point at the same address (fresh chain default), `pairs()` returns each pair exactly once.
- [ ] A failing reserve call on one instance does not remove results from other instances.
- [ ] If a reserve is (invariant-breaking) registered in two instances, `find_pool_for_asset` returns the first match and emits a warning log.
- [ ] (Option B) `add_aave_pool_contract` / `remove_aave_pool_contract` are gated by `AuthorityOrigin` and emit events.
- [ ] (Option B) Migration seeds the registry with the distinct existing contracts; runtime boots without manual intervention.
## Open Questions
- **Which option?** A (ship now, revisit) or B (one migration, done).
- **`PoolData` typo.** Fix `liqudity_in` / `liqudity_out` → `liquidity_in` / `liquidity_out` (`aave_trade_executor.rs:609-610`) in this PR or a separate cleanup? Fixing here is a breaking field rename; deferring is cheap.
- **Caching `find_pool_for_asset`.** Worth caching the `(asset → pool)` map in a lazy-init runtime cache, or is the O(instances × reserves) scan fine given expected bounds?
- **Flash-loan precompile.** Keep `BorrowingContract`-pinned or route per asset?
- **`BorrowingContract` deprecation (Option B).** Remove old storage aliases in the same release or two-step?
- **Liquidation worker.** Follow-up to iterate both pools in `node/src/liquidation_worker.rs`, or remain single-PAP?
## References
- Hydration protocol overview: https://github.com/galacticcouncil/hydration/blob/main/general/hydration.md
- Omnipool design: https://github.com/galacticcouncil/hydration/blob/main/general/omnipool.md
Contributor guide
Assessment
This issue has not been assessed yet.