Skip to content

fix(app): ledger-time expiry, sequential epoch reads, strict network config - #429

Open
luchobonatti wants to merge 5 commits into
feedbackfrom
fix/416-418-app-findings
Open

luchobonatti wants to merge 5 commits into
feedbackfrom
fix/416-418-app-findings

Conversation

@luchobonatti

Copy link
Copy Markdown
Member

Summary

Closes #416
Closes #417
Closes #418

Three small app fixes: an attestation expiry that depended on the local clock, epoch reads that burst every RPC call at once, and a build that silently pointed testnet or mainnet at localhost.

Changes

  • expires_at comes from ledger time: the cached cycle read, aged by the time since it was read, or one getHealth when nothing is cached. Wall clock only if both fail.
  • Epoch and epoch-request reads send one getLedgerEntries chunk at a time. Every epoch is still read.
  • A non-local build or dev server with missing or invalid PUBLIC_STELLAR_* variables fails and names them. Local, or no network set, still falls back to localhost.

Acceptance criteria

  • useAttest derives expires_at from ledger time plus the freshness duration
  • With the wall clock behind ledger time, expires_at is still in the future
  • readEpochs and readEpochRequests send one chunk at a time
  • With several chunks, at most one read is in flight
  • A non-local build with invalid variables throws an error that names them
  • A local build without the variables still falls back to localhost

Test plan

Automated tests

  1. npx vitest run: 2,280 passed.
  2. npm run lint, npm run format:check, npm run typecheck -w app: clean.

Manual verification

  1. PUBLIC_STELLAR_NETWORK=TESTNET npm run build -w app: fails with Invalid Stellar network configuration: PUBLIC_STELLAR_NETWORK_PASSPHRASE, PUBLIC_STELLAR_RPC_URL, PUBLIC_STELLAR_HORIZON_URL.
  2. npm run build -w app with no variables: passes.
  3. A testnet build with all four variables set: passes.

Breaking changes

A non-local build with an incomplete config now fails instead of running against localhost. .env.example's mainnet sample now says PUBLIC, the accepted value.

Checklist

  • Self-reviewed my own diff
  • Tests added or updated
  • Docs updated (if applicable)
  • No unrelated changes bundled in

Screenshots

None.

Copilot AI balanced review requested due to automatic review settings November 2, 2026 20:48
@luchobonatti luchobonatti self-assigned this Nov 2, 2026
@vercel

vercel Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
strata-vault-kit-app Ready Ready Preview Nov 3, 2026 12:42pm UTC

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.

🟡 Changes recommended

Changes are needed because review findings identify a critical runtime schema-validation bug in networkEnv.ts (incorrect z.enum(Networks) usage) and a time-aging edge case in useAttest.ts where negative elapsed time can corrupt the cached ledger expiry calculation.

2 open issues
What changed in this PR

This pull request fixes three frontend/runtime reliability issues: attestation expiry now derives from ledger time (instead of local wall clock), epoch/epoch-request reads are serialized to avoid RPC bursting, and non-local builds/dev runs fail fast when PUBLIC_STELLAR_* network configuration is missing or invalid.

Changes:

  • Compute expires_at from cached/queried ledger time (wall clock only as a last resort).
  • Fetch epoch and epoch-request getLedgerEntries chunks sequentially (max one in flight).
  • Centralize and enforce strict non-local PUBLIC_STELLAR_* validation (while preserving localhost fallback for local/unset network) and update .env.example.
File Summary
app/​vite.config.ts Validates PUBLIC_ network env during Vite config evaluation to fail fast on bad non-local config.
app/​src/​hooks/​useAttest.ts Derives attestation expiry from ledger time (cached + aged, or ledger-time read; wall-clock fallback). Unresolved: cached aging can go negative if system clock moves backward; clamp elapsed ≥ 0 (moderate).
app/​src/​hooks/​useAttest.test.ts Adds tests for cached-ledger-time expiry, aging behavior, and fallback ledger-time read.
app/​.env.example Updates mainnet sample network name to PUBLIC.
app-lib/​readContract.ts Changes epoch and epoch-request reads to process ledger-entry chunks sequentially instead of in parallel.
app-lib/​readContract.test.ts Adds tests asserting chunk serialization (max 1 in-flight) while still returning all epochs.
app-lib/​networkEnv.ts Adds shared env parsing/validation for Stellar network configuration with local fallback. Unresolved: z.enum(Networks) is invalid for @stellar/stellar-sdk Networks object; should use z.nativeEnum(Networks) or explicit Object.values(Networks) (critical).
app-lib/​networkEnv.test.ts Tests local fallback and non-local failure that names invalid/missing variables.
app-lib/​env.ts Switches env handling to the shared parseNetworkEnv implementation.

This pull request requires changes.

Review effort: Balanced

Comment thread app-lib/networkEnv.ts
"LOCAL",
"STANDALONE", // deprecated in favor of LOCAL
] as const),
PUBLIC_STELLAR_NETWORK_PASSPHRASE: z.enum(Networks),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zod 4 accepts enum-like objects in z.enum; z.nativeEnum is deprecated there. Same line as env.ts on feedback.

Comment on lines +46 to +50
const ledgerTime =
cached && cachedLedgerTime != null
? cachedLedgerTime +
BigInt(Math.floor((Date.now() - cached.dataUpdatedAt) / 1000))
: await readLedgerTime()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clamped to 0 in eea0a0b, with a test.

This branch was successfully deployed

1 active deployment
Preview — eea0a0b5 Deployed Nov 3, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants