Skip to content

fix(credits): one baseline for every stream credit, and totals only on funded entries - #28

Merged
sirpy merged 14 commits into
mainfrom
fix/stream-credit-accounting
Oct 9, 2026
Merged

sirpy merged 14 commits into
mainfrom
fix/stream-credit-accounting

Conversation

@blueogin

@blueogin blueogin commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Account 0x2CeADe86…0627 reports 6,229,114 G$ deposited against a $4.25 credit balance, $730.54 stuck as outstanding, and an 8,900 G$/mo stream rate for streams that closed on 2026-09-17. Superfluid says 7,134.63 G$ ever streamed to the vault from that account; the backend funded 19,283.45 G$ against it

1. The credit window was unbounded

elapsedSeconds came from lastStreamCreditAt || stream.lastUpdateAt with no validation. With the profile defaulting that timestamp to 1970-01-01, "time since the last credit" became "time since 1970" — 56.5 years in one entry:

3472222222222222 wei/s × 1783016571 s = 6,191,029 G$

99.4% of the account's reported total, from a single entry whose funding reverted.

Now: the window starts at the later of lastStreamCreditAt and the stream's updatedAtTimestamp. That moves forward on every create, rate change and close, so a stale clock is unreachable — a dormant gap between a closed stream and its replacement is never billed, and a brief stream cannot be charged across weeks. The window itself is not capped: a long gap means the stream really did flow, and a stalled cron should still pay out.

Because updatedAtTimestamp is the last on-chain flow change, the rate is provably constant across the window — so rate × window is exact, not an estimate.

2. Totals were credited before funding landed

G$ totals were added in recordGdCredit while USD totals moved only in markFundingResult on success, so a reverted funding inflated the G$ counters permanently and never released totalOutstandingFundingUsd. Now: all lifetime totals move together, only on funded entries; outstanding is released on either terminal state. This is what produced the 6.2M-vs-$4.25 divergence.

3. StreamUpdated double-counted

It credited event.totalFlowWei, measured from the last on-chain flow change, while the scheduled run measures from lastStreamCreditAt — overlapping windows:

T1 rate change · T2 cron pays T1→T2 · T3 rate change, event pays T1→T3   ← T1→T2 twice

It also advanced the clock without paying for the window: on 2026-08-28 it credited 0.18 G$ while moving the clock past 3.39 days, dropping 1,005 G$. Now: the event credits the window actually owed, from the same baseline as the scheduled run.

4. Closed streams were invisible, and their final interval unpaid

The scheduled run filtered currentFlowRate_gt: "0", so a closed stream vanished and was never revisited — its rate was never cleared (the only writer of 0 was a push-only event, and input.flowRate ? … discarded 0n as falsy), and the interval between the last credit and the close was never paid.

Now: closed streams are fetched, rates sync every run (not only when a credit clears the cooldown and the 4,000 G$ minimum — a fortnight apart at 8,900 G$/mo), summed per account so a closed revision can't clobber its live replacement, and the final interval is credited in both the scheduled run and POST /stream-credits.

5. Root profiles held the wrong flow rate

Totals reach a GoodID root by mirroring in updateUser, correct because they accumulate. A flow rate is absolute, so mirroring left the root holding whichever sub-account wrote last. Now: excluded from the mirror and summed onto the root.

This PR still reconstructs amounts rather than reading them

Every remaining limitation below has one cause: the credit amount is computed as flowRate × window instead of being read from Superfluid.

The subgraph already holds the exact figures, and this PR does not use them. streamPeriods returns one row per span of constant flow rate, with exact start, stop, rate and amount:

stream  started              stopped                        flowRate   streamed G$
a-0.0   2026-07-02 18:22:46  2026-07-02 18:23:37    3472222222222222          0.18
a-0.0   2026-07-02 18:23:37  2026-07-06 15:48:29    3472222222222222      1,167.68
a-1.0   2026-08-28 15:18:07  2026-08-28 15:19:00    3356481481481481          0.18
a-1.0   2026-08-28 15:19:00  2026-09-17 18:00:26    3433641975308641      5,966.59
                                                               TOTAL      7,134.63

Summing flowRate × overlap across the periods after the last credit is exact — the rate is constant within a period by definition — and removes the floor, the remembered rate, any need for a ceiling, and the live/closed branching entirely. analytics.ts:546 already queries this entity; the credit path never adopted it. Deliberately left as a follow-up to keep this PR to the defects above.

Consequences carried in the meantime:

The final interval has one chance to be paid. The scheduled run captures the pre-close rate before zeroing it, so only the first run after a close can price that window. If that run is blocked by the cooldown or the 4,000 G$ minimum, the next sees rate 0 and the interval is lost. POST /stream-credits cannot recover it, since it reads the already-zeroed rate — correct when the cron credited, a dead end when it didn't. Final intervals are often short, so this will frequently not pay out. Under-credit, not loss of funds.

The closed-stream branch can over-credit. It prices lastCredit → termination at one remembered rate, with no floor (a closed stream's last update is its termination) and no ceiling. If the rate dropped and the change wasn't ingested, the remembered rate is the older, higher one. A streamedUntilUpdatedAt cap was tried and removed: it bounds the stream's lifetime total rather than what's owed, so it guards the wrong quantity and usually doesn't bind — false assurance was worse than none.

Rate changes under-credit when the event is missed. updatedAtTimestamp records only the most recent change, so the window before it cannot be priced and is skipped. /v1/celo/events/record is push-only with no retry. Deliberate — errs towards under-crediting.

Not included

Existing KV profiles are already corrupt and are not repaired here; the account above keeps reporting 6.2M G$ until its stored totals are rewritten.

About #27

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • PR title matches follow: (Feature|Bug|Chore) Task Name
  • My code follows the style guidelines of this project
  • I have followed all the instructions described in the initial task (check Definitions of Done)
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have added reference to a related issue in the repository
  • I have added a detailed description of the changes proposed in the pull request. I am as descriptive as possible, assisting reviewers as much as possible.
  • I have added screenshots related to my pull request (for frontend tasks)
  • I have pasted a gif showing the feature.
  • @mentions of the person or team responsible for reviewing proposed changes

Copilot AI balanced review requested due to automatic review settings October 8, 2026 17:38

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.

🟡 Changes recommended

Closed intervals, missed rate changes, root aggregation, and concurrent total updates remain incorrect.

5 open findings
What changed in this PR

Fixes Worker stream-credit accounting and funded-total tracking without changing the payment boundary.

Changes:

  • Unifies stream-credit baselines and synchronizes closed-stream rates.
  • Moves lifetime totals to successful funding completion.
  • Adds regression tests and architecture documentation.
File Description
backend/​src/​worker.ts Revises stream fetching, timing, event, and cron logic.
backend/​src/​kv-credit-store.ts Updates flow-rate and funded-total persistence.
backend/​test/​worker.test.ts Adds stream-accounting regressions.
backend/​test/​kv-credit-store.test.ts Tests terminal funding and rate persistence.
docs/​ARCHITECTURE.md Documents revised accounting behavior.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +178 to +180
totalGdDepositedWei: credited ? addDecimalStrings(current.totalGdDepositedWei, entry.gdAmountWei) : current.totalGdDepositedWei,
totalGDStreamedWei:
credited && entry.source.startsWith("stream") ? addDecimalStrings(current.totalGDStreamedWei, entry.gdAmountWei) : current.totalGDStreamedWei,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@sirpy
Valid. This race already exists on main for the USD totals;
The proper fix is to split per-wallet profiles from the identity aggregate.
I do not fix this in the pr

Comment thread backend/src/worker.ts Outdated
Comment thread backend/src/worker.ts Outdated
Comment thread backend/src/kv-credit-store.ts Outdated
Comment thread docs/ARCHITECTURE.md
blueogin and others added 2 commits October 8, 2026 14:07
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@blueogin

blueogin commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot
please review again

Copilot AI commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

@copilot
please review again

Reviewing the available tree, I found three remaining accounting issues:

  • backend/src/worker.ts:535-536: a StreamUpdated event uses the subgraph’s new updatedAtTimestamp as the floor, so the interval from the last credit through the rate change is skipped. The test at worker.test.ts:2503-2514 asserts that skipped interval but does not verify that the event handler settles it.
  • backend/src/worker.ts:241-248: cron first syncs the summed account rate, then recordGdCredit writes each individual row’s flowRate. An eligible closed row can therefore replace the account’s summed rate with zero, depending on row order.
  • backend/src/kv-credit-store.ts:292-307 (called concurrently by cron funding at worker.ts:266-270): updateUser is a KV read/modify/write for both wallet and root profiles. Concurrent funded entries sharing a root can overwrite each other’s totals and outstanding-balance updates.

The checkout is shallow (no base ancestry), so this review is against the available current tree rather than a complete ancestry-based PR diff.

@sirpy
sirpy merged commit 2279ebc into main Oct 9, 2026
3 checks passed
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.

4 participants