galacticcouncil / galacticcouncil/hydration-node

`router.sell` / `buy` / `sell_all` charge the declared weight in full — no post-dispatch refund

Open
#1,524 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

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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.