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

feat(permit): allowance warning #3184

Merged
merged 23 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6216f3a
refactor: remove unnecessary toString()
alfetopito Oct 6, 2023
52ca3b5
chore: fix typo
alfetopito Oct 6, 2023
d7b4a96
fix: transform BigNumber instances into hex strings for compatibility…
alfetopito Oct 6, 2023
8732312
feat: add fullAppData to local order instance
alfetopito Oct 6, 2023
ab3e5c5
feat: add decodeAppData to appData module
alfetopito Oct 6, 2023
5dc302a
feat: add getAppDataHooks to appData module
alfetopito Oct 6, 2023
1573e5c
feat: add useCheckHasValidPendingPermit to permit module
alfetopito Oct 6, 2023
ae952f0
feat: add permit checking to OrderRow allowance warning
alfetopito Oct 6, 2023
a6f3f1c
refactor: rename getParsedOrderFromItem to getParsedOrderFromTableItem
alfetopito Oct 9, 2023
161e637
refactor: change isParsed order to be a bit more semantic
alfetopito Oct 9, 2023
e34bc56
feat: add ordersPermitStatusAtom
alfetopito Oct 9, 2023
92f7eef
feat: add PendingPermitUpdater
alfetopito Oct 9, 2023
4aeb8bf
feat: add useGetOrdersPermitStatus
alfetopito Oct 9, 2023
243abce
feat: pass down ordersPermitStatus
alfetopito Oct 9, 2023
7e8f905
feat: add PendingPermitUpdater to OrdersTableWidget
alfetopito Oct 9, 2023
8fa8ddf
chore: remove unused export
alfetopito Oct 9, 2023
aed5c0e
refactor: sort exports
alfetopito Oct 9, 2023
3263718
fix: add back export. I removed the wrong one 🤦
alfetopito Oct 9, 2023
a432bf6
refactor: extract useGetOrdersToCheckPendingPermit
alfetopito Oct 10, 2023
21c3f16
chore: debug statements
alfetopito Oct 10, 2023
2ae222a
fix: force atoms to load so stored value is respected
alfetopito Oct 10, 2023
113788f
refactor: use the handy atomWithPartialUpdate
alfetopito Oct 11, 2023
f874fc6
refactor: move useGetOrdersToCheckPendingPermit to its own file
alfetopito Oct 11, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Order, OrderStatus } from 'legacy/state/orders/actions'
import { useAddOrUpdateOrders, useClearOrdersStorage } from 'legacy/state/orders/hooks'
import { classifyOrder, OrderTransitionStatus } from 'legacy/state/orders/utils'

import { useTokensForOrdersList, getTokensListFromOrders } from 'modules/orders'
import { getTokensListFromOrders, useTokensForOrdersList } from 'modules/orders'
import { apiOrdersAtom } from 'modules/orders/state/apiOrdersAtom'

import { useGpOrders } from 'api/gnosisProtocol/hooks'
Expand Down Expand Up @@ -80,6 +80,7 @@ function _transformGpOrderToStoreOrder(
summary: '',
status,
receiver: receiver || '',
fullAppData: order.fullAppData,
apiAdditionalInfo: order,
isCancelling: apiStatus === 'pending' && order.invalidated, // already cancelled in the API, not yet in the UI
// EthFlow related
Expand Down
4 changes: 4 additions & 0 deletions apps/cowswap-frontend/src/legacy/state/orders/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ export interface BaseOrder extends Omit<OrderCreation, 'signingScheme'> {

// Additional information from the order available in the API
apiAdditionalInfo?: OrderInfoApi
// De-normalizing it as this is known at order placement time as `appData`,
// but when returned from the api is replaced with the `appDataHash`
// See this order response for example https://barn.api.cow.fi/goerli/api/v1/orders/0xc170856a42f38ba07a7af3ea8f299ea724ec0aa22445eb741cbad7f9dd4fcda05b0abe214ab7875562adee331deff0fe1912fe4265269bb1
fullAppData?: EnrichedOrder['fullAppData']

// Wallet specific
presignGnosisSafeTxHash?: string // Gnosis Safe tx
Expand Down
6 changes: 4 additions & 2 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_BUY_ADDRESS } from '@cowprotocol/common-const'
import { isAddress, shortenAddress, formatTokenAmount, formatSymbol } from '@cowprotocol/common-utils'
import { NATIVE_CURRENCY_BUY_ADDRESS, RADIX_DECIMAL } from '@cowprotocol/common-const'
import { formatSymbol, formatTokenAmount, isAddress, shortenAddress } from '@cowprotocol/common-utils'
import {
EcdsaSigningScheme,
OrderClass,
Expand Down Expand Up @@ -156,6 +156,7 @@ export function mapUnsignedOrderToOrder({ unsignedOrder, additionalParams }: Map
sellAmountBeforeFee,
orderCreationHash,
quoteId,
appData: { fullAppData },
} = additionalParams
const status = _getOrderStatus(allowsOffchainSigning, isOnChain)

Expand All @@ -170,6 +171,7 @@ export function mapUnsignedOrderToOrder({ unsignedOrder, additionalParams }: Map
outputToken: buyToken,
quoteId,
class: additionalParams.class,
fullAppData,

// Status
status,
Expand Down
1 change: 1 addition & 0 deletions apps/cowswap-frontend/src/modules/appData/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export * from './updater/AppDataUpdater'
export { useAppData, useUploadAppData } from './hooks'
export { updateHooksOnAppData, buildAppData } from './utils/buildAppData'
export { buildAppDataHooks } from './utils/buildAppDataHooks'
export * from './utils/getAppDataHooks'
export type { AppDataInfo, UploadAppDataParams } from './types'
24 changes: 24 additions & 0 deletions apps/cowswap-frontend/src/modules/appData/utils/decodeAppData.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { AnyAppDataDocVersion } from '@cowprotocol/app-data'

import { Nullish } from 'types'

/**
* Decode appData from a string to a AnyAppDataDocVersion instance
* Keep in mind it can be a valid JSON but not necessarily a valid AppDataDoc
*
* Returns undefined if the given appData is not a valid JSON
*/
export function decodeAppData(appData: Nullish<string>): AnyAppDataDocVersion | undefined {
if (!appData) {
return undefined
}

try {
// TODO: returned value can be a valid JSON but not necessarily a valid AppDataDoc
return JSON.parse(appData)
} catch (e) {
console.info(`[decodeAppData] given appData is not a valid JSON`, appData)

return undefined
}
}
21 changes: 21 additions & 0 deletions apps/cowswap-frontend/src/modules/appData/utils/getAppDataHooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { AnyAppDataDocVersion } from '@cowprotocol/app-data'

import { Nullish } from 'types'

import { decodeAppData } from './decodeAppData'

import { AppDataHooks } from '../types'

/**
* Get hooks from fullAppData, which can be JSON stringified or the instance
*
* Returns undefined if the fullAppData is falsy or if there are no hooks
*/
export function getAppDataHooks(fullAppData: Nullish<AnyAppDataDocVersion | string>): AppDataHooks | undefined {
const decodedAppData = typeof fullAppData === 'string' ? decodeAppData(fullAppData) : fullAppData

if (!decodedAppData || !('hooks' in decodedAppData.metadata)) return undefined

// TODO: this requires app-data v0.9.0. Might not work for newer versions...
return decodedAppData.metadata.hooks as AppDataHooks
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import { OrdersReceiptModal } from 'modules/ordersTable/containers/OrdersReceipt
import { useSelectReceiptOrder } from 'modules/ordersTable/containers/OrdersReceiptModal/hooks'
import { OrderActions } from 'modules/ordersTable/pure/OrdersTableContainer/types'
import { buildOrdersTableUrl, parseOrdersTableUrl } from 'modules/ordersTable/utils/buildOrdersTableUrl'
import { useCheckHasValidPendingPermit } from 'modules/permit'
import { useBalancesAndAllowances } from 'modules/tokens'

import { useCancelOrder } from 'common/hooks/useCancelOrder'
import { useCategorizeRecentActivity } from 'common/hooks/useCategorizeRecentActivity'
import { ordersToCancelAtom, updateOrdersToCancelAtom } from 'common/hooks/useMultipleOrdersCancellation/state'
import { CancellableOrder } from 'common/utils/isOrderCancellable'
import { ParsedOrder } from 'utils/orderUtils/parseOrder'
Expand All @@ -28,7 +30,6 @@ import { OrdersTableList, useOrdersTableList } from './hooks/useOrdersTableList'
import { useOrdersTableTokenApprove } from './hooks/useOrdersTableTokenApprove'
import { useValidatePageUrlParams } from './hooks/useValidatePageUrlParams'

import { useCategorizeRecentActivity } from '../../../../common/hooks/useCategorizeRecentActivity'
import { OrdersTableContainer, TabOrderTypes } from '../../pure/OrdersTableContainer'
import { getParsedOrderFromItem, OrderTableItem, tableItemsToOrders } from '../../utils/orderTableGroupUtils'

Expand Down Expand Up @@ -73,6 +74,7 @@ export function OrdersTableWidget({
const getSpotPrice = useGetSpotPrice()
const selectReceiptOrder = useSelectReceiptOrder()
const isSafeViaWc = useIsSafeViaWc()
const checkHasValidPendingPermit = useCheckHasValidPendingPermit()

const spender = useMemo(() => (chainId ? GP_VAULT_RELAYER[chainId] : undefined), [chainId])

Expand Down Expand Up @@ -168,6 +170,7 @@ export function OrdersTableWidget({
allowsOffchainSigning={allowsOffchainSigning}
orderType={orderType}
pendingActivities={pendingActivity}
checkHasValidPendingPermit={checkHasValidPendingPermit}
>
{isOpenOrdersTab && orders.length && <MultipleCancellationMenu pendingOrders={tableItemsToOrders(orders)} />}
</OrdersTableContainer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from 'modules/ordersTable/pure/OrdersTableContainer/styled'
import { OrderActions } from 'modules/ordersTable/pure/OrdersTableContainer/types'
import { OrderStatusBox } from 'modules/ordersTable/pure/OrderStatusBox'
import { CheckHasValidPendingPermit } from 'modules/permit'
import { getIsEthFlowOrder } from 'modules/swap/containers/EthFlowStepper'

import { useSafeMemo } from 'common/hooks/useSafeMemo'
Expand Down Expand Up @@ -148,6 +149,7 @@ export interface OrderRowProps {
orderParams: OrderParams
onClick: () => void
orderActions: OrderActions
checkHasValidPendingPermit?: CheckHasValidPendingPermit | undefined
children?: JSX.Element
}

Expand All @@ -164,6 +166,7 @@ export function OrderRow({
prices,
spotPrice,
children,
checkHasValidPendingPermit,
}: OrderRowProps) {
const { buyAmount, rateInfoParams, hasEnoughAllowance, hasEnoughBalance, chainId } = orderParams
const { creationTime, expirationTime, status } = order
Expand All @@ -173,8 +176,17 @@ export function OrderRow({

const showCancellationModal = orderActions.getShowCancellationModal(order)

const [hasValidPendingPermit, setHasValidPendingPermit] = useState<boolean | undefined>(undefined)

// TODO: do this properly! Maybe an atom & updater on modules/permit and consume it from there?
useEffect(() => {
if (checkHasValidPendingPermit && orderParams.hasEnoughAllowance === false) {
checkHasValidPendingPermit(order).then(setHasValidPendingPermit)
}
}, [checkHasValidPendingPermit, order, orderParams.hasEnoughAllowance])
alfetopito marked this conversation as resolved.
Show resolved Hide resolved

const withWarning =
(hasEnoughBalance === false || hasEnoughAllowance === false) &&
(hasEnoughBalance === false || (hasEnoughAllowance === false && hasValidPendingPermit === false)) &&
// show the warning only for pending and scheduled orders
(status === OrderStatus.PENDING || status === OrderStatus.SCHEDULED)
const theme = useContext(ThemeContext)
Expand Down Expand Up @@ -357,7 +369,7 @@ export function OrderRow({
{hasEnoughBalance === false && (
<BalanceWarning symbol={inputTokenSymbol} isScheduled={isOrderScheduled} />
)}
{hasEnoughAllowance === false && (
{hasEnoughAllowance === false && hasValidPendingPermit === false && (
<AllowanceWarning
approve={() => orderActions.approveOrderToken(order.inputToken)}
symbol={inputTokenSymbol}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useState, useEffect, useMemo, useRef } from 'react'
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'

import iconOrderExecution from '@cowprotocol/assets/cow-swap/orderExecution.svg'
import { SupportedChainId } from '@cowprotocol/cow-sdk'
Expand All @@ -11,19 +11,19 @@ import SVG from 'react-inlinesvg'
import { useLocation } from 'react-router-dom'
import styled from 'styled-components/macro'

import { QuestionWrapper } from 'legacy/components/QuestionHelper'
import QuestionHelper from 'legacy/components/QuestionHelper'
import QuestionHelper, { QuestionWrapper } from 'legacy/components/QuestionHelper'

import { PendingOrdersPrices } from 'modules/orders/state/pendingOrdersPricesAtom'
import { SpotPricesKeyParams } from 'modules/orders/state/spotPricesAtom'
import { ORDERS_TABLE_PAGE_SIZE } from 'modules/ordersTable/const/tabs'
import {
CheckboxCheckmark,
TableHeader,
TableRowCheckbox,
TableRowCheckboxWrapper,
CheckboxCheckmark,
} from 'modules/ordersTable/pure/OrdersTableContainer/styled'
import { OrderActions } from 'modules/ordersTable/pure/OrdersTableContainer/types'
import { CheckHasValidPendingPermit } from 'modules/permit'
import { BalancesAndAllowances } from 'modules/tokens'

import { ordersTableFeatures } from 'common/constants/featureFlags'
Expand Down Expand Up @@ -206,6 +206,7 @@ export interface OrdersTableProps {
balancesAndAllowances: BalancesAndAllowances
getSpotPrice: (params: SpotPricesKeyParams) => Price<Currency, Currency> | null
orderActions: OrderActions
checkHasValidPendingPermit: CheckHasValidPendingPermit
}

export function OrdersTable({
Expand All @@ -219,6 +220,7 @@ export function OrdersTable({
getSpotPrice,
orderActions,
currentPageNumber,
checkHasValidPendingPermit,
}: OrdersTableProps) {
const location = useLocation()
const [isRateInverted, setIsRateInverted] = useState(false)
Expand Down Expand Up @@ -423,6 +425,7 @@ export function OrdersTable({
isRateInverted={isRateInverted}
orderActions={orderActions}
onClick={() => orderActions.selectReceiptOrder(order)}
checkHasValidPendingPermit={checkHasValidPendingPermit}
/>
)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export function OrdersTableContainer({
children,
orderType,
pendingActivities,
checkHasValidPendingPermit,
}: OrdersProps) {
const content = () => {
if (!isWalletConnected) {
Expand Down Expand Up @@ -233,6 +234,7 @@ export function OrdersTableContainer({
balancesAndAllowances={balancesAndAllowances}
getSpotPrice={getSpotPrice}
orderActions={orderActions}
checkHasValidPendingPermit={checkHasValidPendingPermit}
/>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export function getOrderParams(
balancesAndAllowances: BalancesAndAllowances,
order: ParsedOrder
): OrderParams {
const sellAmount = CurrencyAmount.fromRawAmount(order.inputToken, order.sellAmount.toString())
const buyAmount = CurrencyAmount.fromRawAmount(order.outputToken, order.buyAmount.toString())
const sellAmount = CurrencyAmount.fromRawAmount(order.inputToken, order.sellAmount)
const buyAmount = CurrencyAmount.fromRawAmount(order.outputToken, order.buyAmount)

const rateInfoParams: RateInfoParams = {
chainId,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import { useCallback } from 'react'

import { SupportedChainId } from '@cowprotocol/cow-sdk'
import { useWalletInfo } from '@cowprotocol/wallet'
import { Web3Provider } from '@ethersproject/providers'
import { useWeb3React } from '@web3-react/core'

import { DAI_PERMIT_SELECTOR, Eip2612PermitUtils, EIP_2612_PERMIT_SELECTOR } from '@1inch/permit-signed-approvals-utils'

import { getAppDataHooks } from 'modules/appData'

import { ParsedOrder } from 'utils/orderUtils/parseOrder'

import { CheckHasValidPendingPermit } from '../types'
import { getPermitUtilsInstance } from '../utils/getPermitUtilsInstance'

export function useCheckHasValidPendingPermit(): CheckHasValidPendingPermit {
const { chainId } = useWalletInfo()
const { provider } = useWeb3React()

return useCallback(
async (order: ParsedOrder): Promise<boolean | undefined> => {
if (!provider) {
// Missing required params, we can't tell
return undefined
}

return checkHasValidPendingPermit(order, provider, chainId)
},
[chainId, provider]
)
}

async function checkHasValidPendingPermit(
order: ParsedOrder,
provider: Web3Provider,
chainId: SupportedChainId
): Promise<boolean> {
const { fullAppData, partiallyFillable, executionData } = order
const preHooks = getAppDataHooks(fullAppData)?.pre

if (
// No hooks === no permit
!preHooks ||
// Permit is only executed for partially fillable orders in the first execution
// Thus, if there is any amount executed, partiallyFillable permit is no longer valid
(partiallyFillable && executionData.filledAmount.gt('0'))
) {
// These cases we know for sure permit isn't valid or there is no permit
return false
}

const eip2162Utils = getPermitUtilsInstance(chainId, provider, order.owner)

const tokenAddress = order.inputToken.address
const tokenName = order.inputToken.name || tokenAddress

const checkedHooks = await Promise.all(
preHooks.map(({ callData }) =>
checkIsSingleCallDataAValidPermit(order, chainId, eip2162Utils, tokenAddress, tokenName, callData)
)
)

const validPermits = checkedHooks.filter((v) => v !== undefined)

if (!validPermits.length) {
// No permits means no preHook permits, we can say that there is no valid permit
return false
}

// Only when all permits are valid, then the order permits are still valid
return validPermits.every(Boolean)
}

async function checkIsSingleCallDataAValidPermit(
order: ParsedOrder,
chainId: SupportedChainId,
eip2162Utils: Eip2612PermitUtils,
tokenAddress: string,
tokenName: string,
callData: string
): Promise<boolean | undefined> {
const params = { chainId, tokenName, tokenAddress, callData }

let recoverPermitOwnerPromise: Promise<string> | undefined = undefined

// If pre-hook doesn't start with either selector, it's not a permit
if (callData.startsWith(EIP_2612_PERMIT_SELECTOR)) {
recoverPermitOwnerPromise = eip2162Utils.recoverPermitOwnerFromCallData({
...params,
// I don't know why this was removed, ok?
// We added it back on buildPermitCallData.ts
// But it looks like this is needed 🤷
// Check the test for this method https://github.com/1inch/permit-signed-approvals-utils/blob/master/src/eip-2612-permit.test.ts#L85-L106
callData: callData.replace(EIP_2612_PERMIT_SELECTOR, '0x'),
})
} else if (callData.startsWith(DAI_PERMIT_SELECTOR)) {
recoverPermitOwnerPromise = eip2162Utils.recoverDaiLikePermitOwnerFromCallData({
...params,
callData: callData.replace(DAI_PERMIT_SELECTOR, '0x'),
})
}

if (!recoverPermitOwnerPromise) {
// The callData doesn't match any known permit type
return undefined
}

try {
const recoveredOwner = await recoverPermitOwnerPromise

// Permit is valid when recovered owner matches order owner
return recoveredOwner.toLowerCase() === order.owner.toLowerCase()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is it.
If recovered owner matches order owner, permit is still valid!

} catch (e) {
console.debug(
`bug--[checkHasValidPendingPermit] Failed to check permit validity for order ${order.id} with callData ${callData}`,
e
)
return false
}
}
Loading
Loading