diff --git a/src/components/Swap/Summary/index.test.tsx b/src/components/Swap/Summary/index.test.tsx index 193e32432..b830f4e7f 100644 --- a/src/components/Swap/Summary/index.test.tsx +++ b/src/components/Swap/Summary/index.test.tsx @@ -47,10 +47,8 @@ function getInitialTradeState(trade: Partial = {}) { } } -// eslint-disable-next-line @typescript-eslint/no-empty-function -const EMPTY_ASYNC_FUCTION = async () => {} -const EMPTY_PROMISE_FUNCTION = async () => { - return new Promise(EMPTY_ASYNC_FUCTION) +const noopAsync = async () => { + return new Promise(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, diff --git a/src/components/Swap/Summary/index.tsx b/src/components/Swap/Summary/index.tsx index dddaaed70..22358befc 100644 --- a/src/components/Swap/Summary/index.tsx +++ b/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, 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 - 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: Price updated, - icon: AlertTriangle, - tooltipContent: ( - - - - ), - onClick: onAcknowledgeClick, - children: Accept, - }, - ] - default: - return [] + case ReviewState.REVIEWING: + return doesTradeDiffer + ? [ + { + color: 'accent', + message: Price updated, + icon: AlertTriangle, + tooltipContent: ( + + + + ), + onClick: onAcknowledgeClick, + children: Swap, + }, + ] + : [] } }, [ allowance.state, currentState, + doesTradeDiffer, isApprovalLoading, onAcknowledgeClick, onCancel, @@ -257,15 +256,17 @@ 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(() => { @@ -273,10 +274,11 @@ export function SummaryDialog(props: SummaryDialogProps) { setShowSpeedbump(false) } }, [ackPriceImpact, props.impact, showSpeedbump]) + return ( <> {showSpeedbump && props.impact ? ( - + {t`This transaction will result in a`} {props.impact.toString()} {t`price impact on the market price of this pool. Do you wish to continue? `} @@ -286,7 +288,7 @@ export function SummaryDialog(props: SummaryDialogProps) {
- + )} diff --git a/src/constants/misc.ts b/src/constants/misc.ts index ca5a7533e..e39053653 100644 --- a/src/constants/misc.ts +++ b/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) diff --git a/src/utils/tradeMeaningFullyDiffer.ts b/src/utils/tradeMeaningFullyDiffer.ts index 444be3b9d..2233784cc 100644 --- a/src/utils/tradeMeaningFullyDiffer.ts +++ b/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)) ) }