Add GET /api/streams/:id/claimable#485
Merged
Merged
Conversation
|
@mikevill20 is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@mikevill20 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Warning Review limit reached
More reviews will be available in 52 minutes and 19 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements GET /api/streams/:id/claimable to return a stream's real-time claimable amount via Soroban contract simulation; it adds on-chain helpers, a per-IP rate limiter, Swagger docs, integration tests with RPC mocking, and a small dependency/devDependency update. ChangesOn-chain claimable amount endpoint
Sequence DiagramsequenceDiagram
participant Client
participant Endpoint as GET /api/streams/:id/claimable
participant streamStore
participant SorobanRPC as Soroban RPC
Client->>Endpoint: GET /api/streams/:id/claimable
Endpoint->>streamStore: getOnChainClaimableAmount(id)
streamStore->>streamStore: getLatestLedgerTime()
streamStore->>SorobanRPC: simulateTransaction(claimable at ledger time)
SorobanRPC-->>streamStore: simulation result (ScVal)
streamStore-->>Endpoint: {claimableAmount, at}
Endpoint-->>Client: {streamId, claimableAmount, assetCode, at}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/services/streamStore.ts (1)
86-87:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDuplicate object key overwrites null-coalescing fallback.
The second
pausedDurationassignment on line 87 overwrites the first on line 86, discarding the?? 0fallback. Ifrow.paused_durationisnull, the record will havepausedDuration: nullinstead of0, causing arithmetic errors downstream.🐛 Proposed fix
pausedAt: row.paused_at ?? undefined, - pausedDuration: row.paused_duration ?? 0, pausedDuration: row.paused_duration, + pausedDuration: row.paused_duration ?? 0, };🤖 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 `@backend/src/services/streamStore.ts` around lines 86 - 87, Remove the duplicate pausedDuration property so the null-coalescing fallback is preserved: keep the assignment using row.paused_duration ?? 0 and delete the second pausedDuration: row.paused_duration that overwrites it; update the object mapping in streamStore.ts (the code that maps DB row to the output record where pausedDuration and row.paused_duration appear) to only use pausedDuration: row.paused_duration ?? 0 so downstream arithmetic sees 0 instead of null.
🧹 Nitpick comments (5)
backend/src/index.ts (2)
454-463: 💤 Low valueConsider handling
completedAtsimilarly to paused/canceled.The endpoint returns
claimableAmount: 0early for paused and canceled streams, avoiding an RPC call. Completed streams still callgetOnChainClaimableAmount. If completed streams always have 0 claimable, short-circuiting them here would reduce unnecessary RPC calls.🤖 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 `@backend/src/index.ts` around lines 454 - 463, The early-return checks for pausedAt and canceledAt should also include completedAt to avoid unnecessary RPCs: update the condition that checks stream.pausedAt !== undefined || stream.canceledAt !== undefined to also check stream.completedAt !== undefined so completed streams return the same short-circuit response (using getLatestLedgerTime() and res.json with claimableAmount: 0) instead of calling getOnChainClaimableAmount.
468-468: 💤 Low valueRedundant
Number()conversion.
claimableAmountis already anumberfromgetOnChainClaimableAmount. TheNumber()wrapper is unnecessary.♻️ Proposed fix
- claimableAmount: Number(claimableAmount), + claimableAmount,🤖 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 `@backend/src/index.ts` at line 468, The field assignment wraps claimableAmount in an unnecessary Number() call; remove the Number(...) wrapper and assign claimableAmount directly (the value returned from getOnChainClaimableAmount is already a number). Update the object property (the line containing "claimableAmount: Number(claimableAmount)," in backend/src/index.ts) to use "claimableAmount: claimableAmount," (or the shorthand "claimableAmount," if the surrounding object allows) so no runtime conversion is performed.backend/src/services/streamStore.ts (1)
425-425: 💤 Low valueRefine
closeTimehandling ingetOnChainClaimableAmount
rpcServer.getLatestLedger().closeTimeis returned as a string Unix timestamp in@stellar/stellar-sdkv14.x, soparseInt(latestLedger.closeTime, 10)is appropriate and not subject to bigint truncation. Low-effort readability:Number(latestLedger.closeTime)is a cleaner alternative.🤖 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 `@backend/src/services/streamStore.ts` at line 425, The code in getOnChainClaimableAmount uses parseInt on latestLedger.closeTime which is a string Unix timestamp; replace parseInt(latestLedger.closeTime, 10) with Number(latestLedger.closeTime) (or equivalent explicit numeric cast) when computing the variable at to improve readability and clarity while preserving correct numeric conversion of latestLedger.closeTime.backend/src/swagger.ts (1)
697-711: 💤 Low valueConsider documenting the rate limit in the description.
The PR objectives specify a 30 calls per minute per IP rate limit, but this isn't mentioned in the endpoint description. While rate limiting details aren't always included in OpenAPI specs, adding a note would improve developer experience.
📝 Suggested description enhancement
- description: "Retrieves the current real-time claimable amount for a stream using Soroban contract simulation. Returns 0 if paused, canceled, or before the cliff.", + description: "Retrieves the current real-time claimable amount for a stream using Soroban contract simulation. Returns 0 if paused, canceled, or before the cliff. Rate limited to 30 requests per minute per IP.",🤖 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 `@backend/src/swagger.ts` around lines 697 - 711, Update the OpenAPI description for the "/api/streams/{id}/claimable" GET operation to mention the endpoint's rate limit (30 calls per minute per IP); locate the path object for "/api/streams/{id}/claimable" in backend/src/swagger.ts and append a concise note to the existing description (or add a "x-rate-limit" vendor extension) so consumers see "Rate limit: 30 requests per minute per IP" alongside the existing caveats about paused/canceled/cliff states.backend/src/integration.test.ts (1)
440-550: ⚡ Quick winAdd an explicit before-cliff integration case for claimable=0.
The suite covers paused/canceled, but it does not explicitly validate the “before cliff returns 0” contract from the PR objective.
Proposed test addition
+ it("should return 0 before cliff/start conditions", async () => { + const db = getDb(); + const future = Math.floor(Date.now() / 1000) + 7200; + db.prepare("UPDATE streams SET start_at = ? WHERE id = ?").run(future, mockStream.id); + + mockGetLatestLedger.mockResolvedValue({ + sequence: 12345, + closeTime: "1716812160", + }); + + const response = await request(app).get(`/api/streams/${mockStream.id}/claimable`); + + expect(response.status).toBe(200); + expect(response.body.claimableAmount).toBe(0); + });🤖 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 `@backend/src/integration.test.ts` around lines 440 - 550, Add a new integration test in the "GET /api/streams/:id/claimable" suite (e.g., it("should return 0 before cliff", ...)) that updates the test stream via getDb() to set cliff_at to a timestamp in the future (or set start_at earlier and cliff_at > now) so the current time is before the cliff; call mockGetLatestLedger.mockResolvedValue(...) to supply the ledger closeTime; then hit GET /api/streams/${mockStream.id}/claimable and assert status 200, response.body.claimableAmount === 0, response.body.streamId === mockStream.id and response.body.assetCode === mockStream.assetCode, and assert mockSimulateTransaction was not called. Use the existing patterns in the file (mockGetLatestLedger, mockSimulateTransaction, getDb, mockStream) to implement this test.
🤖 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 `@backend/src/integration.test.ts`:
- Around line 440-550: The rate-limit test is flaky because claimableLimiter
(created once in backend/src/index.ts) retains consumed tokens across earlier
tests; reset the limiter before the 31-request loop by forcing module reload and
re-creating the app so a fresh claimableLimiter is instantiated (use
vi.resetModules() then re-import the app used by request(app)), or add and call
a clear/reset function on claimableLimiter before the loop; ensure you reference
claimableLimiter and the app instance used in the test so the limiter state is
fresh when asserting 30 OK and 1 429.
In `@backend/src/swagger.ts`:
- Around line 713-728: The 200 response schema for the
/api/streams/{id}/claimable endpoint is missing a required array; update the
object schema (the response block that currently defines properties streamId,
claimableAmount, assetCode, and at) to include a required:
["streamId","claimableAmount","assetCode","at"] entry so the OpenAPI contract
explicitly marks those fields as mandatory.
---
Outside diff comments:
In `@backend/src/services/streamStore.ts`:
- Around line 86-87: Remove the duplicate pausedDuration property so the
null-coalescing fallback is preserved: keep the assignment using
row.paused_duration ?? 0 and delete the second pausedDuration:
row.paused_duration that overwrites it; update the object mapping in
streamStore.ts (the code that maps DB row to the output record where
pausedDuration and row.paused_duration appear) to only use pausedDuration:
row.paused_duration ?? 0 so downstream arithmetic sees 0 instead of null.
---
Nitpick comments:
In `@backend/src/index.ts`:
- Around line 454-463: The early-return checks for pausedAt and canceledAt
should also include completedAt to avoid unnecessary RPCs: update the condition
that checks stream.pausedAt !== undefined || stream.canceledAt !== undefined to
also check stream.completedAt !== undefined so completed streams return the same
short-circuit response (using getLatestLedgerTime() and res.json with
claimableAmount: 0) instead of calling getOnChainClaimableAmount.
- Line 468: The field assignment wraps claimableAmount in an unnecessary
Number() call; remove the Number(...) wrapper and assign claimableAmount
directly (the value returned from getOnChainClaimableAmount is already a
number). Update the object property (the line containing "claimableAmount:
Number(claimableAmount)," in backend/src/index.ts) to use "claimableAmount:
claimableAmount," (or the shorthand "claimableAmount," if the surrounding object
allows) so no runtime conversion is performed.
In `@backend/src/integration.test.ts`:
- Around line 440-550: Add a new integration test in the "GET
/api/streams/:id/claimable" suite (e.g., it("should return 0 before cliff",
...)) that updates the test stream via getDb() to set cliff_at to a timestamp in
the future (or set start_at earlier and cliff_at > now) so the current time is
before the cliff; call mockGetLatestLedger.mockResolvedValue(...) to supply the
ledger closeTime; then hit GET /api/streams/${mockStream.id}/claimable and
assert status 200, response.body.claimableAmount === 0, response.body.streamId
=== mockStream.id and response.body.assetCode === mockStream.assetCode, and
assert mockSimulateTransaction was not called. Use the existing patterns in the
file (mockGetLatestLedger, mockSimulateTransaction, getDb, mockStream) to
implement this test.
In `@backend/src/services/streamStore.ts`:
- Line 425: The code in getOnChainClaimableAmount uses parseInt on
latestLedger.closeTime which is a string Unix timestamp; replace
parseInt(latestLedger.closeTime, 10) with Number(latestLedger.closeTime) (or
equivalent explicit numeric cast) when computing the variable at to improve
readability and clarity while preserving correct numeric conversion of
latestLedger.closeTime.
In `@backend/src/swagger.ts`:
- Around line 697-711: Update the OpenAPI description for the
"/api/streams/{id}/claimable" GET operation to mention the endpoint's rate limit
(30 calls per minute per IP); locate the path object for
"/api/streams/{id}/claimable" in backend/src/swagger.ts and append a concise
note to the existing description (or add a "x-rate-limit" vendor extension) so
consumers see "Rate limit: 30 requests per minute per IP" alongside the existing
caveats about paused/canceled/cliff states.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8ede560-f14e-455b-bf6b-616ec62faf85
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
backend/package.jsonbackend/src/index.tsbackend/src/integration.test.tsbackend/src/services/streamStore.tsbackend/src/swagger.ts
Comment on lines
+440
to
+550
| describe("GET /api/streams/:id/claimable", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("should return 200 and the claimable amount from Soroban simulation", async () => { | ||
| mockGetLatestLedger.mockResolvedValue({ | ||
| sequence: 12345, | ||
| closeTime: "1716812160", | ||
| }); | ||
|
|
||
| mockSimulateTransaction.mockResolvedValue({ | ||
| kind: "success", | ||
| result: { | ||
| retval: 450, | ||
| }, | ||
| }); | ||
|
|
||
| const response = await request(app).get(`/api/streams/${mockStream.id}/claimable`); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body).toEqual({ | ||
| streamId: mockStream.id, | ||
| claimableAmount: 450, | ||
| assetCode: mockStream.assetCode, | ||
| at: 1716812160, | ||
| }); | ||
|
|
||
| expect(mockGetLatestLedger).toHaveBeenCalled(); | ||
| expect(mockSimulateTransaction).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should return 0 when stream is paused", async () => { | ||
| const db = getDb(); | ||
| db.prepare("UPDATE streams SET paused_at = ? WHERE id = ?").run(Math.floor(Date.now() / 1000) - 1000, mockStream.id); | ||
|
|
||
| mockGetLatestLedger.mockResolvedValue({ | ||
| sequence: 12345, | ||
| closeTime: "1716812160", | ||
| }); | ||
|
|
||
| const response = await request(app).get(`/api/streams/${mockStream.id}/claimable`); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body).toEqual({ | ||
| streamId: mockStream.id, | ||
| claimableAmount: 0, | ||
| assetCode: mockStream.assetCode, | ||
| at: 1716812160, | ||
| }); | ||
|
|
||
| expect(mockSimulateTransaction).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should return 0 when stream is canceled", async () => { | ||
| const db = getDb(); | ||
| db.prepare("UPDATE streams SET canceled_at = ? WHERE id = ?").run(Math.floor(Date.now() / 1000) - 1000, mockStream.id); | ||
|
|
||
| mockGetLatestLedger.mockResolvedValue({ | ||
| sequence: 12345, | ||
| closeTime: "1716812160", | ||
| }); | ||
|
|
||
| const response = await request(app).get(`/api/streams/${mockStream.id}/claimable`); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body).toEqual({ | ||
| streamId: mockStream.id, | ||
| claimableAmount: 0, | ||
| assetCode: mockStream.assetCode, | ||
| at: 1716812160, | ||
| }); | ||
|
|
||
| expect(mockSimulateTransaction).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should return 404 for non-existent stream", async () => { | ||
| const response = await request(app).get("/api/streams/999/claimable"); | ||
|
|
||
| expect(response.status).toBe(404); | ||
| expect(response.body.error).toBe("Stream not found."); | ||
| }); | ||
|
|
||
| it("should return 400 for invalid stream ID", async () => { | ||
| const response = await request(app).get("/api/streams/invalid-id/claimable"); | ||
|
|
||
| expect(response.status).toBe(400); | ||
| expect(response.body.error).toContain("Stream ID must be"); | ||
| }); | ||
|
|
||
| it("should enforce rate limit of 30 requests per minute", async () => { | ||
| mockGetLatestLedger.mockResolvedValue({ | ||
| sequence: 12345, | ||
| closeTime: "1716812160", | ||
| }); | ||
| mockSimulateTransaction.mockResolvedValue({ | ||
| kind: "success", | ||
| result: { retval: 10 }, | ||
| }); | ||
|
|
||
| for (let i = 0; i < 31; i++) { | ||
| const response = await request(app).get(`/api/streams/${mockStream.id}/claimable`); | ||
| if (i < 30) { | ||
| expect(response.status).toBe(200); | ||
| } else { | ||
| expect(response.status).toBe(429); | ||
| expect(response.body.code).toBe("RATE_LIMIT_EXCEEDED"); | ||
| } | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify claimable limiter definition and whether tests reset limiter state.
rg -n -C3 'claimableLimiter|rateLimit\(|/api/streams/:id/claimable|RATE_LIMIT_EXCEEDED' backend/src/index.ts backend/src/integration.test.ts
rg -n -C2 'beforeEach|afterEach|clearAllMocks|resetAllMocks' backend/src/integration.test.tsRepository: ritik4ever/stellar-stream
Length of output: 5960
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show claimable limiter config and middleware usage
nl -ba backend/src/index.ts | sed -n '160,240p'
nl -ba backend/src/index.ts | sed -n '420,480p'
# Show test suite top-level beforeEach(s) and any rate-limit cleanup
# around the claimable describe and also earlier common beforeEach
nl -ba backend/src/integration.test.ts | sed -n '1,140p'
nl -ba backend/src/integration.test.ts | sed -n '420,620p'
# Search for any explicit reset/clear of express-rate-limit store
rg -n "claimableLimiter|express-rate-limit|rateLimit\(|reset|clear.*rate|limiter" backend/src/integration.test.tsRepository: ritik4ever/stellar-stream
Length of output: 112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Claimable limiter definition and route wiring
echo "### backend/src/index.ts: claimableLimiter config"
sed -n '180,220p' backend/src/index.ts
echo "### backend/src/index.ts: /claimable route wiring"
sed -n '420,465p' backend/src/index.ts
# Test suite initialization/fixtures
echo "### backend/src/integration.test.ts: imports + global hooks"
sed -n '1,120p' backend/src/integration.test.ts
echo "### backend/src/integration.test.ts: around claimable describe"
sed -n '400,590p' backend/src/integration.test.ts
# Search for limiter store reset/clear or re-instantiation
echo "### Search for express-rate-limit / limiter reset in tests"
rg -n "express-rate-limit|rateLimit\(|claimableLimiter|limiter|resetKey|reset|clear.*limiter|clear.*rate" backend/src/integration.test.tsRepository: ritik4ever/stellar-stream
Length of output: 13170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "vi\.resetModules\(|resetModules|claimableLimiter|rateLimit" backend/src/integration.test.tsRepository: ritik4ever/stellar-stream
Length of output: 51
Fix flaky /api/streams/:id/claimable rate-limit test caused by shared limiter state.
claimableLimiteris created once inbackend/src/index.tsand attached as middleware for/api/streams/:id/claimable; the test only doesvi.clearAllMocks(), which won’t reset the limiter’s in-memory bucket.- The earlier tests in the same
GET /api/streams/:id/claimableblock already hit the endpoint, consuming quota—so the expectation that the first 30 looped requests are always200(and the 31st is always429) can fail. - Reset/clear the claimable limiter bucket before the rate-limit loop (e.g., re-import server/app after
vi.resetModules()or expose a way to clear the limiter store), or adjust assertions to account for already-consumed quota.
🤖 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 `@backend/src/integration.test.ts` around lines 440 - 550, The rate-limit test
is flaky because claimableLimiter (created once in backend/src/index.ts) retains
consumed tokens across earlier tests; reset the limiter before the 31-request
loop by forcing module reload and re-creating the app so a fresh
claimableLimiter is instantiated (use vi.resetModules() then re-import the app
used by request(app)), or add and call a clear/reset function on
claimableLimiter before the loop; ensure you reference claimableLimiter and the
app instance used in the test so the limiter state is fresh when asserting 30 OK
and 1 429.
Comment on lines
+713
to
+728
| "200": { | ||
| description: "Real-time claimable amount and query context.", | ||
| content: { | ||
| "application/json": { | ||
| schema: { | ||
| type: "object", | ||
| properties: { | ||
| streamId: { type: "string", example: "1" }, | ||
| claimableAmount: { type: "number", example: 450.123456 }, | ||
| assetCode: { type: "string", example: "USDC" }, | ||
| at: { type: "integer", description: "Ledger timestamp at which query was simulated", example: 1716812160 }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the claimable endpoint response structure
# Search for the claimable endpoint handler implementation
rg -nP -A 10 "get.*['\"/]api/streams/:id/claimable" --type=ts
# Also check for response construction patterns
rg -nP -C 3 "streamId.*claimableAmount.*assetCode" --type=tsRepository: ritik4ever/stellar-stream
Length of output: 731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the full claimable handler block (approx) around the route definition
sed -n '420,520p' backend/src/index.ts
# Narrow search for how the response is sent within the claimable handler
rg -n "claimableLimiter|/api/streams/:id/claimable|claimable" backend/src/index.ts | head -n 50
# Find where the handler sends the 200 JSON (res.json / res.send / helper calls)
rg -n "app\.get\(\"/api/streams/:id/claimable\"|res\.json\(|res\.send\(|sendSuccess|sendApiSuccess" backend/src/index.ts -nRepository: ritik4ever/stellar-stream
Length of output: 4438
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the swagger schema around the referenced lines
sed -n '690,760p' backend/src/swagger.ts
# Locate and inspect the helper functions that produce `at` and `claimableAmount`
rg -n "function getLatestLedgerTime|getLatestLedgerTime\s*=" backend/src -S
rg -n "function getOnChainClaimableAmount|getOnChainClaimableAmount\s*=" backend/src -SRepository: ritik4ever/stellar-stream
Length of output: 2491
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '380,520p' backend/src/services/streamStore.tsRepository: ritik4ever/stellar-stream
Length of output: 4326
Update claimable swagger contract: structure matches implementation; add required fields
The /api/streams/{id}/claimable 200 response is correctly defined as an unwrapped root object (streamId, claimableAmount, assetCode, at) and the endpoint implementation returns the same shape (no data wrapper).
The schema still omits required; add it to make the contract explicit.
Option 2: Add required fields
schema: {
type: "object",
+ required: ["streamId", "claimableAmount", "assetCode", "at"],
properties: {
streamId: { type: "string", example: "1" },
claimableAmount: { type: "number", example: 450.123456 },
assetCode: { type: "string", example: "USDC" },
at: { type: "integer", description: "Ledger timestamp at which query was simulated", example: 1716812160 },
},
},📝 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.
Suggested change
| "200": { | |
| description: "Real-time claimable amount and query context.", | |
| content: { | |
| "application/json": { | |
| schema: { | |
| type: "object", | |
| properties: { | |
| streamId: { type: "string", example: "1" }, | |
| claimableAmount: { type: "number", example: 450.123456 }, | |
| assetCode: { type: "string", example: "USDC" }, | |
| at: { type: "integer", description: "Ledger timestamp at which query was simulated", example: 1716812160 }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| "200": { | |
| description: "Real-time claimable amount and query context.", | |
| content: { | |
| "application/json": { | |
| schema: { | |
| type: "object", | |
| required: ["streamId", "claimableAmount", "assetCode", "at"], | |
| properties: { | |
| streamId: { type: "string", example: "1" }, | |
| claimableAmount: { type: "number", example: 450.123456 }, | |
| assetCode: { type: "string", example: "USDC" }, | |
| at: { type: "integer", description: "Ledger timestamp at which query was simulated", example: 1716812160 }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, |
🤖 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 `@backend/src/swagger.ts` around lines 713 - 728, The 200 response schema for
the /api/streams/{id}/claimable endpoint is missing a required array; update the
object schema (the response block that currently defines properties streamId,
claimableAmount, assetCode, and at) to include a required:
["streamId","claimableAmount","assetCode","at"] entry so the OpenAPI contract
explicitly marks those fields as mandatory.
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
2 participants
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.
I Added GET /api/streams/:id/claimable that calls Soroban claimable(stream_id, now) and returns the live value.
This PR closes #455
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Refactor