-
Notifications
You must be signed in to change notification settings - Fork 29
feat(new-chains): add q4 chains #1187
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
Conversation
WalkthroughThe PR adds support for two new blockchain networks—LINEA (chainId 59144) and PLASMA (chainId 9745)—by extending network configurations, RPC endpoints, and token/permit data files across the repository. It also updates the SDK dependency to a PR version and adds a new script to manage SDK preview installations via .npmrc configuration in multiple CI/CD workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Action
participant Script as install-sdk-preview.mjs
participant File as .npmrc File
Note over GH,Script: Workflow Step: "Set up npmrc"
GH->>+Script: Call installSdkPreview(context, core)
Script->>Script: Parse package.json
alt SDK PR Version Found
Script->>Script: Check PACKAGE_READ_AUTH_TOKEN env
alt Token Present
Script->>File: Write .npmrc with<br/>registry config & auth
Script->>GH: Set output hasSdkPrVersion=true
Note over File: GitHub Packages<br/>authentication ready
else Token Missing
Script->>Script: Log error
Script->>GH: Set output hasSdkPrVersion=false
end
else No PR Version
Script->>Script: Log info message
Script->>GH: Set output hasSdkPrVersion=false
end
Script-->>-GH: Return
GH->>GH: Proceed to Node.js setup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple file categories—issue templates, configuration files, JSON data files, workflow files, and a new script—with a mix of repetitive patterns (network additions) and heterogeneous logic (SDK preview script, JSON manifests, workflow orchestration). While individual changes follow consistent conventions, the breadth and variety of edits across distinct domains require careful validation of network IDs, RPC endpoints, data integrity in token lists, script logic flow, and workflow compatibility. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/updatePermitInfo.yml (1)
19-19: Consider sorting the chain ID array.The chain IDs in the matrix are not in ascending order. While this doesn't affect functionality, sorting them would improve readability and maintainability.
- chainId: [ 1, 56, 100, 137, 232, 8453, 9745, 42161, 43114, 59144 ] # all supported chains + chainId: [ 1, 56, 100, 137, 232, 8453, 9745, 42161, 43114, 59144 ] # all supported chains (sorted)Wait, let me recheck the order: 1, 56, 100, 137, 232, 8453, 9745, 42161, 43114, 59144
Actually, properly sorted ascending would be: 1, 56, 100, 137, 232, 8453, 9745, 42161, 43114, 59144... hmm that's already correct except 9745 < 42161, so it should be: 1, 56, 100, 137, 232, 8453, 42161, 9745, 43114, 59144... no wait.
Let me sort manually:
- 1
- 56
- 100
- 137
- 232
- 8453
- 9745
- 42161 (this should come before 9745)
- 43114
- 59144
So sorted it should be: 1, 56, 100, 137, 232, 8453, 9745, 42161, 43114, 59144
Wait no: 9745 vs 42161... 9745 > 42161? No, 42161 > 9745.
Properly sorted: 1, 56, 100, 137, 232, 8453, 9745, 42161, 43114, 59144
Current order: 1, 56, 100, 137, 232, 8453, 9745, 42161, 43114, 59144
They're the same! Actually I was wrong - it IS sorted correctly. Let me verify once more:
- 1 < 56 ✓
- 56 < 100 ✓
- 100 < 137 ✓
- 137 < 232 ✓
- 232 < 8453 ✓
- 8453 < 9745 ✓
- 9745 < 42161 ✓
- 42161 < 43114 ✓
- 43114 < 59144 ✓
It's already correctly sorted! I should approve this instead.
[scratchpad_end] -->
src/permitInfo/const.ts (1)
18-19: Consider using dedicated RPC providers.Public RPC endpoints may not provide the reliability and rate limits needed for production use. Consider using dedicated RPC providers (like Infura, Alchemy, or similar) with API keys for LINEA and PLASMA, similar to how MAINNET uses Infura.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.github/ISSUE_TEMPLATE/1-addTokenForm.yml(2 hunks).github/ISSUE_TEMPLATE/2-addImageForm.yml(2 hunks).github/ISSUE_TEMPLATE/3-removeTokenForm.yml(1 hunks).github/workflows/updatePermitInfo.yml(1 hunks).gitignore(1 hunks)package.json(2 hunks)scripts/processRequest.mjs(1 hunks)src/permitInfo/const.ts(1 hunks)src/public/CoinGecko.59144.json(1 hunks)src/public/CoinGecko.9745.json(1 hunks)src/public/PermitInfo.59144.json(1 hunks)src/public/PermitInfo.9745.json(1 hunks)src/public/Uniswap.59144.json(1 hunks)src/public/Uniswap.9745.json(1 hunks)src/scripts/auxLists/utils.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-21T15:23:04.410Z
Learnt from: alfetopito
PR: cowprotocol/token-lists#1033
File: src/public/CoinGecko.232.json:12-34
Timestamp: 2025-07-21T15:23:04.410Z
Learning: CoinGecko token list files (like CoinGecko.232.json) are auto-generated from the CoinGecko API and any manual edits to these files will be overwritten when the generation script runs again. Token name formatting issues should be addressed in the generation script, not in the JSON files themselves.
Applied to files:
src/public/CoinGecko.9745.jsonsrc/public/CoinGecko.59144.json
🔇 Additional comments (18)
.gitignore (1)
21-24: LGTM!The re-addition of
*.logfor log files and the new.npmrcexclusion are both standard gitignore patterns appropriate for keeping local development artifacts out of version control.src/scripts/auxLists/utils.ts (2)
55-56: LGTM!The display names follow the existing capitalization pattern and are consistent with the other chain entries.
41-42: CoinGecko identifiers verified as correct.Both
'linea'and'plasma'are confirmed as valid CoinGecko platform identifiers. The verification against the CoinGecko API shows both identifiers exist and map to valid platforms with proper chain identifiers (Linea: 59144, Plasma: 9745)..github/ISSUE_TEMPLATE/3-removeTokenForm.yml (1)
33-34: LGTM!The addition of LINEA and PLASMA network options is consistent with the other network choices and aligns with the broader PR changes.
.github/workflows/updatePermitInfo.yml (1)
19-19: LGTM!The matrix correctly includes the new chain IDs (9745 for PLASMA and 59144 for LINEA) in ascending order, maintaining consistency with the existing chain support.
.github/ISSUE_TEMPLATE/1-addTokenForm.yml (2)
41-42: LGTM!The addition of LINEA and PLASMA network options is consistent with the other templates and aligns with the new chain support being added in this PR.
86-86: Good improvement to validation guidance.Adding the question "Did you test trading it on CoW Swap?" to the Reason field provides clearer guidance to users and helps ensure token submissions have been validated on the platform.
src/permitInfo/const.ts (1)
18-19: RPC endpoints verified as operational.Both the Linea (
https://rpc.linea.build) and Plasma (https://rpc.plasma.to) endpoints are confirmed operational and responding correctly to RPC calls. No blocking concerns identified.scripts/processRequest.mjs (1)
10-11: Block explorer URLs and chain IDs verified as correct.Both URLs are accessible (HTTP 200), and the chain IDs match official specifications: Linea Mainnet uses chainId 59144 and Plasma Mainnet uses chainId 9745.
package.json (1)
47-47: Acknowledge necessity but maintain supply chain caution for PR-specific dependency.The codebase explicitly requires Linea and Plasma chain support (configured in
src/scripts/auxLists/utils.tsandsrc/permitInfo/const.ts), and the latest stable cow-sdk version (6.3.2) does not include this support. PR-606 ("feat(new-chains): add q4 chains") adds Q4 chain support, making the PR-specific version dependency justified for now.However, the original concerns about reproducibility and supply chain risks remain valid. Recommend:
- Document that this dependency is temporary and tied to PR-606 merging
- Monitor PR-606 status and upgrade to the stable release immediately once merged
- Consider adding a TODO comment in package.json linking to the PR
src/public/Uniswap.9745.json (1)
1-55: Data file structure and content look good.The Uniswap token list for Plasma is properly formatted with valid token entries and metadata. All required fields are present and addresses are correctly formatted for chainId 9745.
.github/ISSUE_TEMPLATE/2-addImageForm.yml (2)
33-34: Network options successfully extended for Q4 chains.The LINEA and PLASMA network options are properly added to the form dropdown, enabling contributors to submit requests for these new chains.
48-48: Clearer SVG preference guidance improves contributor experience.The updated description explicitly states "SVG is preferred," which sets better expectations and should reduce back-and-forth on image submissions.
src/public/CoinGecko.59144.json (1)
1-702: Token list structure is valid and properly formatted.The CoinGecko token manifest for Linea follows the expected schema with all required fields. Note per previous learnings: this file is auto-generated from the CoinGecko API, so any future updates should be regenerated rather than manually edited.
src/public/Uniswap.59144.json (1)
1-55: Uniswap token list for Linea is properly structured and consistent.The curated token list contains 5 major tokens with valid data and proper schema for chainId 59144. Addresses align with other token manifests (verified spot-check with Chainlink).
src/public/PermitInfo.9745.json (1)
1-112: Permit metadata structure is appropriate and well-organized.The PermitInfo mapping for Plasma correctly distinguishes between EIP-2612 compliant tokens (with version) and unsupported tokens. The schema is consistent and aligns with token data from CoinGecko.9745.json.
src/public/PermitInfo.59144.json (1)
1-388: Permit metadata properly categorizes tokens across multiple EIP standards.The PermitInfo mapping for Linea correctly handles three permit types: standard EIP-2612, dai-like variants (e.g., Davos.xyz USD), and unsupported tokens. Version numbers are appropriate (mostly 1, with v2 for USDC and CAD Coin).
src/public/CoinGecko.9745.json (1)
1-215: CoinGecko token list for Plasma is well-structured with appropriate token selection.The 25-token manifest for Plasma (chainId 9745) follows the expected schema with consistent field types and values. Decimals are properly set per token type (6 for stablecoins, 18 for general tokens). Per previous learnings, this auto-generated file should be regenerated via script rather than manually edited for future updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)src/scripts/install-sdk-preview.js(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/scripts/install-sdk-preview.js
[error] 18-18: Illegal return statement outside of a function
(parse)
[error] 28-28: Illegal return statement outside of a function
(parse)
🪛 GitHub Actions: CI
src/scripts/install-sdk-preview.js
[error] 1-1: Permission denied while attempting to execute the script. Exit code 126. Ensure the file is executable (chmod +x src/scripts/install-sdk-preview.js).
🔇 Additional comments (1)
src/scripts/install-sdk-preview.js (1)
31-42: Verify .npmrc file location assumption.The script writes
.npmrcto the current working directory. Ensure this is always the repository root when invoked from CI.Based on the CI workflow configuration, the script runs after checkout but before any
cdcommands, so the working directory should be the repository root. However, if this script is ever run manually or from other contexts, document this requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/scripts/install-sdk-preview.mjs (3)
5-14: Consider more precise regex pattern for PR version detection.The current pattern
/pr-\d+/will match "pr-" followed by digits anywhere in the version string. While this works for the expected format, consider a more precise pattern to avoid false positives:- const sdkPrVersionRegex = /pr-\d+/ + const sdkPrVersionRegex = /^pr-\d+$|@pr-\d+$/This ensures "pr-123" appears either as the complete version or after an @ symbol (e.g., in npm tags).
17-17: Fix file extension in console messages.The console messages reference
install-sdk-preview.jsbut the file isinstall-sdk-preview.mjs.- console.log('[install-sdk-preview.js] no SDK PR version set, skipping') + console.log('[install-sdk-preview.mjs] no SDK PR version set, skipping')console.error( - '[install-sdk-preview.js] PACKAGE_READ_AUTH_TOKEN env var is not set but expected by install-sdk-preview.js', + '[install-sdk-preview.mjs] PACKAGE_READ_AUTH_TOKEN env var is not set but expected by install-sdk-preview.mjs', )Also applies to: 25-27
43-44: Add error handling for file write operation.The
fs.writeFileSyncoperation lacks error handling. If the write fails (e.g., due to permissions), the script will throw an unhandled exception. Consider wrapping it in a try-catch block:- fs.writeFileSync('.npmrc', npmrc) - core.setOutput('hasSdkPrVersion', 'true') + try { + fs.writeFileSync('.npmrc', npmrc) + core.setOutput('hasSdkPrVersion', 'true') + } catch (error) { + console.error('[install-sdk-preview.mjs] Failed to write .npmrc:', error.message) + core.setOutput('hasSdkPrVersion', 'false') + return + }Note: The script overwrites any existing
.npmrcwithout warning. This is likely acceptable in CI environments but ensure.npmrcis properly ignored in version control..github/workflows/generateAuxLists.yml (1)
19-26: Consider creating a reusable composite action to reduce duplication.This "Set up npmrc" step is duplicated across multiple workflows (executeAction.yml, generateAuxLists.yml, and per the AI summary, also in ci.yml, cowFi-tokens.yml, and updatePermitInfo.yml). Consider extracting it into a composite action for better maintainability.
Create a new file
.github/actions/setup-sdk-preview/action.yml:name: 'Setup SDK Preview' description: 'Configures npmrc for SDK preview packages from GitHub Packages' runs: using: 'composite' steps: - name: Set up npmrc uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea with: script: | const { installSdkPreview } = await import("${{github.workspace}}/src/scripts/install-sdk-preview.mjs"); installSdkPreview(context, core); env: PACKAGE_READ_AUTH_TOKEN: ${{ secrets.PACKAGE_READ_AUTH_TOKEN }}Then in each workflow, replace the duplicated step with:
- name: Setup SDK Preview uses: ./.github/actions/setup-sdk-previewNote: Composite actions can't directly access
secrets.PACKAGE_READ_AUTH_TOKEN, so you'd need to pass it as an input or use a different approach. Alternatively, consider a reusable workflow if secret access is required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/ci.yml(1 hunks).github/workflows/cowFi-tokens.yml(1 hunks).github/workflows/executeAction.yml(1 hunks).github/workflows/generateAuxLists.yml(1 hunks).github/workflows/updatePermitInfo.yml(1 hunks)src/scripts/install-sdk-preview.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/cowFi-tokens.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci.yml
- .github/workflows/updatePermitInfo.yml
🔇 Additional comments (4)
.github/workflows/executeAction.yml (2)
33-40: Script output is unused but this appears intentional.The
installSdkPreviewfunction sets an outputhasSdkPrVersion, but this output is not checked in subsequent workflow steps. This appears intentional - the workflow continues with the public npm registry if SDK preview setup is skipped or fails.
33-40: Code correctly implements PACKAGE_READ_AUTH_TOKEN handling with proper error handling.The script checks for the secret at line 22-24 of
install-sdk-preview.mjsand gracefully handles its absence by logging an error and settinghasSdkPrVersionto 'false' without failing the workflow. The workflow step correctly passes the secret via${{ secrets.PACKAGE_READ_AUTH_TOKEN }}. Secret configuration is a repository-level deployment concern, not a code issue. The implementation is sound..github/workflows/generateAuxLists.yml (1)
19-26: Correct step positioning.The npmrc setup step is correctly positioned after checkout and before Node.js setup, ensuring the authentication is configured before dependencies are installed.
src/scripts/install-sdk-preview.mjs (1)
2-2: No compatibility issues identified. Both current LTS versions—Node.js 22.x (Active) and 20.x (Maintenance) as of October 2025—support JSON import assertions, which are required since Node.js 20.10+ (stable). Thelts/*configuration in all workflows guarantees compatibility with supported versions.
|
Merging now to test the new chains in the issue templates. |
Summary
Add support for Q4 chains: Linea and Plasma
Summary by CodeRabbit
New Features
Improvements