feat(backend): add ttl_warning field to service list/get responses#193
feat(backend): add ttl_warning field to service list/get responses#193leocagli wants to merge 1 commit into
Conversation
…esponses Closes Stellar-Ecosystem#90 Soroban persistent entries with TTL near zero are archived and become unreachable, but list_services returned no indication of remaining TTL — providers had no way to know their listings were about to expire without monitoring the ledger directly. Changes: - stellar.js: exported getCurrentLedgerSequence() using server.getLatestLedger() - contract.js: added MAX_TTL_LEDGERS = 3_110_400 (mirrors Rust constant) and TTL_WARNING_THRESHOLD = 10% of MAX_TTL (~18 days); hasTtlWarning() helper - listServices(): fetches current ledger in parallel with the RPC simulation call (no added latency) and appends ttl_warning: bool to each ServiceEntry - getService(): same — parallel ledger fetch, ttl_warning in return object If the ledger fetch fails, falls back to 0 (no false warnings). The field is additive and backwards-compatible — clients that ignore it see no change.
📝 WalkthroughWalkthroughA new ChangesSoroban TTL expiry warning on service read APIs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/contract.js`:
- Around line 139-142: The getCurrentLedgerSequence() call wrapped with
.catch(() => 0) silently suppresses RPC failures without logging, and provides
no protection against hanging calls that never reject, allowing Promise.all to
stall indefinitely. Add logging to the catch handler to record the error when it
occurs, and wrap the getCurrentLedgerSequence() call with a timeout mechanism to
protect against non-rejecting promises that hang. Apply this fix to both
occurrences of the pattern (around line 141 and line 176) to ensure both
endpoints are protected from stalling.
🪄 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: 18b9c26e-2636-474f-853d-7d11433fba17
📒 Files selected for processing (2)
backend/src/lib/contract.jsbackend/src/lib/stellar.js
| const [retval, currentLedger] = await Promise.all([ | ||
| simulateRead(callOp), | ||
| getCurrentLedgerSequence().catch(() => 0), | ||
| ]); |
There was a problem hiding this comment.
Ledger fallback is silent and won’t trigger on hangs.
At Line 141 and Line 176, .catch(() => 0) suppresses RPC failures with no log, and it does not protect against a hanging ledger call (no rejection), so Promise.all can still stall both endpoints.
Suggested fix
const TIMEOUT = 30;
+const LEDGER_FETCH_TIMEOUT_MS = TIMEOUT * 1000;
+
+async function getCurrentLedgerSequenceSafe(context) {
+ try {
+ return await Promise.race([
+ getCurrentLedgerSequence(),
+ new Promise((_, reject) =>
+ setTimeout(() => reject(new Error('getCurrentLedgerSequence timeout')), LEDGER_FETCH_TIMEOUT_MS),
+ ),
+ ]);
+ } catch (err) {
+ logger.warn({ err, context }, 'Ledger fetch failed; ttl_warning disabled for this response');
+ return 0;
+ }
+}
...
- const [retval, currentLedger] = await Promise.all([
+ const [retval, currentLedger] = await Promise.all([
simulateRead(callOp),
- getCurrentLedgerSequence().catch(() => 0),
+ getCurrentLedgerSequenceSafe('listServices'),
]);
...
- const [retval, currentLedger] = await Promise.all([
+ const [retval, currentLedger] = await Promise.all([
simulateRead(op),
- getCurrentLedgerSequence().catch(() => 0),
+ getCurrentLedgerSequenceSafe('getService'),
]);Also applies to: 174-177
🤖 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/lib/contract.js` around lines 139 - 142, The
getCurrentLedgerSequence() call wrapped with .catch(() => 0) silently suppresses
RPC failures without logging, and provides no protection against hanging calls
that never reject, allowing Promise.all to stall indefinitely. Add logging to
the catch handler to record the error when it occurs, and wrap the
getCurrentLedgerSequence() call with a timeout mechanism to protect against
non-rejecting promises that hang. Apply this fix to both occurrences of the
pattern (around line 141 and line 176) to ensure both endpoints are protected
from stalling.
Closes #90
Summary
getCurrentLedgerSequence()tostellar.jsviaserver.getLatestLedger()MAX_TTL_LEDGERS = 3_110_400andTTL_WARNING_THRESHOLD = 10%constants tocontract.js(mirrors Rust contract constants)hasTtlWarning(registeredAt, currentLedger)helperlistServices()andgetService()now fetch the current ledger in parallel with the simulation call (zero added latency), then appendttl_warning: booleanto everyServiceEntryTest plan
GET /api/services— each entry includesttl_warning: falsefor freshly-registered servicesGET /api/services/:id— same field presentregistered_at = current_ledger - MAX_TTL + 100, confirmttl_warning: truegetLatestLedger()to throw; confirm endpoint still returns 200 withttl_warning: falseSummary by CodeRabbit