Skip to content

Refactor swap quote state and amount-policy ownership #1456

Description

@0xh3rman

Problem

SwapQuote and the surrounding request, transfer, and signer models mix user intent, provider amount policy, quote results, and transaction execution details. Some state is duplicated, and some consumers infer amount policy from transaction type.

This is an internal refactoring task, not a user-reported incident. PR #1453 is closed; use it as investigation context, not as an implementation to preserve. Start from current main and address the ownership properly in a separate PR.

Scope

  • Max intent: trace use_max_amount through request options, GemSwapTransfer, GemTransferData, SwapQuote, and signer input. Establish an authoritative source at each boundary and remove redundant copies. In particular, review the nested optional quote flag and signers that prefer it over the outer input flag.
  • Amount policy versus execution type: keep provider input requirements separate from SwapQuoteDataType::Contract / Transfer. Existing examples include Swaps.xyz (Fixed + Transfer), NEAR Intents (Flexible + Transfer), Hyperliquid bridge (Flexible + Contract), and Uniswap (Fixed + Contract). Review SwapQuoteDataType::has_fixed_value, which currently assumes all transfers are flexible. A flexible provider does not make an already-built transaction payload safe to edit without rebuilding it.
  • Policy ownership: use the existing Swapper::amount_mode(request) contract as the starting point. Determine whether confirmation can obtain policy from the swap service instead of adding the optional amount_mode snapshot proposed in Swap all but the fee and enforce Minimum Receive on THORChain #1453 to the public quote. Avoid parallel sources of truth and ambiguous runtime defaults.
  • Slippage: distinguish the user's Auto/Manual selection from the effective tolerance returned with a quote. swap_quote currently combines request mode with provider-returned bps, and re-quoting reconstructs settings from that combination. Preserve the original selection and reuse existing domain types where appropriate.
  • Remaining quote fields: trace production readers of addresses, provider metadata, values, and optional fields. Remove unused or redundant state only after checking consumers and serialization requirements. Quoted input, transaction native value, and final spend can legitimately differ, especially for token contract calls.

Investigation evidence

An isolated amount-calculator reproducer against #1453 accepted a final amount of 990 for a Fixed transfer quoted at 995, with balance 1,000 and fee 10. The calculator used execution type rather than provider amount policy. Reproduce against current main and trace the actual confirmation and signing path before concluding which routes are affected.

An earlier double-fee-deduction hypothesis was withdrawn: calling SignerInput::swap_value directly did not exercise the app path. Gemstone's sign_input dispatcher rewrites transfer swaps into ordinary transfers. Do not treat double deduction as an established app defect.

Acceptance criteria

  • Document and implement clear ownership for user intent, provider policy, quote results, and prepared transaction data, with minimal additional abstractions.
  • Fixed-input routes are not silently reduced to cover fees. Any changed amount is handled through the appropriate re-quote/rebuild or an explicit error.
  • Confirmation displays the amount that the actual signer path will send; fee adjustment has a clear owner.
  • Preserve max/non-max behavior, native/token fee handling, and Auto/Manual slippage across refreshes.
  • Add focused regression coverage through the real confirmation-to-signer path for demonstrated defects, including fixed and flexible deposits and contract calls.
  • Inspect persisted/serialized consumers before changing compatibility behavior. Regenerate shared bindings and verify both mobile apps when contracts change.
  • Follow repository cleanup and verification requirements; report exact checks and any untested paths.

Keep unrelated provider changes out of this refactor. Reassess the closed PR's max-fit and THORChain minimum-enforcement proposals separately against the resulting contract rather than carrying them over automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions