galacticcouncil / galacticcouncil/hydration-node
`router.sell` / `buy` / `sell_all` charge the declared weight in full — no post-dispatch refund
- Dominant language
- Rust
- Stars
- 208
- Forks
- 109
- Avg merge
- 6d 3h
- Merged PRs (30d)
- 3
Description
## Summary
The three trading extrinsics in `pallet-route-executor` return `DispatchResult`, so no
`PostDispatchInfo` is produced and `actual_weight` is always `None`. The declared
`#[pallet::weight]` — a per-hop sum of each venue's worst case — is therefore what the user
pays, regardless of what the route actually consumed. The two non-trading extrinsics in the
same pallet (`set_route`, `force_insert_route`) do return `DispatchResultWithPostInfo`.
This was invisible while every venue's declared weight was a benchmark close to its real cost.
v52's Uniswap V3 venue is the first whose declared weight is a hand-written **ceiling**, derived
from EVM gas limits and documented as one, on the stated assumption that only consumption is
charged. That assumption is true of EVM gas metering inside the call and false of the extrinsic
fee — so the ceiling is the fee.
## Where
`pallets/route-executor/src/lib.rs` at `v52.0.0`:
| extrinsic | line | weight attribute | returns |
| -------------------- | ---- | ----------------------------------- | ----------------------------- |
| `sell` | 192 | `T::WeightInfo::sell_weight(route)` | `DispatchResult` |
| `buy` | 219 | `T::WeightInfo::buy_weight(route)` | `DispatchResult` |
| `sell_all` | 426 | `T::WeightInfo::sell_weight(route)` | `DispatchResult` |
| `set_route` | 314 | `set_route_weight(new_route)` | `DispatchResultWithPostInfo` |
| `force_insert_route` | 394 | `force_insert_route_weight()` | `DispatchResultWithPostInfo` |
`runtime/hydradx/src/evm/uniswap_v3_trade_executor.rs:52-54` — the premise the V3 ceilings
rest on:
```rust
// Per-call gas ceilings. These are CEILINGS, not consumption — the chain charges
// what is used — but `trade_weight()` is derived from them, so an inflated ceiling
// makes every v3 trade look more expensive than it is and fewer fit in a block.
```
`trade_weight()` (`:455-459`) maps `worst_case_gas()` = **2,900,000** gas through
`GasWeightMapping::gas_to_weight`, and `runtime/hydradx/src/assets.rs:1106` adds that per V3 hop
into `sell_weight`. With `WEIGHT_PER_GAS = 1e12 / 40_000_000 = 25_000` (`evm/mod.rs:79-81`),
that is **72.5 ms ref_time per V3 hop**, charged whether the swap used 2.9M gas or 300k.
## Effect
- The comment says an inflated ceiling makes a trade "look more expensive" and crowds the
block. With no refund it makes the trade **genuinely** more expensive by the full gap between
ceiling and consumption, on every hop, every time.
- This is not V3-specific. Any venue whose declared weight exceeds its real cost over-charges
today; V3 is just the first where the gap is large and written down.
- Anything that prices a route from `sell_weight` — the SDK, wallets, fee estimators — is
already correct about what will be charged. The gap is between that and what the chain does.
## Ask
Return `DispatchResultWithPostInfo` from `sell`, `buy` and `sell_all`, with `actual_weight`
accumulated per hop from what each executor actually did — the pallet already has the shape
in its sibling extrinsics. For the V3 executor the real gas is available from each
`Executor::call/view` result (`used_gas`), so the refund can be exact rather than estimated.
If a refund is deliberately out of scope, the comment in `uniswap_v3_trade_executor.rs` should
say the opposite of what it says now, so the ceilings are set with the knowledge that they are
the price.
Contributor guide
Research direction
Start with sell, buy, and sell_all in pallets/route-executor/src/lib.rs, comparing their return types with set_route and force_insert_route. Trace each hop's executor call/view result, including used_gas in runtime/hydradx/src/evm/uniswap_v3_trade_executor.rs, and confirm that actual_weight is accumulated per hop and returned as post-dispatch information.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- blockchain, rust
- Domain
- blockchain, performance
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100