Skip to content

fix(tokens): use vault relayer address corr to env#7557

Merged
shoom3301 merged 12 commits into
developfrom
fix/approve-vault-staging
May 25, 2026
Merged

fix(tokens): use vault relayer address corr to env#7557
shoom3301 merged 12 commits into
developfrom
fix/approve-vault-staging

Conversation

@shoom3301

@shoom3301 shoom3301 commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Approval flow doesn't work on the Tokens page on the staging environment.
The cause is that COW_PROTOCOL_VAULT_RELAYER_ADDRESS from SDK is the prod address, but we shoud import it from @cowprotocol/common-utils to have it environment dependent.
p.s. there is existing eslint rule which is supposed to protect against similar issues, but it was not covering cowswap-frontend.

image image

To Test

  1. Open tokens page
  2. Approve some token
  • it should show loader while tx is pending
  • it should show the green mark once tx is mined

Summary by CodeRabbit

  • Refactor

    • Standardized and reformatted imports across the frontend.
    • Consolidated protocol contract address constants and utilities into the common utilities package.
  • Chores

    • Updated linting and import-order rules to enforce the new import conventions and narrowed the special grouping for select packages.
    • Adjusted lint rule severity for tests and added a targeted exception to accommodate the new conventions.

Review Change Stack

@vercel

vercel Bot commented May 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cowfi Ready Ready Preview May 25, 2026 1:17pm
explorer-dev Ready Ready Preview May 25, 2026 1:17pm
storybook Ready Ready Preview May 25, 2026 1:17pm
swap-dev Ready Ready Preview May 25, 2026 1:17pm
widget-configurator Ready Ready Preview May 25, 2026 1:17pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
cosmos Ignored Ignored May 25, 2026 1:17pm
sdk-tools Ignored Ignored Preview May 25, 2026 1:17pm

Request Review

@shoom3301 shoom3301 self-assigned this May 21, 2026
@shoom3301 shoom3301 requested a review from a team May 21, 2026 12:15
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Reformats @cowprotocol/common-utils imports in TokensTableRow and Version, splits/relocates a transactions mock import, adds an eslint-disable in buildTwapParts, and updates eslint.config.js to direct two protocol constants to @cowprotocol/common-utils, narrow import ordering groups, add an app-scoped restriction, and set the rule to warn in tests.

Changes

Frontend import updates

Layer / File(s) Summary
Component import & mock adjustments
apps/cowswap-frontend/src/legacy/components/Tokens/TokensTableRow.tsx, apps/cowswap-frontend/src/legacy/components/Version/index.tsx, apps/cowswap-frontend/src/modules/ethFlow/services/ethFlow/transactionsMocks.ts, apps/cowswap-frontend/src/modules/twap/utils/buildTwapParts.ts
Reformatted @cowprotocol/common-utils named imports in TokensTableRow.tsx, consolidated protocol contract address constants into @cowprotocol/common-utils in Version/index.tsx, moved COW_PROTOCOL_VAULT_RELAYER_ADDRESS import to @cowprotocol/common-utils and SupportedChainId to @cowprotocol/cow-sdk in transactionsMocks.ts, and added an eslint-disable-next-line for @typescript-eslint/no-restricted-imports before a specific SDK import in buildTwapParts.ts.

ESLint import rules

