feat: Implement OCToken NFT Open Collaborator Award mechanism (Fixes #53)#76
Open
sureshchouksey8 wants to merge 5 commits into
Open
feat: Implement OCToken NFT Open Collaborator Award mechanism (Fixes #53)#76sureshchouksey8 wants to merge 5 commits into
sureshchouksey8 wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthrough新增 Award 类型字段，并实现 ChangesAward 颁发
🎯 3 (Moderate) | ⏱️ ~20 分钟
可能相关的 PR
建议标签
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pages/api/Lark/award/issue.ts (1)
13-15: ⚡ Quick win建议补充
walletAddress格式校验，避免脏数据入库。当前只校验非空，非法地址也会被写入
Award记录，后续发放/查询会出现数据一致性问题。建议在参数校验阶段增加地址格式检查。✅ 可直接应用的最小修复
if (!recordId || !walletAddress) { context.throw(400, 'recordId and walletAddress are required'); } + if (!/^0x[a-fA-F0-9]{40}$/.test(walletAddress)) { + context.throw(400, 'walletAddress format is invalid'); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/api/Lark/award/issue.ts` around lines 13 - 15, 当前只对 walletAddress 做了非空校验，应增加格式校验以避免非法地址入库：在 issue.ts 中对 recordId 和 walletAddress 的现有校验位置（与 context.throw(400, ...) 同块）加入地址格式验证（例如以 /^0x[a-fA-F0-9]{40}$/ 或项目统一的 isValidAddress(address) 工具函数进行校验），如果验证失败则用 context.throw(400, 'invalid walletAddress') 返回错误；确保后续写入 Award 记录前通过该校验。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pages/api/Lark/award/issue.ts`:
- Around line 18-19: The current code fabricates transactionHash and tokenId
locally instead of calling the real minting/issuance service; replace the
Math.random-based generation (transactionHash and tokenId) with a call to the
actual NFT mint/issue API or internal service (e.g., call mintService.mint(...)
or nftIssuer.issue(...)), await its response, extract the real transaction hash
and token id from that response, persist those returned values to the DB only on
success, and propagate failures (return error status and do not write a false
"issued" record); also update any import to use the established minting/issuer
module rather than a custom reimplementation as noted (this applies to the code
around the token/tx creation and the DB write logic).
---
Nitpick comments:
In `@pages/api/Lark/award/issue.ts`:
- Around line 13-15: 当前只对 walletAddress 做了非空校验，应增加格式校验以避免非法地址入库：在 issue.ts 中对
recordId 和 walletAddress 的现有校验位置（与 context.throw(400, ...) 同块）加入地址格式验证（例如以
/^0x[a-fA-F0-9]{40}$/ 或项目统一的 isValidAddress(address) 工具函数进行校验），如果验证失败则用
context.throw(400, 'invalid walletAddress') 返回错误；确保后续写入 Award 记录前通过该校验。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 301da9bf-45d2-4afe-a63d-882e323de5e7
📒 Files selected for processing (2)
models/Award.tspages/api/Lark/award/issue.ts
Author
|
Validation update for linked bounty #53:
Current PR head is |
Author
|
Addressed the remaining CodeRabbit wallet validation nit in What changed:
Validation rerun locally:
This keeps PR #76 aligned with issue #53 while preventing malformed wallet data from entering the award table. |
Author
|
Scope clarification for review on #53: While rechecking CodeRabbit's note about the generated So the current PR should be reviewed as the Lark/Bitable award issuance data-flow slice:
I do not want to overclaim this as a real on-chain mint without an existing OCToken service contract/API in the project. If the intended acceptance criterion is real chain minting, please point me to the issuer API/contract/module to integrate; otherwise this is ready for review as the award-table issuance workflow for the |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pages/api/Lark/award/issue.ts (1)
13-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value避免使用
any，请补充强类型以符合 TypeScript 规范。
(context.request as any).body与await response.json()（data为隐式any）绕过了类型检查，data.transactionHash/data.tokenId缺乏校验保障。建议为请求体与铸造响应定义接口类型。🔧 示意：为请求体与响应定义类型
- const { recordId, walletAddress } = (context.request as any).body; + const { recordId, walletAddress } = context.request.body as { + recordId?: string; + walletAddress?: string; + }; @@ - const data = await response.json(); - transactionHash = data.transactionHash; - tokenId = data.tokenId; + const data = (await response.json()) as { + transactionHash?: string; + tokenId?: string; + }; + transactionHash = data.transactionHash; + tokenId = data.tokenId;如代码规范
**/*.{ts,tsx}所述：“Let TypeScript infer types when possible to avoid verbose annotations”，应避免使用any削弱类型安全。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/api/Lark/award/issue.ts` around lines 13 - 40, The issue is that `issue.ts` uses `any` for both the incoming request body and the mint API response, which bypasses TypeScript safety. Update the handler around `context.request` and the `fetch`/`response.json()` flow by introducing explicit interfaces for the request payload and the mint response, then use those types instead of `(context.request as any).body` and untyped `data` in the `walletAddress`, `recordId`, `transactionHash`, and `tokenId` assignments. Keep the validation in `issue`/`mint` flow intact, but ensure the parsed JSON is checked against the typed response shape before reading its fields.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pages/api/Lark/award/issue.ts`:
- Around line 26-47: 外部铸造调用缺少超时控制，`issue.ts` 中的 `fetch(mintApiUrl, ...)` 可能因
`NFT_MINT_API` 慢响应而一直挂起；请在该铸造请求里加入 `AbortController` 或等效超时机制，并让超时时间与默认
`mintApiUrl` 一样可通过配置覆盖。更新 `context.throw` 处理路径，确保超时被明确转换为 502/可读错误，并保留
`transactionHash`、`tokenId` 的现有校验逻辑不变。
- Line 12: The issue is that the /issue endpoint in router.post currently only
authenticates with verifyJWT and lacks authorization checks for the specific
resource being awarded. Add a permission check inside the award handler to
verify the authenticated user is allowed to operate on the targeted
walletAddress or recordId, such as confirming ownership or requiring admin
privileges, and reject unauthorized requests before proceeding with the issue
flow.
- Around line 12-57: Add an idempotency check to the award issuance flow in the
router.post('/issue') handler before calling the Mint API. Use AwardModel to
query whether the incoming recordId already has an existing issuance record, and
if it does, either return the existing transaction/token data or throw a 409
conflict instead of minting again. Keep the check close to the existing fetch
logic so the repeated-submit path is handled before any external side effects.
---
Nitpick comments:
In `@pages/api/Lark/award/issue.ts`:
- Around line 13-40: The issue is that `issue.ts` uses `any` for both the
incoming request body and the mint API response, which bypasses TypeScript
safety. Update the handler around `context.request` and the
`fetch`/`response.json()` flow by introducing explicit interfaces for the
request payload and the mint response, then use those types instead of
`(context.request as any).body` and untyped `data` in the `walletAddress`,
`recordId`, `transactionHash`, and `tokenId` assignments. Keep the validation in
`issue`/`mint` flow intact, but ensure the parsed JSON is checked against the
typed response shape before reading its fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 99777876-4de5-4f5f-8732-247dd4c540f4
📒 Files selected for processing (1)
pages/api/Lark/award/issue.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pages/api/Lark/award/issue.ts (2)
13-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win先校验
recordId类型再访问 Lark 与铸造服务。当前只做 truthy 检查，数组或对象会继续进入
getOne()、mint 请求体和updateOne()。请收窄为非空字符串并复用规范化后的值。建议修复
- const { recordId, walletAddress } = (context.request as any).body; + const { recordId, walletAddress } = (context.request.body ?? {}) as { + recordId?: unknown; + walletAddress?: unknown; + }; - if (!recordId || !walletAddress) { + if (typeof recordId !== 'string' || !recordId.trim() || !walletAddress) { context.throw(400, 'recordId and walletAddress are required'); } + + const awardRecordId = recordId.trim(); - const award = await awardModel.getOne(recordId); + const award = await awardModel.getOne(awardRecordId);并在 mint 请求与
updateOne()中使用awardRecordId。Also applies to: 63-67, 89-96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/api/Lark/award/issue.ts` around lines 13 - 25, The current `recordId` validation in `issue` only checks truthiness, so non-string values can flow into `AwardModel.getOne()`, the Lark mint request, and `updateOne()`. Tighten the input check to require a non-empty string, normalize it once into a canonical variable such as `awardRecordId`, and then use that normalized value everywhere the record ID is passed, including the mint request body and `updateOne()`.
74-80: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win校验 mint 响应 schema 后再写回。
response.json()是不可信边界；仅做 truthy 检查会让对象、错误格式哈希等值进入持久化。至少校验transactionHash为字符串且符合交易哈希格式，并把tokenId规范化为非空字符串。建议修复
- const data = await response.json(); - transactionHash = data.transactionHash; - tokenId = data.tokenId; + const data = (await response.json()) as { + transactionHash?: unknown; + tokenId?: unknown; + }; + + if ( + typeof data.transactionHash !== 'string' || + !/^0x[a-fA-F0-9]{64}$/.test(data.transactionHash) || + (typeof data.tokenId !== 'string' && typeof data.tokenId !== 'number') || + `${data.tokenId}`.trim() === '' + ) { + throw new Error('Invalid response from minting service'); + } + + transactionHash = data.transactionHash; + tokenId = `${data.tokenId}`; - if (!transactionHash || !tokenId) { - throw new Error('Invalid response from minting service'); - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/api/Lark/award/issue.ts` around lines 74 - 80, The mint response handling in issueToken should validate the parsed JSON before assigning transactionHash and tokenId. In the response.json() flow, replace the simple truthy check with explicit schema validation: ensure transactionHash is a string matching the expected transaction-hash format and tokenId is normalized to a non-empty string before any write-back or downstream use. If validation fails, keep throwing the invalid response error from issueToken.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pages/api/Lark/award/issue.ts`:
- Around line 44-52: The current idempotency check in issue flow returns an
existing award purely from transactionHash/tokenId and does not bind the result
to walletAddress, so concurrent requests can still double-mint and later
overwrite each other via updateOne. Update the award issuance path in issue.ts
to make minting idempotent by recordId or by reserving an “issuing” lock/status
before calling the external mint service, then finalize the record after
success; also verify the incoming walletAddress matches the stored wallet and
return 409 on mismatch. Apply the same fix to the duplicated logic around the
referenced issue-processing block so both paths use the same wallet-bound
concurrency guard.
- Around line 36-40: 当前发放授权逻辑在 issueAward 相关校验里使用了 currentUser.name 与 “Robot”
展示名以及 award.nominator/award.nomineeName 做比较，存在重名和篡改风险。请改为基于 JWT 负载中的稳定字段
currentUser.id 做权限判断，并把奖项记录中参与比对的字段切换为内部 ID（包括 Robot 的 ID 0）。确保 issue.ts
中这段授权分支只依赖 ID 级别的身份校验，避免继续使用文本身份。
---
Outside diff comments:
In `@pages/api/Lark/award/issue.ts`:
- Around line 13-25: The current `recordId` validation in `issue` only checks
truthiness, so non-string values can flow into `AwardModel.getOne()`, the Lark
mint request, and `updateOne()`. Tighten the input check to require a non-empty
string, normalize it once into a canonical variable such as `awardRecordId`, and
then use that normalized value everywhere the record ID is passed, including the
mint request body and `updateOne()`.
- Around line 74-80: The mint response handling in issueToken should validate
the parsed JSON before assigning transactionHash and tokenId. In the
response.json() flow, replace the simple truthy check with explicit schema
validation: ensure transactionHash is a string matching the expected
transaction-hash format and tokenId is normalized to a non-empty string before
any write-back or downstream use. If validation fails, keep throwing the invalid
response error from issueToken.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac4efecc-2cb5-4b9a-b13e-5faaebc537c2
📒 Files selected for processing (1)
pages/api/Lark/award/issue.ts
Comment on lines
+36
to
+40
| if ( | ||
| currentUser.name !== 'Robot' && | ||
| currentUser.name !== award.nominator && | ||
| currentUser.name !== award.nomineeName | ||
| ) { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 验证 verifyJWT 写入 context.state.user 的字段，以及是否已有稳定 role/id 可复用。
rg -nP -C4 '\bverifyJWT\b|context\.state\.user|state\.user|isAdmin|role|userId|email|name' pages models --type=tsRepository: Open-Source-Bazaar/Open-Source-Bazaar.github.io
Length of output: 47055
不要用展示名和 Robot 字符串做发放授权。
currentUser.name 是文本身份，存在重名或篡改风险。验证确认 JWT 负载中包含稳定字段 id（其中 Robot 的 ID 为 0）。请改用 currentUser.id 进行权限校验，并将奖项记录中的比对字段对应为内部 ID，确保认证逻辑的稳定性与安全性。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/api/Lark/award/issue.ts` around lines 36 - 40, 当前发放授权逻辑在 issueAward
相关校验里使用了 currentUser.name 与 “Robot” 展示名以及 award.nominator/award.nomineeName
做比较，存在重名和篡改风险。请改为基于 JWT 负载中的稳定字段 currentUser.id 做权限判断，并把奖项记录中参与比对的字段切换为内部 ID（包括
Robot 的 ID 0）。确保 issue.ts 中这段授权分支只依赖 ID 级别的身份校验，避免继续使用文本身份。
Comment on lines
+44
to
+52
| // 2. Idempotency Check | ||
| if (award.transactionHash && award.tokenId) { | ||
| context.body = { | ||
| success: true, | ||
| transactionHash: award.transactionHash as string, | ||
| tokenId: award.tokenId as string, | ||
| }; | ||
| return; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
把幂等性绑定到钱包并关闭并发重复铸造窗口。
当前是 check-then-act：两个并发请求可同时通过 Line 45 后各自调用 mint，最后一次 updateOne() 覆盖结果；另外已颁发记录会无视新请求的 walletAddress 直接返回旧凭证。请让 mint 服务按 recordId 幂等，或先写入发行中状态/锁，再调用外部服务，并对钱包不一致返回 409。
Also applies to: 89-96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/api/Lark/award/issue.ts` around lines 44 - 52, The current idempotency
check in issue flow returns an existing award purely from
transactionHash/tokenId and does not bind the result to walletAddress, so
concurrent requests can still double-mint and later overwrite each other via
updateOne. Update the award issuance path in issue.ts to make minting idempotent
by recordId or by reserving an “issuing” lock/status before calling the external
mint service, then finalize the record after success; also verify the incoming
walletAddress matches the stored wallet and return 409 on mismatch. Apply the
same fix to the duplicated logic around the referenced issue-processing block so
both paths use the same wallet-bound concurrency guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #53
Summary by CodeRabbit
新功能
数据模型变更
Award关键字段（如投票数、钱包地址、交易哈希、代币ID）的类型约束与联合键完整性。💳 Payout Details
Please send the bounty rewards to the following payout methods: