feat(new-chains): add q4 chains #176
Conversation
|
Warning Rate limit exceeded@alfetopito has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughAdds LINEA and PLASMA to multiple chain mappings and datasources, introduces an install-sdk-preview script and a reusable setup-dependencies composite action to generate an .npmrc for SDK PRs, updates CI/workflows and Dockerfiles to optionally use an npmrc secret, bumps dependencies, and adds Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TokenHolderRepo as TokenHolderRepository
participant GoldRushAPI as GoldRush API
Client->>TokenHolderRepo: getTokenHolders(chainId, account)
TokenHolderRepo->>TokenHolderRepo: network = GOLD_RUSH_CLIENT_NETWORK_MAPPING[chainId]
alt network != null
TokenHolderRepo->>GoldRushAPI: GET /v1/{network}/...
GoldRushAPI-->>TokenHolderRepo: token holders data
TokenHolderRepo-->>Client: TokenHolderPoint[]
else network == null
TokenHolderRepo-->>Client: null (early return)
end
sequenceDiagram
participant Workflow
participant SetupAction as setup-dependencies
participant Script as install-sdk-preview.mjs
participant Runner as Job Runner
Workflow->>SetupAction: run with authToken
SetupAction->>Script: installSdkPreview(context, core)
Script-->>SetupAction: writes `.npmrc` and sets output flag (if SDK PR & token)
SetupAction->>Runner: setup-node & yarn install (uses `.npmrc` if present)
Runner->>Workflow: docker build (may pass npmrc secret when present)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
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.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/repositories/src/datasources/orderBookDbPool.ts (1)
26-29: Formatting improvements enhance code consistency.The formatting changes (semicolons, multiline function signatures, const declarations) improve code consistency and readability without altering functionality.
Also applies to: 30-30, 51-51, 53-56, 58-58, 67-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
libs/repositories/src/gen/cow/cow-api-types.tsis excluded by!**/gen/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
.gitignore(1 hunks)libs/repositories/src/const.ts(1 hunks)libs/repositories/src/datasources/coingecko.ts(2 hunks)libs/repositories/src/datasources/ethplorer.ts(1 hunks)libs/repositories/src/datasources/goldRush.ts(2 hunks)libs/repositories/src/datasources/moralis.ts(1 hunks)libs/repositories/src/datasources/orderBookDbPool.ts(3 hunks)libs/repositories/src/datasources/viem.ts(3 hunks)libs/repositories/src/repos/TokenHolderRepository/TokenHolderRepositoryGoldRush.ts(2 hunks)libs/shared/src/const.ts(2 hunks)package.json(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
libs/repositories/src/datasources/orderBookDbPool.ts (1)
apps/api/scripts/test-slippage.js (1)
chainId(111-111)
libs/repositories/src/repos/TokenHolderRepository/TokenHolderRepositoryGoldRush.ts (1)
libs/repositories/src/datasources/goldRush.ts (2)
GOLD_RUSH_CLIENT_NETWORK_MAPPING(9-24)GOLD_RUSH_API_BASE_URL(4-4)
🔇 Additional comments (18)
.gitignore (1)
75-77: LGTM!Adding
.npmrcto.gitignoreis a good practice since it may contain authentication tokens or private registry configurations.package.json (2)
33-33: Verify the PR-specific dependency before merging.The dependency is pointing to a PR-specific version (
npm:@cowprotocol/cow-sdk@pr-606) rather than a published version. Ensure this is updated to a stable release version before merging to main, or confirm this PR-specific version is intentional for testing LINEA and PLASMA support.
71-71: LGTM!The viem version bump from
^2.33.2to^2.38.5is appropriate for adding LINEA and PLASMA chain support, as these chains need to be available in the viem/chains exports.libs/repositories/src/datasources/moralis.ts (1)
21-23: LGTM!The additions are correct for extending Moralis network support. LINEA is mapped to
'linea'per Moralis documentation, and PLASMA is appropriately set tonullindicating Moralis doesn't support this network.libs/shared/src/const.ts (2)
41-42: LGTM!The LINEA and PLASMA entries are consistently added to the explorer network names mapping.
56-57: LGTM!The LINEA and PLASMA entries are consistently added to the COW API network names mapping.
libs/repositories/src/datasources/coingecko.ts (1)
23-24: LGTM!The LINEA and PLASMA platforms are correctly added to the Coingecko supported platforms mapping. These will be automatically included in the broader
COINGECKO_PLATFORMSmapping via the spread operator on line 33.libs/repositories/src/datasources/viem.ts (2)
41-42: LGTM!The LINEA and PLASMA chains are correctly added to the NETWORKS mapping, completing the viem integration for these chains.
18-20: Plasma chain is available in viem v2.38.5 — import is valid.Plasma was added in viem v2.37.4, so the import of
plasmafromviem/chainson line 20 is supported in the specified version.libs/repositories/src/repos/TokenHolderRepository/TokenHolderRepositoryGoldRush.ts (2)
50-54: LGTM!The network lookup and null guard are well-implemented. This correctly handles chains that aren't supported by GoldRush (like PLASMA, which maps to
nullinGOLD_RUSH_CLIENT_NETWORK_MAPPING) by returningnullearly, preventing invalid API calls.
56-56: LGTM!The dynamic URL construction using the
networkvariable correctly adapts the API endpoint based on the chain, enabling support for LINEA and other newly added chains.libs/repositories/src/const.ts (2)
50-53: I need to gather additional information to properly verify the review comment.Based on my verification:
Token address verified: USDT0 contract address on Plasma is 0xb8ce59fc3717ada4c02eadf9682a9e934f625ebb, confirming the address in the code is correct.
Architectural context: Plasma is a Bitcoin sidechain with native USDT support, and Plasma is a sidechain on Bitcoin backed by two key initiatives: building the Legacy Mesh for USDT0 and Plasma. Using USDT for Plasma instead of USDC is by design, not an error.
Code handling: The constant stores chain-specific stablecoin addresses that may be either USDC or USDT. The implementation in
UsdRepositoryCow.tstreats all entries uniformly—it simply destructuresaddressanddecimalswithout distinguishing token type, so the naming inconsistency has no functional impact.Assessment: The inline comment already documents the USDT usage for Plasma. While the constant name
USDCis misleading for Plasma, it accurately reflects other chains. The current documentation is adequate for the specific use case.
46-49: LINEA USDC address and decimals are correct.The USDC token address 0x176211869cA2b568f2A7D4EE941E073a821EE1ff is correct for LINEA mainnet, and USDC on Linea is an ERC‑20 token with 6 decimals. The mapping in the code is accurate and requires no changes.
libs/repositories/src/datasources/orderBookDbPool.ts (2)
43-46: Good addition of error handler for connection pool resilience.The error handler for idle database clients is a valuable improvement that will help diagnose connection pool issues. This defensive programming practice enhances the robustness of the database connection management.
22-24: No code changes needed; this is an infrastructure/deployment concern to verify with ops.The code correctly maps LINEA to
'linea'and PLASMA to'plasma', following the established lowercase naming convention used for all other chains (mainnet, xdai, base, polygon, arbitrum-one, etc.). The mappings are consistent across the codebase.However, your concern about database existence is valid but operational in nature: before deployment, the infrastructure team should confirm that the
lineaandplasmadatabases exist on bothPRODandBARNorderbook database servers, and that the credentials in the environment variables grant access to those databases. This is outside the scope of code review—it's a deployment prerequisite.libs/repositories/src/datasources/goldRush.ts (2)
9-11: Type change appropriately handled with proper null checks in place.The type change from
stringtostring | nullcorrectly reflects the domain requirement, and the consuming code inTokenHolderRepositoryGoldRush.ts(line 50-52) already includes proper null handling.
22-23: Verification complete — both network identifiers are correct.GoldRush supports Linea mainnet with chain name
linea-mainnet(chain ID 59144), and Plasma blockchain (chainId 9745) is not listed as a supported chain in GoldRush/Covalent's public documentation. Thenullvalue for PLASMA is appropriate and consistent with how the codebase handles unsupported networks in this datasource.libs/repositories/src/datasources/ethplorer.ts (1)
18-19: No issues found — code is correct.Verification confirms that:
- The URL
https://api.lineaplorer.build/is the correct official Ethplorer API endpoint for Linea- Null handling is properly implemented: the consuming code in
TokenHolderRepositoryEthplorer.tscorrectly checksif (!baseAPI)and returns null for unsupported chains like PLASMAThe code changes are safe and follow the established pattern.
alfetopito
force-pushed
the
feat/q4-chains
branch
2 times, most recently
from
adfb675 to
e96a877
Compare
alfetopito
force-pushed
the
feat/q4-chains
branch
from
e96a877 to
4867a72
Compare
| "typeorm": "^0.3.17", | ||
| "typeorm-fastify-plugin": "^1.0.5", | ||
| "viem": "^2.33.2" | ||
| "viem": "^2.38.5" |
There was a problem hiding this comment.
Updated so we have Plasma support
There was a problem hiding this comment.
Setup needed to install github packages on CI
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/actions/setup-dependencies/action.yml (2)
5-7: Consider making authToken optional.The
authTokeninput is marked as required, but it's only needed when a PR version of the SDK is detected. Consider making it optional to avoid requiring the secret when it's not needed.Apply this diff:
authToken: description: 'Authentication token for package registry' - required: true + required: falseThen update the script to handle the missing token more gracefully (it already logs an error but doesn't fail the action).
15-16: Consider formatting the script import for clarity.The import and function call are on the same line. While valid, splitting them would improve readability.
- script: const { installSdkPreview } = await import("${{github.workspace}}/src/scripts/install-sdk-preview.mjs"); - installSdkPreview(context, core); + script: | + const { installSdkPreview } = await import("${{github.workspace}}/src/scripts/install-sdk-preview.mjs"); + installSdkPreview(context, core);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/actions/setup-dependencies/action.yml(1 hunks).github/workflows/ci.yml(2 hunks).github/workflows/docker.yml(2 hunks)apps/api/Dockerfile(2 hunks)apps/notification-producer/Dockerfile(2 hunks)apps/telegram/Dockerfile(2 hunks)apps/twap/Dockerfile(2 hunks)src/scripts/install-sdk-preview.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/scripts/install-sdk-preview.mjs (1)
apps/api/scripts/test-slippage.js (1)
fs(1-1)
🪛 actionlint (1.7.8)
.github/workflows/actions/setup-dependencies/action.yml
1-1: "on" section is missing in workflow
(syntax-check)
1-1: "jobs" section is missing in workflow
(syntax-check)
2-2: unexpected key "description" for "workflow" section. expected one of "concurrency", "defaults", "env", "jobs", "name", "on", "permissions", "run-name"
(syntax-check)
4-4: unexpected key "inputs" for "workflow" section. expected one of "concurrency", "defaults", "env", "jobs", "name", "on", "permissions", "run-name"
(syntax-check)
9-9: unexpected key "runs" for "workflow" section. expected one of "concurrency", "defaults", "env", "jobs", "name", "on", "permissions", "run-name"
(syntax-check)
🪛 GitHub Actions: Publish Docker image
apps/notification-producer/Dockerfile
[error] Command failed: yarn --cwd notification-producer install --production --force. Exit code: 1. Reason: 401 Unauthorized when accessing https://npm.pkg.github.com/download/@cowprotocol/sdk-contracts-ts/0.4.1-pr-606-49530e0f.0/08e113f648278a9f754dbfc3f1428a632109d9f2
🔇 Additional comments (15)
apps/twap/Dockerfile (2)
1-1: LGTM: BuildKit syntax directive enables secret mounts.The syntax directive is required for the secret mount feature used below.
23-24: LGTM: Optional npmrc secret mount added.The secret mount is correctly marked as optional (
required=false), allowing the build to proceed whether or not private package authentication is needed.apps/telegram/Dockerfile (2)
1-1: LGTM: BuildKit syntax directive added.
23-24: LGTM: Consistent npmrc secret mount implementation.apps/api/Dockerfile (2)
1-1: LGTM: BuildKit syntax directive added.
24-25: LGTM: Consistent npmrc secret mount implementation.apps/notification-producer/Dockerfile (2)
1-1: LGTM: BuildKit syntax directive added.
23-24: The secret is being passed correctly; verify the PACKAGE_READ_AUTH_TOKEN secret value and permissions instead.The code setup is correct—PACKAGE_READ_AUTH_TOKEN flows properly from GitHub secrets → setup-dependencies action → environment variable → .npmrc file → Docker secret mount. The 401 error is not caused by the passing mechanism, but likely by either the secret value being missing/invalid or the token lacking
read:packagesscope. Check GitHub repository settings to confirm the PACKAGE_READ_AUTH_TOKEN secret exists and has the correct permissions.Likely an incorrect or invalid review comment.
.github/workflows/ci.yml (2)
21-24: LGTM: Centralized dependency setup.Replacing the individual setup-node and install steps with the composite action improves consistency and maintainability across workflows.
55-58: LGTM: Consistent dependency setup for affected projects..github/workflows/docker.yml (1)
25-28: LGTM: Consistent dependency setup..github/workflows/actions/setup-dependencies/action.yml (1)
1-28: Dismiss static analysis false positives.The actionlint warnings are false positives. This is a composite action (not a workflow), which uses a different schema with
description,inputs, andrunssections. The structure is correct.src/scripts/install-sdk-preview.mjs (3)
1-2: LGTM: Modern import syntax.Using ES modules with JSON import assertions is appropriate for Node.js LTS.
4-14: LGTM: Clear logic for detecting PR SDK versions.The regex-based detection correctly identifies PR versions across all @cowprotocol packages.
32-44: LGTM: Correctly configured .npmrc for GitHub Packages.The npmrc configuration properly sets up authentication for @cowprotocol packages from GitHub Packages while using the public registry for other packages.
Note: This file is only created when a PR SDK version is detected, which may cause issues in docker.yml (see my comment on that file).
| if (!PACKAGE_READ_AUTH_TOKEN) { | ||
| console.error( | ||
| '[install-sdk-preview.js] PACKAGE_READ_AUTH_TOKEN env var is not set but expected by install-sdk-preview.js', | ||
| ) | ||
| core.setOutput('hasSdkPrVersion', 'false') | ||
| return | ||
| } |
There was a problem hiding this comment.
Fix incorrect filename and consider failing when authentication is required.
The log message has the wrong filename. Additionally, when a PR version is detected but the token is missing, the script logs an error but continues (setting output to 'false'). This could lead to confusing failures later during dependency installation.
Apply this diff:
if (!PACKAGE_READ_AUTH_TOKEN) {
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 is required when using PR SDK versions',
)
- core.setOutput('hasSdkPrVersion', 'false')
- return
+ core.setFailed('PACKAGE_READ_AUTH_TOKEN environment variable is required for PR SDK versions')
+ process.exit(1)
}This will fail fast with a clear error rather than allowing the build to continue and fail during yarn install with a confusing 401 error.
🤖 Prompt for AI Agents
In src/scripts/install-sdk-preview.mjs around lines 24 to 30, the console error
references the wrong filename and the code currently sets hasSdkPrVersion to
'false' and returns when PACKAGE_READ_AUTH_TOKEN is missing; change the log
message to reference install-sdk-preview.mjs and fail fast instead of continuing
— call core.setFailed with a descriptive error (or throw a new Error) so the
action exits immediately with failure instead of setting the output and allowing
a later 401 during yarn install.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/docker.yml (1)
62-86: Consider upgrading docker/build-push-action to v6.The latest version is v6.18.0, while the workflow currently uses v4 (lines 64 and 81). Update to the latest stable version unless there are specific compatibility concerns with your build setup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/docker.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/docker.yml
64-64: the runner of "docker/build-push-action@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
71-71: property "setup-buildx" is not defined in object type {image_meta: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
🔇 Additional comments (1)
.github/workflows/docker.yml (1)
35-41: ✓ Conditional .npmrc detection resolves prior file-not-found risk.This check elegantly addresses the previous critical issue by detecting whether
.npmrcexists before passing it to the Docker build. The conditional build paths (lines 62–86) properly branch based onHAS_NPMRC.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docker.yml (1)
78-89: Upgrade second build step to docker/build-push-action v6 as well.Keep both branches consistent to avoid PR-only failures.
- uses: docker/build-push-action@v4 + uses: docker/build-push-action@v6
♻️ Duplicate comments (1)
.github/workflows/docker.yml (1)
44-46: Good fix: Buildx step now has an id.Resolves prior failure referencing outputs.
🧹 Nitpick comments (2)
.github/workflows/docker.yml (2)
25-29: Guard secrets for forked PRs (avoid failures).In pull_request runs from forks, secrets are unavailable; passing authToken may break the setup-dependencies step. Gate this step or make the token optional.
Example:
- - name: Setup dependencies + - name: Setup dependencies uses: ./.github/workflows/actions/setup-dependencies with: - authToken: ${{ secrets.PACKAGE_READ_AUTH_TOKEN }} + authToken: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork && '' || secrets.PACKAGE_READ_AUTH_TOKEN }} +# Or: +# if: ${{ !(github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork) }}
35-42: Don’t print full .npmrc; harden redaction.Limit output and redact multiple token patterns to avoid leaking PII/config.
Apply:
- echo "Using .npmrc:" && ls -l .npmrc && echo "---" && sed 's/_authToken=.*/_authToken=***REDACTED***/' .npmrc + echo "Using .npmrc (sanitized):" + ls -l .npmrc + echo "---" + sed -E \ + -e 's#(_authToken=)[^[:space:]]+#\1***REDACTED***#g' \ + -e 's#(//[^/]+/:_authToken=)[^[:space:]]+#\1***REDACTED***#g' \ + -e 's#(token=)[^[:space:]]+#\1***REDACTED***#g' \ + .npmrc | grep -vE '^(email|init\.author\.email)='
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/docker.yml(2 hunks)apps/api/Dockerfile(2 hunks)apps/notification-producer/Dockerfile(2 hunks)apps/telegram/Dockerfile(2 hunks)apps/twap/Dockerfile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api/Dockerfile
- apps/twap/Dockerfile
- apps/notification-producer/Dockerfile
- apps/telegram/Dockerfile
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/docker.yml
66-66: the runner of "docker/build-push-action@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/telegram/Dockerfile (1)
23-31: Fix Docker shell syntax (same issue as other Dockerfiles).Apply the same heredoc-based fix as in apps/notification-producer/Dockerfile:
-RUN --mount=type=secret,id=npmrc,target=/root/.npmrc,required=false \ - sh -eu -c ' - if [ -f /root/.npmrc ]; then - echo "Using .npmrc:" && cat /root/.npmrc | sed "s/_authToken=.*/_authToken=***REDACTED***/" - else - echo ".npmrc not present - building without GitHub Packages auth" - fi - yarn --cwd telegram install --production --force - ' +RUN --mount=type=secret,id=npmrc,target=/root/.npmrc,required=false \ + sh -eu << 'EOF' +if [ -f /root/.npmrc ]; then + echo "Using .npmrc:" && cat /root/.npmrc | sed "s/_authToken=.*/_authToken=***REDACTED***/" +else + echo ".npmrc not present - building without GitHub Packages auth" +fi +yarn --cwd telegram install --production --force +EOFapps/twap/Dockerfile (1)
23-31: Fix Docker shell syntax (same issue as other Dockerfiles).Apply the same heredoc-based fix:
-RUN --mount=type=secret,id=npmrc,target=/root/.npmrc,required=false \ - sh -eu -c ' - if [ -f /root/.npmrc ]; then - echo "Using .npmrc:" && cat /root/.npmrc | sed "s/_authToken=.*/_authToken=***REDACTED***/" - else - echo ".npmrc not present - building without GitHub Packages auth" - fi - yarn --cwd twap --omit=dev -f install - ' +RUN --mount=type=secret,id=npmrc,target=/root/.npmrc,required=false \ + sh -eu << 'EOF' +if [ -f /root/.npmrc ]; then + echo "Using .npmrc:" && cat /root/.npmrc | sed "s/_authToken=.*/_authToken=***REDACTED***/" +else + echo ".npmrc not present - building without GitHub Packages auth" +fi +yarn --cwd twap --omit=dev -f install +EOFapps/api/Dockerfile (1)
24-32: Fix Docker shell syntax (same issue as other Dockerfiles).Apply the same heredoc-based fix:
-RUN --mount=type=secret,id=npmrc,target=/root/.npmrc,required=false \ - sh -eu -c ' - if [ -f /root/.npmrc ]; then - echo "Using .npmrc:" && cat /root/.npmrc | sed "s/_authToken=.*/_authToken=***REDACTED***/" - else - echo ".npmrc not present - building without GitHub Packages auth" - fi - yarn --cwd api --omit=dev -f install - ' +RUN --mount=type=secret,id=npmrc,target=/root/.npmrc,required=false \ + sh -eu << 'EOF' +if [ -f /root/.npmrc ]; then + echo "Using .npmrc:" && cat /root/.npmrc | sed "s/_authToken=.*/_authToken=***REDACTED***/" +else + echo ".npmrc not present - building without GitHub Packages auth" +fi +yarn --cwd api --omit=dev -f install +EOF
🧹 Nitpick comments (1)
.github/workflows/docker.yml (1)
35-48: Remove debug output from "Check for .npmrc" step.Lines 37-40 contain debug logging (pwd, ls, grep) that should be removed to keep workflow logs clean and focused on the essential .npmrc detection logic.
Apply this diff to clean up the debug output:
- name: Check for .npmrc run: | - pwd - echo "---" - ls -la | grep -E "^\\.|npmrc" || echo "No .npmrc found" - echo "---" pwd_val=$PWD if [ -f .npmrc ]; then echo "HAS_NPMRC=true" >> $GITHUB_ENV echo "Full path: ${pwd_val}/.npmrc" echo "Using .npmrc:" && ls -l .npmrc && echo "---" && sed 's/_authToken=.*/_authToken=***REDACTED***/' .npmrc else echo "HAS_NPMRC=false" >> $GITHUB_ENV fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/docker.yml(2 hunks)apps/api/Dockerfile(2 hunks)apps/notification-producer/Dockerfile(2 hunks)apps/telegram/Dockerfile(2 hunks)apps/twap/Dockerfile(2 hunks)
🧰 Additional context used
🪛 Hadolint (2.14.0)
apps/api/Dockerfile
[error] 26-26: unexpected 'i'
expecting '#', '', ADD, ARG, CMD, COPY, ENTRYPOINT, ENV, EXPOSE, FROM, HEALTHCHECK, LABEL, MAINTAINER, ONBUILD, RUN, SHELL, STOPSIGNAL, USER, VOLUME, WORKDIR, a pragma, at least one space, or end of input
(DL1000)
apps/notification-producer/Dockerfile
[error] 25-25: unexpected 'i'
expecting '#', '', ADD, ARG, CMD, COPY, ENTRYPOINT, ENV, EXPOSE, FROM, HEALTHCHECK, LABEL, MAINTAINER, ONBUILD, RUN, SHELL, STOPSIGNAL, USER, VOLUME, WORKDIR, a pragma, at least one space, or end of input
(DL1000)
apps/telegram/Dockerfile
[error] 25-25: unexpected 'i'
expecting '#', '', ADD, ARG, CMD, COPY, ENTRYPOINT, ENV, EXPOSE, FROM, HEALTHCHECK, LABEL, MAINTAINER, ONBUILD, RUN, SHELL, STOPSIGNAL, USER, VOLUME, WORKDIR, a pragma, at least one space, or end of input
(DL1000)
apps/twap/Dockerfile
[error] 25-25: unexpected 'i'
expecting '#', '', ADD, ARG, CMD, COPY, ENTRYPOINT, ENV, EXPOSE, FROM, HEALTHCHECK, LABEL, MAINTAINER, ONBUILD, RUN, SHELL, STOPSIGNAL, USER, VOLUME, WORKDIR, a pragma, at least one space, or end of input
(DL1000)
⏰ 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). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (3)
.github/workflows/docker.yml (2)
70-97: Approve conditional build steps with proper v6 action and cache configuration.The dual build steps correctly handle both scenarios:
- Lines 70-85: Build with .npmrc secret when present (v6, includes cache, secret-files)
- Lines 86-97: Build without .npmrc when absent (v6, includes cache)
The conditional logic using
HAS_NPMRCenvironment variable is sound, and the upgrade from v4 to v6 of docker/build-push-action resolves the Node20 runner compatibility issue.
25-28: setup-dependencies action correctly creates .npmrc, but workflow dependency requires manual verification.The verification confirms the setup-dependencies action successfully creates .npmrc at the repository root when both conditions are met:
- A PR SDK version (matching pattern
pr-\d+) is detected in @cowprotocol/* dependencies- PACKAGE_READ_AUTH_TOKEN environment variable is provided
The action uses
fs.writeFileSync('.npmrc', npmrc)to create the file. However, the concern in the review comment is valid: if package.json contains a PR SDK dependency but the auth token secret is not provided to the workflow, the action will silently skip .npmrc creation and the docker build will fail when yarn attempts to install the SDK from GitHub Packages without authentication.Developers must ensure that whenever PR SDK dependencies are added to package.json, the PACKAGE_READ_AUTH_TOKEN secret is properly configured in the workflow environment. The docker.yml's conditional build paths handle missing .npmrc gracefully but cannot prevent build failure if authenticated SDK dependencies exist without credentials.
apps/notification-producer/Dockerfile (1)
23-31: Current Dockerfile syntax is valid; review claim of parsing error is unsupported.The original
sh -eu -c '...'syntax with backslash continuation and multiline quoted string is standard, valid Docker and works correctly with--mount=type=secret. Both the original approach (sh -eu -c) and heredoc syntax are valid options in Docker, and there is no actual evidence of a Hadolint DL1000 error or parsing failure in the code as written.The proposed heredoc change is an optional style improvement for readability, not a required fix. The review incorrectly frames this as resolving a critical syntax error when none exists.
Likely an incorrect or invalid review comment.
…r to failing step
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/docker.yml (1)
55-68: Clean up debug output in the .npmrc check step.The step contains diagnostic output (
pwd,ls -la,sedredaction display) that adds noise to the workflow logs. For production CI/CD workflows, this should be minimized.Consider either:
- Removing unnecessary diagnostics:
- - name: Check for .npmrc - run: | - pwd - echo "---" - ls -la | grep -E "^\\.|npmrc" || echo "No .npmrc found" - echo "---" - pwd_val=$PWD + - name: Check for .npmrc + run: | if [ -f .npmrc ]; then echo "HAS_NPMRC=true" >> $GITHUB_ENV - echo "Full path: ${pwd_val}/.npmrc" - echo "Using .npmrc:" && ls -l .npmrc && echo "---" && sed 's/_authToken=.*/_authToken=***REDACTED***/' .npmrc else echo "HAS_NPMRC=false" >> $GITHUB_ENV fi
- Or keeping detailed logs only for debugging (optional, if you expect to need these diagnostics):
- - name: Check for .npmrc + - name: Check for .npmrc (with diagnostics) run: | + [ "${{ runner.debug }}" = "1" ] && pwd || true + [ "${{ runner.debug }}" = "1" ] && ls -la | grep -E "^\\.|npmrc" || true if [ -f .npmrc ]; then
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/docker.yml(2 hunks)apps/api/Dockerfile(2 hunks)apps/notification-producer/Dockerfile(2 hunks)apps/telegram/Dockerfile(2 hunks)apps/twap/Dockerfile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/notification-producer/Dockerfile
🧰 Additional context used
🪛 GitHub Actions: Publish Docker image
apps/twap/Dockerfile
[error] 1-1: Yarn install failed during Docker build: 401 Unauthorized when fetching @cowprotocol/sdk-config from npm.pkg.github.com. The operation was canceled.
apps/api/Dockerfile
[error] 1-1: Yarn install failed during Docker build: 401 Unauthorized when fetching @cowprotocol/sdk-config from npm.pkg.github.com. The operation was canceled.
apps/telegram/Dockerfile
[error] 1-1: Yarn install failed during Docker build: 401 Unauthorized when fetching @cowprotocol/sdk-config from npm.pkg.github.com. The operation was canceled.
🔇 Additional comments (9)
.github/workflows/docker.yml (3)
25-28: Verify the setup-dependencies action correctly initializes .npmrc.The custom action
./.github/workflows/actions/setup-dependenciesis invoked with anauthTokensecret. Ensure the action:
- Calls the
install-sdk-preview.mjsscript correctly.- Creates or updates
.npmrcwhen a PR SDK version is detected.- Handles the case gracefully when no PR SDK version exists (the downstream HAS_NPMRC check should handle the absence of
.npmrc, but verify the action doesn't error).Please confirm the action definition at
.github/workflows/actions/setup-dependencies/action.ymlis correctly implemented and tested.
35-37: LGTM!The
setup-buildxstep now correctly includes theidfield, enabling downstream steps to reference its outputs.
70-96: LGTM!The two conditional build-push steps correctly handle both scenarios (with and without .npmrc). The
docker/build-push-action@v6is properly configured:
- Mutually exclusive conditions ensure exactly one step runs.
secret-filesis appropriately used only when.npmrcexists.- GHA cache is configured for faster builds.
- Both steps correctly reference the buildx builder output.
apps/twap/Dockerfile (2)
1-1: Docker BuildKit syntax directive added correctly.The
# syntax=docker/dockerfile:1.4directive enables BuildKit features like secret mounting. This is a best practice addition.
23-24: All verification points are satisfied; the code is correctly implemented.The Dockerfile secret mount syntax is correct, and the CI workflow properly passes the secret using the modern
secret-filesparameter indocker/build-push-action@v6, not a raw--secretflag. The setup-dependencies action triggersinstall-sdk-preview.mjs, which generates.npmrcwith thePACKAGE_READ_AUTH_TOKENsecret when SDK preview versions are detected inpackage.json. The generated.npmrcis then mounted to/root/.npmrcduring the Docker build via thesecret-filesparameter. All three verification points are satisfied:
- The CI workflow passes the secret correctly via
secret-filesto the buildx-based build action- The
PACKAGE_READ_AUTH_TOKENGitHub Actions secret is configured and accessible- The
.npmrcis generated with valid credentials fornpm.pkg.github.comLikely an incorrect or invalid review comment.
apps/api/Dockerfile (2)
1-1: Docker BuildKit syntax directive added correctly.Consistent with other Dockerfiles in the PR; syntax directive enables required BuildKit features.
24-25: The review comment is incorrect and should be dismissed.The implementation is not a critical authentication blocker. Both Dockerfiles correctly use
required=falseon the secret mount, making it optional. The CI workflow (docker.yml) properly handles secret provisioning:
- It checks for
.npmrcfile presence- When present: passes the secret via
secret-files: id=npmrc,src=${{ github.workspace }}/.npmrcto the docker build- When absent: builds without the secret using a fallback step
The docker/build-push-action@v6 with Docker Buildx correctly supports this pattern. The setup is functional and has no authentication blocker.
Likely an incorrect or invalid review comment.
apps/telegram/Dockerfile (2)
1-1: Docker BuildKit syntax directive added correctly.Consistent pattern across all three app Dockerfiles.
23-24: The review comment is incorrect. The workflow is properly configured to handle.npmrcsecret passing.Based on code inspection:
- Secret mount syntax is correct in both Dockerfiles
- Workflow properly passes secrets via
docker/build-push-action@v6usingsecret-files: id=npmrc,src=${{ github.workspace }}/.npmrc(conditional on.npmrcexistence).npmrccreation is automated byinstall-sdk-preview.mjs, which readsPACKAGE_READ_AUTH_TOKENfrom the environment and writes the file at the repository root- Conditional logic in docker.yml prevents build failures when secrets are not needed
The setup functions as intended:
setup-dependenciesaction passesPACKAGE_READ_AUTH_TOKEN→install-sdk-preview.mjscreates.npmrc→docker buildconditionally mounts it via BuildKit secret syntax. There is no systemic CI workflow issue or code defect to fix.The only external configuration requirement is that
PACKAGE_READ_AUTH_TOKENmust be set as a GitHub Actions repository secret—this is not a code review issue.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/docker.yml (2)
55-68: Solid npmrc detection with redacted token logging.The check properly detects .npmrc presence, redacts sensitive tokens in logs for security, and sets the HAS_NPMRC environment variable for conditional build steps downstream. However, note the path handling inconsistency: this step uses a relative path check (
if [ -f .npmrc ]), while the secrets mount (line 85) uses an absolute path (${{ github.workspace }}/.npmrc). Both should reference the repo root, so this is likely safe but could be made more explicit for clarity.As a minor note, consider whether this debugging output (ls, pwd, redacted cat) is necessary in all workflow runs or should be conditionally logged only on failure.
Consider normalizing path references to always use
${{ github.workspace }}for consistency:pwd_val=$PWD - if [ -f .npmrc ]; then + if [ -f "${{ github.workspace }}/.npmrc" ]; then echo "HAS_NPMRC=true" >> $GITHUB_ENV echo "Full path: ${pwd_val}/.npmrc" echo "Using .npmrc:" && ls -l .npmrc && echo "---" && sed 's/_authToken=.*/_authToken=***REDACTED***/' .npmrc
81-82: Minor: Trailing whitespace after cache configuration.Lines 81–82 have trailing spaces after
type=ghaon both cache-from and cache-to lines. While this does not affect functionality, it may violate linting rules if the project has a whitespace policy.- cache-from: type=gha - cache-to: type=gha,mode=max + cache-from: type=gha + cache-to: type=gha,mode=max
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/docker.yml(2 hunks)
🔇 Additional comments (4)
.github/workflows/docker.yml (4)
25-28: Clean replacement of inline Node.js setup with composite action.The custom setup-dependencies action improves reusability across workflows. The authToken is correctly passed as a parameter to enable authenticated npm installs when needed.
70-86: Conditional build with .npmrc uses v6 and includes cache configuration.The step correctly gates on
env.HAS_NPMRC == 'true'and uses docker/build-push-action@v6 with GitHub Actions cache. The secrets mount syntax appears correct for passing .npmrc to Docker.One observation: Line 83's comment mentions "Provide both possible input names to be robust across action versions," but only a single
secretsentry is shown. Verify this comment is still accurate, or clarify what the alternative input name would be.
70-98: Verifybuilderparameter support in docker/build-push-action@v6.The workflow passes
builder: ${{ steps.setup-buildx.outputs.name }}to both build-push steps (lines 79 and 96). While docker/build-push-action@v6 examples show build and push uses, the official documentation and standard examples do not reference abuilderinput parameter. The action typically auto-detects the buildx instance created by setup-buildx-action.Verify that this parameter is supported in v6. If unsupported, the action will likely ignore it and auto-detect buildx anyway, but it's worth confirming this is intentional and not a copy-paste error from an older version.
70-98: docker/build-push-action@v6 correctly uses the builder parameter.The builder parameter is a documented input that accepts a Buildx builder name/output to specify which BuildKit/buildx builder instance the action should use. The code correctly references
${{ steps.setup-buildx.outputs.name }}from setup-buildx-action, which is the intended usage pattern. Both conditional steps properly handle the npmrc scenarios.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/scripts/install-sdk-preview.mjs (2)
16-20: Fix incorrect filename in log message.The log message references "install-sdk-preview.js" but the file is "install-sdk-preview.mjs".
Apply this diff:
if (!hasSdkPrVersion) { - console.log('[install-sdk-preview.js] no SDK PR version set, skipping') + console.log('[install-sdk-preview.mjs] no SDK PR version set, skipping') core.setOutput('hasSdkPrVersion', 'false') return }
24-30: Fix incorrect filename and fail fast when authentication is required.The log message references the wrong filename. Additionally, when a PR version is detected but the token is missing, the script should fail immediately rather than continuing, which could lead to confusing 401 errors during dependency installation.
Apply this diff:
if (!PACKAGE_READ_AUTH_TOKEN) { 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 is required when using PR SDK versions', ) - core.setOutput('hasSdkPrVersion', 'false') - return + core.setFailed('PACKAGE_READ_AUTH_TOKEN environment variable is required for PR SDK versions') + process.exit(1) }
🧹 Nitpick comments (2)
src/scripts/install-sdk-preview.mjs (2)
4-4: Remove unusedcontextparameter.The
contextparameter is declared but never used in the function body.Apply this diff:
-export function installSdkPreview(context, core) { +export function installSdkPreview(core) {
46-47: Add error handling for file write operation.The
fs.writeFileSynccall can throw exceptions (e.g., permission denied, disk full) that would crash the script with an unclear error message.Apply this diff:
- fs.writeFileSync('.npmrc', npmrc) - core.setOutput('hasSdkPrVersion', 'true') + try { + fs.writeFileSync('.npmrc', npmrc) + core.setOutput('hasSdkPrVersion', 'true') + } catch (error) { + core.setFailed(`Failed to write .npmrc: ${error.message}`) + process.exit(1) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/scripts/install-sdk-preview.mjs(1 hunks)
🔇 Additional comments (1)
src/scripts/install-sdk-preview.mjs (1)
32-44: LGTM!The .npmrc configuration correctly sets up authentication for GitHub Packages while maintaining public npm as the default registry.
| const PACKAGE_READ_AUTH_TOKEN = process.env.PACKAGE_READ_AUTH_TOKEN | ||
|
|
||
| if (!PACKAGE_READ_AUTH_TOKEN) { | ||
| console.error( | ||
| '[install-sdk-preview.js] PACKAGE_READ_AUTH_TOKEN env var is not set but expected by install-sdk-preview.js', | ||
| ) | ||
| core.setOutput('hasSdkPrVersion', 'false') | ||
| return | ||
| } |
There was a problem hiding this comment.
Validate that the token is non-empty.
The current check only verifies that PACKAGE_READ_AUTH_TOKEN is truthy, but an empty string would pass this check and result in an invalid .npmrc with a blank auth token.
Apply this diff:
const PACKAGE_READ_AUTH_TOKEN = process.env.PACKAGE_READ_AUTH_TOKEN
- if (!PACKAGE_READ_AUTH_TOKEN) {
+ if (!PACKAGE_READ_AUTH_TOKEN || PACKAGE_READ_AUTH_TOKEN.trim() === '') {
console.error(
'[install-sdk-preview.mjs] PACKAGE_READ_AUTH_TOKEN is required when using PR SDK versions',
)
core.setFailed('PACKAGE_READ_AUTH_TOKEN environment variable is required for PR SDK versions')
process.exit(1)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const PACKAGE_READ_AUTH_TOKEN = process.env.PACKAGE_READ_AUTH_TOKEN | |
| if (!PACKAGE_READ_AUTH_TOKEN) { | |
| console.error( | |
| '[install-sdk-preview.js] PACKAGE_READ_AUTH_TOKEN env var is not set but expected by install-sdk-preview.js', | |
| ) | |
| core.setOutput('hasSdkPrVersion', 'false') | |
| return | |
| } | |
| const PACKAGE_READ_AUTH_TOKEN = process.env.PACKAGE_READ_AUTH_TOKEN | |
| if (!PACKAGE_READ_AUTH_TOKEN || PACKAGE_READ_AUTH_TOKEN.trim() === '') { | |
| console.error( | |
| '[install-sdk-preview.mjs] PACKAGE_READ_AUTH_TOKEN is required when using PR SDK versions', | |
| ) | |
| core.setFailed('PACKAGE_READ_AUTH_TOKEN environment variable is required for PR SDK versions') | |
| process.exit(1) | |
| } |
🤖 Prompt for AI Agents
In src/scripts/install-sdk-preview.mjs around lines 22 to 30, the env token
check only tests truthiness which allows an empty string; update the guard to
treat empty/whitespace-only values as missing by checking that
PACKAGE_READ_AUTH_TOKEN is defined and its trimmed length is > 0 (e.g.,
PACKAGE_READ_AUTH_TOKEN && PACKAGE_READ_AUTH_TOKEN.trim() !== ''), and if it
fails, log the same error, set core output 'hasSdkPrVersion' to 'false', and
return.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/Dockerfile(2 hunks)apps/notification-producer/Dockerfile(2 hunks)apps/telegram/Dockerfile(2 hunks)apps/twap/Dockerfile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/notification-producer/Dockerfile
🔇 Additional comments (2)
apps/twap/Dockerfile (1)
1-1: LGTM!The syntax directive and secret-mount pattern are well-implemented. The nonrequired secret mount, conditional file handling, always-auth enforcement, and post-install cleanup are all correct. The
--cwd twapreference is accurate for this application.Also applies to: 23-33
apps/api/Dockerfile (1)
1-1: LGTM!The syntax directive and secret-mount pattern are well-implemented. The nonrequired secret mount, conditional file handling, always-auth enforcement, and post-install cleanup are all correct. The
--cwd apireference is accurate for this application.Also applies to: 24-34
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/Dockerfile (1)
1-1: Implementation consistent with other Dockerfiles.The BuildKit secret authentication pattern is correctly applied with
--cwd apifor the api directory. The implementation follows the same secure credential handling as twap and telegram: token extraction, npm configuration, immediate cleanup, and proper error handling.Optional: Consider centralizing BuildKit RUN logic.
The three Dockerfiles (api, twap, telegram) now contain nearly identical 24-line RUN blocks for secret-based authentication. If this pattern will be replicated across additional Dockerfiles or microservices, consider extracting this as a shared build stage or a reusable BuildKit pattern. This would reduce maintenance burden and ensure consistency as the pattern evolves.
For now, the individual implementations are correct and maintainable.
Also applies to: 25-48
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/Dockerfile(2 hunks)apps/notification-producer/Dockerfile(2 hunks)apps/telegram/Dockerfile(2 hunks)apps/twap/Dockerfile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/notification-producer/Dockerfile
🔇 Additional comments (2)
apps/twap/Dockerfile (1)
1-1: BuildKit secret authentication implementation looks solid.The refactored RUN instruction properly mounts an optional npmrc secret, extracts credentials safely, and cleans up immediately after use. The implementation correctly:
- Uses BuildKit 1.4 syntax for secret mounting with
required=false- Conditionally applies the secret only when available
- Extracts the token without exposing it in build logs or layers
- Sets proper permissions (chmod 600) and environment variables
- Cleans up credentials with
rm -fto minimize exposure- Uses correct
--cwd twapfor the application directoryThe error handling with
|| trueis appropriate for optional secrets.Also applies to: 24-47
apps/telegram/Dockerfile (1)
46-46: Past copy-paste error has been corrected.The previous review flagged an incorrect
--cwd notification-producerreference. The current implementation correctly uses--cwd telegram, ensuring telegram service dependencies are installed in the telegram directory. The BuildKit secret implementation is consistent with twap/Dockerfile and follows all security best practices.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (6)
.github/workflows/docker.yml(3 hunks)apps/api/Dockerfile(1 hunks)apps/notification-producer/Dockerfile(2 hunks)apps/telegram/Dockerfile(2 hunks)apps/twap/Dockerfile(1 hunks)package.json(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/twap/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/Dockerfile
- package.json
- apps/telegram/Dockerfile
⏰ 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). (1)
- GitHub Check: Test and Lint Affected Projects
🔇 Additional comments (3)
.github/workflows/docker.yml (3)
35-37: Good—step ID enables reuse in builder reference.Adding
id: setup-buildxcorrectly allows line 64 to reference${{ steps.setup-buildx.outputs.name }}for the builder parameter.
66-67: Good—GHA cache improves build performance.Adding cache-from/cache-to with GitHub Actions backend is a best practice that will improve subsequent build times without additional infrastructure.
68-70: Verify: How .npmrc is made available to Docker build if secrets handling is disabled.The secret-files configuration is disabled as unresolved ("Disabled as I couldn't get it to work with GH pkgs"), but the build still conditionally depends on
.npmrcexisting (viaif: HAS_NPMRC == 'true'). Without secret-files injection or a COPY in the Dockerfile, the Docker build won't have access to.npmrc.Clarify how .npmrc is passed to the Docker build:
- Is
.npmrccopied into dist/ or the build context before the Docker build step?- Should secret-files be re-enabled once the GitHub Packages issue is resolved?
- Or should the dependency on HAS_NPMRC be removed entirely?
| [57]: 'syscoin', | ||
| [570]: 'rollux', | ||
| [57073]: 'ink', | ||
| [59144]: 'linea', |
There was a problem hiding this comment.
Because it's now added above to the supported ones:
Summary
Setup Linea and Plasma
Testing
Summary by CodeRabbit
New Features
Chores
New Tools