Prepare Stellar Binding #197
Conversation
WalkthroughReplaced the NFTController stub with a publicly exported class exposing two static endpoints: prepareClaim and submitClaim. These endpoints read request body fields, invoke prepareClaimTx and submitSignedClaim use-cases, and return 200 and 202 responses respectively. Removed prior CRUD stub methods. Left a TODO about authenticated user validation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant Ctr as NFTController.prepareClaim
participant UC as prepareClaimTx (use-case)
C->>Ctr: POST /nft/claim/prepare { nftId, claimantPublicKey }
Note over Ctr: Validate input (TODO: auth validation)
Ctr->>UC: prepareClaimTx({ nftId, claimantPublicKey })
UC-->>Ctr: Prepared claim data
Ctr-->>C: 200 OK + data
sequenceDiagram
autonumber
participant C as Client
participant Ctr as NFTController.submitClaim
participant UC as submitSignedClaim (use-case)
C->>Ctr: POST /nft/claim/submit { nftId, signedXdr }
Ctr->>UC: submitSignedClaim({ nftId, signedXdr })
UC-->>Ctr: Submission result
Ctr-->>C: 202 Accepted + data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/modules/nft/presentation/controllers/NFTController.stub.ts (1)
14-17: Guard against duplicate/replayed submissions (idempotency).Consider: (a) Idempotency-Key header, (b) dedupe by hash of {nftId, signedXdr}, (c) respond with Location to a status endpoint.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/modules/nft/presentation/controllers/NFTController.stub.ts(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/modules/nft/presentation/controllers/NFTController.stub.ts
[error] 2-2: Shouldn't redeclare 'Request'. Consider to delete it or rename it.
'Request' is defined here:
(lint/suspicious/noRedeclare)
[error] 2-2: Shouldn't redeclare 'Response'. Consider to delete it or rename it.
'Response' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (1)
src/modules/nft/presentation/controllers/NFTController.stub.ts (1)
14-17: Good choice of 202 for async submission.202 Accepted fits the “queued/pending” flow.
| @@ -1,37 +1,20 @@ | |||
| import { Request, Response } from "express"; | |||
| import { Request, Response } from 'express'; | |||
There was a problem hiding this comment.
Fix duplicate Express import (Biome noRedeclare).
You’re importing Request/Response twice; Biome flags this and CI will fail.
Apply:
-import { Request, Response } from 'express';📝 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.
| import { Request, Response } from 'express'; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 2-2: Shouldn't redeclare 'Request'. Consider to delete it or rename it.
'Request' is defined here:
(lint/suspicious/noRedeclare)
[error] 2-2: Shouldn't redeclare 'Response'. Consider to delete it or rename it.
'Response' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In src/modules/nft/presentation/controllers/NFTController.stub.ts around line 2,
there is a duplicate import of Request and Response from 'express' causing a
Biome noRedeclare error; remove the redundant import (or consolidate any
separate express imports into a single import statement that includes Request
and Response) so only one import from 'express' remains in the file, then save
and run the linter to confirm the noRedeclare issue is resolved.
| export class NFTController { | ||
| static async prepareClaim(req: Request, res: Response) { | ||
| // TODO(security): Validar que el usuario autenticado puede reclamar el NFT. | ||
| const { nftId, claimantPublicKey } = req.body; | ||
| const result = await prepareClaimTx({ nftId, claimantPublicKey }); | ||
| res.status(200).json(result); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add authZ, input validation, and error handling; wire NextFunction and try/catch.
Current handlers trust req.body and can throw unhandled rejections. Validate inputs, surface 400s, and forward unexpected errors to the error middleware.
Apply within these ranges:
- static async prepareClaim(req: Request, res: Response) {
- // TODO(security): Validar que el usuario autenticado puede reclamar el NFT.
- const { nftId, claimantPublicKey } = req.body;
- const result = await prepareClaimTx({ nftId, claimantPublicKey });
- res.status(200).json(result);
- }
+ static async prepareClaim(req: Request, res: Response, next: NextFunction) {
+ try {
+ // TODO(security): Validate that the authenticated user can claim the NFT.
+ const { nftId, claimantPublicKey } = req.body ?? {};
+ if (typeof nftId !== 'string' || typeof claimantPublicKey !== 'string' || !nftId || !claimantPublicKey) {
+ return res.status(400).json({
+ error: 'Invalid payload',
+ details: { nftId: 'required string', claimantPublicKey: 'required string' },
+ });
+ }
+ const result = await prepareClaimTx({ nftId, claimantPublicKey });
+ return res.status(200).json(result);
+ } catch (err) {
+ return next(err);
+ }
+ }- static async submitClaim(req: Request, res: Response) {
- const { nftId, signedXdr } = req.body;
- const result = await submitSignedClaim({ nftId, signedXdr });
- res.status(202).json(result);
- }
+ static async submitClaim(req: Request, res: Response, next: NextFunction) {
+ try {
+ const { nftId, signedXdr } = req.body ?? {};
+ if (typeof nftId !== 'string' || typeof signedXdr !== 'string' || !nftId || !signedXdr) {
+ return res.status(400).json({
+ error: 'Invalid payload',
+ details: { nftId: 'required string', signedXdr: 'required base64 XDR string' },
+ });
+ }
+ const result = await submitSignedClaim({ nftId, signedXdr });
+ return res.status(202).json(result);
+ } catch (err) {
+ return next(err);
+ }
+ }Also update the import (outside the changed ranges):
-import { Request, Response } from "express";
+import { Request, Response, NextFunction } from "express";Also applies to: 14-17
🤖 Prompt for AI Agents
In src/modules/nft/presentation/controllers/NFTController.stub.ts around lines 6
to 12 (and also apply the same pattern to lines 14 to 17), the controller
currently trusts req.body and can produce unhandled rejections; update the
method signature to accept (req: Request, res: Response, next: NextFunction),
wrap the handler body in a try/catch, perform explicit input validation for
nftId and claimantPublicKey (return res.status(400).json({ error: '...' }) for
missing/invalid values), perform an authorization check to ensure req.user (or
an injected authZ helper) is allowed to claim the NFT and return 403 if not
allowed, call prepareClaimTx inside the try and on success
res.status(200).json(result), and on unexpected errors call next(err) to forward
to error middleware; also update the imports at the top to include NextFunction
(and any validator/authZ helpers you use).
💡 Verification agent
🧩 Analysis chain
Static-only class vs default instance export will break callers.
You export an instance (export default new NFTController()), but static methods aren’t available on instances, so legacy call sites like defaultExport.prepareClaim will be undefined at runtime.
Safer bridge fix (keeps static methods and a callable default):
-export default new NFTController();
+export default {
+ prepareClaim: NFTController.prepareClaim,
+ submitClaim: NFTController.submitClaim,
+};Alternatively, remove static and keep an instance API, or export the class as default and update all routes to call NFTController.prepareClaim.
🏁 Script executed:
#!/bin/bash
# Find usages that import the default and call controller methods
rg -nP -C2 "import\\s+\\w+\\s+from\\s+['\"]\\.?\\.?/.*NFTController" -g '!**/dist/**'
rg -nP -C2 "\\w+\\.prepareClaim\\(|\\w+\\.submitClaim\\(" -g '!**/dist/**'Length of output: 837
Export mismatch: static methods vs default instance
In src/modules/nft/presentation/controllers/NFTController.stub.ts you declare prepareClaim/submitClaim as static but do export default new NFTController(), so callers doing
import NFTController from ".../NFTController.stub";
NFTController.prepareClaim(...)will get undefined at runtime. Fix by exporting the class instead of an instance:
-export default new NFTController();
+export default NFTController;Alternatively, convert these to instance methods (remove static) or explicitly export a bridge object:
export default {
prepareClaim: NFTController.prepareClaim,
submitClaim: NFTController.submitClaim,
};🤖 Prompt for AI Agents
In src/modules/nft/presentation/controllers/NFTController.stub.ts around lines
6-12 there is a mismatch: methods are declared static but the module exports an
instance, causing callers like `NFTController.prepareClaim(...)` to be
undefined; fix by either (A) exporting the class itself (replace the exported
instance with `export default NFTController`) so static calls work, or (B)
convert the static methods to instance methods (remove `static`) and keep
exporting an instance, or (C) keep static methods and export a bridge object
mapping those methods to the class (e.g., `export default { prepareClaim:
NFTController.prepareClaim, submitClaim: NFTController.submitClaim }`); also
update any imports/usage to match the chosen approach.
| }); | ||
| export class NFTController { | ||
| static async prepareClaim(req: Request, res: Response) { | ||
| // TODO(security): Validar que el usuario autenticado puede reclamar el NFT. |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Implement authorization now; don’t trust claimantPublicKey from the client.
Enforce that the authenticated user is allowed to claim this NFT (e.g., tie claimantPublicKey to the session/user and verify nftId is in a claimable state). Return 403 when not authorized.
🏁 Script executed:
#!/bin/bash
# Check routing and auth gating for these endpoints
rg -nP -C2 "(router|app)\\.(post|use)\\(.*nft/claim/(prepare|submit)" -g '!**/dist/**'
rg -nP -C2 "NFTController\\.(prepareClaim|submitClaim)|new\\s+NFTController\\(\\)"
rg -nP -C2 "(auth|passport|jwt|ensure.*Auth|verify.*Token|requireAuth)"Length of output: 101504
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' src/routes/nftRoutes.tsLength of output: 805
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' src/modules/nft/presentation/controllers/NFTController.stub.tsLength of output: 897
Enforce authorization on NFT claim flows
-
In src/routes/nftRoutes.ts, add authenticated routes for preparing and submitting claims, e.g.:
import { authMiddleware } from "../middleware/authMiddleware"; router.post("/nfts/claim/prepare", authMiddleware, NFTController.prepareClaim); router.post("/nfts/claim/submit", authMiddleware, NFTController.submitClaim);
-
In src/modules/nft/presentation/controllers/NFTController.stub.ts, replace the
TODOwith logic that readsreq.user.id(populated byauthMiddleware), verifies thatclaimantPublicKeybelongs to that user and thatnftIdis in a claimable state, and returns 403 on any authorization failure.
🤖 Prompt for AI Agents
In src/modules/nft/presentation/controllers/NFTController.stub.ts around line 8,
replace the TODO with authorization logic that reads req.user.id (set by
authMiddleware), fetches/validates that the claimantPublicKey belongs to that
user (e.g. via userService or keysRepository), checks that the nftId is in a
claimable state (via nftService/status check), and if either check fails
immediately respond with res.status(403). If both checks pass, continue the
existing flow; surface any service errors as appropriate but do not proceed on
authorization failures.
1 participant
🚀 Volunchain Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
• Enabled and implemented NFTController for claim flow.
• Added endpoints:
• Added DTOs, use-cases, and Stellar binding scaffold with clear TODOs.
• Added basic controller tests for both endpoints.
• Removed stub/disabled controller files.
⏰ Time spent breakdown
• Controller & routes: 20 min
• DTOs & use-cases: 15 min
• Stellar binding scaffold: 10 min
• Tests: 15 min
• Review & formatting: 10 min
Summary by CodeRabbit
New Features
Refactor