fix(lifecycle-service): invalidate cached account reads after merge - #384
Merged
davedumto merged 2 commits intoAug 30, 2026
Conversation
|
@onuibeblessing2019-hash 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! 🚀 |
|
@onuibeblessing2019-hash is attempting to deploy a commit to the david's projects Team on Vercel. A member of the Team first needs to authorize it. |
2 tasks
Author
|
closes #287 |
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
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.
Closes #287
Investigation
lifecycle-service had zero caching of any kind — every
getAccountcall hit Horizon live. The only cache anywhere in this repo (wallet-service/src/cache.ts) is real, well-built infrastructure, but is wired intodepsand never actually called from any route — unused scaffolding, and unrelated to account merges (wallet-servicehas no merge concept at all; merges are purely a lifecycle-service/classic-account thing). So #287's premise ("cached data is not invalidated") first needed a cache to exist before there was anything to invalidate.Also found, while investigating:
src/server.test.tshas literal syntax errors (TS1128, a stray uncloseddescribeblock with a function illegally nested inside it) that prevent the whole file — and therefore the whole package's typecheck — from compiling, andsrc/account-merge.integration.test.tsreferences aAccountRecordtype that doesn't exist anywhere and fails to even load (unrelatedsac-sdktransitive-import error on top of that). Both are pre-existing and out of scope for this PR — left untouched and disclosed here rather than attempted, per the actual scope of #287.What's here
src/account-cache.ts(new) —createCachedAccountReader, a short-TTL (default 30s) decorator around anyAccountReader. Caches both successful lookups and not-found (undefined) results, so a merged-away account correctly reads as gone once invalidated. Exposesinvalidate(accountId)andinvalidateAll().src/server.ts—POST /lifecycle/mergenow invalidates both the source (merged away) and destination (balance changed) account's cache entries once the merge step is built and audit-logged, via a newisCachedAccountReadertype guard (a plain, uncachedAccountReader— what every existing test in this package uses — gets no invalidation call at all, since there's nothing to invalidate). Logs a newlifecycle.merge.cache_invalidatedevent.README.md.src/index.ts— production composition now wraps the real Horizon reader with the cache (previously just used the raw, uncached reader).README.md— new "Account read caching and invalidation on merge" section per the issue's "document the invalidation step" requirement, including the optimistic-invalidation caveat.workerProcessingLagSeconds is not defined" crash inpackages/service-kit/src/metrics.tsalready fixed independently in #381 (#326/#327) — needed again here since this branch is based directly onupstream/dev, which doesn't have that fix yet; without it, importing@vellar/service-kit(and therefore this service's ownserver.ts`) throws at module load time.Test plan
account-cache.test.ts(9 new tests) — cache hit/miss, TTL expiry, not-found caching, per-account-id isolation, and the exact end-to-end scenario [medium] Fix stale cache after account merge in lifecycle-service #287 describes: a stale cache entry showing the pre-merge source-exists/dest-old-balance state, confirmed still stale without invalidation, then confirmed correct (source gone, dest's new balance) afterinvalidate.merge-cache-invalidation.test.ts(3 new tests) — exercises the realPOST /lifecycle/mergeroute (not just the cache module in isolation): confirms both accounts are evicted after a successful merge, confirms a plain uncached reader is unaffected (no invalidation attempted, no throw), confirms a rejected merge (blockers remaining) does NOT invalidate anything.lifecycle-servicesuite (excluding the two pre-existing broken files above, andworker/loop.test.ts's 4 pre-existing unrelated concurrency failures, confirmed identical with my changes stashed out): 58/58 passing.packages/service-kit: 85/85 passing.npx tsc --noEmitinlifecycle-service: clean except the pre-existingserver.test.tssyntax errors (identical before/after this PR).Closes #287