feat(news): Stellar rich embeds in articles (#27)#34
Open
Kingvic300 wants to merge 1 commit into
Open
Conversation
Detect Stellar entities in article content and render rich, inline, theme-aware embed cards. Entities are resolved server-side and cached. - Parser + StrKey checksum validation (pure, client-safe) that detects transaction hashes, contract ids, account addresses and CODE:ISSUER assets, skipping code blocks, anchors, URLs and tag interiors; also understands explicit [[stellar:...]] reference tags. - Server-side resolution (Horizon REST + Soroban RPC) with a stellar_embeds_cache table (migration 0003), type-aware TTLs and stale-while-revalidate. Resolution never throws or blocks rendering; failures fall back to an explorer link, not-found shows a subtle hint. - Embed components (transaction/contract/account/asset) with copy chips, responsive layout and dark/light support; ArticleContent replaces the raw content render in NewsDetail. - Admin editor: "Insert Stellar Reference" button + modal with live preview via an admin-gated /api/stellar/resolve route. - Vitest unit tests for the parser and StrKey validation. Env: NEXT_PUBLIC_STELLAR_NETWORK selects testnet (default) or mainnet.
|
@Kingvic300 is attempting to deploy a commit to the ACTA Team on Vercel. A member of the Team first needs to authorize it. |
|
@Kingvic300 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! 🚀 |
📝 WalkthroughWalkthroughThis PR introduces end-to-end Stellar entity rich embeds for news articles. Admins can insert ChangesStellar Rich Embeds & Admin Workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/@types/stellar.ts (1)
102-109: ⚡ Quick winMake
ResolvedStellarEntitydiscriminated sostatus: 'ok'always includes payload.Right now the type allows
status: 'ok'withoutresolved, which weakens the contract and can leak undefined into embed render paths.Suggested type tightening
-export interface ResolvedStellarEntity { - ref: StellarEntityRef; - status: 'ok' | 'not_found' | 'error'; - resolved?: StellarResolvedData; - resolvedAt: string; - /** True when served from cache past its TTL while a refresh is scheduled. */ - stale?: boolean; -} +export type ResolvedStellarEntity = + | { + ref: StellarEntityRef; + status: 'ok'; + resolved: StellarResolvedData; + resolvedAt: string; + /** True when served from cache past its TTL while a refresh is scheduled. */ + stale?: boolean; + } + | { + ref: StellarEntityRef; + status: 'not_found' | 'error'; + resolvedAt: string; + /** True when served from cache past its TTL while a refresh is scheduled. */ + stale?: boolean; + resolved?: never; + };🤖 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 `@src/`@types/stellar.ts around lines 102 - 109, The ResolvedStellarEntity should be a discriminated union so that status === 'ok' always includes a resolved payload: replace the single interface with a union such as (1) { ref: StellarEntityRef; status: 'ok'; resolved: StellarResolvedData; resolvedAt: string; stale?: boolean } and (2) { ref: StellarEntityRef; status: 'not_found' | 'error'; resolved?: never; resolvedAt: string; stale?: boolean }, ensuring functions expecting ResolvedStellarEntity can safely access resolved when status is 'ok' (refer to the ResolvedStellarEntity, status and resolved symbols).
🤖 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 `@README.md`:
- Line 65: Add an entry for the test:watch npm script to the README table:
document that `test:watch` runs the Vitest test runner in watch mode (re-running
tests on file changes) so it mirrors the existing `test` row; update the table
row near the `npm run test` entry to include a new line like "`npm run
test:watch` | Run unit tests in watch mode (Vitest)`" so readers can see both
`test` and `test:watch` behaviors (refer to the `test:watch` script in
package.json).
In `@src/components/modules/admin/ui/StellarReferenceModal.tsx`:
- Line 3: Stale async preview responses from the preview request (in the
StellarReferenceModal component) can overwrite modal state after it’s closed;
fix by invalidating in-flight requests when the modal is closed or a new request
starts — add a mutable ref (e.g., previewRequestIdRef) or use an
AbortController, increment/abort it on modal close/reset (the block that resets
state around lines 42-53) and capture the current id/signal when launching the
async preview (the function around lines 64-91); before calling state setters
like setPreview (and any setIsLoading/setPreviewError), verify the request id
matches the current ref (or that the AbortSignal is not aborted) so late
responses are ignored.
In `@src/components/modules/news/ui/embeds/ArticleContent.tsx`:
- Around line 32-33: ArticleContent currently injects raw HTML into the DOM via
dangerouslySetInnerHTML in both the normal render (dangerouslySetInnerHTML={{
__html: content }}) and the catch/fallback path (lines ~42-44), risking XSS if
upstream sanitization regresses; fix by importing/using a robust sanitizer
(e.g., DOMPurify or a project sanitizeHtml utility) and pass the output of
sanitizeHtml(content) and sanitizeHtml(fallbackContent) into
dangerouslySetInnerHTML inside the ArticleContent component so both code paths
always render sanitized HTML.
In `@src/components/modules/news/ui/embeds/StellarEmbed.tsx`:
- Around line 24-33: The switch on resolved.type in the StellarEmbed component
lacks a default branch so unknown or malformed types silently render nothing;
update the switch in StellarEmbed (the block that returns TransactionEmbed,
AccountEmbed, ContractEmbed, AssetEmbed) to include a default case that
defensively handles unexpected values by rendering a clear fallback UI (e.g., an
UnknownEmbed/fallback component or a minimal placeholder) and log or warn about
the unexpected resolved.type so the issue can be traced.
In `@src/lib/stellar/cache.ts`:
- Around line 47-56: readCache currently only queries using refs[0].network,
missing other networks in the refs array; instead compute unique networks and
query the cache for all networks and ids. Replace the single network usage with
something like networks = [...new Set(refs.map(r => r.network))] and change the
Supabase query on 'stellar_embeds_cache' to include .in('network', networks) (or
.in('network', networks).in('entity_id', ids)) so results for all provided
StellarEntityRef.network values are returned; update any references to the
removed single 'network' variable accordingly in readCache and keep using
createPublicClient and the same table/columns.
In `@src/lib/stellar/horizon.ts`:
- Around line 86-92: The asset field for non-native operations currently uses
only op.asset_code which is ambiguous; update the operation assignment (the
operation object built from op in horizon.ts) so that when op.asset_type !==
'native' you include the issuer as well (e.g., combine op.asset_code and
op.asset_issuer into a single identity like "CODE:ISSUER" or a structured object
{code, issuer}) instead of relying solely on asset_code, leaving native assets
as 'XLM'.
In `@src/lib/stellar/soroban.ts`:
- Around line 23-27: The current withTimeout<T> wrapper only rejects the caller
but doesn't cancel the underlying RPC (e.g., server.getContractData), so
timed-out requests keep running; update the Soroban client call site to use an
abortable fetch transport (AbortController) or the SDK's cancellation mechanism:
create an AbortController, pass its signal into a custom fetch/transport used by
the Soroban client or SDK call (instead of plain server.getContractData), wire
the timeout to call controller.abort() when elapsed, and ensure withTimeout
either invokes that abort or is replaced by a small helper that starts the
controller and rejects after ms while aborting the in-flight request; reference
withTimeout, AbortController, and server.getContractData in the change.
---
Nitpick comments:
In `@src/`@types/stellar.ts:
- Around line 102-109: The ResolvedStellarEntity should be a discriminated union
so that status === 'ok' always includes a resolved payload: replace the single
interface with a union such as (1) { ref: StellarEntityRef; status: 'ok';
resolved: StellarResolvedData; resolvedAt: string; stale?: boolean } and (2) {
ref: StellarEntityRef; status: 'not_found' | 'error'; resolved?: never;
resolvedAt: string; stale?: boolean }, ensuring functions expecting
ResolvedStellarEntity can safely access resolved when status is 'ok' (refer to
the ResolvedStellarEntity, status and resolved symbols).
🪄 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: e0f3754c-2813-4680-af55-f93cd2feb8c6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (36)
.env.exampleREADME.mdpackage.jsonsrc/@types/stellar.tssrc/app/api/stellar/resolve/route.tssrc/components/modules/admin/pages/AdminNewsEditorPage.tsxsrc/components/modules/admin/ui/StellarReferenceField.tsxsrc/components/modules/admin/ui/StellarReferenceModal.tsxsrc/components/modules/news/index.tssrc/components/modules/news/ui/NewsDetail.tsxsrc/components/modules/news/ui/embeds/AccountEmbed.tsxsrc/components/modules/news/ui/embeds/ArticleContent.tsxsrc/components/modules/news/ui/embeds/AssetEmbed.tsxsrc/components/modules/news/ui/embeds/ContractEmbed.tsxsrc/components/modules/news/ui/embeds/CopyChip.tsxsrc/components/modules/news/ui/embeds/EmbedFallback.tsxsrc/components/modules/news/ui/embeds/EmbedShell.tsxsrc/components/modules/news/ui/embeds/EmbedSkeleton.tsxsrc/components/modules/news/ui/embeds/StellarEmbed.tsxsrc/components/modules/news/ui/embeds/TransactionEmbed.tsxsrc/components/modules/news/ui/embeds/index.tssrc/lib/stellar/__tests__/parser.test.tssrc/lib/stellar/__tests__/strkey.test.tssrc/lib/stellar/cache.tssrc/lib/stellar/config.tssrc/lib/stellar/explorer.tssrc/lib/stellar/format.tssrc/lib/stellar/horizon.tssrc/lib/stellar/index.tssrc/lib/stellar/parser.tssrc/lib/stellar/resolver.tssrc/lib/stellar/soroban.tssrc/lib/stellar/strkey.tssrc/lib/supabase/database.types.tssupabase/migrations/0003_stellar_embeds_cache.sqlvitest.config.ts
| | `npm run build` | Production build | | ||
| | `npm run start` | Run the production build | | ||
| | `npm run lint` | Lint the codebase | | ||
| | `npm run test` | Run unit tests (Vitest) | |
There was a problem hiding this comment.
Document the test:watch script.
The package.json defines both test and test:watch scripts, but only test is documented here. Consider adding test:watch to the table for completeness.
📝 Suggested addition
| `npm run test` | Run unit tests (Vitest) |
+| `npm run test:watch`| Run unit tests in watch mode |
| `npm run format` | Format with Prettier |🤖 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 `@README.md` at line 65, Add an entry for the test:watch npm script to the
README table: document that `test:watch` runs the Vitest test runner in watch
mode (re-running tests on file changes) so it mirrors the existing `test` row;
update the table row near the `npm run test` entry to include a new line like
"`npm run test:watch` | Run unit tests in watch mode (Vitest)`" so readers can
see both `test` and `test:watch` behaviors (refer to the `test:watch` script in
package.json).
| @@ -0,0 +1,165 @@ | |||
| 'use client'; | |||
|
|
|||
| import { useCallback, useEffect, useState } from 'react'; | |||
There was a problem hiding this comment.
Prevent stale preview responses from mutating modal state after close/reopen.
Line 42 resets state, but the async path from Line 64 can still commit late results. If the modal is closed mid-request, stale data can reappear on reopen and enable inserting the previous reference.
Suggested fix (invalidate in-flight preview responses)
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback, useEffect, useRef, useState } from 'react';
@@
const [entity, setEntity] = useState<ResolvedStellarEntity | null>(null);
const [loading, setLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
+ const previewSeqRef = useRef(0);
@@
const reset = useCallback(() => {
+ previewSeqRef.current += 1; // invalidate in-flight preview responses
setValue('');
setRef(null);
setEntity(null);
setLoading(false);
setError(null);
}, []);
@@
const handlePreview = useCallback(async () => {
+ const seq = ++previewSeqRef.current;
setError(null);
setEntity(null);
const detected = detectEntity(value.trim(), network);
@@
setRef(detected);
setLoading(true);
try {
const res = await fetch(
`/api/stellar/resolve?entity=${encodeURIComponent(detected.id)}&network=${network}`
);
const json = await res.json();
+ if (previewSeqRef.current !== seq) return;
if (json.ok) {
setEntity(json.entity as ResolvedStellarEntity);
} else {
// The ref is valid; resolution failed — still allow insert with a fallback.
setEntity({ ref: detected, status: 'error', resolvedAt: new Date().toISOString() });
}
} catch {
+ if (previewSeqRef.current !== seq) return;
setEntity({ ref: detected, status: 'error', resolvedAt: new Date().toISOString() });
} finally {
- setLoading(false);
+ if (previewSeqRef.current === seq) setLoading(false);
}
}, [value, network]);Also applies to: 42-53, 64-91
🤖 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 `@src/components/modules/admin/ui/StellarReferenceModal.tsx` at line 3, Stale
async preview responses from the preview request (in the StellarReferenceModal
component) can overwrite modal state after it’s closed; fix by invalidating
in-flight requests when the modal is closed or a new request starts — add a
mutable ref (e.g., previewRequestIdRef) or use an AbortController,
increment/abort it on modal close/reset (the block that resets state around
lines 42-53) and capture the current id/signal when launching the async preview
(the function around lines 64-91); before calling state setters like setPreview
(and any setIsLoading/setPreviewError), verify the request id matches the
current ref (or that the AbortSignal is not aborted) so late responses are
ignored.
Comment on lines
+32
to
+33
| return <div className={proseClass} dangerouslySetInnerHTML={{ __html: content }} />; | ||
| } |
There was a problem hiding this comment.
Sanitize HTML on both dangerouslySetInnerHTML paths.
Both the catch fallback and normal segment rendering inject raw HTML without an in-component sanitizer. If upstream sanitization regresses, this becomes an immediate XSS sink.
Suggested hardening pattern
+import { sanitizeHtml } from '`@/lib/security/sanitizeHtml`';
...
- return <div className={proseClass} dangerouslySetInnerHTML={{ __html: content }} />;
+ return <div className={proseClass} dangerouslySetInnerHTML={{ __html: sanitizeHtml(content) }} />;
...
- dangerouslySetInnerHTML={{ __html: segment.html }}
+ dangerouslySetInnerHTML={{ __html: sanitizeHtml(segment.html) }}Also applies to: 42-44
🤖 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 `@src/components/modules/news/ui/embeds/ArticleContent.tsx` around lines 32 -
33, ArticleContent currently injects raw HTML into the DOM via
dangerouslySetInnerHTML in both the normal render (dangerouslySetInnerHTML={{
__html: content }}) and the catch/fallback path (lines ~42-44), risking XSS if
upstream sanitization regresses; fix by importing/using a robust sanitizer
(e.g., DOMPurify or a project sanitizeHtml utility) and pass the output of
sanitizeHtml(content) and sanitizeHtml(fallbackContent) into
dangerouslySetInnerHTML inside the ArticleContent component so both code paths
always render sanitized HTML.
Comment on lines
+24
to
+33
| switch (resolved.type) { | ||
| case 'transaction': | ||
| return <TransactionEmbed data={resolved.data} network={ref.network} />; | ||
| case 'account': | ||
| return <AccountEmbed data={resolved.data} network={ref.network} />; | ||
| case 'contract': | ||
| return <ContractEmbed data={resolved.data} network={ref.network} />; | ||
| case 'asset': | ||
| return <AssetEmbed data={resolved.data} network={ref.network} />; | ||
| } |
There was a problem hiding this comment.
Handle unknown resolved.type defensively.
The switch has no fallback. If cached or malformed data produces an unexpected type, this function returns nothing and silently drops the embed instead of rendering a fallback.
Suggested patch
switch (resolved.type) {
case 'transaction':
return <TransactionEmbed data={resolved.data} network={ref.network} />;
case 'account':
return <AccountEmbed data={resolved.data} network={ref.network} />;
case 'contract':
return <ContractEmbed data={resolved.data} network={ref.network} />;
case 'asset':
return <AssetEmbed data={resolved.data} network={ref.network} />;
+ default:
+ return <EmbedFallback entityRef={ref} notFound={false} />;
}📝 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
| switch (resolved.type) { | |
| case 'transaction': | |
| return <TransactionEmbed data={resolved.data} network={ref.network} />; | |
| case 'account': | |
| return <AccountEmbed data={resolved.data} network={ref.network} />; | |
| case 'contract': | |
| return <ContractEmbed data={resolved.data} network={ref.network} />; | |
| case 'asset': | |
| return <AssetEmbed data={resolved.data} network={ref.network} />; | |
| } | |
| switch (resolved.type) { | |
| case 'transaction': | |
| return <TransactionEmbed data={resolved.data} network={ref.network} />; | |
| case 'account': | |
| return <AccountEmbed data={resolved.data} network={ref.network} />; | |
| case 'contract': | |
| return <ContractEmbed data={resolved.data} network={ref.network} />; | |
| case 'asset': | |
| return <AssetEmbed data={resolved.data} network={ref.network} />; | |
| default: | |
| return <EmbedFallback entityRef={ref} notFound={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 `@src/components/modules/news/ui/embeds/StellarEmbed.tsx` around lines 24 - 33,
The switch on resolved.type in the StellarEmbed component lacks a default branch
so unknown or malformed types silently render nothing; update the switch in
StellarEmbed (the block that returns TransactionEmbed, AccountEmbed,
ContractEmbed, AssetEmbed) to include a default case that defensively handles
unexpected values by rendering a clear fallback UI (e.g., an
UnknownEmbed/fallback component or a minimal placeholder) and log or warn about
the unexpected resolved.type so the issue can be traced.
Comment on lines
+47
to
+56
| const network = refs[0].network; | ||
| const ids = [...new Set(refs.map((r) => r.id))]; | ||
|
|
||
| try { | ||
| const supabase = createPublicClient(); | ||
| const { data, error } = await supabase | ||
| .from('stellar_embeds_cache') | ||
| .select('*') | ||
| .eq('network', network) | ||
| .in('entity_id', ids); |
There was a problem hiding this comment.
Query cache rows per network, not just refs[0].network.
readCache() accepts arbitrary StellarEntityRef[], but this query only looks at the first ref's network. A mixed testnet/mainnet batch will miss cache for every later network and re-hit Horizon/Soroban unnecessarily.
💡 Suggested fix
export async function readCache(
refs: StellarEntityRef[]
): Promise<Map<string, StellarEmbedCacheRow>> {
const map = new Map<string, StellarEmbedCacheRow>();
if (refs.length === 0) return map;
-
- const network = refs[0].network;
- const ids = [...new Set(refs.map((r) => r.id))];
try {
const supabase = createPublicClient();
- const { data, error } = await supabase
- .from('stellar_embeds_cache')
- .select('*')
- .eq('network', network)
- .in('entity_id', ids);
-
- if (error || !data) return map;
-
- for (const row of data as unknown as StellarEmbedCacheRow[]) {
- map.set(cacheKey(row.network, row.entity_id), row);
+ const idsByNetwork = new Map<StellarNetwork, string[]>();
+ for (const ref of refs) {
+ const ids = idsByNetwork.get(ref.network) ?? [];
+ ids.push(ref.id);
+ idsByNetwork.set(ref.network, ids);
+ }
+
+ for (const [network, rawIds] of idsByNetwork) {
+ const ids = [...new Set(rawIds)];
+ const { data, error } = await supabase
+ .from('stellar_embeds_cache')
+ .select('*')
+ .eq('network', network)
+ .in('entity_id', ids);
+
+ if (error || !data) continue;
+
+ for (const row of data as unknown as StellarEmbedCacheRow[]) {
+ map.set(cacheKey(row.network, row.entity_id), row);
+ }
}
} catch {
// Treat any cache read failure as a cache miss.
}
return map;🤖 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 `@src/lib/stellar/cache.ts` around lines 47 - 56, readCache currently only
queries using refs[0].network, missing other networks in the refs array; instead
compute unique networks and query the cache for all networks and ids. Replace
the single network usage with something like networks = [...new Set(refs.map(r
=> r.network))] and change the Supabase query on 'stellar_embeds_cache' to
include .in('network', networks) (or .in('network', networks).in('entity_id',
ids)) so results for all provided StellarEntityRef.network values are returned;
update any references to the removed single 'network' variable accordingly in
readCache and keep using createPublicClient and the same table/columns.
Comment on lines
+86
to
+92
| operation = { | ||
| type: op.type, | ||
| from: op.from, | ||
| to: op.to, | ||
| amount: op.amount, | ||
| asset: op.asset_type === 'native' ? 'XLM' : op.asset_code ? `${op.asset_code}` : undefined, | ||
| }; |
There was a problem hiding this comment.
Include the issuer in non-native operation assets.
Using only asset_code makes transaction summaries ambiguous because Stellar asset codes are not unique across issuers. This can display the wrong asset identity in the embed.
💡 Suggested fix
operation = {
type: op.type,
from: op.from,
to: op.to,
amount: op.amount,
- asset: op.asset_type === 'native' ? 'XLM' : op.asset_code ? `${op.asset_code}` : undefined,
+ asset:
+ op.asset_type === 'native'
+ ? 'XLM'
+ : op.asset_code && op.asset_issuer
+ ? `${op.asset_code}:${op.asset_issuer}`
+ : op.asset_code,
};📝 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
| operation = { | |
| type: op.type, | |
| from: op.from, | |
| to: op.to, | |
| amount: op.amount, | |
| asset: op.asset_type === 'native' ? 'XLM' : op.asset_code ? `${op.asset_code}` : undefined, | |
| }; | |
| operation = { | |
| type: op.type, | |
| from: op.from, | |
| to: op.to, | |
| amount: op.amount, | |
| asset: | |
| op.asset_type === 'native' | |
| ? 'XLM' | |
| : op.asset_code && op.asset_issuer | |
| ? `${op.asset_code}:${op.asset_issuer}` | |
| : op.asset_code, | |
| }; |
🤖 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 `@src/lib/stellar/horizon.ts` around lines 86 - 92, The asset field for
non-native operations currently uses only op.asset_code which is ambiguous;
update the operation assignment (the operation object built from op in
horizon.ts) so that when op.asset_type !== 'native' you include the issuer as
well (e.g., combine op.asset_code and op.asset_issuer into a single identity
like "CODE:ISSUER" or a structured object {code, issuer}) instead of relying
solely on asset_code, leaving native assets as 'XLM'.
Comment on lines
+23
to
+27
| function withTimeout<T>(promise: Promise<T>, ms: number): Promise<T> { | ||
| return Promise.race([ | ||
| promise, | ||
| new Promise<T>((_, reject) => setTimeout(() => reject(new Error('Soroban RPC timeout')), ms)), | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does @stellar/stellar-sdk's rpc.Server/getContractData support AbortSignal, custom fetch injection, or a built-in request timeout that cancels the underlying HTTP request?
💡 Result:
@stellar/stellar-sdk rpc.Server/getContractData does not expose an AbortSignal parameter, does not document custom fetch injection, and (in the public API for getContractData itself) does not offer a per-request timeout that would cancel the underlying HTTP request. What is supported instead: 1) Built-in request timeout exists at a higher level (CLI/generation tooling), not in getContractData’s signature. - The package’s CLI options include --timeout “RPC request timeout in milliseconds” [1]. This indicates there is a timeout concept in some usage contexts, but it is not stated as part of Server/getContractData’s API. 2) Custom headers are supported via Server/client options. - The contract client creates rpc.Server(rpcUrl, { allowHttp, headers }) [2], and the Server documentation (for the Horizon/RPC Server class) documents custom header-related options through its options object (e.g., allowHttp/app headers in the generated docs) [3]. - However, this is not the same as injecting a custom fetch function or providing AbortSignal cancellation. Direct evidence from getContractData docs/signature: - The generated docs for rpc.Server#getContractData list parameters: contract, key, and durability (with default Durability.Persistent) [4]. There is no AbortSignal, no fetch override, and no timeout parameter shown for this method. - The server implementation for getContractData similarly takes only contract/key/durability in the exported method signature [5]. Conclusion for your specific question: - AbortSignal: Not supported by getContractData (no parameter documented/implemented) [4][5]. - Custom fetch injection: Not supported by getContractData (no documented option to pass a fetch implementation to this method, and only headers/allowHttp are shown in related options) [2][4]. - Built-in request timeout that cancels the underlying HTTP request: Not provided via getContractData’s API (no timeout parameter in signature/docs) [4][5]. A timeout exists elsewhere (CLI option --timeout), but there’s no evidence that it cancels the underlying HTTP request for getContractData [1].
Citations:
- 1: https://www.npmjs.com/package/@stellar/stellar-sdk
- 2: https://github.com/stellar/js-stellar-sdk/blob/2f52d0e7/src/contract/client.ts
- 3: https://stellar.github.io/js-stellar-sdk/libdocs_server.js.html
- 4: https://stellar.github.io/js-stellar-sdk/module-rpc.Server.html
- 5: https://github.com/stellar/js-stellar-sdk/blob/2f52d0e7/src/rpc/server.ts
Make the Soroban timeout actually cancel the RPC call (current Promise.race won’t).
In src/lib/stellar/soroban.ts:23-27, Promise.race only rejects the caller; it does not stop the underlying server.getContractData() request. The SDK’s getContractData API does not expose AbortSignal, custom fetch injection, or a per-request timeout that would cancel the in-flight HTTP request, so timed-out calls can keep running in the background during outages. Switch to an abortable transport/custom fetch (via AbortController) or use an SDK mechanism that supports real cancellation.
🤖 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 `@src/lib/stellar/soroban.ts` around lines 23 - 27, The current withTimeout<T>
wrapper only rejects the caller but doesn't cancel the underlying RPC (e.g.,
server.getContractData), so timed-out requests keep running; update the Soroban
client call site to use an abortable fetch transport (AbortController) or the
SDK's cancellation mechanism: create an AbortController, pass its signal into a
custom fetch/transport used by the Soroban client or SDK call (instead of plain
server.getContractData), wire the timeout to call controller.abort() when
elapsed, and ensure withTimeout either invokes that abort or is replaced by a
small helper that starts the controller and rejects after ms while aborting the
in-flight request; reference withTimeout, AbortController, and
server.getContractData in the change.
Author
|
@JosueBrenes kindly review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Detect Stellar entities in article content and render rich, inline, theme-aware embed cards. Entities are resolved server-side and cached.
Env: NEXT_PUBLIC_STELLAR_NETWORK selects testnet (default) or mainnet.
🚀 ACTA Pull Request
Mark with an
xall the checkboxes that apply (like[x])📌 Type of Change
📝 Changes description
📸 Evidence (A Loom/Cap video is required as evidence, we WON'T merge if there's no proof)
⏰ Time spent breakdown
🌌 Comments
Thank you for contributing to ACTA! We hope you can continue contributing to this project.
Closes : #27
Summary by CodeRabbit
Release Notes
New Features
Configuration
Tests