Layer / File(s) Summary
Restricted-imports and import ordering
eslint.config.js
Replaced a global @typescript-eslint/no-restricted-imports entry to instruct importing COW_PROTOCOL_SETTLEMENT_CONTRACT_ADDRESS and COW_PROTOCOL_VAULT_RELAYER_ADDRESS from @cowprotocol/common-utils, narrowed import/order pathGroups to match only @cowprotocol, @uniswap, and @safe-global, added an apps/cowswap-frontend scoped restriction blocking direct import of those constants from @cowprotocol/cow-sdk (and the same restriction under apps/cowswap-frontend/src/common/**/*), and changed the rule to 'warn' for **/*.test.* files.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • cowprotocol/cowswap#7482: Changes addressing where COW_PROTOCOL_SETTLEMENT_CONTRACT_ADDRESS and COW_PROTOCOL_VAULT_RELAYER_ADDRESS are sourced, similar to this PR's import centralization.

Suggested reviewers

  • kernelwhisperer
  • elena-zh
  • limitofzero

Poem

🐰 A tidy hop through imports I prance,
Constants gather where they take their stance.
Linted lines whisper rules that softly chime,
Mocks point true, a comment stills the rhyme.
Rabbity cheers for tidy code and time.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly relates to the main change: updating imports to use the environment-dependent vault relayer address from common-utils instead of the SDK.
Description check ✅ Passed The description includes a clear summary of the problem, screenshots, and testing steps; it matches the template structure for Summary and To Test sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/approve-vault-staging

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 21, 2026

Copy link
Copy Markdown

Deploying swap-dev with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6584955
Status: ✅  Deploy successful!
Preview URL: https://3f9d6e91.swap-dev-5u6.pages.dev
Branch Preview URL: https://fix-approve-vault-staging.swap-dev-5u6.pages.dev

View logs

@elena-zh elena-zh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TY!

@shoom3301 shoom3301 requested a review from Danziger May 22, 2026 11:57
@fairlighteth

fairlighteth commented May 22, 2026

Copy link
Copy Markdown
Contributor

In general it works. But what I notice is, if I give approval it doesn't (always) show the spinner and often just out of the blue, shows that is approved (with delay). Can this be addressed?

Screencast_20260522_155531.webm

Other automated Findings

  1. eslint.config.js:339 - The new @cowprotocol/cow-sdk restriction is not enforced under apps/cowswap-frontend/src/common/**. That later override replaces @typescript-eslint/no-restricted-imports with only the modules/* restriction, so src/common can still import
    COW_PROTOCOL_* directly from the SDK. eslint --print-config apps/cowswap-frontend/src/common/hooks/useContract.ts confirms this. The cow-sdk restriction should be duplicated/merged into this override, with explicit disables only for intentional prod-only exceptions.
  2. eslint.config.js:153 - This PR drops the existing ethers restricted-import entry, so direct import ... from 'ethers' is allowed again. That looks unrelated to the vault relayer change and weakens the existing tree-shaking guardrail. Restore the ethers path restriction unless this is a deliberate policy change.

@fairlighteth fairlighteth self-requested a review May 22, 2026 15:02

@fairlighteth fairlighteth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added a few comments.

@shoom3301

shoom3301 commented May 25, 2026

Copy link
Copy Markdown
Collaborator Author

@fairlighteth thanks!

  1. The issue was not introduced in this PR. I would keep it out of the scope
  2. common/** eslint - fixed
  3. Yes, we don't ban ethers imports because we don't have the package anymore

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 25, 2026

Copy link
Copy Markdown

Deploying explorer-dev with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6584955
Status: ✅  Deploy successful!
Preview URL: https://b7517c4f.explorer-dev-dxz.pages.dev
Branch Preview URL: https://fix-approve-vault-staging.explorer-dev-dxz.pages.dev

View logs

@Danziger

Copy link
Copy Markdown
Contributor

Tested it and it works, after I refresh the page.

This is after accepting the tx request in the wallet:

Captura de pantalla 2026-05-25 152114

If you wait a bit, the spinner goes away and you'll see "Approve" again. However, if you refresh, it shows the right status:

Captura de pantalla 2026-05-25 152147

In any case, nothing to do with this PR, just a different issue in the same feature.

@shoom3301 shoom3301 merged commit d9228ea into develop May 25, 2026
21 checks passed
@shoom3301 shoom3301 deleted the fix/approve-vault-staging branch May 25, 2026 14:15
@github-actions github-actions Bot locked and limited conversation to collaborators May 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants