Fix router minimum_receive on unwrap path (#469) #1003

2026-07-01T14:04:58Z

PlasticDigits commented

2026-07-01 14:04:58 +00:00

(Migrated from gitlab.com)

Summary

Fixes GitLab #469: on the router unwrap_output path, minimum_receive is now asserted against the native amount delivered after wrap-mapper fee, not the pre-unwrap CW20 hop output.

Final hop queries wrap-mapper Config { fee_bps } and compares hop_output − floor(hop_output × fee_bps / 10_000) to minimum_receive.
CW20 transfer path (no unwrap) unchanged.
Documented invariant R3 update, new agent playbook skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md, and regression tests including the #469 repro (fee_bps = 50, floor = wrapped output → revert).

Acceptance checklist

Criterion	Verification	Result
`minimum_receive` on unwrap path uses post-mapper net, not wrapped hop output	`cd smartcontracts && cargo test unwrap_minimum_receive`	PASS
Non-unwrap `minimum_receive` unchanged	`cd smartcontracts && cargo test test_router_minimum_receive_assertion`	PASS
#469 repro: wrapped output equals floor but mapper fee delivers less → revert	`test_unwrap_minimum_receive_rejects_when_mapper_fee_skims_below_floor`	PASS
Floor at post-unwrap net allows swap; recipient gets expected native	`test_unwrap_minimum_receive_succeeds_at_post_unwrap_net`	PASS
Invariant R3 documented and cross-linked	`docs/contracts-security-audit.md`, `skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md`	PASS
`cargo fmt` / pre-commit hooks	commit hooks on this branch	PASS

Verification checklist (third parties)

git fetch origin issue-469-router-unwrap-minimum-receive
git checkout issue-469-router-unwrap-minimum-receive
cd smartcontracts
cargo test minimum_receive

Expected: 5 tests pass (test_router_minimum_receive_assertion, test_unwrap_output_minimum_receive, three test_unwrap_minimum_receive_*).

Manual invariant read: reply_swap_hop in smartcontracts/contracts/router/src/contract.rs — delivered_amount uses net_after_wrap_mapper_unwrap_fee when unwrap_output is true.

## Summary Fixes GitLab #469: on the router `unwrap_output` path, `minimum_receive` is now asserted against the **native amount delivered after wrap-mapper fee**, not the pre-unwrap CW20 hop output. - Final hop queries wrap-mapper `Config { fee_bps }` and compares `hop_output − floor(hop_output × fee_bps / 10_000)` to `minimum_receive`. - CW20 transfer path (no unwrap) unchanged. - Documented invariant **R3** update, new agent playbook [`skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md`](skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md), and regression tests including the #469 repro (`fee_bps = 50`, floor = wrapped output → revert). ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | `minimum_receive` on unwrap path uses post-mapper net, not wrapped hop output | `cd smartcontracts && cargo test unwrap_minimum_receive` | PASS | | Non-unwrap `minimum_receive` unchanged | `cd smartcontracts && cargo test test_router_minimum_receive_assertion` | PASS | | #469 repro: wrapped output equals floor but mapper fee delivers less → revert | `test_unwrap_minimum_receive_rejects_when_mapper_fee_skims_below_floor` | PASS | | Floor at post-unwrap net allows swap; recipient gets expected native | `test_unwrap_minimum_receive_succeeds_at_post_unwrap_net` | PASS | | Invariant R3 documented and cross-linked | `docs/contracts-security-audit.md`, `skills/AGENTS_ROUTER_MINIMUM_RECEIVE.md` | PASS | | `cargo fmt` / pre-commit hooks | commit hooks on this branch | PASS | ## Verification checklist (third parties) ```bash git fetch origin issue-469-router-unwrap-minimum-receive git checkout issue-469-router-unwrap-minimum-receive cd smartcontracts cargo test minimum_receive ``` Expected: 5 tests pass (`test_router_minimum_receive_assertion`, `test_unwrap_output_minimum_receive`, three `test_unwrap_minimum_receive_*`). Manual invariant read: `reply_swap_hop` in `smartcontracts/contracts/router/src/contract.rs` — `delivered_amount` uses `net_after_wrap_mapper_unwrap_fee` when `unwrap_output` is true.

PlasticDigits commented

2026-07-01 14:08:06 +00:00

(Migrated from gitlab.com)

Security review

Commit reviewed: c4335137decd7192f603bf51f86bf3853766c941
Scope: Router minimum_receive enforcement on unwrap_output final-hop path (net_after_wrap_mapper_unwrap_fee, reply_swap_hop), docs/playbook, regression tests (#469).

Outcome: FINDINGS: 0 medium+

Summary

This MR closes GitLab #469 (slippage floor checked against pre-unwrap CW20 hop output while the recipient receives post–wrap-mapper native net). The fix is directionally correct and does not introduce a new exploitable path in the reviewed diff.

Analysis (no medium+ issues)

Fee math parity — net_after_wrap_mapper_unwrap_fee uses the same truncating multiply_ratio(fee_bps, 10_000) + subtract pattern as wrap-mapper calculate_fee / unwrap InstantWithdraw amount. Checked subtraction handles pathological fee_bps responses safely (revert, no partial payout).
Attacker-controlled input → sink — minimum_receive remains user-supplied; the settlement sink now compares against delivered_amount derived from on-chain hop_output (R4 balance delta) and governance-configured wrap-mapper Config.fee_bps. No unprivileged actor can lower the floor check or redirect unwrap proceeds.
TOCTOU on fee_bps — Config is queried in reply_swap_hop and unwrap executes as the next message in the same atomic transaction; SetFeeBps is governance-only. A fee change mid-swap across blocks either reverts (fee up) or over-delivers (fee down); not a theft vector for external attackers.
Malicious wrap-mapper — A mapper that lies on Config vs Unwrap could break slippage semantics, but WRAP_MAPPER is set only by router governance (SetWrapMapper). That is an deployment/trust-boundary concern outside this diff, not a new permission bypass.
Atomicity — minimum_receive failure reverts the full transaction (including prior hops in the SubMsg chain). Unwrap failure after a passing check also rolls back the swap chain.
Prior threads — No earlier security-review comments on this MR.

Inline threads: none (no medium+ findings).

Security review: no medium+ findings on this diff.

## Security review **Commit reviewed:** `c4335137decd7192f603bf51f86bf3853766c941` **Scope:** Router `minimum_receive` enforcement on `unwrap_output` final-hop path (`net_after_wrap_mapper_unwrap_fee`, `reply_swap_hop`), docs/playbook, regression tests (#469). **Outcome:** `FINDINGS: 0` medium+ ### Summary This MR closes GitLab #469 (slippage floor checked against pre-unwrap CW20 hop output while the recipient receives post–wrap-mapper native net). The fix is directionally correct and does not introduce a new exploitable path in the reviewed diff. ### Analysis (no medium+ issues) 1. **Fee math parity** — `net_after_wrap_mapper_unwrap_fee` uses the same truncating `multiply_ratio(fee_bps, 10_000)` + subtract pattern as wrap-mapper `calculate_fee` / unwrap `InstantWithdraw` amount. Checked subtraction handles pathological `fee_bps` responses safely (revert, no partial payout). 2. **Attacker-controlled input → sink** — `minimum_receive` remains user-supplied; the settlement sink now compares against `delivered_amount` derived from on-chain `hop_output` (R4 balance delta) and governance-configured wrap-mapper `Config.fee_bps`. No unprivileged actor can lower the floor check or redirect unwrap proceeds. 3. **TOCTOU on `fee_bps`** — Config is queried in `reply_swap_hop` and unwrap executes as the next message in the same atomic transaction; `SetFeeBps` is governance-only. A fee change mid-swap across blocks either reverts (fee up) or over-delivers (fee down); not a theft vector for external attackers. 4. **Malicious wrap-mapper** — A mapper that lies on `Config` vs `Unwrap` could break slippage semantics, but `WRAP_MAPPER` is set only by router governance (`SetWrapMapper`). That is an deployment/trust-boundary concern outside this diff, not a new permission bypass. 5. **Atomicity** — `minimum_receive` failure reverts the full transaction (including prior hops in the SubMsg chain). Unwrap failure after a passing check also rolls back the swap chain. 6. **Prior threads** — No earlier security-review comments on this MR. **Inline threads:** none (no medium+ findings). Security review: no medium+ findings on this diff.