fix(approve): reset custom approve amount when trade changes #6421
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThis PR threads a new Changes
Sequence DiagramsequenceDiagram
participant OrderPartialApprove as OrderPartialApprove
participant PartialApproveAmountModal as PartialApproveAmountModal
participant ChangeApproveAmountModal as ChangeApproveAmountModal
participant ChangeApproveAmountModalPure as ChangeApproveAmountModalPure
participant ApprovalAmountInput as ApprovalAmountInput
OrderPartialApprove->>PartialApproveAmountModal: amountToSwap={amountToApprove}<br/>initialAmountToApprove=...
PartialApproveAmountModal->>ChangeApproveAmountModal: amountToSwap={amountToSwap}
ChangeApproveAmountModal->>ChangeApproveAmountModalPure: amountToSwap={amountToSwap}
ChangeApproveAmountModalPure->>ApprovalAmountInput: initialApproveAmount={initialAmount}<br/>amountToSwap={amountToSwap}
Note over ApprovalAmountInput: customAmountValue = userValue ??<br/>initialApproveAmount ?? amountToSwap
Note over ApprovalAmountInput: isInvalid = amountToSwap ?<br/>value.lessThan(amountToSwap) : false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
fairlighteth
approved these changes
Oct 23, 2025
Contributor
There was a problem hiding this comment.
Functionally (manual UI testing) it works for case:
(1) partial-approval flag on, (2) swap 6 COW → USDC where COW still needs approval, (3) set a custom allowance of 100 COW and confirm, (4) execute the swap so 94 COW allowance remains, (5) start a new swap for less than 94 COW. In that loop, it correctly recognized the existing 100 COW allowance and didn’t prompt for another approval.
fairlighteth
requested changes
Oct 23, 2025
Contributor
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
Tested this scenario and here I do see an issue:
Problematic scenario: pick a token that needs approval, enable partial approvals, open the partial-approval modal, set a custom amount (say 100), and confirm. Without clearing that allowance, reopen the modal—e.g., click “Change approval amount.” Instead of showing the saved 100, the modal falls back to the current trade amount, so the user must re-enter their custom value.
Contributor
|
Hey @shoom3301 , apart the issue that @fairlighteth reported above, I have 1 question: is it OK that 'partial approval' block is displayed ~30-40 seconds after a trade, however, a token is already approved? Thank you! |
Collaborator
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Base automatically changed from
fix/partial-approve-loading-state
to
fix/affected-order-partial-approve
October 31, 2025 11:27
Contributor
Could you correct me if I'm wrong. As I get it, you sign approve from another window? It can be improved by integrating moralis api for allowance and reducing time for checking this allowance from the frontend side. right now if an approve was signed not directly from "approve flow"(f.e. another window or somewhere else) we don't know that we should force allowance update |
Contributor
|
@elena-zh @fairlighteth could you pls check again, I've added saving the amount if user once confirmed it |
Contributor
|
Hey @limitofzero , thank you!
No, from the same. I follow the steps to reproduce in the original issue, and after the 1st trade I still see 'approve and swap' even if a sell amount is already approved. This state is displayed up to 20-30 seconds.
Now, new issues that I've faced (all of them are recorded on this video)
Could you please fix it? |
Contributor
|
@elena-zh thank you for checking, fixed |
Base automatically changed from
fix/affected-order-partial-approve
to
develop
November 1, 2025 04:03
limitofzero
force-pushed
the
fix/update-allowance-earlier
branch
from
1643c47 to
0d20851
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsx (1)
54-54: Valid observation on code duplication.The TODO comment correctly identifies that OrderPartialApprove and PartialApproveContainer share similar logic. Consider consolidating this in a follow-up refactoring.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsx(1 hunks)apps/cowswap-frontend/src/modules/erc20Approve/containers/ApprovalAmountInput/ApprovalAmountInput.tsx(2 hunks)apps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsx(3 hunks)apps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModalPure.tsx(3 hunks)apps/cowswap-frontend/src/modules/erc20Approve/containers/Erc20ApproveWidget/index.tsx(2 hunks)apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveAmountModal/PartialApproveAmountModal.tsx(1 hunks)apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveContainer/PartialApproveContainer.tsx(2 hunks)apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx(1 hunks)apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsx(1 hunks)apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetAmountToSignApprove.tsx(2 hunks)apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/index.tsx(0 hunks)apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/styled.ts(0 hunks)apps/cowswap-frontend/src/modules/erc20Approve/state/customApproveAmountInputState.ts(2 hunks)
💤 Files with no reviewable changes (2)
- apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/index.tsx
- apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/styled.ts
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/containers/Erc20ApproveWidget/index.tsxapps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetAmountToSignApprove.tsxapps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveContainer/PartialApproveContainer.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveAmountModal/PartialApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/ApprovalAmountInput/ApprovalAmountInput.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModalPure.tsx
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/containers/Erc20ApproveWidget/index.tsxapps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetAmountToSignApprove.tsxapps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveContainer/PartialApproveContainer.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveAmountModal/PartialApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/ApprovalAmountInput/ApprovalAmountInput.tsxapps/cowswap-frontend/src/modules/erc20Approve/state/customApproveAmountInputState.tsapps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModalPure.tsx
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/containers/Erc20ApproveWidget/index.tsxapps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveContainer/PartialApproveContainer.tsx
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/containers/Erc20ApproveWidget/index.tsxapps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveAmountModal/PartialApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModalPure.tsx
📚 Learning: 2025-08-12T05:57:08.021Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6138
File: libs/hook-dapp-lib/src/hookDappsRegistry.ts:1-1
Timestamp: 2025-08-12T05:57:08.021Z
Learning: The matchHooksToDapps function in libs/hook-dapp-lib/src/utils.ts provides backward compatibility for permit hooks through function selector detection (EIP_2612_PERMIT_SELECTOR and DAI_PERMIT_SELECTOR) rather than dappId matching, making it robust against dappId changes.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetAmountToSignApprove.tsx
📚 Learning: 2025-08-08T13:56:18.009Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/updaters/TokensListsUpdater/index.tsx:29-31
Timestamp: 2025-08-08T13:56:18.009Z
Learning: In libs/tokens/src/updaters/TokensListsUpdater/index.tsx, the project’s current Jotai version requires using `unstable_getOnInit` (not `getOnInit`) in atomWithStorage options; keep `{ unstable_getOnInit: true }` until Jotai is upgraded.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetAmountToSignApprove.tsxapps/cowswap-frontend/src/modules/erc20Approve/state/customApproveAmountInputState.ts
📚 Learning: 2025-08-08T13:55:17.528Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/state/tokens/allTokensAtom.ts:78-78
Timestamp: 2025-08-08T13:55:17.528Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts (TypeScript/Jotai), the team prefers to wait for token lists to initialize (listsStatesListAtom non-empty) before returning tokens. No fallback to favorites/user-added/native tokens should be used when listsStatesList is empty.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetAmountToSignApprove.tsxapps/cowswap-frontend/src/modules/erc20Approve/state/customApproveAmountInputState.ts
📚 Learning: 2025-06-23T07:03:50.760Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsx
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsx
📚 Learning: 2025-06-16T15:58:00.268Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5830
File: apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx:1-2
Timestamp: 2025-06-16T15:58:00.268Z
Learning: JSX can be imported as a named export from React in modern React versions (React 17+). The import `import { JSX } from 'react'` is valid and does not cause compilation errors.
Applied to files:
apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveContainer/PartialApproveContainer.tsx
🧬 Code graph analysis (10)
apps/cowswap-frontend/src/modules/erc20Approve/containers/Erc20ApproveWidget/index.tsx (3)
apps/cowswap-frontend/src/modules/trade/hooks/useDerivedTradeState.ts (1)
useDerivedTradeState(7-9)apps/cowswap-frontend/src/modules/erc20Approve/state/useSetUserApproveAmountModalState.ts (1)
useSetUserApproveAmountModalState(5-7)libs/common-utils/src/isSellOrder.ts (1)
isSellOrder(3-5)
apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetAmountToSignApprove.tsx (1)
apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsPartialApprovalModeSelected.ts (1)
useIsPartialApprovalModeSelected(5-7)
apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsx (2)
apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveAmountModal/PartialApproveAmountModal.tsx (1)
PartialApproveAmountModal(15-28)apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/styled.ts (1)
OrderActionsWrapper(3-7)
apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx (2)
apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts (1)
useIsApprovalOrPermitRequired(31-64)apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsPartialApprovalModeSelected.ts (1)
useIsPartialApprovalModeSelected(5-7)
apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsx (3)
apps/cowswap-frontend/src/modules/erc20Approve/state/useSetUserApproveAmountModalState.ts (1)
useSetUserApproveAmountModalState(5-7)apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetPartialAmountToSignApprove.ts (1)
useGetPartialAmountToSignApprove(15-31)apps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsx (1)
ChangeApproveAmountModal(18-57)
apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveContainer/PartialApproveContainer.tsx (4)
apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsPartialApprovalModeSelected.ts (1)
useIsPartialApprovalModeSelected(5-7)apps/cowswap-frontend/src/modules/erc20Approve/constants/maxApproveAmount.ts (1)
MAX_APPROVE_AMOUNT(3-3)apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveAmountModal/PartialApproveAmountModal.tsx (1)
PartialApproveAmountModal(15-28)apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveToggle/TradeApproveToggle.tsx (1)
TradeApproveToggle(13-25)
apps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsx (1)
apps/cowswap-frontend/src/modules/erc20Approve/state/customApproveAmountInputState.ts (2)
useCustomApproveAmountInputState(32-34)useUpdateOrResetCustomApproveAmountInputState(23-30)
apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveAmountModal/PartialApproveAmountModal.tsx (2)
apps/cowswap-frontend/src/modules/erc20Approve/state/partialApproveAmountModalState.ts (1)
useUpdatePartialApproveAmountModalState(21-23)apps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsx (1)
ChangeApproveAmountModal(18-57)
apps/cowswap-frontend/src/modules/erc20Approve/containers/ApprovalAmountInput/ApprovalAmountInput.tsx (1)
apps/cowswap-frontend/src/modules/erc20Approve/state/customApproveAmountInputState.ts (2)
useUpdateOrResetCustomApproveAmountInputState(23-30)useCustomApproveAmountInputState(32-34)
apps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModalPure.tsx (1)
apps/cowswap-frontend/src/modules/erc20Approve/containers/ApprovalAmountInput/ApprovalAmountInput.tsx (1)
ApprovalAmountInput(27-97)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (13)
apps/cowswap-frontend/src/modules/erc20Approve/state/customApproveAmountInputState.ts (1)
2-2: Good practice: memoizing the reset function.Wrapping the reset function with
useCallbackensures referential stability for consumers of this hook, which aligns well with the PR's goal of stabilizing callbacks to support prop-driven approval flows.Also applies to: 28-28
apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeChangeApproveAmountModal/TradeChangeApproveAmountModal.tsx (1)
3-18: LGTM!The addition of
amountToSwapprop correctly threads the maximum sell amount from the quote through to the modal for validation.apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGetAmountToSignApprove.tsx (1)
9-44: LGTM!Refactoring from atom-based to hook-based partial approval check improves encapsulation and aligns with the broader changes in this PR.
apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveContainer/PartialApproveContainer.tsx (1)
41-41: Verify prop values for PartialApproveAmountModal.Both
initialAmountToApproveandamountToSwapare set toamountToApprove. Based on the pattern in OrderPartialApprove.tsx (line 48),initialAmountToApproveshould likely bepartialAmountToApproveFinal(which includes user customization viaamountSetByUser), whileamountToSwapremainsamountToApprove.Apply this diff if the pattern should match OrderPartialApprove:
- return <PartialApproveAmountModal initialAmountToApprove={amountToApprove} amountToSwap={amountToApprove} /> + return <PartialApproveAmountModal initialAmountToApprove={partialAmountToApproveFinal} amountToSwap={amountToApprove} />apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsx (1)
47-49: LGTM!The addition of
amountToSwap={amountToApprove}correctly threads the swap amount through to the modal.apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx (1)
8-19: LGTM!Removal of
currentAllowancedisplay and migration to hook-based partial approval checking aligns with PR objectives to fix the partial approval bug.apps/cowswap-frontend/src/modules/erc20Approve/containers/PartialApproveAmountModal/PartialApproveAmountModal.tsx (1)
3-25: LGTM!The addition of
amountToSwapprop with a clear explanatory comment improves the API's clarity and enables proper validation in downstream components.apps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModal.tsx (1)
4-50: LGTM!The addition of
amountToSwapprop and adjustment toonBackbehavior (removingamountSetByUserreset) correctly implements the intended flow where the custom amount is preserved when navigating back but cleared when confirming or resetting.apps/cowswap-frontend/src/modules/erc20Approve/containers/ChangeApproveAmountModal/ChangeApproveAmountModalPure.tsx (2)
13-23: LGTM! Clear semantic improvements.The interface changes improve clarity by distinguishing between the initial approve amount and the amount to swap. The added comments are helpful, and the new
amountToSwapandonResetprops enable the partial approval reset functionality described in the PR objectives.
46-51: Props correctly threaded to ApprovalAmountInput.The prop renaming from
initialAmounttoinitialApproveAmountis consistent with the interface changes in ApprovalAmountInput.tsx, and the new propsamountToSwapandonResetare correctly passed through.apps/cowswap-frontend/src/modules/erc20Approve/containers/ApprovalAmountInput/ApprovalAmountInput.tsx (3)
18-32: LGTM! Improved semantic clarity.The prop renaming from
initialAmounttoinitialApproveAmountand the addition ofamountToSwapmake the distinction between "amount to approve" and "amount to swap" much clearer. The added comments are helpful.
55-71: LGTM! Correct validation logic.The validation now correctly checks if the entered amount is less than
amountToSwaprather thaninitialAmount. This ensures users cannot approve less than they need to swap, which aligns with the partial approval fixes mentioned in the PR objectives. The edge case whereamountToSwapis null/undefined is properly handled.
36-39: AdduseEffectto reset custom approve amount when trade changes.The custom approve amount state does not automatically reset when the trade changes. Currently, the fallback chain
customAmountValueState.amount ?? initialApproveAmount ?? amountToSwapwill use a persisted custom amount even after the user changes their trade (updatingamountToSwap).To fix this, add a
useEffectinChangeApproveAmountModalthat monitorsamountToSwapand callsresetCustomApproveAmountInput()when it changes:useEffect(() => { resetCustomApproveAmountInput() }, [amountToSwap, resetCustomApproveAmountInput])Alternatively, close the modal automatically when the trade changes by including it in the
resetAllScreenscallback inTradeWidgetModals.tsx.⛔ Skipped due to learnings
Learnt from: shoom3301 Repo: cowprotocol/cowswap PR: 5859 File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82 Timestamp: 2025-06-23T07:03:50.760Z Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.Learnt from: fairlighteth Repo: cowprotocol/cowswap PR: 6347 File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49 Timestamp: 2025-10-10T20:28:16.565Z Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior. <!-- [add_learning] When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import the container as outdated when the pure view is renamed or refactored.Learnt from: limitofzero Repo: cowprotocol/cowswap PR: 6351 File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121 Timestamp: 2025-10-13T19:41:31.440Z Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
apps/cowswap-frontend/src/modules/erc20Approve/containers/Erc20ApproveWidget/index.tsx
Show resolved
Hide resolved
elena-zh
approved these changes
Nov 3, 2025
Contributor
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
Hey @limitofzero , thank you. I'm approving the PR.
However, I've been able to catch a weird issue here:
- Placed an order with a permittable token, approval amount >sell amount
- the order has been traded
- in a while, I specified an order with a sell amount > approved one --> approve button did not appear, I placed an unfillable order in the end.
This issue stopped to be reproducible in ~30 seconds, so, I assume, it might be related to allowances check that runs with a delay.
alfetopito
approved these changes
Nov 3, 2025
Collaborator
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
Tested and it's working as described.
Except that the first time, it showed the approval again.
By accident I refreshed the page and it didn't show the approval anymore.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Fixes https://www.notion.so/cownation/App-asks-to-approve-a-partial-amount-when-token-is-approved-2878da5f04ca801e8a0fc4e4d00cea04?source=copy_link
To Test
Summary by CodeRabbit
Bug Fixes
Improvements