Router minimum_receive on unwrap path checks the wrapped amount, not what the recipient actually gets after the mapper fee #469

New issue

Closed

opened 2026-07-01 12:57:07 +00:00 by Brouie · 5 comments

Brouie commented

2026-07-01 12:57:07 +00:00

(Migrated from gitlab.com)

Came out of the security sweep on the router. On the unwrap_output path the minimum_receive slippage floor is checked against the pre-unwrap (wrapped) amount, so the mapper's fee gets skimmed after the guard and the recipient can land below the floor they set.

Where it is: smartcontracts/contracts/router/src/contract.rs, reply_swap_hop, the final-hop block ~349-398.

Mechanism:

hop_output is the router's CW20 balance delta of the wrapped output token (:349-351).
On the final hop, minimum_receive is asserted against that hop_output (:356-363).
Then on the state.unwrap_output branch the router builds a Cw20ExecuteMsg::Send for the full hop_output to the wrap-mapper with wrap_mapper::Cw20HookMsg::Unwrap { recipient } (:365-382) and fires it as a fire-and-forget message via add_message (:394-395). SWAP_STATE was already removed at :354, so there's no reply/second check on what actually gets delivered.
The wrap-mapper carries a governance-settable fee: dex-common::wrap_mapper has SetFeeBps { fee_bps } (packages/dex-common/src/wrap_mapper.rs:41) and exposes fee_bps in ConfigResponse (:71). The Unwrap hook (:48) delivers amount - fee to the recipient.

So the check passes on the larger wrapped number, the mapper takes fee_bps, and the recipient gets less than minimum_receive — no revert.

Repro:

Mapper fee_bps = 50 (0.5%).
User submits a route with unwrap_output = true and minimum_receive = 1_000_000.
Final hop yields exactly 1_000_000 wrapped. Check at :357 passes (1_000_000 < 1_000_000 is false).
Unwrap Send goes out for 1_000_000; mapper skims 5_000; recipient receives 995_000 native — below their floor, and nothing reverts.

Impact is low: the shortfall is bounded by the mapper fee (fee_bps), which is governance-controlled and expected to be small, and it only bites on the unwrap path. But it does mean the minimum_receive guarantee is not actually honored end to end — the number the user signed off on isn't the number they can receive. If governance ever bumps fee_bps, the gap widens silently for anyone using the unwrap path.

Fix direction: apply minimum_receive to the delivered (post-unwrap) amount. Cleanest is to compute the expected net after the mapper fee — query the mapper Config{} for fee_bps and check hop_output - fee(hop_output, fee_bps) >= min before sending — or, if we don't want the extra query, do the minimum_receive assertion in a reply on the unwrap Send against the recipient's actual balance delta. The transfer (non-unwrap) path is fine as-is since there's no fee there.

Filing under the #381 security-hardening umbrella (router slippage-invariant hardening).

Came out of the security sweep on the router. On the `unwrap_output` path the `minimum_receive` slippage floor is checked against the pre-unwrap (wrapped) amount, so the mapper's fee gets skimmed *after* the guard and the recipient can land below the floor they set. Where it is: `smartcontracts/contracts/router/src/contract.rs`, `reply_swap_hop`, the final-hop block ~349-398. Mechanism: - `hop_output` is the router's CW20 balance delta of the **wrapped** output token (`:349-351`). - On the final hop, `minimum_receive` is asserted against that `hop_output` (`:356-363`). - Then on the `state.unwrap_output` branch the router builds a `Cw20ExecuteMsg::Send` for the full `hop_output` to the wrap-mapper with `wrap_mapper::Cw20HookMsg::Unwrap { recipient }` (`:365-382`) and fires it as a fire-and-forget message via `add_message` (`:394-395`). `SWAP_STATE` was already removed at `:354`, so there's no reply/second check on what actually gets delivered. - The wrap-mapper carries a governance-settable fee: `dex-common::wrap_mapper` has `SetFeeBps { fee_bps }` (`packages/dex-common/src/wrap_mapper.rs:41`) and exposes `fee_bps` in `ConfigResponse` (`:71`). The `Unwrap` hook (`:48`) delivers `amount - fee` to the recipient. So the check passes on the larger wrapped number, the mapper takes `fee_bps`, and the recipient gets less than `minimum_receive` — no revert. Repro: - Mapper `fee_bps = 50` (0.5%). - User submits a route with `unwrap_output = true` and `minimum_receive = 1_000_000`. - Final hop yields exactly `1_000_000` wrapped. Check at `:357` passes (`1_000_000 < 1_000_000` is false). - Unwrap Send goes out for `1_000_000`; mapper skims 5_000; recipient receives `995_000` native — below their floor, and nothing reverts. Impact is low: the shortfall is bounded by the mapper fee (`fee_bps`), which is governance-controlled and expected to be small, and it only bites on the unwrap path. But it does mean the `minimum_receive` guarantee is not actually honored end to end — the number the user signed off on isn't the number they can receive. If governance ever bumps `fee_bps`, the gap widens silently for anyone using the unwrap path. Fix direction: apply `minimum_receive` to the **delivered** (post-unwrap) amount. Cleanest is to compute the expected net after the mapper fee — query the mapper `Config{}` for `fee_bps` and check `hop_output - fee(hop_output, fee_bps) >= min` before sending — or, if we don't want the extra query, do the `minimum_receive` assertion in a reply on the unwrap Send against the recipient's actual balance delta. The transfer (non-unwrap) path is fine as-is since there's no fee there. Filing under the #381 security-hardening umbrella (router slippage-invariant hardening).

