feat: threshold and state simplification (#475)

* feat: threshold and state simplification

* refactor: polish

* fix: pr comments

* fix: test functions

src/components/Swap/Summary/index.test.tsx

@@ -47,10 +47,8 @@ function getInitialTradeState(trade: Partial<Swap> = {}) {
   }
 }
 
-// eslint-disable-next-line @typescript-eslint/no-empty-function
-const EMPTY_ASYNC_FUCTION = async () => {}
-const EMPTY_PROMISE_FUNCTION = async () => {
-  return new Promise<void>(EMPTY_ASYNC_FUCTION)
+const noopAsync = async () => {
+  return new Promise<void>(jest.fn)
 }
 function Summary({ allowance }: { allowance: usePermit2Allowance.Allowance }) {
   return (
@@ -62,8 +60,8 @@ function Summary({ allowance }: { allowance: usePermit2Allowance.Allowance }) {
           tradeType: TradeType.EXACT_INPUT,
         })
       }
-      onConfirm={EMPTY_PROMISE_FUNCTION}
-      onAcknowledgeNewTrade={EMPTY_ASYNC_FUCTION}
+      onConfirm={noopAsync}
+      triggerImpactSpeedbump={() => false}
       allowance={allowance}
       slippage={{
         auto: true,

src/components/Swap/Summary/index.tsx

@@ -27,7 +27,6 @@ enum ReviewState {
   REVIEWING,
   ALLOWING,
   ALLOWANCE_FAILED,
-  TRADE_CHANGED,
   SWAP_PENDING,
 }
 
@@ -45,25 +44,22 @@ function useReviewState(onSwap: () => Promise<void>, allowance: Allowance, doesT
       }
       // if the user finishes permit2 allowance flow, onStartSwapFlow() will be called again by useEffect below to trigger swap
     } else if (allowance.state === AllowanceState.ALLOWED) {
-      if (doesTradeDiffer) {
-        setCurrentState(ReviewState.TRADE_CHANGED)
+      // Prevents immediate swap if trade has updated mid permit2 flow
+      if (currentState === ReviewState.ALLOWING && doesTradeDiffer) {
+        setCurrentState(ReviewState.REVIEWING)
         return
+      } else {
+        setCurrentState(ReviewState.SWAP_PENDING)
+        await onSwap()
+        setCurrentState(ReviewState.REVIEWING)
       }
-      setCurrentState(ReviewState.SWAP_PENDING)
-      await onSwap()
-      setCurrentState(ReviewState.REVIEWING)
     }
-  }, [allowance, doesTradeDiffer, onSwap])
+  }, [allowance, currentState, doesTradeDiffer, onSwap])
 
   // Automatically triggers signing swap tx if allowance requirements are met
   useEffect(() => {
-    // Prevents swap if trade has updated mid permit2 flow
-    if (doesTradeDiffer && currentState === ReviewState.REVIEWING) {
-      setCurrentState(ReviewState.TRADE_CHANGED)
-    } else if (currentState === ReviewState.ALLOWING && allowance.state === AllowanceState.ALLOWED) {
+    if (currentState === ReviewState.ALLOWING && allowance.state === AllowanceState.ALLOWED) {
       onStartSwapFlow()
-    } else if (!doesTradeDiffer && currentState === ReviewState.TRADE_CHANGED) {
-      setCurrentState(ReviewState.REVIEWING)
     }
   }, [allowance, currentState, doesTradeDiffer, onStartSwapFlow])
 
@@ -146,20 +142,20 @@ export function ConfirmButton({
   trade,
   slippage,
   onConfirm,
-  onAcknowledgeNewTrade,
+  triggerImpactSpeedbump,
   allowance,
 }: {
   trade: InterfaceTrade
   slippage: Slippage
   onConfirm: () => Promise<void>
-  onAcknowledgeNewTrade: () => void
+  triggerImpactSpeedbump: () => boolean
   allowance: Allowance
 }) {
   const { onSwapPriceUpdateAck, onSubmitSwapClick } = useAtomValue(swapEventHandlersAtom)
   const [ackTrade, setAckTrade] = useState(trade)
   const doesTradeDiffer = useMemo(
-    () => Boolean(trade && ackTrade && tradeMeaningfullyDiffers(trade, ackTrade)),
-    [ackTrade, trade]
+    () => Boolean(trade && ackTrade && tradeMeaningfullyDiffers(trade, ackTrade, slippage.allowed)),
+    [ackTrade, trade, slippage]
   )
   const onSwap = useCallback(async () => {
     onSubmitSwapClick?.(trade)
@@ -180,9 +176,11 @@ export function ConfirmButton({
   const onAcknowledgeClick = useCallback(() => {
     onSwapPriceUpdateAck?.(ackTrade, trade)
     setAckTrade(trade)
-    // Prompts parent to show speedbump if new trade has high impact
-    onAcknowledgeNewTrade()
-  }, [ackTrade, onAcknowledgeNewTrade, onSwapPriceUpdateAck, trade])
+
+    const wasInterrupted = triggerImpactSpeedbump()
+    // Prevents immeadiate swap if price impact speedbump was triggered
+    if (!wasInterrupted) onStartSwapFlow()
+  }, [ackTrade, triggerImpactSpeedbump, onStartSwapFlow, onSwapPriceUpdateAck, trade])
 
   const [action, color] = useMemo((): [Action?, ActionButtonColor?] => {
     switch (currentState) {
@@ -205,27 +203,28 @@ export function ConfirmButton({
           getAllowanceFailedAction(shouldRequestApproval, onStartSwapFlow, trade.inputAmount.currency),
           'warningSoft',
         ]
-      case ReviewState.TRADE_CHANGED:
-        return [
-          {
-            color: 'accent',
-            message: <Trans>Price updated</Trans>,
-            icon: AlertTriangle,
-            tooltipContent: (
-              <SmallToolTipBody>
-                <SwapInputOutputEstimate trade={trade} slippage={slippage} />
-              </SmallToolTipBody>
-            ),
-            onClick: onAcknowledgeClick,
-            children: <Trans>Accept</Trans>,
-          },
-        ]
-      default:
-        return []
+      case ReviewState.REVIEWING:
+        return doesTradeDiffer
+          ? [
+              {
+                color: 'accent',
+                message: <Trans>Price updated</Trans>,
+                icon: AlertTriangle,
+                tooltipContent: (
+                  <SmallToolTipBody>
+                    <SwapInputOutputEstimate trade={trade} slippage={slippage} />
+                  </SmallToolTipBody>
+                ),
+                onClick: onAcknowledgeClick,
+                children: <Trans>Swap</Trans>,
+              },
+            ]
+          : []
     }
   }, [
     allowance.state,
     currentState,
+    doesTradeDiffer,
     isApprovalLoading,
     onAcknowledgeClick,
     onCancel,
@@ -257,26 +256,29 @@ export function SummaryDialog(props: SummaryDialogProps) {
   const [ackPriceImpact, setAckPriceImpact] = useState(false)
   const [showSpeedbump, setShowSpeedbump] = useState(props.impact?.warning === 'error')
 
-  const onAcknowledgePriceImpact = useCallback(() => {
+  const onAcknowledgeSpeedbump = useCallback(() => {
     setAckPriceImpact(true)
     setShowSpeedbump(false)
   }, [])
 
-  const onAcknowledgeNewTrade = useCallback(() => {
+  const triggerImpactSpeedbump = useCallback((): boolean => {
     if (!showSpeedbump && !ackPriceImpact && props.impact?.warning === 'error') {
       setShowSpeedbump(true)
+      return true
     }
+    return false
   }, [ackPriceImpact, props.impact?.warning, showSpeedbump])
 
   useEffect(() => {
     if (showSpeedbump && props.impact?.warning !== 'error') {
       setShowSpeedbump(false)
     }
   }, [ackPriceImpact, props.impact, showSpeedbump])
+
   return (
     <>
       {showSpeedbump && props.impact ? (
-        <SpeedBumpDialog onAcknowledge={onAcknowledgePriceImpact}>
+        <SpeedBumpDialog onAcknowledge={onAcknowledgeSpeedbump}>
           {t`This transaction will result in a`} <PriceImpactText>{props.impact.toString()} </PriceImpactText>
           {t`price impact on the market price of this pool. Do you wish to continue? `}
         </SpeedBumpDialog>
@@ -286,7 +288,7 @@ export function SummaryDialog(props: SummaryDialogProps) {
           <Body flex align="stretch">
             <Details {...props} />
           </Body>
-          <ConfirmButton {...props} onAcknowledgeNewTrade={onAcknowledgeNewTrade} />
+          <ConfirmButton {...props} triggerImpactSpeedbump={triggerImpactSpeedbump} />
         </>
       )}
     </>

src/constants/misc.ts

@@ -17,9 +17,6 @@ export const L2_DEADLINE_FROM_NOW = 60 * 5
 export const DEFAULT_TXN_DISMISS_MS = 25000
 export const L2_TXN_DISMISS_MS = 5000
 
-// threshold for when to show a price changed warning
-export const DEFAULT_PRICE_CHANGED_THRESHOLD = 0.01
-
 // used for rewards deadlines
 export const BIG_INT_SECONDS_IN_WEEK = JSBI.BigInt(60 * 60 * 24 * 7)
 

src/utils/tradeMeaningFullyDiffer.ts

@@ -1,16 +1,15 @@
+import { Percent } from '@uniswap/sdk-core'
 import { InterfaceTrade } from 'state/routing/types'
 
 /**
  * Returns true if the trade requires a confirmation of details before we can submit it
  * @param args either a pair of V2 trades or a pair of V3 trades
  */
-export function tradeMeaningfullyDiffers(...args: [InterfaceTrade, InterfaceTrade]): boolean {
-  const [tradeA, tradeB] = args
+export function tradeMeaningfullyDiffers(tradeA: InterfaceTrade, tradeB: InterfaceTrade, slippage: Percent): boolean {
   return (
     tradeA.tradeType !== tradeB.tradeType ||
     !tradeA.inputAmount.currency.equals(tradeB.inputAmount.currency) ||
-    !tradeA.inputAmount.equalTo(tradeB.inputAmount) ||
     !tradeA.outputAmount.currency.equals(tradeB.outputAmount.currency) ||
-    !tradeA.outputAmount.equalTo(tradeB.outputAmount)
+    tradeB.executionPrice.lessThan(tradeA.worstExecutionPrice(slippage))
   )
 }