fix : added validation in add image form. #1184
fix : added validation in add image form. #1184minato32 wants to merge 9 commits intocowprotocol:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds YAML validation fields to an issue template and a new GitHub Actions workflow that extracts Network and Address from issue bodies, validates address format, optionally queries chain RPC getCode, and comments/labels issues on validation failure. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant GH as GitHub Issues
participant GA as GitHub Actions
participant Script as Validation Script
participant RPC as Chain RPC (JsonRpcProvider)
participant API as GitHub API
User->>GH: Open/Edit issue labeled "addImage"
GH-->>GA: Trigger workflow
GA->>Script: Checkout, setup Node, install ethers, run validation
Script->>GH: Read issue body (Network, Address)
Script->>Script: Check Address presence and ^0x[a-fA-F0-9]{40}$
alt Missing or invalid format
Script-->>GA: Fail validation
else RPC mapping exists
Script->>RPC: provider.getCode(Address)
alt getCode == "0x" or provider error indicating invalid
Script-->>GA: Fail validation
else Code present
Script-->>GA: Pass
end
else No RPC mapping
Script-->>GA: Skip on-chain check (treat as pass)
end
opt On failure
GA->>API: Post comment with error details
GA->>API: Add "invalid-address" label
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip Migrating from UI to YAML configuration.Use the |
|
I have read the CLA Document and I hereby sign the CLA |
|
closes #109 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
.github/ISSUE_TEMPLATE/2-addImageForm.yml (2)
43-43: Broaden address regex to accept 0X prefix (optional)Some users paste addresses with 0X. Consider allowing both 0x/0X; checksum enforcement can remain out-of-band.
- pattern: "^0x[a-fA-F0-9]{40}$" + pattern: "^0[xX][a-fA-F0-9]{40}$"
53-53: Relax URL regex; current pattern rejects valid URLs with query/fragment and allows underscores in hostSimplify to accept any http(s) URL without spaces; reduces false negatives and avoids host underscore ambiguity.
- pattern: "^(https?://[\\w.-]+(?:/[\\w\\-.~!$&'()*+,;=:@%]*)*)$" + pattern: "^https?://\\S+$".github/workflows/validate-token-address.yml (5)
20-22: Avoid modifying repo deps; pin ethers and don’t savePrevent npm from touching package.json/lockfile and pin a known-major.
- - name: Install ethers.js - run: npm install ethers + - name: Install ethers.js + run: npm install --no-save ethers@6
23-40: Harden extraction and sanitize outputs
- Make matching more specific to form headers to avoid accidental captures.
- Constrain network to the allowed set before exporting.
- name: Extract address and network id: extract uses: actions/github-script@v7 with: script: | const body = context.payload.issue.body; - // Extract Network - const networkMatch = body.match(/Network\s*\n\s*(.*)/); + // Extract Network (expects "### Network" section in issue form) + const networkMatch = body.match(/#+\s*Network\s*\n\s*([A-Z0-9_]+)/); const network = networkMatch ? networkMatch[1].trim() : null; - // Extract Address - const addressMatch = body.match(/Address\s*\n\s*(0x[a-fA-F0-9]{40})/); + // Extract Address (expects "### Address" section) + const addressMatch = body.match(/#+\s*Address\s*\n\s*(0x[a-fA-F0-9]{40})/); const address = addressMatch ? addressMatch[1].trim() : null; - core.setOutput('network', network || ''); + const allowed = new Set(['MAINNET','GNOSIS_CHAIN','ARBITRUM_ONE','BASE','POLYGON','AVALANCHE','BNB','LENS']); + core.setOutput('network', (network && allowed.has(network)) ? network : ''); core.setOutput('address', address || '');
55-64: RPC map OK; consider fallbacks (optional)Public RPCs can rate-limit. Optional: add multiple URLs per network or a fallback map.
72-85: Add explicit timeout and non-zero exit on provider hangYou added a step timeout above; that’s good to avoid hanging runs. Consider retry/backoff if RPC is flaky. Optional.
87-110: Fix step condition syntax and de-duplicate labels/comments
- Use expression syntax for if.
- Optionally remove the invalid-address label when a subsequent edit passes validation.
- - name: Add comment and label if invalid - if: failure() + - name: Add comment and label if invalid + if: ${{ failure() }} uses: actions/github-script@v7 with: script: | const issue_number = context.issue.number; const address = '${{ steps.extract.outputs.address }}'; const network = '${{ steps.extract.outputs.network }}'; // Add comment await github.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number, body: `⚠️ The address \`${address || 'N/A'}\` does not exist or is invalid on network \`${network || 'N/A'}\`. Please verify.` }); // Add label - await github.issues.addLabels({ + await github.issues.addLabels({ owner: context.repo.owner, repo: context.repo.repo, issue_number, labels: ['invalid-address'] });To remove the label on success, add the following step after validation:
- name: Remove invalid-address label on success if: ${{ success() }} uses: actions/github-script@v7 with: script: | const labels = (await github.issues.listLabelsOnIssue({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number })).data.map(l => l.name); if (labels.includes('invalid-address')) { await github.issues.removeLabel({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, name: 'invalid-address' }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/ISSUE_TEMPLATE/2-addImageForm.yml(1 hunks).github/workflows/validate-token-address.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/validate-token-address.yml
13-13: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
16-16: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (2)
.github/ISSUE_TEMPLATE/2-addImageForm.yml (2)
39-39: Address description LGTMClear and helpful guidance for contributors.
49-50: Image field copy/placeholder LGTMGood UX; sets expectation without blocking.
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
Thank you for the contribution!
Please make sure to acknowledge/fix if appropriate comments by CodeRabbit as well.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.yarn/install-state.gzis excluded by!**/.yarn/**,!**/*.gzyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
.github/workflows/validate-token-address.yml(1 hunks).yarnrc.yml(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .yarnrc.yml
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/validate-token-address.yml (1)
36-41: Consider making extraction more robust.The regex patterns assume a specific issue body format. If the issue template changes or users edit the structure, extraction may fail silently.
Consider:
- Adding validation that both fields were extracted successfully
- Logging extracted values for debugging
- Using more forgiving regex patterns or structured parsing
Example improvement:
const addressMatch = body.match(/Address\s*\n\s*(0x[a-fA-F0-9]{40})/); const address = addressMatch ? addressMatch[1].trim() : null; + console.log('Extracted network:', network); + console.log('Extracted address:', address); + core.setOutput('network', network || ''); core.setOutput('address', address || '');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/validate-token-address.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/validate-token-address.yml (1)
112-135: LGTM!The failure handling correctly uses the
failure()condition, provides helpful feedback to users, and adds an appropriate label for triage.
@alfetopito @shoom3301 hey I have resolved all the coderabbit comments as well. |
alfetopito
left a comment
alfetopito
left a comment
There was a problem hiding this comment.
Thanks again for the contribution, and sorry for taking so long to review it.
Do you mind updating the PR so we can accept the change?
Added LINEA and PLASMA chain to rpc map as asked as well
|
@alfetopito hey i have updated the PR. apologies for taking this long |
3 participants
This PR adds a GitHub Action that automatically checks token addresses submitted via the “Add Image” issue form.
What it does:
If invalid or missing:
Posts a comment to let the contributor know.
Adds an invalid-address label.
Summary by CodeRabbit
New Features
Documentation