PlasticDigits commented

2026-07-01 13:57:20 +00:00

(Migrated from gitlab.com)

Recommendation approved

PlasticDigits commented

2026-07-01 14:05:00 +00:00

(Migrated from gitlab.com)

mentioned in merge request !1003

PlasticDigits commented

2026-07-02 02:00:53 +00:00

(Migrated from gitlab.com)

mentioned in commit 5ebd7719e7

mentioned in commit 5ebd7719e766552d29aad06a43f67a25f5aecfe2

PlasticDigits commented

2026-07-07 02:15:19 +00:00

(Migrated from gitlab.com)

Verification — #469 (router `minimum_receive` on unwrap path)

Result: PASS — fix is on main; no repo changes required.

What was verified

Criterion	Result	How
`minimum_receive` on unwrap path compares post–wrap-mapper net, not pre-unwrap CW20 `hop_output`	PASS	Code review: `reply_swap_hop` queries wrap-mapper `Config { fee_bps }`, computes `delivered_amount` via `net_after_wrap_mapper_unwrap_fee`, asserts `delivered_amount >= minimum_receive` before the unwrap `Send` (`smartcontracts/contracts/router/src/contract.rs`)
Issue repro: `fee_bps = 50`, `minimum_receive` = wrapped hop output → swap reverts (recipient would get `hop_output − fee`)	PASS	`cargo test test_unwrap_minimum_receive_rejects_when_mapper_fee_skims_below_floor`
Floor at wrapped sim amount on unwrap path → revert (fee skims below floor)	PASS	`cargo test test_unwrap_minimum_receive_checked_on_post_unwrap_net`
Floor at post-unwrap net → success; native delta matches net	PASS	`cargo test test_unwrap_minimum_receive_succeeds_at_post_unwrap_net`
Non-unwrap CW20 transfer path unchanged (`hop_output` vs floor)	PASS	`cargo test test_router_minimum_receive_assertion`
Invariant R3 documented	PASS	`docs/contracts-security-audit.md` R3 row; `skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md`
Full contract regression suite	PASS	`make test-contracts` — 393 integration tests, 0 failed

Commands run

cd smartcontracts && cargo test test_unwrap_minimum_receive
cd smartcontracts && cargo test test_router_minimum_receive
make test-contracts

Notes

The approved fix direction (query mapper fee_bps and check net before unwrap Send) is implemented. Integrators setting minimum_receive on native-output swaps should subtract the mapper fee from simulated wrapped output (documented in skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md).

## Verification — #469 (router `minimum_receive` on unwrap path) **Result: PASS** — fix is on `main`; no repo changes required. ### What was verified | Criterion | Result | How | |-----------|--------|-----| | `minimum_receive` on unwrap path compares **post–wrap-mapper net**, not pre-unwrap CW20 `hop_output` | **PASS** | Code review: `reply_swap_hop` queries wrap-mapper `Config { fee_bps }`, computes `delivered_amount` via `net_after_wrap_mapper_unwrap_fee`, asserts `delivered_amount >= minimum_receive` before the unwrap `Send` (`smartcontracts/contracts/router/src/contract.rs`) | | Issue repro: `fee_bps = 50`, `minimum_receive` = wrapped hop output → swap **reverts** (recipient would get `hop_output − fee`) | **PASS** | `cargo test test_unwrap_minimum_receive_rejects_when_mapper_fee_skims_below_floor` | | Floor at wrapped sim amount on unwrap path → **revert** (fee skims below floor) | **PASS** | `cargo test test_unwrap_minimum_receive_checked_on_post_unwrap_net` | | Floor at post-unwrap net → **success**; native delta matches net | **PASS** | `cargo test test_unwrap_minimum_receive_succeeds_at_post_unwrap_net` | | Non-unwrap CW20 transfer path unchanged (`hop_output` vs floor) | **PASS** | `cargo test test_router_minimum_receive_assertion` | | Invariant **R3** documented | **PASS** | `docs/contracts-security-audit.md` R3 row; `skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md` | | Full contract regression suite | **PASS** | `make test-contracts` — 393 integration tests, 0 failed | ### Commands run ```bash cd smartcontracts && cargo test test_unwrap_minimum_receive cd smartcontracts && cargo test test_router_minimum_receive make test-contracts ``` ### Notes The approved fix direction (query mapper `fee_bps` and check net before unwrap `Send`) is implemented. Integrators setting `minimum_receive` on native-output swaps should subtract the mapper fee from simulated wrapped output (documented in `skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md`).

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-07-07 02:15:21 +00:00

PlasticDigits commented

2026-07-29 02:07:47 +00:00

(Migrated from gitlab.com)

mentioned in issue #502