Skip to content

Feat/token metadata caching#46

Open
K1NGD4VID wants to merge 3 commits intoMiracle656:mainfrom
K1NGD4VID:feat/token-metadata-caching
Open

Feat/token metadata caching#46
K1NGD4VID wants to merge 3 commits intoMiracle656:mainfrom
K1NGD4VID:feat/token-metadata-caching

Conversation

@K1NGD4VID
Copy link
Copy Markdown
Contributor

Summary

Related issue

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Docs
  • Tests
  • CI / tooling

Checklist

  • I have read CONTRIBUTING.md
  • npx tsc --noEmit passes
  • npm run build passes
  • I added / updated tests where relevant
  • I updated docs where relevant

@drips-wave
Copy link
Copy Markdown

drips-wave Bot commented Apr 25, 2026

@K1NGD4VID 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! 🚀

Learn more about application limits

Copy link
Copy Markdown
Owner

@Miracle656 Miracle656 left a comment

Choose a reason for hiding this comment

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

Token metadata caching is the right approach for #37. Three things to address:

1. Remove scratch/generate_fixtures.js
Same as #45 — this debug script shouldn't be committed. git rm scratch/generate_fixtures.js.

2. Stacked on #45
Your diff includes all of #45's changes (decoder rename, decoder tests, fixtures). If #45 merges first your diff will shrink automatically to just the caching additions. Please either: (a) wait for #45 to merge and rebase, or (b) confirm you want this PR to supersede #45 and we'll close that one.

3. prisma/schema.prisma changes
You're adding to the Prisma schema here and also in #48. Make sure only one PR owns schema changes to avoid conflicts.

@Miracle656 Miracle656 mentioned this pull request Apr 26, 2026
11 tasks
@K1NGD4VID K1NGD4VID force-pushed the feat/token-metadata-caching branch from b0a2f1c to 7555083 Compare April 26, 2026 08:36
@K1NGD4VID
Copy link
Copy Markdown
Contributor Author

I've addressed the feedback for both PR #37 (feat/token-metadata-caching) and PR #45 (test/xdr-decoder-unit-tests).

Here’s the summary of the changes:

Removed scratch/generate_fixtures.js: I've removed this script from the feat/token-metadata-caching branch. I also ensured it was cleaned up on the test/xdr-decoder-unit-tests branch as previously requested.
Stacked on #45: I've rebased feat/token-metadata-caching onto test/xdr-decoder-unit-tests. This ensures that once #45 is merged into main, the diff for the caching PR will automatically shrink to only show the metadata caching logic.
Resolved Prisma Schema Conflict: To address the concern about multiple PRs owning schema changes, I've rebased feat/accounts-balance-endpoint (#48) onto feat/token-metadata-caching (#37). This makes #37 the "owner" of the TokenMetadata model in the schema, while #48 now simply inherits it as part of its base.
Updated Branch State:

feat/token-metadata-caching: Rebased on #45, script removed, force-pushed.
feat/accounts-balance-endpoint: Rebased on #37, force-pushed.
Everything is now cleanly organized and ready for the next round of reviews!

@K1NGD4VID K1NGD4VID requested a review from Miracle656 April 26, 2026 08:45
Copy link
Copy Markdown
Owner

@Miracle656 Miracle656 left a comment

Choose a reason for hiding this comment

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

Good progress — scratch/generate_fixtures.js is gone. The token metadata cache (memory → DB → RPC) is the right architecture.\n\nOne thing to fix before merging:\n\nRebase on main — PR #45 (XDR decoder) merged today, so your branch still carries all of #45's changes in the diff. After rebasing, the diff will shrink to just the caching-related additions (tokenCache.ts, tokenCache.test.ts, schema changes, etc.) making it much easier to review in isolation.\n\n\ngit fetch origin\ngit rebase origin/main\ngit push --force-with-lease\n\n\nOnce rebased I'll approve immediately.

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.

2 participants