Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make wallet log context normal #1598

Closed
huumn opened this issue Nov 17, 2024 · 3 comments
Closed

Make wallet log context normal #1598

huumn opened this issue Nov 17, 2024 · 3 comments
Assignees
Labels
enhancement improvements to existing features wallets

Comments

@huumn
Copy link
Member

huumn commented Nov 17, 2024

Currently we store an anything-goes JSON object as the wallet log context. We shouldn't:

  1. the context object is undocumented (ie not even documented in code in one place)
  2. migrating the context object over time (which is inevitable) is going to be messy
  3. its keys need to iterated over/filtered in JS to tell if the wallet log has utility in whatever function is using wallet logs
  4. context keys can't be properly indexed in the database without making a bigger mess
  5. given the above, it's not worth developing views based on the wallet log's relationship to any data stored in context (e.g. viewing all the wallet logs related to an invoice payment)
  6. people are going to store random denormalized data in the context object over time because its the path of least resistance for them and pushes the burden of their decisions into the future

Keeping vanity data in JSON blobs is fine, but we shouldn't be storing important data this way. The DB is the root of sanity in our architecture. Storing normalized data in it means we can access data declaratively and can mostly liberate ourselves from typechecking most of our code and writing tests for what/how data is being stored.

@huumn huumn added enhancement improvements to existing features wallets labels Nov 17, 2024
@ekzyis
Copy link
Member

ekzyis commented Nov 17, 2024

Something to consider here is that we can only enforce relations for the receive logs since the send logs are stored on the client.

@huumn
Copy link
Member Author

huumn commented Nov 17, 2024

Oh good point. I'm imagining we kind of loosely enforce the same data shape on the client as we do on the server like we do with configs.

@ekzyis ekzyis self-assigned this Nov 28, 2024
@ekzyis ekzyis mentioned this issue Dec 25, 2024
13 tasks
@ekzyis
Copy link
Member

ekzyis commented Mar 25, 2025

This was fixed by #1826 afaict

@ekzyis ekzyis closed this as completed Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improvements to existing features wallets
Projects
None yet
Development

No branches or pull requests

2 participants