-
Notifications
You must be signed in to change notification settings - Fork 176
fix(widget): fire ON_BEFORE_APPROVAL widget hook before permit signing #7697
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
base: develop
Are you sure you want to change the base?
Changes from 5 commits
1567101
f9d6050
4b55dfc
97b1ab5
6653bb1
7d49d28
b0c24cb
40d5d5f
52717ce
c81179d
2db25db
d5dcf3c
0982536
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,16 @@ | ||
| import { captureError, ERROR_TYPES, normalizeError, reportPermitWithDefaultSigner } from '@cowprotocol/common-utils' | ||
| import { SigningScheme } from '@cowprotocol/cow-sdk' | ||
| import { | ||
| captureError, | ||
| COW_PROTOCOL_VAULT_RELAYER_ADDRESS, | ||
| currencyAmountToTokenAmount, | ||
| ERROR_TYPES, | ||
| normalizeError, | ||
| reportPermitWithDefaultSigner, | ||
| } from '@cowprotocol/common-utils' | ||
| import { SigningScheme, SupportedChainId } from '@cowprotocol/cow-sdk' | ||
| import { Percent } from '@cowprotocol/currency' | ||
| import { isSupportedPermitInfo } from '@cowprotocol/permit-utils' | ||
| import { Command, UiOrderType } from '@cowprotocol/types' | ||
| import { WidgetHookEvents } from '@cowprotocol/widget-lib' | ||
|
|
||
| import { tradingSdk } from 'tradingSdk/tradingSdk' | ||
| import { sendTransaction } from 'wagmi/actions' | ||
|
|
@@ -11,8 +19,9 @@ import { PriceImpact } from 'legacy/hooks/usePriceImpact' | |
| import { partialOrderUpdate } from 'legacy/state/orders/utils' | ||
| import { mapUnsignedOrderToOrder, wrapErrorInOperatorError } from 'legacy/utils/trade' | ||
|
|
||
| import { callWidgetHook } from 'modules/injectedWidget' | ||
| import { LOW_RATE_THRESHOLD_PERCENT } from 'modules/limitOrders/const/trade' | ||
| import { PriceImpactDeclineError, TradeFlowContext } from 'modules/limitOrders/services/types' | ||
| import { PriceImpactDeclineError, TradeFlowContext, WidgetHookDeclineError } from 'modules/limitOrders/services/types' | ||
| import { LimitOrdersSettingsState } from 'modules/limitOrders/state/limitOrdersSettingsAtom' | ||
| import { calculateLimitOrdersDeadline } from 'modules/limitOrders/utils/calculateLimitOrdersDeadline' | ||
| import { emitPostedOrderEvent } from 'modules/orders' | ||
|
|
@@ -27,7 +36,7 @@ import { getSwapErrorMessage } from 'common/utils/getSwapErrorMessage' | |
| import type { Hex } from 'viem' | ||
|
|
||
| // TODO: Break down this large function into smaller functions | ||
| // eslint-disable-next-line max-lines-per-function | ||
| // eslint-disable-next-line max-lines-per-function, complexity | ||
| export async function tradeFlow( | ||
| params: TradeFlowContext, | ||
| priceImpact: PriceImpact, | ||
|
|
@@ -73,7 +82,28 @@ export async function tradeFlow( | |
|
|
||
| try { | ||
| logTradeFlow('LIMIT ORDER FLOW', 'STEP 2: handle permit') | ||
| if (isSupportedPermitInfo(permitInfo)) await beforePermit() | ||
| if (isSupportedPermitInfo(permitInfo)) { | ||
| const cachedPermit = await params.getCachedPermit(sellToken.address) | ||
|
|
||
| if (!cachedPermit) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see this logic duplicated in three different places. |
||
| const sellTokenAmount = currencyAmountToTokenAmount(inputAmount) | ||
| const isWidgetHookPassed = await callWidgetHook(WidgetHookEvents.ON_BEFORE_APPROVAL, { | ||
| chainId: sellTokenAmount.currency.chainId, | ||
| sellToken: { | ||
| ...sellTokenAmount.currency, | ||
| name: sellTokenAmount.currency.name || '', | ||
| symbol: sellTokenAmount.currency.symbol || '', | ||
| }, | ||
| sellAmount: (permitAmountToSign ?? 0n).toString(), | ||
| walletAddress: account, | ||
| spenderAddress: COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId as SupportedChainId], | ||
| }) | ||
|
|
||
| if (!isWidgetHookPassed) throw new WidgetHookDeclineError() | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not report widget-hook declines as swap errors. At Line 102, Suggested fix } catch (err: unknown) {
const error = normalizeError(err)
+ if (error instanceof WidgetHookDeclineError) {
+ throw error
+ }
logTradeFlow('LIMIT ORDER FLOW', 'STEP 9: ERROR: ', error)
const swapErrorMessage = getSwapErrorMessage(error)🤖 Prompt for AI Agents |
||
|
|
||
| await beforePermit() | ||
| } | ||
|
|
||
| postOrderParams.appData = await handlePermit({ | ||
| permitInfo, | ||
|
|
@@ -196,6 +226,11 @@ export async function tradeFlow( | |
| } catch (err: unknown) { | ||
| const error = normalizeError(err) | ||
|
|
||
| // Expected abort path: skip generic swap-error analytics so widget-hook declines don't pollute telemetry. | ||
| if (error instanceof WidgetHookDeclineError) { | ||
| throw error | ||
| } | ||
|
|
||
| logTradeFlow('LIMIT ORDER FLOW', 'STEP 9: ERROR: ', error) | ||
| const swapErrorMessage = getSwapErrorMessage(error) | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.