galacticcouncil / galacticcouncil/hydration-node

UniswapV3 `trade_weight()` charges a 1M-gas quote that `sell` never performs, on ceilings 2-10× the measured calls

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

Description

## Summary

`UniswapV3::trade_weight()` prices every V3 hop at `worst_case_gas()` = **2,900,000** gas
(`runtime/hydradx/src/evm/uniswap_v3_trade_executor.rs:445-459` at `v52.0.0`). Two independent problems
inflate it:

1. **The sell path never quotes, but pays for one.** `worst_case_gas()` includes
`QUOTE_GAS_LIMIT = 1_000_000` (34% of the total). `execute_sell` → `do_sell` (`:509-550`,
`:638-671`) does approve → `exactInputSingle` → `getPool` → `balanceOf`×2 → `slot0`. No call
to the quoter. The router already knows this — `assets.rs:1091`:

```rust
let c = 0; // number of times AMM::calculate_sell is executed. It is zero as we don't calculate trade amounts upfront in sell anymore
```

but `sell_weight` then adds `UniswapV3::trade_weight()` (`:1106`), which is a constant and
ignores `c`. `sell_and_calculate_sell_trade_amounts_weight` and `buy_weight` do quote, so the
quote belongs in _those_ figures, not in a single shared constant.

2. **The per-call ceilings are 2-10× the numbers in the comment above them.** From
`:56-58`, measured on zombienet at spec 429, 21k base included:

| call | ceiling (`:63-67`) | measured (`:56-58`) | ratio |
| ---------------------- | ------------------ | ---------------------------------- | ----- |
| `getPool` | 250,000 | 105,509 | 2.4× |
| `slot0` / `liquidity` | 250,000 | 92,661 | 2.7× |
| `balanceOf` | 100,000 | 22,446 | 4.5× |
| `approve` | 100,000 | 23,194 | 4.3× |
| `quoteExactInputSingle`| 1,000,000 | 138,712 (one full-range position) | 7.2× |
| `exactInputSingle` | 1,000,000 | not measured | — |

The comment justifies the two 1M figures — swap and quote walk ticks once real bands exist —
and that reasoning is sound for those two. It does not apply to the four reads, which the
same comment says "are bounded by storage access and do not grow that way".

## Arithmetic

With `WEIGHT_PER_GAS = 25_000` (`evm/mod.rs:79-81`), 2.9M gas = **72.5 ms ref_time per hop**.

What the sell path actually executes, using the file's own measurements and leaving the swap at
its full 1M ceiling since it is unmeasured:

```
approve 23,194
swap 1,000,000 (ceiling kept — tick walk is real)
getPool 105,509
balanceOf×2 44,892
slot0 92,661
---------
1,266,256 vs 2,900,000 charged → 2.3× at the swap's ceiling,
4-7× at any realistic swap
```

Because `sell`/`buy`/`sell_all` return `DispatchResult` — no `PostDispatchInfo`, so no refund — the 2.9M figure is
not a block-space estimate the chain corrects after the fact; it is the fee.

## Ask

Concrete and small, in order of value:

1. **Split the constant by path.** A `sell_gas()` without `QUOTE_GAS_LIMIT`, and a `buy_gas()`
/ `calculate_*` figure with it. `sell_weight` already distinguishes `c` from `e`; give
`trade_weight` the same signature the other venues get (`router_execution_sell(c, e)`).
2. **Tighten the four read ceilings** toward the measured figures with an ordinary safety
margin (2× would still be generous): `getPool`/`slot0` ~200k, `balanceOf`/`approve` ~50k.
Leave `SWAP_GAS_LIMIT` at 1M until it is measured on a pool with real bands.

(1) alone removes ~34% of the per-hop charge on the extrinsic that dominates volume, and the
change is a constant and a match arm.

## Context

`parameters.UniswapV3{Factory,SwapRouter,Quoter}` are still `None` on mainnet, so no V3 trade
has been charged this weight yet. The constants are cheapest to change now, before the venue
has users and a fee change has a constituency.

Contributor guide

Open the contributing guide

Research direction

Start in runtime/hydradx/src/evm/uniswap_v3_trade_executor.rs by reading worst_case_gas(), execute_sell, and do_sell, then trace assets.rs:sell_weight and its c value. Confirm which calls each path performs and compare them with the listed measurements. Done means sell no longer includes quote gas, buy and calculation paths retain it, and the four read ceilings reflect a stated safety margin while the swap ceiling remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
blockchain
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.