Skip to content

fix: switch to sell order if wallet changed from safe to EOA #6610

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.

Sign up for GitHub

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

Merged
shoom3301 merged 1 commit into from
Dec 5, 2025
+11 −1
Merged

fix: switch to sell order if wallet changed from safe to EOA #6610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion apps/cowswap-frontend/project.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
"executor": "@nx/vite:dev-server",
"defaultConfiguration": "development",
"options": {
"buildTarget": "cowswap-frontend:build"
"buildTarget": "cowswap-frontend:build",
"host": true
Comment on lines +38 to +39
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n apps/cowswap-frontend/project.json | sed -n '30,50p'

Repository: cowprotocol/cowswap

Length of output: 722

🏁 Script executed:

git log --oneline -20 -- apps/cowswap-frontend/project.json

Repository: cowprotocol/cowswap

Length of output: 133

🏁 Script executed:

git show HEAD:apps/cowswap-frontend/project.json | head -50

Repository: cowprotocol/cowswap

Length of output: 1246

🏁 Script executed:

git show HEAD~1:apps/cowswap-frontend/project.json | sed -n '34,50p'

Repository: cowprotocol/cowswap

Length of output: 99

🏁 Script executed:

git log -p --all -S '"host": true' -- apps/cowswap-frontend/project.json | head -100

Repository: cowprotocol/cowswap

Length of output: 45

🏁 Script executed:

rg -A 5 -B 5 "host.*true" apps/cowswap-frontend/

Repository: cowprotocol/cowswap

Length of output: 50377

🏁 Script executed:

git log --all --oneline | head -20

Repository: cowprotocol/cowswap

Length of output: 1312

🏁 Script executed:

rg -B 5 -A 5 "host.*true" --type json

Repository: cowprotocol/cowswap

Length of output: 766

🏁 Script executed:

fd -name "*.md" | xargs rg -l "host" 2>/dev/null | head -5

Repository: cowprotocol/cowswap

Length of output: 474

Clarify the relevance of this dev server configuration change.

This change sets "host": true for the Vite dev server, binding it to all network interfaces instead of localhost. This doesn't appear related to the PR's stated objective of fixing ETH-flow order handling when switching from Safe to EOA wallets.

If this configuration is needed for testing the fix (e.g., in CI/CD or specific development setups), please document that in the PR description. Otherwise, consider moving it to a separate PR.

🤖 Prompt for AI Agents 
In apps/cowswap-frontend/project.json around lines 38-39, the dev server "host":
true change binds Vite to all network interfaces but appears unrelated to the
Safe→EOA order-handling fix; either remove/revert this change from this PR (keep
dev server config unchanged) or, if it is required for testing the bugfix
(CI/dev machines, remote device testing, or reproducing the issue), document
that requirement clearly in the PR description (explain why binding to all
interfaces is necessary, where/how tests rely on it, and any security/port
notes); do not leave the unrelated config change in this PR without
justification — move it to a separate config/infra PR if it’s not strictly
needed here.

Copy link
Contributor Author

@kernelwhisperer kernelwhisperer Dec 3, 2025

Choose a reason for hiding this comment

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

True, this was needed for my setup (Windows + Ubuntu running through WSL), otherwise I could not load http://localhost:3000

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 3, 2025

Choose a reason for hiding this comment

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

@kernelwhisperer, thank you for the clarification! That makes perfect sense—WSL setups often require binding to all network interfaces (0.0.0.0) for the Windows host to access services running in the Linux subsystem. The "host": true option is exactly what's needed for this scenario.

This is a reasonable change to include, especially if team members use similar development setups. You might consider adding a brief note in the repository's development documentation (if one exists) mentioning that this configuration supports WSL-based development workflows, which could help other developers in similar environments.

🐰✨

✏️ Learnings added 
Learnt from: kernelwhisperer
Repo: cowprotocol/cowswap PR: 6610
File: apps/cowswap-frontend/project.json:38-39
Timestamp: 2025-12-03T19:23:35.765Z
Learning: In the cowswap-frontend project configuration (apps/cowswap-frontend/project.json), the Vite dev server option `"host": true` is set to support Windows + WSL development environments, where binding to all network interfaces is required for the Windows host to access the dev server running in WSL.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

},
"configurations": {
"dev": {
Expand Down
9 changes: 9 additions & 0 deletions apps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import { useHooksEnabledManager } from 'legacy/state/user/hooks'
import { useTryFindIntermediateToken } from 'modules/bridge'
import { TradeApproveWithAffectedOrderList } from 'modules/erc20Approve'
import { EthFlowModal, EthFlowProps } from 'modules/ethFlow'
import { SELL_ETH_RESET_STATE } from 'modules/swap/consts'
import { AddIntermediateTokenModal } from 'modules/tokensList'
import {
TradeWidget,
TradeWidgetSlots,
useGetReceiveAmountInfo,
useIsEoaEthFlow,
useTradePriceImpact,
useWrapNativeFlow,
} from 'modules/trade'
Expand Down Expand Up @@ -97,12 +99,19 @@ export function SwapWidget({ topContent, bottomContent }: SwapWidgetProps): Reac
const [isHydrated, setIsHydrated] = useState(false)
const handleUnlock = useCallback(() => updateSwapState({ isUnlocked: true }), [updateSwapState])
const isPrimaryValidationPassed = useIsTradeFormValidationPassed()
const isEoaEthFlow = useIsEoaEthFlow()

useEffect(() => {
// Hydration guard: defer lock-screen until persisted state (isUnlocked) loads to prevent initial flash.
setIsHydrated(true)
}, [])

useEffect(() => {
if (isEoaEthFlow && !isSellOrder(orderKind)) {
updateSwapState(SELL_ETH_RESET_STATE)
}
}, [isEoaEthFlow, orderKind, updateSwapState])

const isSellTrade = isSellOrder(orderKind)

const ethFlowProps: EthFlowProps = useSafeMemoObject({
Expand Down