Skip to content

Commit

Permalink
fix(permit): typefy hooks and allow only 1 permit (#4726)
Browse files Browse the repository at this point in the history
* fix(hooks): prevent duplicated hooks

* feat: add typedHooks and types

* feat: generify buildAppDataHooks

* feat: use typedHooks everywhere

* chore: remove merge and duplicate hooks logic

* chore: linter fix

* chore: fix build

* fix: keep hook store hooks when permit is not in use

* fix: use hooks from parameter rather than what's in the current appData

* refactor: rename updateHooksOnAppData to replaceHooksOnAppData

* chore: remove debug logs (#4722)
  • Loading branch information
alfetopito authored Jul 17, 2024
1 parent bfb1f29 commit 0e59ce1
Show file tree
Hide file tree
Showing 22 changed files with 163 additions and 80 deletions.
7 changes: 4 additions & 3 deletions apps/cowswap-frontend/src/modules/appData/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
export { getAppData } from './utils/fullAppData'
export * from './updater/AppDataUpdater'
export { useAppData, useUploadAppData } from './hooks'
export { useAppData, useAppDataHooks, useUploadAppData } from './hooks'
export { filterPermitSignerPermit } from './utils/appDataFilter'
export { updateHooksOnAppData, buildAppData } from './utils/buildAppData'
export { replaceHooksOnAppData, buildAppData } from './utils/buildAppData'
export { buildAppDataHooks } from './utils/buildAppDataHooks'
export * from './utils/getAppDataHooks'
export type { AppDataInfo, UploadAppDataParams } from './types'
export { addPermitHookToHooks, removePermitHookFromHooks } from './utils/typedHooks'
export type { AppDataInfo, UploadAppDataParams, TypedAppDataHooks } from './types'
6 changes: 3 additions & 3 deletions apps/cowswap-frontend/src/modules/appData/state/atoms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import { deepEqual } from '@cowprotocol/common-utils'
import { buildDocFilterFn, buildInverseDocFilterFn } from './utils'

import {
AppDataHooks,
AppDataInfo,
AppDataPendingToUpload,
RemoveAppDataFromUploadQueueParams,
TypedAppDataHooks,
UpdateAppDataOnUploadQueueParams,
UploadAppDataParams,
UploadAppDataParams
} from '../types'
import { updateFullAppData } from '../utils/fullAppData'

Expand Down Expand Up @@ -107,4 +107,4 @@ export const removeAppDataFromUploadQueueAtom = atom(
/**
* In memory atom for storing the current appData hooks info
*/
export const appDataHooksAtom = atom<AppDataHooks | undefined>(undefined)
export const appDataHooksAtom = atom<TypedAppDataHooks | undefined>(undefined)
11 changes: 11 additions & 0 deletions apps/cowswap-frontend/src/modules/appData/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,19 @@ export type UploadAppDataParams = AppDataKeyParams & {
export type UpdateAppDataOnUploadQueueParams = AppDataKeyParams & Partial<AppDataUploadStatus>
export type RemoveAppDataFromUploadQueueParams = AppDataKeyParams

export type CowHook = latest.CoWHook

export type TypedCowHook = CowHook & {
type: 'permit' | 'hookStore'
}

export type AppDataHooks = latest.OrderInteractionHooks

export type TypedAppDataHooks = Omit<AppDataHooks, 'pre' | 'post'> & {
pre?: TypedCowHook[]
post?: TypedCowHook[]
}

export type PreHooks = latest.PreHooks

export type PostHooks = latest.PostHooks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import { useDerivedSwapInfo } from 'modules/swap/hooks/useSwapState'
import { useLimitHasEnoughAllowance } from '../../limitOrders/hooks/useLimitHasEnoughAllowance'
import { useSwapEnoughAllowance } from '../../swap/hooks/useSwapFlowContext'
import { useUpdateAppDataHooks } from '../hooks'
import { TypedAppDataHooks, TypedCowHook } from '../types'
import { buildAppDataHooks } from '../utils/buildAppDataHooks'
import { cowHookToTypedCowHook } from '../utils/typedHooks'

type OrderInteractionHooks = latest.OrderInteractionHooks

Expand Down Expand Up @@ -47,10 +49,16 @@ export function AppDataHooksUpdater(): null {
const isNativeSell = trade?.inputAmount.currency ? getIsNativeToken(trade?.inputAmount.currency) : false

useEffect(() => {
const preInteractionHooks = preHooks.map((hookDetails) => hookDetails.hook)
const postInteractionHooks = postHooks.map((hookDetails) => hookDetails.hook)
const hooks = buildAppDataHooks({
preInteractionHooks: permitData ? preInteractionHooks.concat([permitData]) : preInteractionHooks,
const preInteractionHooks = preHooks.map<TypedCowHook>((hookDetails) =>
cowHookToTypedCowHook(hookDetails.hook, 'hookStore')
)
const postInteractionHooks = postHooks.map<TypedCowHook>((hookDetails) =>
cowHookToTypedCowHook(hookDetails.hook, 'hookStore')
)
const hooks = buildAppDataHooks<TypedCowHook[], TypedAppDataHooks>({
preInteractionHooks: permitData
? preInteractionHooks.concat([cowHookToTypedCowHook(permitData, 'permit')])
: preInteractionHooks,
postInteractionHooks,
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { AppCodeWithWidgetMetadata } from 'modules/injectedWidget/hooks/useAppCo
import { UtmParams } from 'modules/utm'

import { appDataInfoAtom } from '../state/atoms'
import { AppDataHooks, AppDataOrderClass, AppDataPartnerFee } from '../types'
import { AppDataOrderClass, AppDataPartnerFee, TypedAppDataHooks } from '../types'
import { buildAppData, BuildAppDataParams } from '../utils/buildAppData'
import { getAppData } from '../utils/fullAppData'

Expand All @@ -17,7 +17,7 @@ export type UseAppDataParams = {
slippageBips: number
orderClass: AppDataOrderClass
utm: UtmParams | undefined
hooks?: AppDataHooks
typedHooks?: TypedAppDataHooks
volumeFee?: AppDataPartnerFee
replacedOrderUid?: string
}
Expand All @@ -32,7 +32,7 @@ export function AppDataInfoUpdater({
slippageBips,
orderClass,
utm,
hooks,
typedHooks,
volumeFee,
replacedOrderUid,
}: UseAppDataParams): void {
Expand All @@ -56,7 +56,7 @@ export function AppDataInfoUpdater({
environment,
orderClass,
utm,
hooks,
typedHooks,
partnerFee: volumeFee,
widget,
replacedOrderUid,
Expand All @@ -82,7 +82,7 @@ export function AppDataInfoUpdater({
slippageBips,
orderClass,
utm,
hooks,
typedHooks,
volumeFee,
replacedOrderUid,
])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const AppDataUpdater = React.memo(({ slippage, orderClass }: AppDataUpdat
const appCode = useAppCode()
const slippageBips = percentToBps(slippage)
const utm = useUtm()
const hooks = useAppDataHooks()
const typedHooks = useAppDataHooks()
const appCodeWithWidgetMetadata = useAppCodeWidgetAware(appCode)
const volumeFee = useVolumeFee()
const replacedOrderUid = useReplacedOrderUid()
Expand All @@ -40,7 +40,7 @@ export const AppDataUpdater = React.memo(({ slippage, orderClass }: AppDataUpdat
slippageBips={slippageBips}
orderClass={orderClass}
utm={utm}
hooks={hooks}
typedHooks={typedHooks}
volumeFee={volumeFee}
replacedOrderUid={replacedOrderUid}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ export const filterPermitSignerPermit: HooksFilter = (cowHook: CowHook): boolean
const hasFormerAddress = cowHook.callData.toLowerCase().includes(FORMER_PERMIT_HOOK_ADDRESS)
const hasCurrentAddress = cowHook.callData.toLowerCase().includes(PERMIT_SIGNER.address.slice(2).toLowerCase())

console.log(`bug:filterPermitSignerPermit`, cowHook, hasFormerAddress, hasCurrentAddress)

return !hasFormerAddress && !hasCurrentAddress
}

Expand All @@ -30,7 +28,6 @@ export function filterHooks(
postHooksFilter: HooksFilter | undefined
): OrderInteractionHooks | undefined {
if (!hooks) {
console.log(`bug:filterHooks no hooks`)
return hooks
}

Expand All @@ -41,12 +38,9 @@ export function filterHooks(

// Remove metadata completely if nothing is left after filter
if (!filteredPre?.length && !filteredPost?.length) {
console.log(`bug:filterHooks filtered out everything`, hooks)
return undefined
}

console.log(`bug:filterHooks something left after filtering`, hooks, filteredPre, filteredPost)

return {
...rest,
// Filter out if there's nothing in one of them
Expand Down
37 changes: 9 additions & 28 deletions apps/cowswap-frontend/src/modules/appData/utils/buildAppData.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { latest, stringifyDeterministic } from '@cowprotocol/app-data'
import { stringifyDeterministic } from '@cowprotocol/app-data'
import { SupportedChainId } from '@cowprotocol/cow-sdk'

import { metadataApiSDK } from 'cowSdk'
Expand All @@ -7,6 +7,7 @@ import { keccak256, toUtf8Bytes } from 'ethers/lib/utils'
import { UtmParams } from 'modules/utm'

import { filterHooks, HooksFilter } from './appDataFilter'
import { typedAppDataHooksToAppDataHooks } from './typedHooks'

import {
AppDataHooks,
Expand All @@ -15,6 +16,7 @@ import {
AppDataPartnerFee,
AppDataRootSchema,
AppDataWidget,
TypedAppDataHooks,
} from '../types'

export type BuildAppDataParams = {
Expand All @@ -25,14 +27,12 @@ export type BuildAppDataParams = {
orderClass: AppDataOrderClass
referrerAccount?: string
utm: UtmParams | undefined
hooks?: AppDataHooks
typedHooks?: TypedAppDataHooks
widget?: AppDataWidget
partnerFee?: AppDataPartnerFee
replacedOrderUid?: string
}

type OrderInteractionHooks = latest.OrderInteractionHooks

async function generateAppDataFromDoc(
doc: AppDataRootSchema
): Promise<Pick<AppDataInfo, 'fullAppData' | 'appDataKeccak256'>> {
Expand All @@ -49,7 +49,7 @@ export async function buildAppData({
environment,
orderClass: orderClassName,
utm,
hooks,
typedHooks,
widget,
partnerFee,
replacedOrderUid,
Expand All @@ -69,7 +69,7 @@ export async function buildAppData({
quote: quoteParams,
orderClass,
utm,
hooks,
hooks: typedAppDataHooksToAppDataHooks(typedHooks),
widget,
partnerFee,
...{ replacedOrder },
Expand All @@ -85,21 +85,21 @@ export function toKeccak256(fullAppData: string) {
return keccak256(toUtf8Bytes(fullAppData))
}

export async function updateHooksOnAppData(
export async function replaceHooksOnAppData(
appData: AppDataInfo,
hooks: AppDataHooks | undefined,
preHooksFilter?: HooksFilter,
postHooksFilter?: HooksFilter
): Promise<AppDataInfo> {
const { doc } = appData

const existingHooks = filterHooks(doc.metadata.hooks, preHooksFilter, postHooksFilter)
const filteredHooks = filterHooks(hooks, preHooksFilter, postHooksFilter)

const newDoc = {
...doc,
metadata: {
...doc.metadata,
hooks: mergeHooks(existingHooks, hooks),
hooks: filteredHooks,
},
}

Expand All @@ -115,22 +115,3 @@ export async function updateHooksOnAppData(
appDataKeccak256,
}
}

function mergeHooks(
hooks1: OrderInteractionHooks | undefined,
hooks2: OrderInteractionHooks | undefined
): OrderInteractionHooks | undefined {
if (hooks1 && !hooks2) return hooks1

if (!hooks1 && hooks2) return hooks2

if (hooks1 && hooks2) {
return {
version: hooks1.version,
pre: (hooks1.pre || []).concat(hooks2.pre || []),
post: (hooks1.post || []).concat(hooks2.post || []),
}
}

return undefined
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import { AppDataHooks, PostHooks, PreHooks } from '../types'
import { AppDataHooks, CowHook } from '../types'

export function buildAppDataHooks({
export function buildAppDataHooks<T extends CowHook[], V extends AppDataHooks>({
preInteractionHooks,
postInteractionHooks,
}: {
preInteractionHooks?: PreHooks
postInteractionHooks?: PostHooks
}): AppDataHooks | undefined {
preInteractionHooks?: T
postInteractionHooks?: T
}): V | undefined {
if (!preInteractionHooks?.length && !postInteractionHooks?.length) {
return undefined
}

return {
...(preInteractionHooks?.length ? { pre: preInteractionHooks } : undefined),
...(postInteractionHooks?.length ? { post: postInteractionHooks } : undefined),
}
} as V
}
70 changes: 70 additions & 0 deletions apps/cowswap-frontend/src/modules/appData/utils/typedHooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { buildAppDataHooks } from './buildAppDataHooks'

import { AppDataHooks, CowHook, TypedAppDataHooks, TypedCowHook } from '../types'

export function cowHookToTypedCowHook(hook: CowHook, type: TypedCowHook['type']): TypedCowHook {
return { ...hook, type }
}

export function typedAppDataHooksToAppDataHooks(hooks: TypedAppDataHooks | undefined): AppDataHooks | undefined {
if (!hooks) {
return hooks
}

const { pre, post, ...rest } = hooks

return {
...rest,
pre: pre ? pre?.map(typedCowHookToCowHook) : undefined,
post: post ? post?.map(typedCowHookToCowHook) : undefined,
}
}

function typedCowHookToCowHook({ type: _, ...rest }: TypedCowHook): CowHook {
return rest
}

export function addPermitHookToHooks(hooks: TypedAppDataHooks | undefined, permit: CowHook): AppDataHooks | undefined {
if (!hooks) {
return buildAppDataHooks({ preInteractionHooks: [permit] })
}

const { pre, ...rest } = hooks

// Replace permit if existing
let replaced = false

const updatedPreHooks =
pre?.reduce<TypedCowHook[]>((acc, hook) => {
if (hook.type === 'permit') {
replaced = true
acc.push({ ...hook, ...permit })
} else {
acc.push(hook)
}
return acc
}, []) || []

if (!replaced) {
updatedPreHooks.push({ ...permit, type: 'permit' })
}

return typedAppDataHooksToAppDataHooks({ ...rest, pre: updatedPreHooks })
}

export function removePermitHookFromHooks(hooks: TypedAppDataHooks | undefined): AppDataHooks | undefined {
if (!hooks) {
return hooks
}

const { pre, ...rest } = hooks

const filteredPre = pre?.reduce<TypedCowHook[]>((acc, hook) => {
if (hook.type !== 'permit') {
acc.push(hook)
}
return acc
}, [])

return typedAppDataHooksToAppDataHooks({ ...rest, pre: filteredPre })
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const rateInfoParams = {
}

const tradeContext: TradeFlowContext = {
typedHooks: undefined,
permitInfo: undefined,
generatePermitHook: () => Promise.resolve(undefined),
getCachedPermit: () => Promise.resolve(undefined),
Expand Down
Loading

0 comments on commit 0e59ce1

Please sign in to comment.