Repository navigation
Refactor swap quote state and amount-policy ownership #1456
Description
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_amountthrough 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). ReviewSwapQuoteDataType::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 optionalamount_modesnapshot 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_quotecurrently 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.
Activity