From d9cc43415fded1183fa85a06321c340fe73f7cd5 Mon Sep 17 00:00:00 2001 From: sirpy Date: Thu, 25 Jul 2024 08:44:55 +0000 Subject: [PATCH 1/2] fix: working regular token approve when permit2 disabled --- src/components/Swap/Summary/index.tsx | 25 +++++++++++++------ .../Swap/SwapActionButton/SwapButton.tsx | 1 + .../Swap/SwapActionButton/index.tsx | 5 +--- src/cosmos/Swap.fixture.tsx | 4 ++- src/hooks/swap/useSwapApproval.ts | 3 +-- 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/components/Swap/Summary/index.tsx b/src/components/Swap/Summary/index.tsx index 238b2b5ef..9ee9fe76f 100644 --- a/src/components/Swap/Summary/index.tsx +++ b/src/components/Swap/Summary/index.tsx @@ -7,7 +7,7 @@ import { Header, MIN_PAGE_CENTERED_DIALOG_WIDTH, useCloseDialog, useIsDialogPage import { PopoverBoundaryProvider } from 'components/Popover' import { SmallToolTipBody, TooltipText } from 'components/Tooltip' import { UserRejectedRequestError } from 'errors' -import { Allowance, AllowanceState } from 'hooks/usePermit2Allowance' +import { Allowance, AllowanceRequired, AllowanceState } from 'hooks/usePermit2Allowance' import { PriceImpact } from 'hooks/usePriceImpact' import { Slippage } from 'hooks/useSlippage' import { useWindowWidth } from 'hooks/useWindowWidth' @@ -24,6 +24,8 @@ import SpeedBumpDialog from '../Speedbump' import Details from './Details' import SwapInputOutputEstimate from './Estimate' import Summary from './Summary' +import { ApprovalState, SwapApproval, SwapApprovalState } from 'hooks/swap/useSwapApproval' +import { usePermit2 as usePermit2Enabled } from 'hooks/useSyncFlags' export default Summary @@ -34,15 +36,16 @@ enum ReviewState { SWAP_PENDING, } -function useReviewState(onSwap: () => Promise, allowance: Allowance, doesTradeDiffer: boolean) { +function useReviewState(onSwap: () => Promise, allowance: Allowance, approval: SwapApproval, doesTradeDiffer: boolean) { const [currentState, setCurrentState] = useState(ReviewState.REVIEWING) const closeDialog = useCloseDialog() + const permit2Enabled = usePermit2Enabled() const onStartSwapFlow = useCallback(async () => { - if (allowance.state === AllowanceState.REQUIRED) { + if (currentState!== ReviewState.ALLOWING && (allowance.state === AllowanceState.REQUIRED || approval.state === SwapApprovalState.REQUIRES_APPROVAL || approval.state === SwapApprovalState.REQUIRES_SIGNATURE)) { setCurrentState(ReviewState.ALLOWING) try { - await allowance.approveAndPermit?.() + await ((allowance as AllowanceRequired).approveAndPermit ?? approval.approve)?.() } catch (e) { if (e instanceof UserRejectedRequestError) { closeDialog?.() @@ -52,7 +55,7 @@ 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) { + } else if ((permit2Enabled && allowance.state === AllowanceState.ALLOWED) || approval.state === SwapApprovalState.APPROVED) { // Prevents immediate swap if trade has updated mid permit2 flow if (currentState === ReviewState.ALLOWING && doesTradeDiffer) { setCurrentState(ReviewState.REVIEWING) @@ -152,12 +155,14 @@ export function ConfirmButton({ onConfirm, triggerImpactSpeedbump, allowance, + approval }: { trade: InterfaceTrade slippage: Slippage onConfirm: () => Promise triggerImpactSpeedbump: () => boolean allowance: Allowance + approval:SwapApproval }) { const { onSwapPriceUpdateAck, onSubmitSwapClick } = useAtomValue(swapEventHandlersAtom) const [ackTrade, setAckTrade] = useState(trade) @@ -165,20 +170,22 @@ export function ConfirmButton({ () => Boolean(trade && ackTrade && tradeMeaningfullyDiffers(trade, ackTrade, slippage.allowed)), [ackTrade, trade, slippage] ) + const onSwap = useCallback(async () => { onSubmitSwapClick?.(trade) await onConfirm() }, [onConfirm, onSubmitSwapClick, trade]) - const { onStartSwapFlow, onCancel, currentState } = useReviewState(onSwap, allowance, doesTradeDiffer) + const { onStartSwapFlow, onCancel, currentState } = useReviewState(onSwap, allowance,approval, doesTradeDiffer) // Used to determine specific message to render while in ALLOWANCE_PROMPTED state const [shouldRequestApproval, isApprovalLoading] = useMemo( () => + approval.approve ?[false, approval.state === SwapApprovalState.PENDING_APPROVAL || approval.state === SwapApprovalState.PENDING_SIGNATURE] : allowance.state === AllowanceState.REQUIRED ? [allowance.shouldRequestApproval, allowance.isApprovalLoading] : [false, false], - [allowance] + [allowance, approval] ) const onAcknowledgeClick = useCallback(() => { @@ -203,7 +210,7 @@ export function ConfirmButton({ 'interactive', ] case ReviewState.ALLOWING: - return isApprovalLoading || allowance.state === AllowanceState.ALLOWED + return isApprovalLoading || (!approval.approve && allowance.state === AllowanceState.ALLOWED) ? [getApprovalLoadingAction()] : [getAllowancePendingAction(shouldRequestApproval, onCancel, trade.inputAmount.currency)] case ReviewState.ALLOWANCE_FAILED: @@ -231,6 +238,7 @@ export function ConfirmButton({ } }, [ allowance.state, + approval.state, currentState, doesTradeDiffer, isApprovalLoading, @@ -258,6 +266,7 @@ interface SummaryDialogProps { impact?: PriceImpact onConfirm: () => Promise allowance: Allowance + approval: SwapApproval } export function SummaryDialog(props: SummaryDialogProps) { diff --git a/src/components/Swap/SwapActionButton/SwapButton.tsx b/src/components/Swap/SwapActionButton/SwapButton.tsx index e40f92c1e..6f824ddf1 100644 --- a/src/components/Swap/SwapActionButton/SwapButton.tsx +++ b/src/components/Swap/SwapActionButton/SwapButton.tsx @@ -123,6 +123,7 @@ export default function SwapButton({ disabled }: { disabled: boolean }) { impact={impact} onConfirm={onSwap} allowance={allowance} + approval={approval} /> )} diff --git a/src/components/Swap/SwapActionButton/index.tsx b/src/components/Swap/SwapActionButton/index.tsx index f7a6b8e73..35b6aa3c6 100644 --- a/src/components/Swap/SwapActionButton/index.tsx +++ b/src/components/Swap/SwapActionButton/index.tsx @@ -4,7 +4,6 @@ import { SupportedChainId } from 'constants/chains' import { ChainError, useSwapInfo } from 'hooks/swap' import { SwapApprovalState } from 'hooks/swap/useSwapApproval' import { useIsWrap } from 'hooks/swap/useWrapCallback' -import { usePermit2 as usePermit2Enabled } from 'hooks/useSyncFlags' import { useMemo } from 'react' import { Field } from 'state/swap' @@ -23,16 +22,14 @@ export default function SwapActionButton() { trade: { trade }, } = useSwapInfo() const isWrap = useIsWrap() - const permit2Enabled = usePermit2Enabled() const isDisabled = useMemo( () => - (!permit2Enabled && approval.state !== SwapApprovalState.APPROVED) || error !== undefined || (!isWrap && !trade) || !inputCurrencyAmount || // If there is no balance loaded, we should default to isDisabled=false Boolean(inputCurrencyBalance?.lessThan(inputCurrencyAmount)), - [permit2Enabled, approval.state, error, isWrap, trade, inputCurrencyAmount, inputCurrencyBalance] + [approval.state, error, isWrap, trade, inputCurrencyAmount, inputCurrencyBalance] ) if (!account || !isActive) { diff --git a/src/cosmos/Swap.fixture.tsx b/src/cosmos/Swap.fixture.tsx index 827cae2ad..ff88c8f75 100644 --- a/src/cosmos/Swap.fixture.tsx +++ b/src/cosmos/Swap.fixture.tsx @@ -43,6 +43,8 @@ function Fixture() { [] ) + const [permit2] = useValue('permit2Enabled', { defaultValue: true }) + const [convenienceFee] = useValue('convenienceFee', { defaultValue: 0 }) const convenienceFeeRecipient = useOption('convenienceFeeRecipient', { options: [ @@ -98,7 +100,7 @@ function Fixture() { const widget = ( ): SwapApproval await approve() } } catch (e) { - // Swallow approval errors - user rejections do not need to be displayed. - return + throw e } onSwapApprove?.() } From db37c6a2b49c44a47ec0bfa4ef7df096aeb22a95 Mon Sep 17 00:00:00 2001 From: sirpy Date: Mon, 12 Aug 2024 10:35:05 +0000 Subject: [PATCH 2/2] fix: settings reset --- src/components/Swap/Summary/index.test.tsx | 1 + src/components/Swap/Summary/index.tsx | 16 ++++++++-------- src/cosmos/Swap.fixture.tsx | 8 ++++++++ src/hooks/swap/useSyncController.ts | 14 ++++++++------ src/test/index.tsx | 4 ++-- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/components/Swap/Summary/index.test.tsx b/src/components/Swap/Summary/index.test.tsx index b830f4e7f..0bb7cd703 100644 --- a/src/components/Swap/Summary/index.test.tsx +++ b/src/components/Swap/Summary/index.test.tsx @@ -63,6 +63,7 @@ function Summary({ allowance }: { allowance: usePermit2Allowance.Allowance }) { onConfirm={noopAsync} triggerImpactSpeedbump={() => false} allowance={allowance} + approval={undefined} slippage={{ auto: true, allowed: new Percent(1, 100), diff --git a/src/components/Swap/Summary/index.tsx b/src/components/Swap/Summary/index.tsx index 9ee9fe76f..d0aed8bd1 100644 --- a/src/components/Swap/Summary/index.tsx +++ b/src/components/Swap/Summary/index.tsx @@ -36,16 +36,16 @@ enum ReviewState { SWAP_PENDING, } -function useReviewState(onSwap: () => Promise, allowance: Allowance, approval: SwapApproval, doesTradeDiffer: boolean) { +function useReviewState(onSwap: () => Promise, allowance: Allowance, approval: SwapApproval | undefined, doesTradeDiffer: boolean) { const [currentState, setCurrentState] = useState(ReviewState.REVIEWING) const closeDialog = useCloseDialog() const permit2Enabled = usePermit2Enabled() const onStartSwapFlow = useCallback(async () => { - if (currentState!== ReviewState.ALLOWING && (allowance.state === AllowanceState.REQUIRED || approval.state === SwapApprovalState.REQUIRES_APPROVAL || approval.state === SwapApprovalState.REQUIRES_SIGNATURE)) { + if (currentState!== ReviewState.ALLOWING && (allowance.state === AllowanceState.REQUIRED || approval?.state === SwapApprovalState.REQUIRES_APPROVAL || approval?.state === SwapApprovalState.REQUIRES_SIGNATURE)) { setCurrentState(ReviewState.ALLOWING) try { - await ((allowance as AllowanceRequired).approveAndPermit ?? approval.approve)?.() + await ((allowance as AllowanceRequired).approveAndPermit ?? approval?.approve)?.() } catch (e) { if (e instanceof UserRejectedRequestError) { closeDialog?.() @@ -55,7 +55,7 @@ function useReviewState(onSwap: () => Promise, allowance: Allowance, appro } } // if the user finishes permit2 allowance flow, onStartSwapFlow() will be called again by useEffect below to trigger swap - } else if ((permit2Enabled && allowance.state === AllowanceState.ALLOWED) || approval.state === SwapApprovalState.APPROVED) { + } else if ((permit2Enabled && allowance.state === AllowanceState.ALLOWED) || approval?.state === SwapApprovalState.APPROVED) { // Prevents immediate swap if trade has updated mid permit2 flow if (currentState === ReviewState.ALLOWING && doesTradeDiffer) { setCurrentState(ReviewState.REVIEWING) @@ -162,7 +162,7 @@ export function ConfirmButton({ onConfirm: () => Promise triggerImpactSpeedbump: () => boolean allowance: Allowance - approval:SwapApproval + approval?:SwapApproval }) { const { onSwapPriceUpdateAck, onSubmitSwapClick } = useAtomValue(swapEventHandlersAtom) const [ackTrade, setAckTrade] = useState(trade) @@ -181,7 +181,7 @@ export function ConfirmButton({ // Used to determine specific message to render while in ALLOWANCE_PROMPTED state const [shouldRequestApproval, isApprovalLoading] = useMemo( () => - approval.approve ?[false, approval.state === SwapApprovalState.PENDING_APPROVAL || approval.state === SwapApprovalState.PENDING_SIGNATURE] : + approval?.approve ?[false, approval?.state === SwapApprovalState.PENDING_APPROVAL || approval?.state === SwapApprovalState.PENDING_SIGNATURE] : allowance.state === AllowanceState.REQUIRED ? [allowance.shouldRequestApproval, allowance.isApprovalLoading] : [false, false], @@ -210,7 +210,7 @@ export function ConfirmButton({ 'interactive', ] case ReviewState.ALLOWING: - return isApprovalLoading || (!approval.approve && allowance.state === AllowanceState.ALLOWED) + return isApprovalLoading || (!approval?.approve && allowance.state === AllowanceState.ALLOWED) ? [getApprovalLoadingAction()] : [getAllowancePendingAction(shouldRequestApproval, onCancel, trade.inputAmount.currency)] case ReviewState.ALLOWANCE_FAILED: @@ -238,7 +238,7 @@ export function ConfirmButton({ } }, [ allowance.state, - approval.state, + approval?.state, currentState, doesTradeDiffer, isApprovalLoading, diff --git a/src/cosmos/Swap.fixture.tsx b/src/cosmos/Swap.fixture.tsx index ff88c8f75..a64f48708 100644 --- a/src/cosmos/Swap.fixture.tsx +++ b/src/cosmos/Swap.fixture.tsx @@ -5,6 +5,7 @@ import { defaultTheme, DialogAnimationType, lightTheme, + RouterPreference, SupportedChainId, SwapWidget, } from '@uniswap/widgets' @@ -115,6 +116,13 @@ function Fixture() { width={width} routerUrl={routerUrl} brandedFooter={brandedFooter} + settings={ + { + slippage: { auto: false, max: '0.3' }, + routerPreference: RouterPreference.API, + transactionTtl: 30, + } + } dialogOptions={{ animationType: dialogAnimation, pageCentered, diff --git a/src/hooks/swap/useSyncController.ts b/src/hooks/swap/useSyncController.ts index dd3711753..5a18900e1 100644 --- a/src/hooks/swap/useSyncController.ts +++ b/src/hooks/swap/useSyncController.ts @@ -1,7 +1,7 @@ import { useAtom } from 'jotai' import { useEffect, useRef } from 'react' import { controlledAtom as controlledSwapAtom, Swap } from 'state/swap' -import { controlledAtom as controlledSettingsAtom, Settings } from 'state/swap/settings' +import { stateAtom as controlledSettingsAtom, Settings } from 'state/swap/settings' export interface SwapController { value?: Swap @@ -12,6 +12,13 @@ export default function useSyncController({ value, settings }: SwapController): // Log an error if the component changes from uncontrolled to controlled (or vice versa). const isSwapControlled = useRef(Boolean(value)) const isSettingsControlled = useRef(Boolean(settings)) + const [controlledSettings, setControlledSettings] = useAtom(controlledSettingsAtom) + + useEffect(() => { + // only set settings on mount - otherwise settings are being reset to default after every change in UI + setControlledSettings(settings as Settings) + },[]) + useEffect(() => { if (Boolean(value) !== isSwapControlled.current) { warnOnControlChange({ state: 'swap', prop: 'value' }) @@ -25,11 +32,6 @@ export default function useSyncController({ value, settings }: SwapController): if (controlledSwap !== value) { setControlledSwap(value) } - - const [controlledSettings, setControlledSettings] = useAtom(controlledSettingsAtom) - if (controlledSettings !== settings) { - setControlledSettings(settings) - } } function warnOnControlChange({ state, prop }: { state: string; prop: string }) { diff --git a/src/test/index.tsx b/src/test/index.tsx index 225b79d17..c8dd2a1b3 100644 --- a/src/test/index.tsx +++ b/src/test/index.tsx @@ -25,7 +25,7 @@ import { Provider as ReduxProvider } from 'react-redux' import { store } from 'state' import { Provider as ThemeProvider } from 'theme' import JsonRpcConnector from 'utils/JsonRpcConnector' -import { WalletConnectPopup, WalletConnectQR } from 'utils/WalletConnect' +import { WalletConnectQR } from 'utils/WalletConnect' export * from '@testing-library/react' export { default as userEvent } from '@testing-library/user-event' @@ -55,7 +55,7 @@ export function TestableWidget(props: PropsWithChildren) { connectors={{ user: {} as JsonRpcConnector, metaMask: {} as MetaMask, - walletConnect: {} as WalletConnectPopup, + walletConnect: {} as WalletConnectQR, walletConnectQR: {} as WalletConnectQR, network: {} as Network, }}