Skip to content

Commit

Permalink
fix(fee=0): buy order amounts again (#3679)
Browse files Browse the repository at this point in the history
* chore: revert "fix: rely on swapZeroFee feature flag for anything slippage related"

This reverts commit 161dd98.

* fix: always use buy amount with fees

* fix: do not add fee to summary when swapZeroFee flag is on

* fix: buy orders without fee=0 should use sell amount without fee for signature

* fix: pass featureFlags to useCreateTwapOrder

* test: fix unit tests
  • Loading branch information
alfetopito authored Jan 19, 2024
1 parent 2b20e2f commit 72526eb
Show file tree
Hide file tree
Showing 16 changed files with 67 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import TradeGp from 'legacy/state/swap/TradeGp'

import { SwapConfirmState } from 'modules/swap/state/swapConfirmAtom'

import { useFeatureFlags } from 'common/hooks/featureFlags/useFeatureFlags'
import { RateInfoParams } from 'common/pure/RateInfo'
import { TransactionErrorContent } from 'common/pure/TransactionErrorContent'
import { TradeAmounts } from 'common/types'
Expand Down Expand Up @@ -77,9 +76,7 @@ export function ConfirmSwapModal({
rateInfoParams,
])

const { swapZeroFee } = useFeatureFlags()

const slippageAdjustedSellAmount = trade?.maximumAmountIn(allowedSlippage, swapZeroFee)
const slippageAdjustedSellAmount = trade?.maximumAmountIn(allowedSlippage)
const buttonText = useButtonText(slippageAdjustedSellAmount)

const modalBottom = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import { NoImpactWarning } from 'modules/trade/pure/NoImpactWarning'
import { PriceUpdatedBanner } from 'modules/trade/pure/PriceUpdatedBanner'
import { useTradeUsdAmounts } from 'modules/usdAmount'

import { useFeatureFlags } from 'common/hooks/featureFlags/useFeatureFlags'
import { FiatValue } from 'common/pure/FiatValue'
import { BannerOrientation, CustomRecipientWarningBanner } from 'common/pure/InlineBanner/banners'
import { RateInfoParams } from 'common/pure/RateInfo'
Expand Down Expand Up @@ -78,11 +77,9 @@ function SwapModalHeaderComponent({
allowsOffchainSigning,
rateInfoParams,
}: SwapModalHeaderProps) {
const { swapZeroFee } = useFeatureFlags()

const slippageAdjustedAmounts = useMemo(
() => computeSlippageAdjustedAmounts(trade, allowedSlippage, swapZeroFee),
[trade, allowedSlippage, swapZeroFee]
() => computeSlippageAdjustedAmounts(trade, allowedSlippage),
[trade, allowedSlippage]
)

const {
Expand Down
8 changes: 4 additions & 4 deletions apps/cowswap-frontend/src/legacy/state/swap/TradeGp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ export function _minimumAmountOut(pct: Percent, trade: TradeGp) {
return trade.outputAmount.multiply(ONE_FRACTION.subtract(pct))
}

export function _maximumAmountIn(pct: Percent, trade: TradeGp, withoutFee?: boolean) {
export function _maximumAmountIn(pct: Percent, trade: TradeGp) {
if (trade.tradeType === TradeType.EXACT_INPUT) {
return trade.inputAmount
}

return (withoutFee ? trade.inputAmountWithoutFee : trade.inputAmountWithFee).multiply(ONE_FRACTION.add(pct))
return trade.inputAmountWithFee.multiply(ONE_FRACTION.add(pct))
}

interface TradeGpConstructor {
Expand Down Expand Up @@ -143,7 +143,7 @@ export default class TradeGp {
* Get the maximum amount in that can be spent via this trade for the given slippage tolerance
* @param slippageTolerance tolerance of unfavorable slippage from the execution price of this trade
*/
public maximumAmountIn(slippageTolerance: Percent, withoutFee?: boolean): CurrencyAmount<Currency> {
return _maximumAmountIn(slippageTolerance, this, withoutFee)
public maximumAmountIn(slippageTolerance: Percent): CurrencyAmount<Currency> {
return _maximumAmountIn(slippageTolerance, this)
}
}
5 changes: 2 additions & 3 deletions apps/cowswap-frontend/src/legacy/utils/prices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,10 @@ export function warningSeverity(priceImpact: Percent | undefined): WarningSeveri
export function computeSlippageAdjustedAmounts(
// trade: Trade | undefined,
trade: TradeGp | undefined,
allowedSlippage: Percent,
withoutFee?: boolean
allowedSlippage: Percent
): { [field in Field]?: CurrencyAmount<Currency> } {
return {
[Field.INPUT]: trade?.maximumAmountIn(allowedSlippage, withoutFee),
[Field.INPUT]: trade?.maximumAmountIn(allowedSlippage),
[Field.OUTPUT]: trade?.minimumAmountOut(allowedSlippage),
}
}
27 changes: 22 additions & 5 deletions apps/cowswap-frontend/src/legacy/utils/trade.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { RADIX_DECIMAL, NATIVE_CURRENCY_ADDRESS } from '@cowprotocol/common-const'
import { isAddress, shortenAddress, formatTokenAmount, formatSymbol, getIsNativeToken } from '@cowprotocol/common-utils'
import { NATIVE_CURRENCY_ADDRESS, RADIX_DECIMAL } from '@cowprotocol/common-const'
import { formatSymbol, formatTokenAmount, getIsNativeToken, isAddress, shortenAddress } from '@cowprotocol/common-utils'
import {
EcdsaSigningScheme,
OrderClass,
Expand Down Expand Up @@ -57,10 +57,26 @@ export type UnsignedOrderAdditionalParams = PostOrderParams & {
export function getOrderSubmitSummary(
params: Pick<
PostOrderParams,
'kind' | 'account' | 'inputAmount' | 'outputAmount' | 'recipient' | 'recipientAddressOrName' | 'feeAmount'
| 'kind'
| 'account'
| 'inputAmount'
| 'outputAmount'
| 'recipient'
| 'recipientAddressOrName'
| 'feeAmount'
| 'featureFlags'
>
): string {
const { kind, account, inputAmount, outputAmount, recipient, recipientAddressOrName, feeAmount } = params
const {
kind,
account,
inputAmount,
outputAmount,
recipient,
recipientAddressOrName,
feeAmount,
featureFlags: { swapZeroFee },
} = params

const sellToken = inputAmount.currency
const buyToken = outputAmount.currency
Expand All @@ -71,7 +87,8 @@ export function getOrderSubmitSummary(
]
const inputSymbol = formatSymbol(sellToken.symbol)
const outputSymbol = formatSymbol(buyToken.symbol)
const inputAmountValue = formatTokenAmount(feeAmount ? inputAmount.add(feeAmount) : inputAmount)
// this already contains the fee in the fee amount when fee=0
const inputAmountValue = formatTokenAmount(feeAmount && !swapZeroFee ? inputAmount.add(feeAmount) : inputAmount)
const outputAmountValue = formatTokenAmount(outputAmount)

const base = `Swap ${inputQuantifier}${inputAmountValue} ${inputSymbol} for ${outputQuantifier}${outputAmountValue} ${outputSymbol}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,26 @@ import { Fragment, useMemo } from 'react'
import { CHAIN_INFO } from '@cowprotocol/common-const'
import {
getEtherscanLink,
getExplorerAddressLink,
getExplorerLabel,
isMobile,
shortenAddress,
getExplorerAddressLink,
isMobile,
} from '@cowprotocol/common-utils'
import { ExternalLink, MouseoverTooltip } from '@cowprotocol/ui'
import { MouseoverTooltip } from '@cowprotocol/ui'
import { ExternalLink } from '@cowprotocol/ui'
import {
ConnectionType,
useWalletInfo,
WalletDetails,
getConnectionIcon,
getConnectionName,
getIsCoinbaseWallet,
getIsHardWareWallet,
getIsMetaMask,
getWeb3ReactConnection,
Identicon,
useIsWalletConnect,
useWalletDetails,
useWalletInfo,
WalletDetails,
useIsWalletConnect,
getWeb3ReactConnection,
getIsHardWareWallet,
} from '@cowprotocol/wallet'
import { useWeb3React } from '@web3-react/core'
import { Connector } from '@web3-react/types'
Expand All @@ -38,7 +39,6 @@ import {
import Activity from 'modules/account/containers/Transaction'

import { useIsProviderNetworkUnsupported } from 'common/hooks/useIsProviderNetworkUnsupported'
import { useUnsupportedNetworksText } from 'common/hooks/useUnsupportedNetworksText'

import {
AccountControl,
Expand All @@ -55,13 +55,14 @@ import {
WalletActions,
WalletName,
WalletNameAddress,
WalletSecondaryActions,
WalletSelector,
WalletSecondaryActions,
WalletWrapper,
Wrapper,
} from './styled'
import { SurplusCard } from './SurplusCard'

import { useUnsupportedNetworksText } from '../../../../common/hooks/useUnsupportedNetworksText'
import { useDisconnectWallet } from '../../hooks/useDisconnectWallet'
import { CreationDateText } from '../Transaction/styled'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,10 @@ import { RowWithShowHelpersProps } from 'modules/swap/pure/Row/typings'
export interface RowReceivedAfterSlippageProps extends RowWithShowHelpersProps {
trade: TradeGp
allowedSlippage: Percent
withoutFee?: boolean
}

export function RowReceivedAfterSlippage({
trade,
allowedSlippage,
showHelpers,
withoutFee,
}: RowReceivedAfterSlippageProps) {
const slippageAdjustedAmounts = computeSlippageAdjustedAmounts(trade, allowedSlippage, withoutFee)
export function RowReceivedAfterSlippage({ trade, allowedSlippage, showHelpers }: RowReceivedAfterSlippageProps) {
const slippageAdjustedAmounts = computeSlippageAdjustedAmounts(trade, allowedSlippage)

const props = useMemo(
() => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import { LowerSectionWrapper } from 'modules/swap/pure/styled'
import { useIsWrapOrUnwrap } from 'modules/trade/hooks/useIsWrapOrUnwrap'
import { useUsdAmount } from 'modules/usdAmount'

import { useFeatureFlags } from 'common/hooks/featureFlags/useFeatureFlags'

interface TradeBasicDetailsProp extends BoxProps {
allowedSlippage: Percent | string
isExpertMode: boolean
Expand All @@ -35,8 +33,6 @@ export function TradeBasicDetails(props: TradeBasicDetailsProp) {
const isEoaEthFlow = useIsEoaEthFlow()
const isWrapOrUnwrap = useIsWrapOrUnwrap()

const { swapZeroFee } = useFeatureFlags()

const showRowSlippage =
(isEoaEthFlow || isExpertMode || !allowedSlippagePercent.equalTo(INITIAL_ALLOWED_SLIPPAGE_PERCENT)) &&
!isWrapOrUnwrap
Expand All @@ -55,12 +51,7 @@ export function TradeBasicDetails(props: TradeBasicDetailsProp) {
{/* Slippage */}
{showRowSlippage && <RowSlippage allowedSlippage={allowedSlippagePercent} />}
{showRowReceivedAfterSlippage && (
<RowReceivedAfterSlippage
trade={trade}
allowedSlippage={allowedSlippagePercent}
showHelpers={true}
withoutFee={swapZeroFee}
/>
<RowReceivedAfterSlippage trade={trade} allowedSlippage={allowedSlippagePercent} showHelpers={true} />
)}
</LowerSectionWrapper>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import TradeGp from 'legacy/state/swap/TradeGp'
import { TradeSummaryContent } from 'modules/swap/pure/TradeSummary'
import { useUsdAmount } from 'modules/usdAmount'

import { useFeatureFlags } from 'common/hooks/featureFlags/useFeatureFlags'

// Sub-components

export type TradeSummaryProps = {
Expand All @@ -20,15 +18,13 @@ export type TradeSummaryProps = {
export function TradeSummary({ trade, ...restProps }: TradeSummaryProps) {
const { allowsOffchainSigning } = useWalletDetails()
const feeFiatValue = useUsdAmount(trade.fee.feeAsCurrency).value
const { swapZeroFee } = useFeatureFlags()

return (
<TradeSummaryContent
{...restProps}
trade={trade}
fee={feeFiatValue}
allowsOffchainSigning={allowsOffchainSigning}
withoutFee={swapZeroFee}
/>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('getAmountsForSignature()', () => {
})

/**
* Sell amount calculated as: (inputAmount + fee) + slippage
* Sell amount calculated as: (inputAmount) + slippage
* Output amount is just what user entered
*/
it('Buy order', () => {
Expand All @@ -86,8 +86,8 @@ describe('getAmountsForSignature()', () => {
kind: OrderKind.BUY,
})

// inputAmount = (3012 + 8) + 5% = 3171.000000
expect(result.inputAmount.toFixed()).toEqual('3171.000000')
// inputAmount = (3012) + 5% = 3,162.6
expect(result.inputAmount.toFixed()).toEqual('3162.600000')
// Output amount the same, because this is buy order
expect(result.outputAmount.toFixed()).toEqual('2.000000000000000000')
})
Expand Down Expand Up @@ -119,7 +119,7 @@ describe('getAmountsForSignature()', () => {
})

/**
* Sell amount calculated as: inputAmount + slippage
* Sell amount calculated as: (inputAmount + fee) + slippage
* Output amount is just what user entered
*/
it('Buy order', () => {
Expand All @@ -132,8 +132,8 @@ describe('getAmountsForSignature()', () => {
kind: OrderKind.BUY,
})

// inputAmount = 3012 + 5% = 3162.600000
expect(result.inputAmount.toFixed()).toEqual('3162.600000')
// inputAmount = (3012 + 8) + 5% = 3,171
expect(result.inputAmount.toFixed()).toEqual('3171.000000')

// Output amount the same, because this is buy order
expect(result.outputAmount.toFixed()).toEqual('2.000000000000000000')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ function inputAmountForSignature(params: AmountForSignatureParams): CurrencyAmou
if (kind === OrderKind.SELL) {
return trade.inputAmount
} else {
return trade.inputAmountWithoutFee.multiply(slippageCoeff) // add slippage
return trade.inputAmountWithFee.multiply(slippageCoeff) // add slippage
}
}

if (kind === OrderKind.SELL) {
return trade.inputAmountWithFee
} else {
return trade.inputAmountWithFee.multiply(slippageCoeff) // add slippage
return trade.inputAmountWithoutFee.multiply(slippageCoeff) // add slippage
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,8 @@ export function useSwapAmountsWithSlippage(): [
CurrencyAmount<Currency> | undefined
] {
const { v2Trade: trade, allowedSlippage } = useDerivedSwapInfo()
const { swapZeroFee } = useFeatureFlags()

const { INPUT, OUTPUT } = computeSlippageAdjustedAmounts(trade, allowedSlippage, swapZeroFee)
const { INPUT, OUTPUT } = computeSlippageAdjustedAmounts(trade, allowedSlippage)

return [INPUT, OUTPUT]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ import { useWrappedToken } from 'modules/trade/hooks/useWrappedToken'

import { useFeatureFlags } from 'common/hooks/featureFlags/useFeatureFlags'
import { useApproveState } from 'common/hooks/useApproveState'
import { useSafeMemo } from 'common/hooks/useSafeMemo'

import { useSafeBundleEthFlowContext } from './useSafeBundleEthFlowContext'
import { useDerivedSwapInfo, useSwapActionHandlers } from './useSwapState'

import { useSafeMemo } from '../../../common/hooks/useSafeMemo'
import { getAmountsForSignature } from '../helpers/getAmountsForSignature'
import { logSwapParams } from '../helpers/logSwapParams'

Expand Down
8 changes: 3 additions & 5 deletions apps/cowswap-frontend/src/modules/swap/hooks/useSwapState.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { FEE_SIZE_THRESHOLD } from '@cowprotocol/common-const'
import { formatSymbol, getIsNativeToken, isAddress, tryParseCurrencyAmount } from '@cowprotocol/common-utils'
import { SupportedChainId } from '@cowprotocol/cow-sdk'
import { useENS } from '@cowprotocol/ens'
import { useAreThereTokensWithSameSymbol, useTokenBySymbolOrAddress } from '@cowprotocol/tokens'
import { useTokenBySymbolOrAddress } from '@cowprotocol/tokens'
import { useAreThereTokensWithSameSymbol } from '@cowprotocol/tokens'
import { useWalletInfo } from '@cowprotocol/wallet'
import { Currency, CurrencyAmount, Percent } from '@uniswap/sdk-core'

Expand All @@ -25,7 +26,6 @@ import { useIsExpertMode } from 'legacy/state/user/hooks'
import { useNavigateOnCurrencySelection } from 'modules/trade/hooks/useNavigateOnCurrencySelection'
import { useTradeNavigate } from 'modules/trade/hooks/useTradeNavigate'

import { useFeatureFlags } from 'common/hooks/featureFlags/useFeatureFlags'
import { useIsProviderNetworkUnsupported } from 'common/hooks/useIsProviderNetworkUnsupported'

import { useSwapSlippage } from './useSwapSlippage'
Expand Down Expand Up @@ -272,14 +272,12 @@ export function useDerivedSwapInfo(): DerivedSwapInfo {
[inputCurrencyBalance, outputCurrencyBalance]
)

const { swapZeroFee } = useFeatureFlags()

// allowed slippage is either auto slippage, or custom user defined slippage if auto slippage disabled
// TODO: check whether we want to enable auto slippage tolerance
// const autoSlippageTolerance = useAutoSlippageTolerance(trade.trade) // mod
// const allowedSlippage = useUserSlippageToleranceWithDefault(autoSlippageTolerance) // mod
const allowedSlippage = useSwapSlippage()
const slippageAdjustedSellAmount = v2Trade?.maximumAmountIn(allowedSlippage, swapZeroFee) || null
const slippageAdjustedSellAmount = v2Trade?.maximumAmountIn(allowedSlippage) || null

const inputError = useMemo(() => {
let inputError: string | undefined
Expand Down
Loading

0 comments on commit 72526eb

Please sign in to comment.