Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix swapping when permit2 disabled #639

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/Swap/Summary/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
25 changes: 17 additions & 8 deletions src/components/Swap/Summary/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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

Expand All @@ -34,15 +36,16 @@ enum ReviewState {
SWAP_PENDING,
}

function useReviewState(onSwap: () => Promise<void>, allowance: Allowance, doesTradeDiffer: boolean) {
function useReviewState(onSwap: () => Promise<void>, allowance: Allowance, approval: SwapApproval | undefined, 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?.()
Expand All @@ -52,7 +55,7 @@ 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) {
} 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)
Expand Down Expand Up @@ -152,33 +155,37 @@ export function ConfirmButton({
onConfirm,
triggerImpactSpeedbump,
allowance,
approval
}: {
trade: InterfaceTrade
slippage: Slippage
onConfirm: () => Promise<void>
triggerImpactSpeedbump: () => boolean
allowance: Allowance
approval?:SwapApproval
}) {
const { onSwapPriceUpdateAck, onSubmitSwapClick } = useAtomValue(swapEventHandlersAtom)
const [ackTrade, setAckTrade] = useState(trade)
const doesTradeDiffer = useMemo(
() => 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(() => {
Expand All @@ -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:
Expand Down Expand Up @@ -231,6 +238,7 @@ export function ConfirmButton({
}
}, [
allowance.state,
approval?.state,
currentState,
doesTradeDiffer,
isApprovalLoading,
Expand Down Expand Up @@ -258,6 +266,7 @@ interface SummaryDialogProps {
impact?: PriceImpact
onConfirm: () => Promise<void>
allowance: Allowance
approval: SwapApproval
}

export function SummaryDialog(props: SummaryDialogProps) {
Expand Down
1 change: 1 addition & 0 deletions src/components/Swap/SwapActionButton/SwapButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export default function SwapButton({ disabled }: { disabled: boolean }) {
impact={impact}
onConfirm={onSwap}
allowance={allowance}
approval={approval}
/>
</ResponsiveDialog>
)}
Expand Down
5 changes: 1 addition & 4 deletions src/components/Swap/SwapActionButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion src/cosmos/Swap.fixture.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
defaultTheme,
DialogAnimationType,
lightTheme,
RouterPreference,
SupportedChainId,
SwapWidget,
} from '@uniswap/widgets'
Expand Down Expand Up @@ -43,6 +44,8 @@ function Fixture() {
[]
)

const [permit2] = useValue('permit2Enabled', { defaultValue: true })

const [convenienceFee] = useValue('convenienceFee', { defaultValue: 0 })
const convenienceFeeRecipient = useOption('convenienceFeeRecipient', {
options: [
Expand Down Expand Up @@ -98,7 +101,7 @@ function Fixture() {

const widget = (
<SwapWidget
permit2
permit2={permit2}
convenienceFee={convenienceFee}
convenienceFeeRecipient={convenienceFeeRecipient}
defaultInputTokenAddress={defaultInputToken}
Expand All @@ -113,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,
Expand Down
3 changes: 1 addition & 2 deletions src/hooks/swap/useSwapApproval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ export function useSwapApproval(amount?: CurrencyAmount<Currency>): SwapApproval
await approve()
}
} catch (e) {
// Swallow approval errors - user rejections do not need to be displayed.
return
throw e
}
onSwapApprove?.()
}
Expand Down
14 changes: 8 additions & 6 deletions src/hooks/swap/useSyncController.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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' })
Expand All @@ -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 }) {
Expand Down
4 changes: 2 additions & 2 deletions src/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -55,7 +55,7 @@ export function TestableWidget(props: PropsWithChildren<TestableWidgetProps>) {
connectors={{
user: {} as JsonRpcConnector,
metaMask: {} as MetaMask,
walletConnect: {} as WalletConnectPopup,
walletConnect: {} as WalletConnectQR,
walletConnectQR: {} as WalletConnectQR,
network: {} as Network,
}}
Expand Down
Loading