Agent/fix sync views link orb security#229
Open
sirgawain0x wants to merge 33 commits into
Open
Conversation
optimizations
…cel-deployment fix: resolve Vercel TypeScript failure in Tour component
…n-tour.tsx Fix react-joyride step property mismatch in Tour
…n-tour.tsx-4rieek Fix Joyride tour TypeScript build errors
…uery-error fix(predictions): use valid Reality.eth subgraph query field
Updates predict page
working on updating the preditctions
update view
predictions
Co-authored-by: G2 <sirgawain0x@users.noreply.github.com>
Co-authored-by: G2 <sirgawain0x@users.noreply.github.com>
Co-authored-by: G2 <sirgawain0x@users.noreply.github.com>
…ts-7bd0 Fix dynamic Livepeer view metrics
Prevent cron and sync routes from overwriting Supabase views_count with Livepeer zeros; return max(Livepeer, DB) on the read API via service role. Co-authored-by: Cursor <cursoragent@cursor.com>
Wire mainnet/testnet Lens RPC resolution, Grove ACL by env, and viem client helper; document Sepolia vs mainnet in .env.example. Co-authored-by: Cursor <cursoragent@cursor.com>
…nd profile linking. Add Orb QR login alongside Account Kit, Grove media resolver in GatewayImage, creator_profiles Orb/Lens fields, and optional Orb Grove thumbnail uploads. Co-authored-by: Cursor <cursoragent@cursor.com>
Fix zero video views, Lens mainnet RPC, and Orb modules integration
Always sync views from Livepeer (never client body), verify Lens account from Orb token with wallet auth, and add route tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
WalkthroughAdds Orb/Lens auth and creator linking, refactors Livepeer metrics with no-store APIs and hooks, introduces dual subgraph provider and a new Creative Platform subgraph, enforces prediction quotas, overhauls IPFS pipeline and gateways, migrates to Base chain utilities, and updates build/patch configuration. ChangesOrb/Lens session integration and creator linking
Livepeer view metrics and sync refactor
Dual subgraph provider and Creative Platform subgraph
Prediction-market monthly quota and canonical arbitrator
Grove-first IPFS pipeline, Helia fallback, and gateway normalization
Base chain utilities and adoption
Build system, patching, and assets
Sequence Diagram(s)sequenceDiagram
participant ComponentA
participant ComponentB
ComponentA->>ComponentB: observable interaction
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Orb and Lens protocol integrations, enabling sovereign identity linking and decentralized storage via Lens Grove. It introduces a quota management system for prediction markets, enhances Livepeer view metric synchronization, and adds support for Graph Studio subgraphs alongside Goldsky. Additionally, the PR includes several infrastructure improvements, such as dynamic Helia initialization to prevent SSR crashes and local chain definitions to optimize build performance. Review feedback identifies high-priority security issues concerning the potential spoofing of Orb identifiers and unverified transaction recording for quotas, along with a recommendation to optimize buffer handling in the IPFS service.
| ); | ||
| } | ||
|
|
||
| const orbAccountId = authenticationId || resolvedLensAccount || ownerAddress; |
There was a problem hiding this comment.
The authenticationId is taken directly from the request body and used as a unique identifier (orb_account_id) in the database. Since this field is not verified against the accessToken or an external Orb API, a malicious user could potentially spoof this ID to cause conflicts or squat on identifiers belonging to other users. Consider extracting this ID from a verified token or verifying it via an Orb SDK method if it is intended to be a trusted sovereign identifier.
Comment on lines
+77
to
+83
| const { error: insertError } = await supabaseService | ||
| .from("prediction_market_creations") | ||
| .insert({ | ||
| creator_address: normalized, | ||
| transaction_hash: transactionHash.toLowerCase(), | ||
| question_id: questionId ?? null, | ||
| }); |
There was a problem hiding this comment.
The endpoint records a transactionHash to track monthly quotas without verifying on-chain that the transaction is valid, successful, and actually corresponds to a prediction market creation by the specified address. This allows users to potentially exhaust their quota with invalid hashes or pollute the database with arbitrary transaction data. Consider adding a server-side verification step using a public client to confirm the transaction's validity and event logs before recording it.
Comment on lines
+179
to
+183
| const bytes = | ||
| buffer instanceof ArrayBuffer | ||
| ? new Uint8Array(buffer) | ||
| : Uint8Array.from(buffer); | ||
| const blob = new Blob([bytes]); |
There was a problem hiding this comment.
Using Uint8Array.from(buffer) when the input is already a Buffer or Uint8Array creates an unnecessary copy of the data. For ArrayBuffer, Buffer, or Uint8Array, you can directly use the Uint8Array constructor which is more efficient as it creates a view over the existing memory.
const bytes = buffer instanceof Uint8Array ? buffer : new Uint8Array(buffer);
const blob = new Blob([bytes]);![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/api/livepeer/views/[playbackId]/route.ts (1)
59-59:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix undefined Supabase client factory call.
Line 59 calls
createClient(), but this module importscreateServiceClient. This will throw when fallback executes.Proposed fix
- const supabase = await createClient(); + const supabase = createServiceClient();🤖 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 `@app/api/livepeer/views/`[playbackId]/route.ts at line 59, The code calls createClient() but the module only exports createServiceClient; replace the incorrect factory call with createServiceClient() so the Supabase client is constructed from the imported symbol (update the reference where const supabase = await createClient() appears to use createServiceClient instead and ensure any required arguments/typing for createServiceClient are passed through).app/api/video-assets/sync-views/[playbackId]/route.ts (1)
99-115:⚠️ Potential issue | 🟠 Major | ⚡ Quick winProtect mutating GET sync endpoint with auth/rate controls.
This GET path writes
views_countbut has no bot check, auth gate, or rate limiting. It is publicly triggerable and can drive uncontrolled DB/API load.Proposed fix
export async function GET( _request: NextRequest, { params }: { params: Promise<{ playbackId: string }> } ) { + const verification = await checkBotId(); + if (verification.isBot) { + return NextResponse.json({ error: 'Access denied' }, { status: 403 }); + } + const rl = await rateLimiters.standard(_request); + if (rl) return rl; + try { const { playbackId } = await params;🤖 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 `@app/api/video-assets/sync-views/`[playbackId]/route.ts around lines 99 - 115, The public GET handler (function GET) calls syncViewsForPlayback and mutates views_count without any protection; add an authentication and rate-control gate before calling createServiceClient/syncViewsForPlayback: require and validate an auth token or service secret header (reject with 401 if missing/invalid) or verify current session, and implement rate limiting (per playbackId or per client IP) that returns 429 when limits exceeded; also consider a lightweight bot-check (User-Agent/recaptcha) for public clients. Update the checks inside the GET handler (route.ts) and short-circuit to NextResponse with appropriate status codes before creating the Supabase service client or invoking syncViewsForPlayback.package.json (1)
1-6: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd a project-level Node.js version pin.
The project lacks a Node.js runtime specification. Add either an
enginesorvoltafield topackage.json, or create a companion file (.nvmrc,.node-version, or.tool-versions) at the repository root to declare the required Node.js version. This ensures consistent builds and installs across all environments and contributors.🤖 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 `@package.json` around lines 1 - 6, Add a project-level Node.js version pin by updating package.json to include either an "engines" field (e.g., "engines": {"node": ">=16 <19" or a specific "16.x"}) or a "volta" field (e.g., "volta": {"node": "18.16.0"}) right under the top-level keys (near "name", "version", "private", "type") so all contributors and CI use the same runtime; alternatively, create a companion file at repo root (.nvmrc, .node-version, or .tool-versions) containing the exact Node version string to satisfy the reviewer.
🧹 Nitpick comments (7)
components/predictions/CreatePrediction.tsx (1)
473-479: 💤 Low valueVerify upgrade link destination.
The upgrade links point to
"/"(home page). Consider pointing to a dedicated membership/pricing page if one exists, to provide a better upgrade experience for users hitting quota limits.Also applies to: 483-489
🤖 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 `@components/predictions/CreatePrediction.tsx` around lines 473 - 479, The upgrade Link components in CreatePrediction.tsx currently point to "/" (homepage) which is misleading; update the Link hrefs (the Link elements around "Upgrade to Investor or Brand") to the correct membership/pricing path used by the app (e.g. "/pricing" or your dedicated billing/membership route) so users hitting quota limits go directly to the upgrade page, and verify both occurrences (the Link at the block around "Upgrade to Investor or Brand" and the second occurrence noted in the review) are changed to the same canonical upgrade URL.lib/sdk/ipfs/service.ts (2)
211-231: 💤 Low valueFire-and-forget archival could silently fail for valid CIDs.
maybeFilecoinArchivalswallows failures via.catch()and only logs a warning. While this is intentional to avoid blocking uploads, consider emitting a structured telemetry event or metric for archival failures so operators can detect patterns (e.g., FilecoinFirst API down).🤖 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 `@lib/sdk/ipfs/service.ts` around lines 211 - 231, The fire-and-forget method maybeFilecoinArchival currently swallows errors from createFilecoinDeal and only logs them via serverLogger.warn, which can hide systemic failures; modify maybeFilecoinArchival so that inside the .catch(err) handler it emits a structured telemetry/metrics event (e.g., call Telemetry.trackEvent or Metrics.increment with a distinct key like "filecoin_archival_failure") including the hash, error message/stack, and any contextual flags (filecoinFirstService/enableFilecoinArchival) in addition to the existing warning log, so operators can alert on and aggregate archival failures; reference maybeFilecoinArchival, createFilecoinDeal, and serverLogger.warn when adding the telemetry/metrics call.
116-125: 💤 Low valueUpload options are declared but never used.
The
optionsparameter (pin,wrapWithDirectory,maxSize) is explicitly voided but not forwarded to Grove, Lighthouse, or Helia. Callers invokinguploadFile(file, { pin: true })will have no effect.If these options are intentionally unsupported in the new pipeline, consider removing them from the signature to avoid misleading callers. Otherwise, forward them to the underlying services where applicable.
🤖 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 `@lib/sdk/ipfs/service.ts` around lines 116 - 125, The uploadFile function currently declares but voids the options parameter, so pin/wrapWithDirectory/maxSize are ignored; remove the "void options" and either (A) remove these options from the uploadFile signature if the new pipeline intentionally doesn't support them, or (B) forward them to the underlying upload implementation(s) used by uploadFile (e.g., pass pin/wrapWithDirectory/maxSize to the Grove/Lighthouse/Helia upload calls or helper functions invoked after toUploadableFile). Also add any necessary validation for maxSize before uploading and propagate pin/wrapWithDirectory through the same call chain so callers like uploadFile(file, { pin: true }) actually affect the underlying service.components/ui/gateway-image.tsx (1)
41-80: 💤 Low valueConsider memoizing or hoisting
normalizeSrc.
normalizeSrcis redefined on every render but only depends on external functions (not props/state). This causesnormalizeSrc(src || '')on line 62 to recalculate on every render even whensrchasn't changed.Since the function is pure and stateless, you could hoist it outside the component or use
useCallback. This is a minor optimization—current behavior is correct.🤖 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 `@components/ui/gateway-image.tsx` around lines 41 - 80, Hoist or memoize normalizeSrc to avoid re-creating it on every render: move the function out of the component (or wrap it with useCallback) so it no longer closes over render scope; ensure it still calls resolveOrbMediaForGateway and convertFailingGateway and returns the converted value, then update usages (initial useState call and the useEffect body that calls normalizeSrc(src || '')) to reference the hoisted/memoized normalizeSrc; keep triedGatewaysRef/allGatewaysExhaustedRef/consecutiveFailuresRef logic unchanged.app/api/ai/generate-thumbnail/route.ts (1)
219-219: 💤 Low valueReplace deprecated
substrwithslice.The
substr()method is deprecated. Useslice()instead for future compatibility.♻️ Suggested fix
- const imageId = `gemini-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + const imageId = `gemini-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;🤖 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 `@app/api/ai/generate-thumbnail/route.ts` at line 219, The generated imageId uses the deprecated substr method; update the imageId creation (variable imageId in route.ts) to replace Math.random().toString(36).substr(2, 9) with the non-deprecated slice variant (e.g., .slice(2, 11)) so the random string length remains the same and avoids deprecated API usage.lib/utils/suppressDevWarnings.ts (1)
20-21: ⚡ Quick winNarrow this suppression pattern to the known dependency path.
This regex is broad enough to hide unrelated webpack critical-dependency warnings, which can mask real regressions in dev.
Suggested narrowing
- /Critical dependency: the request of a dependency is an expression/i, + /Critical dependency: the request of a dependency is an expression.*(viem|ox|tempo)/i,🤖 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 `@lib/utils/suppressDevWarnings.ts` around lines 20 - 21, The current suppression in suppressDevWarnings.ts uses a too-broad pattern that hides all "Critical dependency: the request of a dependency is an expression" warnings; narrow it so it only suppresses warnings about the known viem dynamic-import path (e.g., messages that also mention "viem" / "viem/chains" / "viem/ox"). Update the pattern in the suppression list (the array or variable that holds the regexes inside suppressDevWarnings or the function that builds them) to require the viem-specific module path text in the warning before silencing it, leaving other critical-dependency warnings uncaught.scripts/graph-studio/parity-check.ts (1)
4-11: ⚡ Quick winAdd HTTP/status guarding in
query()for actionable parity failures.
res.json()is called unconditionally, so non-JSON/non-2xx responses fail without endpoint/status context. Add explicitres.okhandling (with response text snippet) to make parity debugging reliable.Proposed fix
async function query<T>(url: string, queryText: string): Promise<GraphResponse<T>> { const res = await fetch(url, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ query: queryText }) }); - return res.json(); + if (!res.ok) { + const body = await res.text(); + throw new Error(`Query failed (${res.status}) for ${url}: ${body.slice(0, 300)}`); + } + return res.json(); }🤖 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 `@scripts/graph-studio/parity-check.ts` around lines 4 - 11, The query function currently calls res.json() unconditionally which obscures non-2xx or non-JSON responses; update the async function query<T>(url, queryText) to check res.ok first and, if false, read a small snippet of the response text (or full text if short), then throw or return a rejected Error containing the HTTP status, statusText and the response snippet for actionable parity failure diagnostics; when res.ok is true, proceed to parse and return JSON as GraphResponse<T>, and handle JSON parse errors with an explicit, descriptive error including the response body snippet.
🤖 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 `@app/api/creator-profiles/link-orb/route.ts`:
- Around line 104-107: The code only sets payload.avatar_url when lensAvatarUri
exists and body.avatar_url is falsy, so explicit avatar values from the request
are ignored; update the logic in the route handler to persist body.avatar_url
when provided (e.g., if (body.avatar_url) payload.avatar_url = body.avatar_url),
and keep the existing lensAvatarUri fallback (e.g., if (lensAvatarUri) {
payload.lens_avatar_uri = lensAvatarUri; if (!payload.avatar_url)
payload.avatar_url = lensAvatarUri; }) so that payload.avatar_url prefers
body.avatar_url but falls back to lensAvatarUri.
- Around line 9-17: The bot check and rate limiter calls (checkBotId and
rateLimiters.standard) are executed outside the try block in the POST handler so
their failures can escape the JSON error contract; move the verification step
(verification = await checkBotId()) and the rate limiter call (rl = await
rateLimiters.standard(request)) inside the existing try block in the POST
function, and ensure any early returns (NextResponse.json({ error: 'Access
denied' }, { status: 403 }) and rate-limiter response) remain inside that try so
thrown errors are caught and handled by your JSON error response path.
In `@app/api/metokens-subgraph/route.ts`:
- Around line 98-105: The retry loop currently aborts on thrown fetch/network
errors and uses substring checks to decide whether to send Goldsky secrets; fix
by (1) creating a fresh headers copy per endpoint (e.g., const endpointHeaders =
{ ...headers } or new object) so you don't mutate shared headers, (2) derive the
endpoint hostname strictly via new URL(endpoint).hostname and only allow
Authorization when hostname === 'goldsky.com' (remove Authorization otherwise),
and (3) wrap the performFetch(endpoint, endpointHeaders) call in try/catch so
network exceptions are caught, set response only on success, and continue to the
next candidate on error instead of breaking the failover loop; keep using
subgraphEndpoint and response as before.
In `@app/api/reality-eth-subgraph/route.ts`:
- Around line 86-96: The loop currently stops failover when performFetch throws
and uses string includes to detect Goldsky hosts; fix by parsing each candidate
endpoint's hostname with new URL(endpoint).hostname and use a strict check like
hostname === 'goldsky.com' or hostname.endsWith('.goldsky.com') to decide
whether to attach Authorization, build a fresh endpointHeaders object per
iteration (so you don't mutate shared headers), then wrap the performFetch call
in try/catch inside the for loop (log the error with serverLogger.debug/error
and continue to the next candidate on exception) and only set subgraphEndpoint
and break when response.ok; reference candidateEndpoints, endpointHeaders,
performFetch, serverLogger, subgraphEndpoint, and response when making these
changes.
In `@ENVIRONMENT_SETUP.md`:
- Around line 324-328: Update the note so it clarifies that SUBGRAPH_QUERY_KEY
is not needed only when using Goldsky public endpoints (e.g., when
SUBGRAPH_PROVIDER_MODE is set to goldsky/default), and explicitly state that the
GRAPH_STUDIO_CREATIVE_PLATFORM_URL example requires a SUBGRAPH_QUERY_KEY when
using Studio or SUBGRAPH_PROVIDER_MODE=dual; reference the variables
SUBGRAPH_QUERY_KEY, SUBGRAPH_PROVIDER_MODE, and
GRAPH_STUDIO_CREATIVE_PLATFORM_URL in the updated sentence so readers know when
the key is required versus when Goldsky endpoints are used.
In `@lib/livepeer/sync-view-count.ts`:
- Around line 8-14: getStoredViewsCount currently ignores Supabase query errors
and returns 0, masking outages; update the function to check the Supabase
response error (the returned "error" field alongside "data") after calling
supabase.from('video_assets').select(...).eq(...).maybeSingle(), and if error is
present log the error (include playbackId and error.message) and surface it to
the caller (throw or return a rejected Promise) instead of defaulting to 0; keep
returning data?.views_count when no error. Use the symbols getStoredViewsCount,
data, and error to locate and modify the logic.
- Around line 28-32: The current read-merge-write can overwrite a newer higher
views_count; change the update so it only applies when the DB value is less than
the computed merged value. Replace the unconditional .update({ views_count:
merged }).eq('playback_id', playbackId) call with a conditional update that adds
a WHERE clause comparing the stored column (e.g. .lt('views_count', merged) or
equivalent) so the update only runs when views_count < merged for the given
playbackId (and keep error handling for the operation). Ensure you reference the
merged and playbackId variables and the supabase .from('video_assets') update
flow when making this change.
In `@lib/sdk/orb/login.ts`:
- Around line 35-42: The saveStoredOrbSession function currently persists full
StoredOrbSession (including access/refresh/id tokens) into localStorage via
ORB_SESSION_STORAGE_KEY; change this so no sensitive tokens are stored
client-side—remove localStorage.setItem usage for tokens and instead send the
session (or token material) to a backend endpoint that sets secure, httpOnly,
SameSite cookies, or avoid storing refresh/id tokens on the client entirely;
update saveStoredOrbSession to only store non-sensitive UI state locally (or
clear the key) and call a new server API to set/clear httpOnly cookies for
access/refresh tokens, and remove other code paths that read/writes tokens from
localStorage.
In `@lib/sdk/orb/media.ts`:
- Around line 16-20: The HTTP(S) branch always returns false making the inner
checks dead; change the branch in lib/sdk/orb/media.ts so that instead of the
unconditional "return false" it returns the result that respects the two checks:
evaluate the /\.(m3u8|mp4|mov|webm)(\?|$)/i regex against the variable trimmed
and the trimmed.includes('livepeer') test, and return the negation/appropriate
boolean so the function outcome depends on those checks (i.e., return the
condition that combines the regex match and the includes('livepeer') result
rather than always false).
In `@lib/sdk/story/ipa-metadata.ts`:
- Around line 106-118: The current ipMetadataURI construction can yield
"ipfs://undefined" when result.url is a non-URI string (e.g., a bare IPFS cid
like "Qm...") and result.hash is missing; modify the logic that computes
ipMetadataURI (the result and ipMetadataURI variables) so that: if result.url
starts with "http://", "https://", or "ipfs://", use result.url; else if
result.hash is present use `ipfs://` + result.hash; else if result.url is
present (non-URI) treat it as a raw CID and use `ipfs://` + result.url;
otherwise throw or surface the error—this ensures a valid IPFS URI instead of
"ipfs://undefined".
In `@subgraphs/creative-platform/package.json`:
- Around line 1-17: Add explicit Node runtime pinning to the
creative-platform-subgraph package.json by adding an engines.node field (e.g.,
"node": ">=18 <19" or a specific 18.x/20.x version your repo uses) and/or a
Volta pin (volta.node set to the exact toolchain version) at the top-level of
package.json so Graph CLI/build behavior is reproducible; update the
package.json root (the file containing the "name": "creative-platform-subgraph"
entry) to include either an "engines" object and/or a "volta" object with the
agreed project Node version.
In `@subgraphs/creative-platform/src/realityEth.ts`:
- Around line 46-47: The new-question handler is incorrectly initializing
question.history_hash from event.params.content_hash; instead initialize
question.history_hash to an empty/zero Bytes value (no history) and keep
event.params.content_hash assigned to question.content_hash; find the assignment
to question.history_hash and replace the content_hash usage with an empty Bytes
(e.g., Bytes.empty() / zero value) so history starts empty until an answer
exists (leave question.arbitrator and other assignments as-is).
---
Outside diff comments:
In `@app/api/livepeer/views/`[playbackId]/route.ts:
- Line 59: The code calls createClient() but the module only exports
createServiceClient; replace the incorrect factory call with
createServiceClient() so the Supabase client is constructed from the imported
symbol (update the reference where const supabase = await createClient() appears
to use createServiceClient instead and ensure any required arguments/typing for
createServiceClient are passed through).
In `@app/api/video-assets/sync-views/`[playbackId]/route.ts:
- Around line 99-115: The public GET handler (function GET) calls
syncViewsForPlayback and mutates views_count without any protection; add an
authentication and rate-control gate before calling
createServiceClient/syncViewsForPlayback: require and validate an auth token or
service secret header (reject with 401 if missing/invalid) or verify current
session, and implement rate limiting (per playbackId or per client IP) that
returns 429 when limits exceeded; also consider a lightweight bot-check
(User-Agent/recaptcha) for public clients. Update the checks inside the GET
handler (route.ts) and short-circuit to NextResponse with appropriate status
codes before creating the Supabase service client or invoking
syncViewsForPlayback.
In `@package.json`:
- Around line 1-6: Add a project-level Node.js version pin by updating
package.json to include either an "engines" field (e.g., "engines": {"node":
">=16 <19" or a specific "16.x"}) or a "volta" field (e.g., "volta": {"node":
"18.16.0"}) right under the top-level keys (near "name", "version", "private",
"type") so all contributors and CI use the same runtime; alternatively, create a
companion file at repo root (.nvmrc, .node-version, or .tool-versions)
containing the exact Node version string to satisfy the reviewer.
---
Nitpick comments:
In `@app/api/ai/generate-thumbnail/route.ts`:
- Line 219: The generated imageId uses the deprecated substr method; update the
imageId creation (variable imageId in route.ts) to replace
Math.random().toString(36).substr(2, 9) with the non-deprecated slice variant
(e.g., .slice(2, 11)) so the random string length remains the same and avoids
deprecated API usage.
In `@components/predictions/CreatePrediction.tsx`:
- Around line 473-479: The upgrade Link components in CreatePrediction.tsx
currently point to "/" (homepage) which is misleading; update the Link hrefs
(the Link elements around "Upgrade to Investor or Brand") to the correct
membership/pricing path used by the app (e.g. "/pricing" or your dedicated
billing/membership route) so users hitting quota limits go directly to the
upgrade page, and verify both occurrences (the Link at the block around "Upgrade
to Investor or Brand" and the second occurrence noted in the review) are changed
to the same canonical upgrade URL.
In `@components/ui/gateway-image.tsx`:
- Around line 41-80: Hoist or memoize normalizeSrc to avoid re-creating it on
every render: move the function out of the component (or wrap it with
useCallback) so it no longer closes over render scope; ensure it still calls
resolveOrbMediaForGateway and convertFailingGateway and returns the converted
value, then update usages (initial useState call and the useEffect body that
calls normalizeSrc(src || '')) to reference the hoisted/memoized normalizeSrc;
keep triedGatewaysRef/allGatewaysExhaustedRef/consecutiveFailuresRef logic
unchanged.
In `@lib/sdk/ipfs/service.ts`:
- Around line 211-231: The fire-and-forget method maybeFilecoinArchival
currently swallows errors from createFilecoinDeal and only logs them via
serverLogger.warn, which can hide systemic failures; modify
maybeFilecoinArchival so that inside the .catch(err) handler it emits a
structured telemetry/metrics event (e.g., call Telemetry.trackEvent or
Metrics.increment with a distinct key like "filecoin_archival_failure")
including the hash, error message/stack, and any contextual flags
(filecoinFirstService/enableFilecoinArchival) in addition to the existing
warning log, so operators can alert on and aggregate archival failures;
reference maybeFilecoinArchival, createFilecoinDeal, and serverLogger.warn when
adding the telemetry/metrics call.
- Around line 116-125: The uploadFile function currently declares but voids the
options parameter, so pin/wrapWithDirectory/maxSize are ignored; remove the
"void options" and either (A) remove these options from the uploadFile signature
if the new pipeline intentionally doesn't support them, or (B) forward them to
the underlying upload implementation(s) used by uploadFile (e.g., pass
pin/wrapWithDirectory/maxSize to the Grove/Lighthouse/Helia upload calls or
helper functions invoked after toUploadableFile). Also add any necessary
validation for maxSize before uploading and propagate pin/wrapWithDirectory
through the same call chain so callers like uploadFile(file, { pin: true })
actually affect the underlying service.
In `@lib/utils/suppressDevWarnings.ts`:
- Around line 20-21: The current suppression in suppressDevWarnings.ts uses a
too-broad pattern that hides all "Critical dependency: the request of a
dependency is an expression" warnings; narrow it so it only suppresses warnings
about the known viem dynamic-import path (e.g., messages that also mention
"viem" / "viem/chains" / "viem/ox"). Update the pattern in the suppression list
(the array or variable that holds the regexes inside suppressDevWarnings or the
function that builds them) to require the viem-specific module path text in the
warning before silencing it, leaving other critical-dependency warnings
uncaught.
In `@scripts/graph-studio/parity-check.ts`:
- Around line 4-11: The query function currently calls res.json()
unconditionally which obscures non-2xx or non-JSON responses; update the async
function query<T>(url, queryText) to check res.ok first and, if false, read a
small snippet of the response text (or full text if short), then throw or return
a rejected Error containing the HTTP status, statusText and the response snippet
for actionable parity failure diagnostics; when res.ok is true, proceed to parse
and return JSON as GraphResponse<T>, and handle JSON parse errors with an
explicit, descriptive error including the response body snippet.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 43f211f2-fe9c-49ff-8963-dbf8a2dae914
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsubgraphs/creative-platform/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (97)
.env.exampleENVIRONMENT_SETUP.mdGRAPH_STUDIO_MIGRATION.mdapp/api/ai/generate-thumbnail/route.tsapp/api/ai/upload-to-ipfs/route.tsapp/api/creator-profiles/link-orb/route.test.tsapp/api/creator-profiles/link-orb/route.tsapp/api/creator-profiles/upsert/route.tsapp/api/livepeer/views.test.tsapp/api/livepeer/views.tsapp/api/livepeer/views/[playbackId]/route.tsapp/api/market/tokens/[address]/fresh/route.tsapp/api/market/tokens/[address]/price-history/route.tsapp/api/market/tokens/sync/cron/route.tsapp/api/metokens-subgraph/route.tsapp/api/predictions/quota/route.tsapp/api/predictions/record/route.tsapp/api/reality-eth-subgraph/route.tsapp/api/video-assets/published/route.tsapp/api/video-assets/sync-views/[playbackId]/route.test.tsapp/api/video-assets/sync-views/[playbackId]/route.tsapp/api/video-assets/sync-views/cron/route.tsapp/layout.tsxapp/predict/[id]/page.tsxapp/providers.tsxcomponents/Navbar.tsxcomponents/Player/PreviewPlayer.tsxcomponents/Player/ViewsComponent.tsxcomponents/Tour/Tour.tsxcomponents/UserProfile/useProfile.tscomponents/Videos/VideoViewMetrics.tsxcomponents/account-dropdown/AccountDropdown.tsxcomponents/auth/OrbLoginModal.tsxcomponents/home-page/Featured.tsxcomponents/predictions/CreatePrediction.tsxcomponents/predictions/PredictionList.tsxcomponents/proposal-list/ProposalListSkeleton.tsxcomponents/ui/gateway-image.tsxconfig.tscontext/HeliaContext.tsxcontext/OrbSessionContext.tsxhooks/useLens.tsinstrumentation-client.tslib/hooks/ipfs/useIpfsService.tslib/hooks/livepeer/useLivepeerViewMetrics.test.tslib/hooks/livepeer/useLivepeerViewMetrics.tslib/hooks/orb/useOrbSession.tslib/livepeer/sync-view-count.tslib/livepeer/view-count.tslib/predictions/prediction-quota.tslib/sdk/accountKit/createWalletClient.tslib/sdk/alchemy/metoken-service.tslib/sdk/grove/service.tslib/sdk/ipfs/lighthouse-service.tslib/sdk/ipfs/service.tslib/sdk/lens/chains.tslib/sdk/lens/viem-client.tslib/sdk/metokens/subgraph.tslib/sdk/orb/config.tslib/sdk/orb/grove-upload.tslib/sdk/orb/login.tslib/sdk/orb/media.tslib/sdk/reality-eth/reality-eth-arbitrator.tslib/sdk/reality-eth/reality-eth-client.tslib/sdk/reality-eth/reality-eth-question-wrapper.tslib/sdk/reality-eth/reality-eth-subgraph.tslib/sdk/story/ipa-metadata.tslib/sdk/supabase/client.tslib/sdk/supabase/creator-profiles.tslib/sdk/supabase/metokens.tslib/services/thumbnail-upload.tslib/utils/chains/base.tslib/utils/image-gateway.tslib/utils/metokenUtils.tslib/utils/published-videos-client.tslib/utils/suppressDevWarnings.tslib/utils/transactionPolling.tsnext.config.mjspackage.jsonpatches/next@16.2.4.patchpnpm-workspace.yamlpublic/workbox-1bb06f5e.jspublic/workbox-e9849328.jsscripts/graph-studio/deploy-creative-platform.shscripts/graph-studio/parity-check.tssubgraphs/creative-platform/README.mdsubgraphs/creative-platform/abis/MeTokens.jsonsubgraphs/creative-platform/abis/RealityETH-3.0-tmp.jsonsubgraphs/creative-platform/abis/RealityETH-3.0.jsonsubgraphs/creative-platform/package.jsonsubgraphs/creative-platform/schema.graphqlsubgraphs/creative-platform/src/metokens.tssubgraphs/creative-platform/src/realityEth.tssubgraphs/creative-platform/subgraph.yamlsupabase/migrations/20260505120000_create_prediction_market_creations.sqlsupabase/migrations/20260519170000_add_orb_lens_to_creator_profiles.sqlverify_market_script.ts
💤 Files with no reviewable changes (1)
- public/workbox-1bb06f5e.js
Comment on lines
+9
to
+17
| export async function POST(request: NextRequest) { | ||
| const verification = await checkBotId(); | ||
| if (verification.isBot) { | ||
| return NextResponse.json({ error: 'Access denied' }, { status: 403 }); | ||
| } | ||
| const rl = await rateLimiters.standard(request); | ||
| if (rl) return rl; | ||
|
|
||
| try { |
Contributor
There was a problem hiding this comment.
Move bot/rate-limit checks inside the guarded error path.
Line 10 and Line 14 run outside the try, so failures there can bypass your JSON error response contract and bubble as framework-level 500s.
Suggested fix
export async function POST(request: NextRequest) {
- const verification = await checkBotId();
- if (verification.isBot) {
- return NextResponse.json({ error: 'Access denied' }, { status: 403 });
- }
- const rl = await rateLimiters.standard(request);
- if (rl) return rl;
-
try {
+ const verification = await checkBotId();
+ if (verification.isBot) {
+ return NextResponse.json({ error: 'Access denied' }, { status: 403 });
+ }
+ const rl = await rateLimiters.standard(request);
+ if (rl) return rl;
+
let body: Record<string, unknown>;
try {
body = await request.json();📝 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
| export async function POST(request: NextRequest) { | |
| const verification = await checkBotId(); | |
| if (verification.isBot) { | |
| return NextResponse.json({ error: 'Access denied' }, { status: 403 }); | |
| } | |
| const rl = await rateLimiters.standard(request); | |
| if (rl) return rl; | |
| try { | |
| export async function POST(request: NextRequest) { | |
| try { | |
| const verification = await checkBotId(); | |
| if (verification.isBot) { | |
| return NextResponse.json({ error: 'Access denied' }, { status: 403 }); | |
| } | |
| const rl = await rateLimiters.standard(request); | |
| if (rl) return rl; | |
| let body: Record<string, unknown>; |
🤖 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 `@app/api/creator-profiles/link-orb/route.ts` around lines 9 - 17, The bot
check and rate limiter calls (checkBotId and rateLimiters.standard) are executed
outside the try block in the POST handler so their failures can escape the JSON
error contract; move the verification step (verification = await checkBotId())
and the rate limiter call (rl = await rateLimiters.standard(request)) inside the
existing try block in the POST function, and ensure any early returns
(NextResponse.json({ error: 'Access denied' }, { status: 403 }) and rate-limiter
response) remain inside that try so thrown errors are caught and handled by your
JSON error response path.
Comment on lines
+104
to
+107
| if (lensAvatarUri) { | ||
| payload.lens_avatar_uri = lensAvatarUri; | ||
| if (!body.avatar_url) payload.avatar_url = lensAvatarUri; | ||
| } |
Contributor
There was a problem hiding this comment.
Persist avatar_url explicitly when provided.
At Line 106, body.avatar_url is only used as a guard; explicit avatar values are never written to payload, so they are silently ignored.
Suggested fix
const lensAvatarUri = body.lens_avatar_uri
? String(body.lens_avatar_uri).trim()
: undefined;
+ const avatarUrl = body.avatar_url ? String(body.avatar_url).trim() : undefined;
@@
- if (lensAvatarUri) {
- payload.lens_avatar_uri = lensAvatarUri;
- if (!body.avatar_url) payload.avatar_url = lensAvatarUri;
- }
+ if (lensAvatarUri) payload.lens_avatar_uri = lensAvatarUri;
+ if (avatarUrl) payload.avatar_url = avatarUrl;
+ else if (lensAvatarUri) payload.avatar_url = lensAvatarUri;🤖 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 `@app/api/creator-profiles/link-orb/route.ts` around lines 104 - 107, The code
only sets payload.avatar_url when lensAvatarUri exists and body.avatar_url is
falsy, so explicit avatar values from the request are ignored; update the logic
in the route handler to persist body.avatar_url when provided (e.g., if
(body.avatar_url) payload.avatar_url = body.avatar_url), and keep the existing
lensAvatarUri fallback (e.g., if (lensAvatarUri) { payload.lens_avatar_uri =
lensAvatarUri; if (!payload.avatar_url) payload.avatar_url = lensAvatarUri; })
so that payload.avatar_url prefers body.avatar_url but falls back to
lensAvatarUri.
Comment on lines
+42
to
+45
| orb_account_id, | ||
| lens_account_id, | ||
| lens_handle, | ||
| lens_avatar_uri, |
Contributor
There was a problem hiding this comment.
Block direct writes to Orb/Lens identity fields in this generic upsert route.
These fields are accepted and persisted without Orb-token verification, which bypasses the /api/creator-profiles/link-orb trust flow and allows arbitrary self-assignment of Orb/Lens identity data.
🔒 Suggested fix
@@
const {
owner_address,
username,
bio,
avatar_url,
twin_enabled,
twin_address,
twin_avatar_glb_url,
twin_chat_endpoint,
orb_account_id,
lens_account_id,
lens_handle,
lens_avatar_uri,
} = body;
+
+ if (
+ orb_account_id !== undefined ||
+ lens_account_id !== undefined ||
+ lens_handle !== undefined ||
+ lens_avatar_uri !== undefined
+ ) {
+ return NextResponse.json(
+ {
+ success: false,
+ error: 'Use /api/creator-profiles/link-orb to link Orb/Lens identity fields',
+ },
+ { status: 400 }
+ );
+ }
@@
- if (orb_account_id !== undefined) payload.orb_account_id = orb_account_id;
- if (lens_account_id !== undefined) payload.lens_account_id = lens_account_id;
- if (lens_handle !== undefined) payload.lens_handle = lens_handle;
- if (lens_avatar_uri !== undefined) payload.lens_avatar_uri = lens_avatar_uri;Also applies to: 84-87
Comment on lines
+98
to
+105
| for (const endpoint of candidateEndpoints) { | ||
| subgraphEndpoint = endpoint; | ||
| const endpointHeaders = endpoint.includes('goldsky.com') ? headers : { 'Content-Type': 'application/json' }; | ||
| if (!endpoint.includes('goldsky.com') && endpointHeaders['Authorization']) { | ||
| delete endpointHeaders['Authorization']; | ||
| } | ||
| response = await performFetch(endpoint, endpointHeaders); | ||
| if (response.ok) break; |
Contributor
There was a problem hiding this comment.
Harden endpoint failover and auth-header scoping in the retry loop.
A thrown fetch/network error on the first candidate currently aborts failover, and the Goldsky auth-header gate should use strict hostname matching instead of substring checks before sending secrets.
Proposed fix
- for (const endpoint of candidateEndpoints) {
- subgraphEndpoint = endpoint;
- const endpointHeaders = endpoint.includes('goldsky.com') ? headers : { 'Content-Type': 'application/json' };
- if (!endpoint.includes('goldsky.com') && endpointHeaders['Authorization']) {
- delete endpointHeaders['Authorization'];
- }
- response = await performFetch(endpoint, endpointHeaders);
- if (response.ok) break;
- }
+ for (const endpoint of candidateEndpoints) {
+ subgraphEndpoint = endpoint;
+ try {
+ const hostname = new URL(endpoint).hostname;
+ const isGoldskyHost = hostname === 'api.goldsky.com' || hostname.endsWith('.goldsky.com');
+ const endpointHeaders = isGoldskyHost ? headers : { 'Content-Type': 'application/json' };
+
+ response = await fetch(endpoint, {
+ method: 'POST',
+ headers: endpointHeaders,
+ body: JSON.stringify(body),
+ signal: AbortSignal.timeout(10_000),
+ });
+
+ if (response.ok) break;
+ } catch (endpointError) {
+ serverLogger.warn('Subgraph endpoint attempt failed', { endpoint, endpointError });
+ continue;
+ }
+ }🤖 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 `@app/api/metokens-subgraph/route.ts` around lines 98 - 105, The retry loop
currently aborts on thrown fetch/network errors and uses substring checks to
decide whether to send Goldsky secrets; fix by (1) creating a fresh headers copy
per endpoint (e.g., const endpointHeaders = { ...headers } or new object) so you
don't mutate shared headers, (2) derive the endpoint hostname strictly via new
URL(endpoint).hostname and only allow Authorization when hostname ===
'goldsky.com' (remove Authorization otherwise), and (3) wrap the
performFetch(endpoint, endpointHeaders) call in try/catch so network exceptions
are caught, set response only on success, and continue to the next candidate on
error instead of breaking the failover loop; keep using subgraphEndpoint and
response as before.
Comment on lines
+86
to
+96
| for (const endpoint of candidateEndpoints) { | ||
| subgraphEndpoint = endpoint; | ||
| const endpointHeaders = endpoint.includes('goldsky.com') ? headers : { 'Content-Type': 'application/json' }; | ||
| if (!endpoint.includes('goldsky.com') && endpointHeaders['Authorization']) { | ||
| delete endpointHeaders['Authorization']; | ||
| } | ||
| serverLogger.debug(`Forwarding Reality.eth query to endpoint: ${endpoint}`); | ||
| response = await performFetch(endpoint, endpointHeaders); | ||
| // Try every candidate until one succeeds. (Previous logic stopped on any 4xx, | ||
| // so a misconfigured Graph Studio URL blocked Goldsky fallback entirely.) | ||
| if (response.ok) break; |
Contributor
There was a problem hiding this comment.
Make fallback robust against network failures and tighten Goldsky host validation.
The current loop stops failover if fetch throws on any candidate, and Authorization routing should be based on parsed hostname (not includes) before sending API-key-bearing headers.
Proposed fix
- for (const endpoint of candidateEndpoints) {
- subgraphEndpoint = endpoint;
- const endpointHeaders = endpoint.includes('goldsky.com') ? headers : { 'Content-Type': 'application/json' };
- if (!endpoint.includes('goldsky.com') && endpointHeaders['Authorization']) {
- delete endpointHeaders['Authorization'];
- }
- serverLogger.debug(`Forwarding Reality.eth query to endpoint: ${endpoint}`);
- response = await performFetch(endpoint, endpointHeaders);
- // Try every candidate until one succeeds. (Previous logic stopped on any 4xx,
- // so a misconfigured Graph Studio URL blocked Goldsky fallback entirely.)
- if (response.ok) break;
- }
+ for (const endpoint of candidateEndpoints) {
+ subgraphEndpoint = endpoint;
+ try {
+ const hostname = new URL(endpoint).hostname;
+ const isGoldskyHost = hostname === 'api.goldsky.com' || hostname.endsWith('.goldsky.com');
+ const endpointHeaders = isGoldskyHost ? headers : { 'Content-Type': 'application/json' };
+
+ serverLogger.debug(`Forwarding Reality.eth query to endpoint: ${endpoint}`);
+ response = await fetch(endpoint, {
+ method: 'POST',
+ headers: endpointHeaders,
+ body: JSON.stringify(body),
+ signal: AbortSignal.timeout(10_000),
+ });
+ if (response.ok) break;
+ } catch (endpointError) {
+ serverLogger.warn('Reality.eth endpoint attempt failed', { endpoint, endpointError });
+ continue;
+ }
+ }🤖 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 `@app/api/reality-eth-subgraph/route.ts` around lines 86 - 96, The loop
currently stops failover when performFetch throws and uses string includes to
detect Goldsky hosts; fix by parsing each candidate endpoint's hostname with new
URL(endpoint).hostname and use a strict check like hostname === 'goldsky.com' or
hostname.endsWith('.goldsky.com') to decide whether to attach Authorization,
build a fresh endpointHeaders object per iteration (so you don't mutate shared
headers), then wrap the performFetch call in try/catch inside the for loop (log
the error with serverLogger.debug/error and continue to the next candidate on
exception) and only set subgraphEndpoint and break when response.ok; reference
candidateEndpoints, endpointHeaders, performFetch, serverLogger,
subgraphEndpoint, and response when making these changes.
Comment on lines
+35
to
+42
| export function saveStoredOrbSession(session: StoredOrbSession | null): void { | ||
| if (typeof window === 'undefined') return; | ||
| if (!session?.accessToken) { | ||
| localStorage.removeItem(ORB_SESSION_STORAGE_KEY); | ||
| return; | ||
| } | ||
| localStorage.setItem(ORB_SESSION_STORAGE_KEY, JSON.stringify(session)); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid persisting Orb tokens in localStorage.
Line 35-Line 42 stores bearer tokens (including refresh/id tokens) in localStorage, which is directly readable by injected scripts. Please move token persistence to secure, httpOnly cookies (or avoid storing refresh/id tokens client-side entirely).
🤖 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 `@lib/sdk/orb/login.ts` around lines 35 - 42, The saveStoredOrbSession function
currently persists full StoredOrbSession (including access/refresh/id tokens)
into localStorage via ORB_SESSION_STORAGE_KEY; change this so no sensitive
tokens are stored client-side—remove localStorage.setItem usage for tokens and
instead send the session (or token material) to a backend endpoint that sets
secure, httpOnly, SameSite cookies, or avoid storing refresh/id tokens on the
client entirely; update saveStoredOrbSession to only store non-sensitive UI
state locally (or clear the key) and call a new server API to set/clear httpOnly
cookies for access/refresh tokens, and remove other code paths that read/writes
tokens from localStorage.
Comment on lines
+16
to
+20
| if (trimmed.startsWith('http://') || trimmed.startsWith('https://')) { | ||
| if (/\.(m3u8|mp4|mov|webm)(\?|$)/i.test(trimmed)) return false; | ||
| if (trimmed.includes('livepeer')) return false; | ||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
HTTP(S) exception checks are currently dead logic.
Line 16-Line 20 always returns false for HTTP(S), so Line 17-Line 18 can never change behavior. If HTTP URLs with specific markers should be resolved, return that condition instead of unconditional false.
🤖 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 `@lib/sdk/orb/media.ts` around lines 16 - 20, The HTTP(S) branch always returns
false making the inner checks dead; change the branch in lib/sdk/orb/media.ts so
that instead of the unconditional "return false" it returns the result that
respects the two checks: evaluate the /\.(m3u8|mp4|mov|webm)(\?|$)/i regex
against the variable trimmed and the trimmed.includes('livepeer') test, and
return the negation/appropriate boolean so the function outcome depends on those
checks (i.e., return the condition that combines the regex match and the
includes('livepeer') result rather than always false).
Comment on lines
+106
to
+118
| if (!result.success || (!result.hash && !result.url)) { | ||
| throw new Error( | ||
| result.error ?? "Failed to upload IPA metadata to IPFS" | ||
| ); | ||
| } | ||
|
|
||
| const ipMetadataURI = `ipfs://${result.hash}`; | ||
| const ipMetadataURI = | ||
| result.url && | ||
| (result.url.startsWith("http://") || | ||
| result.url.startsWith("https://") || | ||
| result.url.startsWith("ipfs://")) | ||
| ? result.url | ||
| : `ipfs://${result.hash}`; |
Contributor
There was a problem hiding this comment.
Edge case: non-URI url with missing hash produces invalid ipMetadataURI.
If result.url is set to something like "QmHash..." (truthy but not http/https/ipfs), the check on line 106 passes, but lines 112-118 fall through to ipfs://${result.hash}. If result.hash is undefined, this yields ipfs://undefined.
Proposed fix
- if (!result.success || (!result.hash && !result.url)) {
+ const hasValidUrl =
+ result.url &&
+ (result.url.startsWith("http://") ||
+ result.url.startsWith("https://") ||
+ result.url.startsWith("ipfs://"));
+ if (!result.success || (!result.hash && !hasValidUrl)) {
throw new Error(
result.error ?? "Failed to upload IPA metadata to IPFS"
);
}
const ipMetadataURI =
- result.url &&
- (result.url.startsWith("http://") ||
- result.url.startsWith("https://") ||
- result.url.startsWith("ipfs://"))
+ hasValidUrl
? result.url
: `ipfs://${result.hash}`;🤖 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 `@lib/sdk/story/ipa-metadata.ts` around lines 106 - 118, The current
ipMetadataURI construction can yield "ipfs://undefined" when result.url is a
non-URI string (e.g., a bare IPFS cid like "Qm...") and result.hash is missing;
modify the logic that computes ipMetadataURI (the result and ipMetadataURI
variables) so that: if result.url starts with "http://", "https://", or
"ipfs://", use result.url; else if result.hash is present use `ipfs://` +
result.hash; else if result.url is present (non-URI) treat it as a raw CID and
use `ipfs://` + result.url; otherwise throw or surface the error—this ensures a
valid IPFS URI instead of "ipfs://undefined".
Comment on lines
+1
to
+17
| { | ||
| "name": "creative-platform-subgraph", | ||
| "private": true, | ||
| "version": "0.0.1", | ||
| "scripts": { | ||
| "codegen": "graph codegen", | ||
| "build": "graph build", | ||
| "create:studio": "graph create creative-platform -g https://api.studio.thegraph.com/deploy/", | ||
| "deploy:studio": "graph deploy creative-platform subgraph.yaml", | ||
| "deploy:studio:tag": "graph deploy creative-platform subgraph.yaml --version-label ${VERSION_LABEL:-v0.0.1}", | ||
| "auth:studio": "graph auth ${GRAPH_STUDIO_DEPLOY_KEY}" | ||
| }, | ||
| "devDependencies": { | ||
| "@graphprotocol/graph-cli": "^0.98.1", | ||
| "@graphprotocol/graph-ts": "^0.38.2" | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add explicit Node.js version pinning for this package.
This new package manifest should declare the project’s required Node runtime (e.g., via engines and/or volta) so Graph CLI/build behavior is reproducible across environments.
Suggested change
{
"name": "creative-platform-subgraph",
"private": true,
"version": "0.0.1",
+ "engines": {
+ "node": ">=20 <21"
+ },
"scripts": {As per coding guidelines, **/{package.json,.nvmrc,.node-version,.tool-versions}: Ensure the project specifies and uses the correct Node.js version (via package.json engines/volta, .nvmrc, .node-version, or .tool-versions).
🤖 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 `@subgraphs/creative-platform/package.json` around lines 1 - 17, Add explicit
Node runtime pinning to the creative-platform-subgraph package.json by adding an
engines.node field (e.g., "node": ">=18 <19" or a specific 18.x/20.x version
your repo uses) and/or a Volta pin (volta.node set to the exact toolchain
version) at the top-level of package.json so Graph CLI/build behavior is
reproducible; update the package.json root (the file containing the "name":
"creative-platform-subgraph" entry) to include either an "engines" object and/or
a "volta" object with the agreed project Node version.
Comment on lines
+46
to
+47
| question.history_hash = event.params.content_hash; | ||
| question.arbitrator = changetype<Bytes>(event.params.arbitrator); |
Contributor
There was a problem hiding this comment.
Fix incorrect history_hash initialization in new questions.
On Line 46, history_hash is being initialized from content_hash, which is a different field in RealityETH semantics. This pollutes question history state before any answer exists.
Suggested fix
- question.history_hash = event.params.content_hash;
+ question.history_hash = Bytes.empty();📝 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
| question.history_hash = event.params.content_hash; | |
| question.arbitrator = changetype<Bytes>(event.params.arbitrator); | |
| question.history_hash = Bytes.empty(); | |
| question.arbitrator = changetype<Bytes>(event.params.arbitrator); |
🤖 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 `@subgraphs/creative-platform/src/realityEth.ts` around lines 46 - 47, The
new-question handler is incorrectly initializing question.history_hash from
event.params.content_hash; instead initialize question.history_hash to an
empty/zero Bytes value (no history) and keep event.params.content_hash assigned
to question.content_hash; find the assignment to question.history_hash and
replace the content_hash usage with an empty Bytes (e.g., Bytes.empty() / zero
value) so history starts empty until an answer exists (leave question.arbitrator
and other assignments as-is).
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.
Summary by CodeRabbit
New Features
Improvements