preview: mirror fork PR #7697#7699
Draft
github-actions[bot] wants to merge 8 commits into
Draft
Conversation
For permittable tokens, CowSwap signs an EIP-2612 permit instead of performing an on-chain ERC-20 approval. The ON_BEFORE_APPROVAL widget hook, however, was only fired from the on-chain approval path (useApproveCurrency), so widget integrators that subscribed to onBeforeApproval never saw the event when the user was about to sign a permit. From the integrator's perspective the user authorized spending without their pre-approval handler ever running. Fire ON_BEFORE_APPROVAL in the swap and limit order trade flows right before requesting the permit signature, mirroring the payload shape used by useApproveCurrency (chainId, sellToken, sellAmount, walletAddress, spenderAddress). The hook is skipped when a cached permit is reused, matching the behaviour of the existing permit request UI step. If the integrator's hook returns false, the relevant flow aborts cleanly without dispatching a permit request: - swap flow returns false, just like a declined price impact; - limit order flow throws a new WidgetHookDeclineError which is swallowed by useHandleOrderPlacement and only dismisses the trade confirmation modal. Closes #7685
github-actions
Bot
added
the
DONT_MERGE
10 tasks
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Deploying explorer-dev with
|
| Latest commit: |
40d5d5f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://74f747f8.explorer-dev-dxz.pages.dev |
| Branch Preview URL: | https://cf-preview-pr-7697.explorer-dev-dxz.pages.dev |
Deploying swap-dev with
|
| Latest commit: |
40d5d5f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ffce30c6.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://cf-preview-pr-7697.swap-dev-5u6.pages.dev |
Treat the widget-hook decline as an expected abort path rather than a generic swap error: re-throw before the catch block routes it through captureError and analytics, so declined ON_BEFORE_APPROVAL hooks no longer pollute error telemetry. Addresses @coderabbitai's feedback on #7697.
The previous fix only wired ON_BEFORE_APPROVAL into the permit-signing paths (limit orders + swap permit flow). On the Swap page with a non-permittable token, the approval still goes through `useTradeApproveCallback` -> `approveCallback`, which never fired the hook. Result: the widget's onBeforeApproval listener never ran on the Swap page in that scenario. Call the hook in `useTradeApproveCallback` right before the ERC20 `approve` transaction. If the host widget declines, reset the approve progress modal and bail out without sending the tx, matching the behaviour of the other hook call sites. Addresses @elena-zh's review on #7697.
On the Swap page, when the user clicks Approve for a permittable token, the trade form pre-signs an EIP-2612 permit via `useGeneratePermitInAdvanceToTrade`. That path skipped the `ON_BEFORE_APPROVAL` widget hook, so the host widget's `onBeforeApproval` listener never ran on Swap with permittable tokens (it worked on Limit Orders because the permit is signed inside the limit-order `tradeFlow`, which already calls the hook). Call the hook in `useGeneratePermitInAdvanceToTrade` right before `generatePermit`. If the host widget declines, bail out without signing, matching the behaviour of the other hook call sites. Also drop the speculative hook call I added to `useTradeApproveCallback` in 97b1ab5: `useApproveCurrency` (the only path that consumes `useTradeApproveCallback` for the Swap form) already fires the hook, so the extra call would double-prompt. Addresses @elena-zh's follow-up review on #7697.
If \`tradeSpenderAddress\` was unavailable, the previous version skipped the \`ON_BEFORE_APPROVAL\` widget hook but still called \`generatePermit\`, letting a permit be signed without the pre-approval gate. Treat a missing spender as a hard precondition (same as missing \`account\` / \`permitInfo\`) so the hook is always evaluated before any permit signature. Addresses @coderabbitai's review on #7697.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR mirrors fork PR #7697 for Cloudflare Pages preview builds.
Source PR: #7697
Source fork branch:
tenderdeve/cowswap:fix/7685-widget-on-before-approvalMirrored SHA:
40d5d5f5a014Do not merge this PR. Close the source PR to clean up this mirror.