SIG-605: add SignalsRouter for atomic swap-and-bet

[worjs]

Context

MarketCore's openPosition only accepts ctUSD. Users holding other tokens (USDC.e, WCBTC) must manually swap → approve → open in 3 separate transactions, with price slippage risk between the swap and bet. SIG-605

This PR adds SignalsRouter, a non-upgradeable peripheral contract that atomically swaps input tokens to ctUSD via Algebra V2 (Satsuma DEX) and executes the corresponding Core operation in a single transaction.

Decisions

• Non-upgradeable, peripheral-only: The router holds no protocol state — it swaps, calls Core, and returns NFT + change to the caller. No proxy, no storage slots.
• Owner-gated token allowlist: Only explicitly allowed tokens can be used as input/output. ctUSD is excluded from the allowlist (direct Core calls handle that path). This narrows the attack surface to vetted Algebra pools only.
• NFT transfer pattern for existing positions: increasePositionWithSwap, decreasePositionWithSwap, closePositionWithSwap, claimPayoutWithSwap temporarily transfer the position NFT to the router (via transferFrom) so the router becomes ownerOf for the Core ownership check, then return it post-operation. onERC721Received rejects non-mint transfers to prevent accidental NFT deposits.
• Balance-delta accounting: Uses balanceOf(this) before/after pattern rather than trusting return values, ensuring pre-existing ctUSD dust in the router is never drained to users.
• poolDeployer set to address(0) in deploy script: The Algebra V2 pool deployer address on Citrea is not yet registered in environment configs. The deploy script passes address(0) — this needs to be updated before mainnet deployment.
• Rescue functions: rescueToken and rescueNFT allow the owner to recover stuck assets — defensive measure since the router should never hold assets between transactions.

Impact

This is a new contract with a new ABI. No existing contract interfaces or events are modified.

• v1-sdk: New SignalsRouter ABI needs to be added for frontend integration (ABI exported via export-abis.js)
• signals-app: Betting flow needs to integrate router calls for non-ctUSD token paths (Satsuma swap infra already exists in src/core/satsuma/)
• No sibling PRs found — downstream integration is not yet started

Risk Areas

