Raise router multi-hop gas floor to 950k/hop — 2-hop still OOGs at 1,810,000 (#353 reopen) #869

Merged

Brouie merged 1 commit from qa/353-2hop-router-gas-floor into main

2026-06-12 07:54:31 +00:00

Brouie commented

2026-06-12 04:34:12 +00:00

(Migrated from gitlab.com)

#353 closed having fixed the wrap half, but the 2-hop half never got its gas floor raised — it's still deterministically out-of-gas at HEAD (8408689).

Live on-chain evidence (EMBER->JADE->RUBY, dapp Expert Mode, today):

tx	code	gasUsed / gasWanted
6CE61EEB…	11 OOG	1,810,064 / 1,810,000
E106667E…	11 OOG	1,810,134 / 1,810,000
C6FB8192…	11 OOG	1,810,206 / 1,810,000

Root cause: gasLimitForRouterExecuteSwapOperations(2) = ROUTER_SWAP_OPS_MIN_GAS_PER_HOP (900,000) × 2 + SWAP_GAS_SAFETY_MARGIN (10,000) = exactly 1,810,000 — the same number the swaps need, zero headroom, so it tips over every time. The existing 2-hop test only guarded > 1,718,000 (stale), so it never caught the knife-edge.

Fix: raise the per-hop floor to 950,000 → 2-hop 1,910,000 (~100k / 5.5% headroom over the worst observed 1,810,206), 3-hop 2,860,000. Single-hop (ROUTER_SINGLE_HOP_GAS_LIMIT 1.4M) and direct-pair swap paths are untouched. Updated the 2-hop/3-hop assertions and re-floored the regression guard above the live observed 1,810,206 so it actually catches this class now.

transactions.test.ts 51/51. Change is constant + test only.

Note: this MR does not address the separate, pre-existing tsc-only errors in terraTxHash.ts / terraWalletSignTxRaw.ts that a fresh-cache npm run build surfaces on main — flagging that separately.

#353 closed having fixed the wrap half, but the **2-hop half never got its gas floor raised** — it's still deterministically out-of-gas at HEAD (8408689). Live on-chain evidence (EMBER->JADE->RUBY, dapp Expert Mode, today): | tx | code | gasUsed / gasWanted | |----|------|---------------------| | 6CE61EEB… | 11 OOG | 1,810,064 / 1,810,000 | | E106667E… | 11 OOG | 1,810,134 / 1,810,000 | | C6FB8192… | 11 OOG | 1,810,206 / 1,810,000 | Root cause: `gasLimitForRouterExecuteSwapOperations(2)` = `ROUTER_SWAP_OPS_MIN_GAS_PER_HOP (900,000) × 2 + SWAP_GAS_SAFETY_MARGIN (10,000)` = exactly **1,810,000** — the same number the swaps need, zero headroom, so it tips over every time. The existing 2-hop test only guarded `> 1,718,000` (stale), so it never caught the knife-edge. Fix: raise the per-hop floor to **950,000** → 2-hop **1,910,000** (~100k / 5.5% headroom over the worst observed 1,810,206), 3-hop 2,860,000. Single-hop (`ROUTER_SINGLE_HOP_GAS_LIMIT` 1.4M) and direct-pair `swap` paths are untouched. Updated the 2-hop/3-hop assertions and re-floored the regression guard above the live observed 1,810,206 so it actually catches this class now. transactions.test.ts 51/51. Change is constant + test only. Note: this MR does **not** address the separate, pre-existing tsc-only errors in terraTxHash.ts / terraWalletSignTxRaw.ts that a fresh-cache `npm run build` surfaces on main — flagging that separately.

PlasticDigits (Migrated from gitlab.com) approved these changes

2026-06-12 04:34:12 +00:00

PlasticDigits commented

2026-06-12 04:36:25 +00:00

(Migrated from gitlab.com)

Security review

Commit reviewed: 6afad1657c166a6e4942b13889b7db9e7845d61b
Scope: frontend-dapp/src/utils/constants.ts (ROUTER_SWAP_OPS_MIN_GAS_PER_HOP 900k → 950k) and matching assertions in transactions.test.ts.

Method: Diff review plus trace of gasLimitForRouterExecuteSwapOperations → getGasLimitForTx → buildTerraClassicFee / broadcast. No prior security-review threads on this MR.

Outcome: FINDINGS: 0 medium+

This change only raises the client-side multi-hop router gas floor so 2-hop execute_swap_operations grants headroom above live observed usage (~1.81M). Single-hop (ROUTER_SINGLE_HOP_GAS_LIMIT) and direct-pair swap paths are unchanged. Hop count still derives from the signed operations array length (pre-existing); no new attacker-controlled sinks, authn/authz changes, injection surfaces, secret handling, or dependency changes.

Raising gas_wanted increases the fee quote users sign when the per-hop floor binds (~100k gas/hop); that is intentional reliability headroom, not an exploitable mis-estimation vector in this diff.

Inline threads: none (no medium+ findings).

block:security not applied.

## Security review **Commit reviewed:** `6afad1657c166a6e4942b13889b7db9e7845d61b` **Scope:** `frontend-dapp/src/utils/constants.ts` (`ROUTER_SWAP_OPS_MIN_GAS_PER_HOP` 900k → 950k) and matching assertions in `transactions.test.ts`. **Method:** Diff review plus trace of `gasLimitForRouterExecuteSwapOperations` → `getGasLimitForTx` → `buildTerraClassicFee` / broadcast. No prior security-review threads on this MR. **Outcome:** **FINDINGS: 0** medium+ This change only raises the client-side multi-hop router gas floor so 2-hop `execute_swap_operations` grants headroom above live observed usage (~1.81M). Single-hop (`ROUTER_SINGLE_HOP_GAS_LIMIT`) and direct-pair `swap` paths are unchanged. Hop count still derives from the signed `operations` array length (pre-existing); no new attacker-controlled sinks, authn/authz changes, injection surfaces, secret handling, or dependency changes. Raising `gas_wanted` increases the fee quote users sign when the per-hop floor binds (~100k gas/hop); that is intentional reliability headroom, not an exploitable mis-estimation vector in this diff. **Inline threads:** none (no medium+ findings). `block:security` not applied.

Brouie commented

2026-06-12 04:39:15 +00:00

(Migrated from gitlab.com)

mentioned in issue #353