feat: admin clawback endpoint (#326) - #651
Conversation
|
@rhemaolamiju295-sudo is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@baedboibidex-cmyk 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! 🚀 |
📝 WalkthroughWalkthroughAdds an admin-authenticated stream clawback endpoint that validates requests, submits Soroban clawback transactions, records ChangesAdmin stream clawback
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant AdminClient
participant AdminRoute
participant clawbackStream
participant SorobanContract
participant EventHistory
AdminClient->>AdminRoute: POST clawback request
AdminRoute->>clawbackStream: validated stream id and amount
clawbackStream->>SorobanContract: signed clawback transaction
SorobanContract-->>clawbackStream: finalized transaction result
clawbackStream-->>AdminRoute: txHash and actualAmount
AdminRoute->>EventHistory: record clawed_back event
AdminRoute-->>AdminClient: clawback response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.5)backend/src/services/streamStore.tsFile contains syntax errors that prevent linting: Line 812: expected 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: 7
🧹 Nitpick comments (1)
backend/src/auth-protected-routes.integration.test.ts (1)
558-570: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an assertion on the recorded
clawed_backevent.The success test checks the HTTP response but never queries
stream_eventsto verify the recordedactor/amount/metadata. Adding this would have caught the actor-attribution bug flagged inindex.ts(the event'sactoris set tostream.senderinstead of the admin address).🤖 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/auth-protected-routes.integration.test.ts` around lines 558 - 570, Extend the success case in the test around “valid admin key + Soroban success” to query the recorded clawed_back event from stream_events and assert its actor is the admin address, amount matches the clawback amount, and metadata contains the expected transaction details. Use the existing event-query helpers and fixtures rather than changing the HTTP response assertions.
🤖 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/index.ts`:
- Around line 1952-1976: After a successful clawback in the handler surrounding
clawbackStream, persist the returned clawback amount and any required stream
progress fields to the streams table so subsequent calculations reflect the
mutation without reconciliation. Also access the in-memory cache via getCache()
and call resetStatsCache() after the database update, while preserving the
existing event recording and response behavior.
- Around line 1925-1927: Move clawbackBodySchema from index.ts into
backend/src/validation/schemas.ts, export it, and import it where the clawback
route handler uses it. Add integer validation alongside the existing positive
amount constraint so validated values satisfy the Swagger and downstream i128
contract requirements.
In `@backend/src/services/streamStore.ts`:
- Around line 1417-1454: Serialize all admin transaction flows using
serverKeypair, including createStream, estimateCreateStreamFee, cancelStream,
and clawbackStream, with a shared coroutine-level mutex or per-signer task queue
spanning account fetch, transaction preparation/build, and signing. Ensure
concurrent operations execute these sequence-sensitive steps one at a time while
preserving existing transaction behavior.
- Around line 1486-1489: Update the clawback result construction in the
surrounding clawback method to preserve the i128 amount as the bigint returned
by scValToNative, removing the Number conversion, and include the existing
adminAddress value in the returned ClawbackResult. Extend the result type as
needed so callers can access both actualAmount without precision loss and
adminAddress.
- Around line 1439-1442: Update the clawback request encoding near
nativeToScVal(amount, { type: "i128" }) so the amount is converted to an exact
BigInt before serialization, using the validated parsed amount or integer string
and truncating only as required. Preserve the i128 type and existing streamIdU64
and adminAddress arguments.
In `@backend/src/swagger.ts`:
- Around line 1872-1877: Update the amount schema in the Swagger definition and
the corresponding clawbackBodySchema to enforce positive integers, not
fractional numbers, while preserving the existing minimum constraint and
description. Align the validation with the i128 conversion used by streamStore
so all clawback amount boundaries are consistent.
- Around line 1921-1928: Update the 401 response documented near adminAuth to
match the actual `{ message: "Unauthorized" }` payload: replace the
`#/components/schemas/Error` reference with a schema requiring a string
`message`, or adjust adminAuth to return the existing Error-shaped payload. Keep
the documented response and runtime behavior consistent.
---
Nitpick comments:
In `@backend/src/auth-protected-routes.integration.test.ts`:
- Around line 558-570: Extend the success case in the test around “valid admin
key + Soroban success” to query the recorded clawed_back event from
stream_events and assert its actor is the admin address, amount matches the
clawback amount, and metadata contains the expected transaction details. Use the
existing event-query helpers and fixtures rather than changing the HTTP response
assertions.
🪄 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 Plus
Run ID: a9dfce6d-c19e-4977-b19f-3252dcf64759
📒 Files selected for processing (5)
backend/src/auth-protected-routes.integration.test.tsbackend/src/index.tsbackend/src/services/eventHistory.tsbackend/src/services/streamStore.tsbackend/src/swagger.ts
| const clawbackBodySchema = z.object({ | ||
| amount: z.number().positive("amount must be positive"), | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move clawbackBodySchema into backend/src/validation/schemas.ts.
This schema is defined inline in index.ts. As per coding guidelines, "In backend/src/index.ts, keep Express route handlers in the API layer and validate requests with Zod schemas from backend/src/validation/schemas.ts before passing data to services" and "apply validation by parsing, transforming, and refining Zod schemas in backend/src/validation/schemas.ts before passing data to services."
Additionally, the schema doesn't enforce integer amounts, but the Swagger doc (and the downstream i128 contract call) expect an integer — consider adding .int() here.
♻️ Suggested fix
-const clawbackBodySchema = z.object({
- amount: z.number().positive("amount must be positive"),
-});
+// in backend/src/validation/schemas.ts
+export const clawbackBodySchema = z.object({
+ amount: z.number().int().positive("amount must be a positive integer"),
+});🤖 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 1925 - 1927, Move clawbackBodySchema from
index.ts into backend/src/validation/schemas.ts, export it, and import it where
the clawback route handler uses it. Add integer validation alongside the
existing positive amount constraint so validated values satisfy the Swagger and
downstream i128 contract requirements.
Source: Coding guidelines
| try { | ||
| const result = await clawbackStream(parsedId.value, parsedBody.data.amount); | ||
|
|
||
| const db = (await import("./services/db")).getDb(); | ||
| const { recordEventWithDb } = await import("./services/eventHistory"); | ||
| const now = Math.floor(Date.now() / 1000); | ||
|
|
||
| recordEventWithDb( | ||
| db, | ||
| stream.id, | ||
| "clawed_back", | ||
| now, | ||
| stream.sender, | ||
| result.actualAmount, | ||
| { txHash: result.txHash, requestedAmount: parsedBody.data.amount }, | ||
| ); | ||
|
|
||
| res.json({ | ||
| result: { | ||
| txHash: result.txHash, | ||
| actualAmount: result.actualAmount, | ||
| requestedAmount: parsedBody.data.amount, | ||
| }, | ||
| }); | ||
| } catch (error: any) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Missing resetStatsCache() and no local stream-record update after a successful clawback.
As per coding guidelines, "access the in-memory LRU cache through getCache() and call resetStatsCache() after mutations that affect stats." A clawback reduces the stream's remaining/vested balance on-chain, which affects aggregate stats (totalVested, totalAmount, etc.) — yet resetStatsCache() is never called here.
Separately, the handler only records an event; it never updates the streams table itself (unlike, e.g., cancel/complete flows which appear to persist fields such as refundedAmount/completedAt). Until an admin manually calls /api/streams/{id}/reconcile, stream progress/vested-amount calculations served by other endpoints will not reflect the clawback.
🤖 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 1952 - 1976, After a successful clawback
in the handler surrounding clawbackStream, persist the returned clawback amount
and any required stream progress fields to the streams table so subsequent
calculations reflect the mutation without reconciliation. Also access the
in-memory cache via getCache() and call resetStatsCache() after the database
update, while preserving the existing event recording and response behavior.
Source: Coding guidelines
| export async function clawbackStream(id: string, amount: number): Promise<ClawbackResult> { | ||
| const contractId = process.env.CONTRACT_ID; | ||
| const netPass = | ||
| process.env.NETWORK_PASSPHRASE || "Test SDF Network ; September 2015"; | ||
|
|
||
| if (!contractId || !rpcServer || !serverKeypair) { | ||
| throw new Error("Backend not configured for Soroban."); | ||
| } | ||
|
|
||
| const streamIdU64 = parseInt(id, 10); | ||
| if (isNaN(streamIdU64) || streamIdU64 < 0) { | ||
| const err: any = new Error("Invalid stream ID for on-chain clawback."); | ||
| err.statusCode = 400; | ||
| throw err; | ||
| } | ||
|
|
||
| const adminAddress = serverKeypair.publicKey(); | ||
|
|
||
| const sourceAccount = await rpcServer.getAccount(adminAddress); | ||
| const contract = new Contract(contractId); | ||
| const tx = contract.call( | ||
| "clawback", | ||
| nativeToScVal(streamIdU64, { type: "u64" }), | ||
| nativeToScVal(amount, { type: "i128" }), | ||
| new Address(adminAddress).toScVal(), | ||
| ); | ||
|
|
||
| const built = await rpcServer.prepareTransaction( | ||
| new TransactionBuilder(sourceAccount, { | ||
| fee: "1000", | ||
| networkPassphrase: netPass, | ||
| }) | ||
| .addOperation(tx) | ||
| .setTimeout(30) | ||
| .build(), | ||
| ); | ||
|
|
||
| built.sign(serverKeypair); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether other Soroban-signing functions in this file serialize access to serverKeypair
rg -n "serverKeypair" backend/src/services/streamStore.ts -B3 -A3Repository: ritik4ever/stellar-stream
Length of output: 3376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant streamStore functions =="
sed -n '800,850p' backend/src/services/streamStore.ts
sed -n '900,935p' backend/src/services/streamStore.ts
sed -n '1125,1170p' backend/src/services/streamStore.ts
sed -n '1410,1475p' backend/src/services/streamStore.ts
echo
echo "== routes using clawbackStream/createStream/cancelStream =="
rg -n "clawbackStream|createStream\\(|cancelStream|cancel_stream" backend/src -B2 -A4
echo
echo "== middleware/rate limiter references =="
rg -n "rate limit|rateLimit|60|10|req|res|next" backend/src -g '*.ts' | sed -n '1,220p'Repository: ritik4ever/stellar-stream
Length of output: 34200
Serialize admin transactions signed by serverKeypair.
getAccount(...) in createStream, estimateCreateStreamFee, cancelStream, and clawbackStream is followed by prepareTransaction, build, and sign without any shared mutex/queue around serverKeypair. Concurrent admin operations can fetch the same sequence number, causing one on-chain transaction to fail with a bad-sequence error. Add a coroutine-level lock or per-signer serialized task queue around these paths.
🤖 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 1417 - 1454, Serialize all
admin transaction flows using serverKeypair, including createStream,
estimateCreateStreamFee, cancelStream, and clawbackStream, with a shared
coroutine-level mutex or per-signer task queue spanning account fetch,
transaction preparation/build, and signing. Ensure concurrent operations execute
these sequence-sensitive steps one at a time while preserving existing
transaction behavior.
| nativeToScVal(streamIdU64, { type: "u64" }), | ||
| nativeToScVal(amount, { type: "i128" }), | ||
| new Address(adminAddress).toScVal(), | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files around streamStore:"
git ls-files | rg 'backend/src/services/streamStore\.ts|backend/src/.*' || true
echo
echo "Relevant section line count:"
wc -l backend/src/services/streamStore.ts
sed -n '1350,1475p' backend/src/services/streamStore.ts
echo
echo "Search amount parameter declarations/callers:"
rg -n " clawback|amount" backend/src/services/streamStore.ts backend/src -g '*.ts' | head -200Repository: ritik4ever/stellar-stream
Length of output: 29064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Clawback route validation:"
sed -n '1850,1970p' backend/src/index.ts
echo
echo "Lockfile/package references for stellar-sdk:"
for f in package.json backend/package.json package-lock.json backend/package-lock.json pnpm-lock.yaml backend/pnpm-lock.yaml yarn.lock backend/yarn.lock; do
if [ -f "$f" ]; then
echo "=== $f ==="
rg -n "stellar-sdk|`@stellar/stellar-sdk`|packages/stellar-sdk" "$f" | head -80
fi
done
echo
echo "All nativeToScVal uses:"
rg -n "nativeToScVal\\(" backend/src -g '*.ts'Repository: ritik4ever/stellar-stream
Length of output: 3513
🌐 Web query:
Stellar JavaScript SDK migration nativeToScVal numeric values silent rounding i128 amount
💡 Result:
In the Stellar JavaScript SDK, the handling of numeric values when using nativeToScVal has undergone significant improvements to prevent silent data corruption, though developers must remain vigilant regarding how they pass values [1][2]. Key points regarding the migration and numeric safety include: 1. Handling of Out-of-Range Values: Recent versions of the SDK (specifically starting with changes around #1399) have introduced strict bounds-checking for fixed-width integers like u32 and i32 [1][2]. For larger types such as i128, u128, i256, and u256, the SDK has been updated to reject out-of-range inputs rather than silently wrapping or truncating them [1][2]. Previously, some paths—particularly contract client calls using funcArgsToScVals—could lead to silent wrapping of oversized integers into incorrect on-chain values [3]. 2. Avoid JavaScript 'number' for Large Integers: You should strictly avoid passing JavaScript 'number' types for i128 or u128 amounts, as they are subject to floating-point precision limitations [2]. The SDK now warns that using high-precision JS numbers can lead to silent rounding [2]. Always use 'bigint' or string representations for exact integer values to ensure the SDK encodes them correctly without precision loss [2]. 3. Recommended Practices: - Always prefer BigInt for integer-based contract arguments [2]. - If you encounter complex conversion needs, utilize the newer, explicit conversion utilities (such as nativeToI128/i128ToNative) introduced to provide a clearer path between JS types and Soroban integers, reducing common "foot-guns" in earlier SDK versions [4]. - Ensure your SDK is updated to a recent version (e.g., v16.x or later) to benefit from these validation patches [5][1][2]. If you are using generated contract clients, ensure you are providing values that fit within the expected bounds, and prefer BigInt to guarantee that the value passed to the SDK is exactly what ends up on-chain [3][2].
Citations:
- 1: https://github.com/stellar/js-stellar-sdk/releases
- 2: https://github.com/stellar/js-stellar-sdk/releases/tag/v16.0.0-rc.1
- 3: [AI Security] Medium: Oversized integer ABI inputs wrap instead of rejecting in Spec.funcArgsToScVals() stellar/js-stellar-sdk#1358
- 4: Add Soroban number and address utilities stellar/js-stellar-sdk#1471
- 5: https://stellar.github.io/js-stellar-sdk/guides/00-migration/
Encode the clawback request as an exact i128 value.
amount flows through as a Zod number, so large integer request values lose JS floating-point precision before nativeToScVal(amount, { type: "i128" }) encodes the contract argument. Use BigInt(Math.trunc(parsedBody.data.amount)) or a validated integer string before nativeToScVal.
🤖 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 1439 - 1442, Update the
clawback request encoding near nativeToScVal(amount, { type: "i128" }) so the
amount is converted to an exact BigInt before serialization, using the validated
parsed amount or integer string and truncating only as required. Preserve the
i128 type and existing streamIdU64 and adminAddress arguments.
| const actualAmount = Number(scValToNative(txResult.returnValue)); | ||
|
|
||
| return { txHash: sendRes.hash, actualAmount }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Number(scValToNative(...)) can lose precision on the returned amount, and the admin address isn't surfaced in the result.
Two issues here:
scValToNativeconverts ani128return value to abigint; wrapping it inNumber(...)can silently lose precision for large values, same class of issue as the write path above.ClawbackResultdoesn't include the admin address (adminAddress, computed at line 1433) even though the caller needs it to correctly attribute the on-chain action in the audit event (see consolidated comment withindex.ts).
🛠️ Suggested fix
export interface ClawbackResult {
txHash: string;
actualAmount: number;
+ adminAddress: string;
}
...
- const actualAmount = Number(scValToNative(txResult.returnValue));
-
- return { txHash: sendRes.hash, actualAmount };
+ const actualAmount = Number(scValToNative(txResult.returnValue));
+
+ return { txHash: sendRes.hash, actualAmount, adminAddress };🤖 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 1486 - 1489, Update the
clawback result construction in the surrounding clawback method to preserve the
i128 amount as the bigint returned by scValToNative, removing the Number
conversion, and include the existing adminAddress value in the returned
ClawbackResult. Extend the result type as needed so callers can access both
actualAmount without precision loss and adminAddress.
| amount: { | ||
| type: "number", | ||
| description: "Amount to clawback (positive integer). Capped at unclaimed vested amount by the contract.", | ||
| exclusiveMinimum: true, | ||
| minimum: 0, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Doc says "positive integer" but schema allows fractional numbers.
amount is typed number with exclusiveMinimum/minimum only — no integer type or multipleOf constraint, despite the description stating "positive integer." This mirrors the same gap in clawbackBodySchema (index.ts) and the i128 conversion in streamStore.ts — see consolidated comment.
🤖 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 1872 - 1877, Update the amount schema in
the Swagger definition and the corresponding clawbackBodySchema to enforce
positive integers, not fractional numbers, while preserving the existing minimum
constraint and description. Align the validation with the i128 conversion used
by streamStore so all clawback amount boundaries are consistent.
| "401": { | ||
| description: "Unauthorized — missing or invalid admin key.", | ||
| content: { | ||
| "application/json": { | ||
| schema: { $ref: "#/components/schemas/Error" }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate swagger and validation files"
fd -a 'swagger\.ts|schemas\.ts|index\.ts' backend/src 2>/dev/null | sed 's#^\./##' | head -50
echo
echo "Inspect relevant lines in backend/src/swagger.ts"
if [ -f backend/src/swagger.ts ]; then
sed -n '1880,1945p' backend/src/swagger.ts | cat -n
fi
echo
echo "Search adminAuth and Error schema definitions/usages"
rg -n "adminAuth|schemas\.Error|Error"| backend/src -S
echo
echo "If validation schemas exist, show relevant sections"
for f in backend/src/validation/schemas.ts backend/src/swagger.ts; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n "Error|statusCode|error|code|message" "$f" -C 3 -S | head -220
fi
doneRepository: ritik4ever/stellar-stream
Length of output: 3058
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find adminAuth definitions and json responses"
rg -n "adminAuth|Unauthorized|res\.status\(401\)|res\.status\(|JSONError|Error\)|statusCode:" backend/src -S
echo
echo "Show backend/src/swagger.ts Error schema references around the relevant schema object"
rg -n "Error:" backend/src/swagger.ts -A 20 -B 10 -S
echo
echo "Show backend/src/validation/schemas.ts relevant section if present"
if [ -f backend/src/validation/schemas.ts ]; then
rg -n "Error|statusCode|error|code|message" backend/src/validation/schemas.ts -A 8 -B 4 -S | head -240
fi
echo
echo "Inspect route registration around adminAuth uses"
rg -n "adminAuth" backend/src -A 8 -B 8 -SRepository: ritik4ever/stellar-stream
Length of output: 22451
Fix the 401 response schema to match adminAuth.
adminAuth returns { message: "Unauthorized" }, but this references #/components/schemas/Error, which requires error and statusCode. Define a separate 401 response schema or change adminAuth to send an Error-shaped body.
🤖 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 1921 - 1928, Update the 401 response
documented near adminAuth to match the actual `{ message: "Unauthorized" }`
payload: replace the `#/components/schemas/Error` reference with a schema
requiring a string `message`, or adjust adminAuth to return the existing
Error-shaped payload. Keep the documented response and runtime behavior
consistent.
|
This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/src/services/streamStore.ts (3)
1070-1073: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse a named SQL parameter.
Replace the positional placeholder with
@idand bind an object. This follows the requiredbetter-sqlite3binding convention.Proposed fix
- const row = db.prepare("SELECT * FROM streams WHERE id = ?").get(id) as StreamRow | undefined; + const row = db + .prepare("SELECT * FROM streams WHERE id = `@id`") + .get({ id }) as StreamRow | undefined;As per coding guidelines, use
@nameparameter binding syntax forbetter-sqlite3prepared statements instead of?placeholders.🤖 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 1070 - 1073, Update the SQL query in getStreamById to use the named `@id` placeholder, and bind the input through an object keyed by id when calling get. Preserve the existing StreamRow lookup and return behavior.Source: Coding guidelines
197-203: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse the admin key for
clawbackStream.
clawbackStreamcallsclawbackwithserverKeypair.publicKey()asadminAddressand signs withserverKeypair, which comes fromSTELLAR_SECRET_KEYorSERVER_PRIVATE_KEY. AddADMIN_SECRET_KEYas the admin secret, keep the server secret for Soroban operations that need it, and derive the clawback key from the admin secret so contract admin authorization matches the spec.🤖 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 197 - 203, Update the key initialization near serverKeypair so ADMIN_SECRET_KEY is used to derive a separate admin/clawback keypair, while retaining STELLAR_SECRET_KEY or SERVER_PRIVATE_KEY for serverKeypair and Soroban operations. Modify clawbackStream to pass the admin keypair’s public key as adminAddress and sign the clawback transaction with that same admin keypair, preserving the contract’s admin authorization requirements.
811-819: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winComplete the
TransactionBuilderexpression.
txToSimulateis not closed beforebuiltis declared, so this expression cannot compile. Use a single completedTransactionBuildervalue for both simulation and preparation.Proposed fix
const txToSimulate = new TransactionBuilder(sourceAccount, { - const built = await rpcServer.prepareTransaction( - new TransactionBuilder(sourceAccount, { fee: "1000", networkPassphrase: netPass, }) .addOperation(op) .setTimeout(30) .build();🤖 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 811 - 819, Complete the TransactionBuilder expression assigned to txToSimulate before declaring built, ensuring the builder is fully configured and built once. Pass that same txToSimulate value to rpcServer.prepareTransaction instead of starting a second TransactionBuilder expression.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@backend/src/services/streamStore.ts`:
- Around line 1070-1073: Update the SQL query in getStreamById to use the named
`@id` placeholder, and bind the input through an object keyed by id when calling
get. Preserve the existing StreamRow lookup and return behavior.
- Around line 197-203: Update the key initialization near serverKeypair so
ADMIN_SECRET_KEY is used to derive a separate admin/clawback keypair, while
retaining STELLAR_SECRET_KEY or SERVER_PRIVATE_KEY for serverKeypair and Soroban
operations. Modify clawbackStream to pass the admin keypair’s public key as
adminAddress and sign the clawback transaction with that same admin keypair,
preserving the contract’s admin authorization requirements.
- Around line 811-819: Complete the TransactionBuilder expression assigned to
txToSimulate before declaring built, ensuring the builder is fully configured
and built once. Pass that same txToSimulate value to
rpcServer.prepareTransaction instead of starting a second TransactionBuilder
expression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dc756489-9a18-4700-85b8-09a69ec8717c
📒 Files selected for processing (2)
backend/src/index.tsbackend/src/services/streamStore.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/index.ts
@ritik4ever please merge it now its fixed |
3 participants
Summary
Adds \POST /api/admin/streams/:id/clawback\ — admin-compliance endpoint that calls the on-chain \clawback\ function to reclaim unclaimed vested tokens.
Changes
Acceptance Criteria
Closes #326
Summary by CodeRabbit
New Features
Documentation
Tests