Skip to content

remodel paid actions schema #2084

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

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from
Draft

remodel paid actions schema #2084

wants to merge 15 commits into from

Conversation

huumn
Copy link
Member

@huumn huumn commented Apr 9, 2025

Notes for 7e93200:

These are my WIP schema changes. They haven't been integrated with the rest of the schema yet, because the changes will be dramatic.


Notes for 366c069:

I'm finding that it difficult to refactor without practically rewriting - mostly due to supporting multiple payment methods simultaneously and atomically. I'll likely create a minimal parallel implementation of PayIns (formerly "paid actions") first that meets all the requirements, then merge it with the old code. Presumed order of operations:

  • parallel schema/state machine
  • replace existing schema/state machine
  • backend->frontend refactor
  • test a lot
  • migrations from existing schema

Some notable additions to the state machine are:

  • new state PENDING_INVOICE_CREATION which is needed because we don't know the invoice amount in advance when paying partially with CCs/reward sats
    • this also means that we'll have a PayIn record before we know if invoice creation succeeds, so in the case that a P2P invoice is impossible to create, we'll need the ability to retry server side to support CC fallback
      • this will be written in such a way that client side retries can use it
  • recording the reason for entering a FAILED state as an enum
  • recording the time of any state change

Notes for 5f7ad5e:

I've added the flow for wrapping and creating invoice - where/how I think they're best done given our constraints. Obviously, none of this actually works yet but it logically lays out what I'm aiming for enough to proceed with adapting the state machine. Then I'll be rewriting the payInType modules to be consistent with the changes.


Notes for e7c93f4:

State machine is conceptually complete and features additional transitions specifically for withdrawals. PayIn modules are ready for refactoring now.


Notes for 396d643:

It's coming along. Many of the PayIn modules have been refactored. The logic of my draft still has a few holes in it (i.e. composable payIns are needed, e.g. an item created with boost), but I have mods designed for all of these holes that I'm aware of. That'll be what I work on after refactoring zap. Then I'll likely do another pass over api/payIn/index.js before beginning either a refactor of retries or refactoring my way to the frontend.


Notes for c1c8380:

Rework of api/payIn/index.js which resulted in relatively declarative payIn modules (yay ... although it's still not as declarative as I'd like). I also added PayIn beneficiaries (payIns of payIns) to deal with the issue mentioned above. Once I finish refactoring zap, I'll begin working on retries.


Notes for b5f8aa8:

As of this commit, I have retries fairly well figured out. The main thing it's missing is it doesn't yet have a way to signal when manual retries should begin.

This is at the stage now where I can begin a conceptual refactor of code that will make use of the schema changes. Most of what I need to assess is:

  1. the worker transitioning the state machine
  2. the client and graphql api

This PR aims to accomplish:

  1. simple and fine grained satistics gathering - per customer and in aggregate
    • PayIn will have ALL customer spending with a type
    • PayOut will have ALL customer earning with a type
  2. trivial audibility - which assets were spent and earned, and what the effect of the action was on the balance of the assets for each the spender and earner
  3. multiple payment methods for the same "action", e.g. 20 CCs + 20 reward sats + 60 sat invoice for an "action" that costs 100 sats while maintaining audit-ability and atomic refunds
  4. realtime revenue for territory founders (this was not intended but was a bonus of the design), i.e territory revenue is not paid out each night anymore ... it happens when the action happens
    • similarly, actions that increase the rewards pool are no longer deduced but represented by a row in PayOut
      • consideration was given to future plans like territory-specific rewards pools and the design is conducive to that
  5. the design should greatly improve the readability and reliability of retries and auto-retries
    • I haven't explicitly considered this in the design tbh, but given that we aren't overloading Invoice and Withdrawl with state, intuition would suggest this is the case. Regardless, wip/tbd.
      • This should be doable with a predecessorId in PayIn where each retry creates a new PayIn record
  6. "prisms" (to be implemented later using the fact that PayOut is decoupled from PayIn)

@stackernews stackernews deleted a comment from gitguardian bot Apr 21, 2025
Copy link

gitguardian bot commented Apr 22, 2025

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@ekzyis ekzyis mentioned this pull request Apr 25, 2025
18 tasks
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.

1 participant