• Swap slippage + Core cost interaction: if the Algebra swap returns slightly less ctUSD than maxCost, the Core call reverts and the entire transaction rolls back. The minCtUSD parameter controls this boundary but requires correct off-chain quoting.
• _takePosition uses transferFrom (not safeTransferFrom) — caller must have approved the router for the position NFT. Missing approval reverts at ERC721 level.
• The router becomes msg.sender for all Core calls — fee policies and risk gates see the router address as the trader, not the end user. The NatSpec documents this, but any future per-user policy logic must account for router-mediated calls.
• onERC721Received allows from == address(0) (mint) only — this is correct for openPosition flow where Core mints to the router, but means the router cannot receive position NFTs via safeTransferFrom from users (they must use transferFrom or setApprovalForAll + router's _takePosition).

[signals-reviewer]

Summary

This PR adds SignalsRouter, a non-upgradeable peripheral contract that atomically swaps input tokens to ctUSD via Algebra V2 (Satsuma DEX) and executes the corresponding Core operation in a single transaction. The router supports open, increase, decrease, close, and claim operations with token swap. It uses balance-delta accounting for ctUSD input flows and return-value accounting for output swaps. All entry points are nonReentrant, and the token allowlist is owner-gated with ctUSD excluded.

Cross-PR Context

Sibling PRs (SIG-605)

No sibling PRs found. The PR description correctly identifies downstream integration needs:

• v1-sdk: New SignalsRouter ABI export needed (ABI is already generated in abis/SignalsRouter.json)
• signals-app: Betting flow integration for non-ctUSD token paths

Since this is a new additive contract (no existing interface changes), downstream repos continue to function without modification. Sibling PRs can follow independently.

Issues

No issues found.

Suggestions

• 🟡 [Robustness — dangling ctUSD approval to Core after swap operations]: After openPositionWithSwap and increasePositionWithSwap, the router calls ctUSD.forceApprove(address(core), ctUSDReceived) but Core only pulls totalCost which may be less than ctUSDReceived. The remaining approval persists until the next forceApprove call. (contracts/router/SignalsRouter.sol:718)

  Scenario: User calls openPositionWithSwap with inputAmount = 100_000, swap returns 100_000 ctUSD, but maxCost matches actual cost of 80_000. The router approves Core for 100_000, Core pulls 80_000, leaving 20_000 ctUSD approval. The leftover ctUSD balance is correctly returned to the user via the delta refund, and the approval is harmless because Core is a trusted immutable proxy. However, resetting the approval to 0 after the Core call (ctUSD.forceApprove(address(core), 0)) follows the principle of least privilege and eliminates even theoretical risk from a future Core upgrade.
  Evidence: Line 718 approves ctUSDReceived, line 720 calls core.openPosition which pulls only totalCost <= maxCost. forceApprove on line 747 (increasePosition path) exhibits the same pattern. The refund on lines 722-725 correctly handles the balance delta, so no funds are at risk — this is a defense-in-depth suggestion.

• 🟡 [Robustness — _deliverOutput uses swap return value instead of balance-delta for output token transfer]: The _deliverOutput function transfers outputAmount (the return value from _swapExactInput) directly to the recipient, while input-side swaps use balance-delta accounting. (contracts/router/SignalsRouter.sol:827-828)

  Scenario: If the Algebra V2 swap router's exactInputSingle return value amountOut does not exactly match the actual token transfer amount (e.g., a fee-on-transfer output token is added to the allowlist in the future), the safeTransfer(recipient, outputAmount) would revert due to insufficient balance. The input-side already uses balance-delta (ctUSD.balanceOf(address(this)) - ctUSDBefore), which is immune to this class of bug.
  Evidence: Lines 716-717 use balance-delta for ctUSD received from swap. Lines 827-828 use return value for output token. The current allowlisted tokens (USDC.e, WCBTC) are standard ERC20s without transfer fees, so this is not an active bug — but applying the same balance-delta pattern for output tokens would make the router robust against future allowlist additions of fee-on-transfer tokens.

Verdict

REQUEST_CHANGES: Two suggestions remain unresolved.

[signals-reviewer]

Summary

This PR adds SignalsRouter, a non-upgradeable peripheral contract that atomically swaps input tokens to ctUSD via Algebra V2 (Satsuma DEX) and executes the corresponding Core operation in a single transaction. The router supports open, increase, decrease, close, and claim operations with token swap. It uses balance-delta accounting for ctUSD input flows and return-value accounting for output swaps. All entry points are nonReentrant, and the token allowlist is owner-gated with ctUSD excluded.

Cross-PR Context

Sibling PRs (SIG-605)

No sibling PRs found. The PR description correctly identifies downstream integration needs:

• v1-sdk: New SignalsRouter ABI export needed (ABI is already generated in abis/SignalsRouter.json)
• signals-app: Betting flow integration for non-ctUSD token paths

Since this is a new additive contract (no existing interface changes), downstream repos continue to function without modification. Sibling PRs can follow independently.

Issues

No issues found.

Suggestions

• 🟡 [Robustness — dangling ctUSD approval to Core after swap operations]: After openPositionWithSwap and increasePositionWithSwap, the router calls ctUSD.forceApprove(address(core), ctUSDReceived) but Core only pulls totalCost which may be less than ctUSDReceived. The remaining approval persists until the next forceApprove call. (contracts/router/SignalsRouter.sol:718)

  Scenario: User calls openPositionWithSwap with inputAmount = 100_000, swap returns 100_000 ctUSD, but maxCost matches actual cost of 80_000. The router approves Core for 100_000, Core pulls 80_000, leaving 20_000 ctUSD approval. The leftover ctUSD balance is correctly returned to the user via the delta refund, and the approval is harmless because Core is a trusted immutable proxy. However, resetting the approval to 0 after the Core call (ctUSD.forceApprove(address(core), 0)) follows the principle of least privilege and eliminates even theoretical risk from a future Core upgrade.
  Evidence: Line 718 approves ctUSDReceived, line 720 calls core.openPosition which pulls only totalCost <= maxCost. forceApprove on line 747 (increasePosition path) exhibits the same pattern. The refund on lines 722-725 correctly handles the balance delta, so no funds are at risk — this is a defense-in-depth suggestion.

• 🟡 [Robustness — _deliverOutput uses swap return value instead of balance-delta for output token transfer]: The _deliverOutput function transfers outputAmount (the return value from _swapExactInput) directly to the recipient, while input-side swaps use balance-delta accounting. (contracts/router/SignalsRouter.sol:827-828)

  Scenario: If the Algebra V2 swap router's exactInputSingle return value amountOut does not exactly match the actual token transfer amount (e.g., a fee-on-transfer output token is added to the allowlist in the future), the safeTransfer(recipient, outputAmount) would revert due to insufficient balance. The input-side already uses balance-delta (ctUSD.balanceOf(address(this)) - ctUSDBefore), which is immune to this class of bug.
  Evidence: Lines 716-717 use balance-delta for ctUSD received from swap. Lines 827-828 use return value for output token. The current allowlisted tokens (USDC.e, WCBTC) are standard ERC20s without transfer fees, so this is not an active bug — but applying the same balance-delta pattern for output tokens would make the router robust against future allowlist additions of fee-on-transfer tokens.

Verdict

REQUEST_CHANGES: Two suggestions remain unresolved.

[signals-reviewer]

Summary

This PR adds SignalsRouter, a non-upgradeable peripheral contract that atomically swaps input tokens to ctUSD via Algebra V2 (Satsuma DEX) and executes the corresponding Core operation in a single transaction. The router supports open, increase, decrease, close, and claim operations with token swap. It uses balance-delta accounting for ctUSD input flows and return-value accounting for output swaps. All entry points are nonReentrant, and the token allowlist is owner-gated with ctUSD excluded.

Cross-PR Context

Sibling PRs (SIG-605)

No sibling PRs found. The PR description correctly identifies downstream integration needs:

• v1-sdk: New SignalsRouter ABI export needed (ABI is already generated in abis/SignalsRouter.json)
• signals-app: Betting flow integration for non-ctUSD token paths

Since this is a new additive contract (no existing interface changes), downstream repos continue to function without modification. Sibling PRs can follow independently.

Issues

No issues found.

Suggestions

• 🟡 [Robustness — dangling ctUSD approval to Core after swap operations]: After openPositionWithSwap and increasePositionWithSwap, the router calls ctUSD.forceApprove(address(core), ctUSDReceived) but Core only pulls totalCost which may be less than ctUSDReceived. The remaining approval persists until the next forceApprove call. (contracts/router/SignalsRouter.sol:718)

  Scenario: User calls openPositionWithSwap with inputAmount = 100_000, swap returns 100_000 ctUSD, but maxCost matches actual cost of 80_000. The router approves Core for 100_000, Core pulls 80_000, leaving 20_000 ctUSD approval. The leftover ctUSD balance is correctly returned to the user via the delta refund, and the approval is harmless because Core is a trusted immutable proxy. However, resetting the approval to 0 after the Core call (ctUSD.forceApprove(address(core), 0)) follows the principle of least privilege and eliminates even theoretical risk from a future Core upgrade.
  Evidence: Line 718 approves ctUSDReceived, line 720 calls core.openPosition which pulls only totalCost <= maxCost. forceApprove on line 747 (increasePosition path) exhibits the same pattern. The refund on lines 722-725 correctly handles the balance delta, so no funds are at risk — this is a defense-in-depth suggestion.

• 🟡 [Robustness — _deliverOutput uses swap return value instead of balance-delta for output token transfer]: The _deliverOutput function transfers outputAmount (the return value from _swapExactInput) directly to the recipient, while input-side swaps use balance-delta accounting. (contracts/router/SignalsRouter.sol:827-828)

  Scenario: If the Algebra V2 swap router's exactInputSingle return value amountOut does not exactly match the actual token transfer amount (e.g., a fee-on-transfer output token is added to the allowlist in the future), the safeTransfer(recipient, outputAmount) would revert due to insufficient balance. The input-side already uses balance-delta (ctUSD.balanceOf(address(this)) - ctUSDBefore), which is immune to this class of bug.
  Evidence: Lines 716-717 use balance-delta for ctUSD received from swap. Lines 827-828 use return value for output token. The current allowlisted tokens (USDC.e, WCBTC) are standard ERC20s without transfer fees, so this is not an active bug — but applying the same balance-delta pattern for output tokens would make the router robust against future allowlist additions of fee-on-transfer tokens.

Verdict

REQUEST_CHANGES: Two suggestions remain unresolved.

[signals-reviewer]

Summary

This PR adds SignalsRouter, a non-upgradeable peripheral contract that atomically swaps input tokens to ctUSD via Algebra V2 (Satsuma DEX) and executes the corresponding Core operation in a single transaction. The router supports open, increase, decrease, close, and claim operations with token swap. It uses balance-delta accounting for ctUSD input flows and return-value accounting for output swaps. All entry points are nonReentrant, and the token allowlist is owner-gated with ctUSD excluded.

Cross-PR Context

Sibling PRs (SIG-605)

No sibling PRs found. The PR description correctly identifies downstream integration needs:

• v1-sdk: New SignalsRouter ABI export needed (ABI is already generated in abis/SignalsRouter.json)
• signals-app: Betting flow integration for non-ctUSD token paths

Since this is a new additive contract (no existing interface changes), downstream repos continue to function without modification. Sibling PRs can follow independently.

Issues

No issues found.

Suggestions

• 🟡 [Robustness — dangling ctUSD approval to Core after swap operations]: After openPositionWithSwap and increasePositionWithSwap, the router calls ctUSD.forceApprove(address(core), ctUSDReceived) but Core only pulls totalCost which may be less than ctUSDReceived. The remaining approval persists until the next forceApprove call. (contracts/router/SignalsRouter.sol:718)

  Scenario: User calls openPositionWithSwap with inputAmount = 100_000, swap returns 100_000 ctUSD, but maxCost matches actual cost of 80_000. The router approves Core for 100_000, Core pulls 80_000, leaving 20_000 ctUSD approval. The leftover ctUSD balance is correctly returned to the user via the delta refund, and the approval is harmless because Core is a trusted immutable proxy. However, resetting the approval to 0 after the Core call (ctUSD.forceApprove(address(core), 0)) follows the principle of least privilege and eliminates even theoretical risk from a future Core upgrade.
  Evidence: Line 718 approves ctUSDReceived, line 720 calls core.openPosition which pulls only totalCost <= maxCost. forceApprove on line 747 (increasePosition path) exhibits the same pattern. The refund on lines 722-725 correctly handles the balance delta, so no funds are at risk — this is a defense-in-depth suggestion.

• 🟡 [Robustness — _deliverOutput uses swap return value instead of balance-delta for output token transfer]: The _deliverOutput function transfers outputAmount (the return value from _swapExactInput) directly to the recipient, while input-side swaps use balance-delta accounting. (contracts/router/SignalsRouter.sol:827-828)

  Scenario: If the Algebra V2 swap router's exactInputSingle return value amountOut does not exactly match the actual token transfer amount (e.g., a fee-on-transfer output token is added to the allowlist in the future), the safeTransfer(recipient, outputAmount) would revert due to insufficient balance. The input-side already uses balance-delta (ctUSD.balanceOf(address(this)) - ctUSDBefore), which is immune to this class of bug.
  Evidence: Lines 716-717 use balance-delta for ctUSD received from swap. Lines 827-828 use return value for output token. The current allowlisted tokens (USDC.e, WCBTC) are standard ERC20s without transfer fees, so this is not an active bug — but applying the same balance-delta pattern for output tokens would make the router robust against future allowlist additions of fee-on-transfer tokens.

Verdict

REQUEST_CHANGES: Two suggestions remain unresolved.

[signals-reviewer]

Summary

This PR adds SignalsRouter, a non-upgradeable peripheral contract that atomically swaps input tokens to ctUSD via Algebra V2 (Satsuma DEX) and executes the corresponding Core operation in a single transaction. The router supports open, increase, decrease, close, and claim operations with token swap. It uses balance-delta accounting for ctUSD input flows and return-value accounting for output swaps. All entry points are nonReentrant, and the token allowlist is owner-gated with ctUSD excluded.

Cross-PR Context

Sibling PRs (SIG-605)

No sibling PRs found. The PR description correctly identifies downstream integration needs:

• v1-sdk: New SignalsRouter ABI export needed (ABI is already generated in abis/SignalsRouter.json)
• signals-app: Betting flow integration for non-ctUSD token paths

Since this is a new additive contract (no existing interface changes), downstream repos continue to function without modification. Sibling PRs can follow independently.

Issues

No issues found.

Suggestions

• 🟡 [Robustness — dangling ctUSD approval to Core after swap operations]: After openPositionWithSwap and increasePositionWithSwap, the router calls ctUSD.forceApprove(address(core), ctUSDReceived) but Core only pulls totalCost which may be less than ctUSDReceived. The remaining approval persists until the next forceApprove call. (contracts/router/SignalsRouter.sol:718)

  Scenario: User calls openPositionWithSwap with inputAmount = 100_000, swap returns 100_000 ctUSD, but maxCost matches actual cost of 80_000. The router approves Core for 100_000, Core pulls 80_000, leaving 20_000 ctUSD approval. The leftover ctUSD balance is correctly returned to the user via the delta refund, and the approval is harmless because Core is a trusted immutable proxy. However, resetting the approval to 0 after the Core call (ctUSD.forceApprove(address(core), 0)) follows the principle of least privilege and eliminates even theoretical risk from a future Core upgrade.
  Evidence: Line 718 approves ctUSDReceived, line 720 calls core.openPosition which pulls only totalCost <= maxCost. forceApprove on line 747 (increasePosition path) exhibits the same pattern. The refund on lines 722-725 correctly handles the balance delta, so no funds are at risk — this is a defense-in-depth suggestion.

• 🟡 [Robustness — _deliverOutput uses swap return value instead of balance-delta for output token transfer]: The _deliverOutput function transfers outputAmount (the return value from _swapExactInput) directly to the recipient, while input-side swaps use balance-delta accounting. (contracts/router/SignalsRouter.sol:827-828)

  Scenario: If the Algebra V2 swap router's exactInputSingle return value amountOut does not exactly match the actual token transfer amount (e.g., a fee-on-transfer output token is added to the allowlist in the future), the safeTransfer(recipient, outputAmount) would revert due to insufficient balance. The input-side already uses balance-delta (ctUSD.balanceOf(address(this)) - ctUSDBefore), which is immune to this class of bug.
  Evidence: Lines 716-717 use balance-delta for ctUSD received from swap. Lines 827-828 use return value for output token. The current allowlisted tokens (USDC.e, WCBTC) are standard ERC20s without transfer fees, so this is not an active bug — but applying the same balance-delta pattern for output tokens would make the router robust against future allowlist additions of fee-on-transfer tokens.

Verdict

REQUEST_CHANGES: Two suggestions remain unresolved.

[worjs]

Review Response

Applied

Output token balance-delta — _deliverOutput now uses balanceOf delta instead of swap return value, matching input-side accounting. Commit 648b4bc.

Rebutted

ctUSD approval reset after Core call — Core is an immutable UUPS proxy. The only entity that can pull from the Router's ctUSD approval is SignalsCore.openPosition/increasePosition via _pullPayment, which runs inside nonReentrant. A future Core upgrade would require Safe multisig governance — the same governance that controls the Router's Ownable. Adding forceApprove(core, 0) after every Core call costs ~5,200 gas per tx for zero practical security gain. The reviewer acknowledges "no funds are at risk."

[signals-reviewer]

Re-review Summary

Re-reviewing after 2 new commits since last review: f2d5b7e (ABI regen) and 648b4bc (balance-delta fix for output delivery). 1 file changed in the substantive fix commit.

Previous Items

• ✅ [Resolved]: _deliverOutput uses swap return value instead of balance-delta — resolved by commit 648b4bc. The function now reads outToken.balanceOf(address(this)) before and after the swap, transferring only the delta. This matches input-side accounting and protects against fee-on-transfer tokens.

• 🔄 [Accepted]: Dangling ctUSD approval to Core after swap operations — author's rebuttal is technically sound: Core is a UUPS proxy whose upgrades require the same Safe multisig governance that controls the Router's Ownable. The Router holds zero ctUSD between transactions (excess is refunded via balance-delta), so the dangling approval allows pulling 0 tokens. Adding forceApprove(core, 0) costs ~5,200 gas per tx. The threat model (compromised governance) makes this defense moot since governance controls both the Router and Core upgrade path. The cost-benefit analysis is concrete and evidence-based.

New Issues

No new issues found. Fresh review of the full contract confirms:

• All entry points are nonReentrant with correct allowlist enforcement
• Balance-delta accounting is consistent across both input (ctUSD from swap) and output (output token from swap) paths
• NFT lifecycle is correct: _takePosition uses transferFrom (requires prior approval), _returnPositionIfNeeded checks exists() before attempting return, onERC721Received correctly rejects non-mint transfers
• ctUSD shortcut in _deliverOutput correctly bypasses allowlist check (ctUSD is always valid output)
• Dust protection: pre-existing ctUSD balance in the router is never leaked to users (ctUSDBefore snapshot isolates the transaction's delta)
• Test coverage is comprehensive: happy paths, slippage reverts, rollback scenarios, access control, dust preservation, paused-state claim, and losing position claim

Verdict

APPROVE: All previous items are either Resolved (1) or Accepted (1). No new issues found.