-
Notifications
You must be signed in to change notification settings - Fork 6
Exact-sum subscriber attribution via largest-remainder (audit C1) #136
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
Open
brawlaphant
wants to merge
1
commit into
regen-network:main
Choose a base branch
from
brawlaphant:fix/pool-attribution-bigint-math
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| import { describe, it, expect } from "vitest"; | ||
| import { | ||
| allocateProportional, | ||
| creditsToMicro, | ||
| microToCredits, | ||
| ATTRIBUTION_SCALE, | ||
| } from "../services/attribution.js"; | ||
|
|
||
| describe("creditsToMicro / microToCredits", () => { | ||
| it("round-trips integer credit counts exactly", () => { | ||
| for (const n of [0, 1, 100, 1000.5]) { | ||
| expect(microToCredits(creditsToMicro(n))).toBe(n); | ||
| } | ||
| }); | ||
|
|
||
| it("rounds half to nearest micro", () => { | ||
| expect(creditsToMicro(0.0000005)).toBe(1n); | ||
| expect(creditsToMicro(0.0000004)).toBe(0n); | ||
| }); | ||
|
|
||
| it("clamps non-finite or negative input to 0", () => { | ||
| expect(creditsToMicro(NaN)).toBe(0n); | ||
| expect(creditsToMicro(-1)).toBe(0n); | ||
| expect(creditsToMicro(Infinity)).toBe(0n); | ||
| }); | ||
|
|
||
| it("ATTRIBUTION_SCALE is 1_000_000n", () => { | ||
| expect(ATTRIBUTION_SCALE).toBe(1_000_000n); | ||
| }); | ||
| }); | ||
|
|
||
| describe("allocateProportional (audit C1)", () => { | ||
| it("returns zero shares when total is zero", () => { | ||
| expect(allocateProportional(0n, [1n, 2n, 3n])).toEqual([0n, 0n, 0n]); | ||
| }); | ||
|
|
||
| it("returns zero shares when all weights are zero", () => { | ||
| expect(allocateProportional(100n, [0n, 0n, 0n])).toEqual([0n, 0n, 0n]); | ||
| }); | ||
|
|
||
| it("returns empty array for empty weights", () => { | ||
| expect(allocateProportional(100n, [])).toEqual([]); | ||
| }); | ||
|
|
||
| it("rejects negative weights", () => { | ||
| expect(() => allocateProportional(100n, [-1n, 2n])).toThrow(); | ||
| }); | ||
|
|
||
| it("splits 1.0 credit between equal subscribers exactly (no float drift)", () => { | ||
| // Previous float code: 1/3 ≈ 0.333..., 3 * 0.333 = 0.999, 1 micro lost. | ||
| const total = creditsToMicro(1); // 1_000_000n | ||
| const shares = allocateProportional(total, [1n, 1n, 1n]); | ||
| const sum = shares.reduce((a, b) => a + b, 0n); | ||
| expect(sum).toBe(total); | ||
| // Each share is 333_333n, with one bumped to 333_334n by largest-remainder. | ||
| expect(shares.filter((s) => s === 333_333n).length).toBe(2); | ||
| expect(shares.filter((s) => s === 333_334n).length).toBe(1); | ||
| }); | ||
|
|
||
| it("the leftover micro goes to the largest-residual share, deterministically", () => { | ||
| // weights [1, 2] with total 1n: floor shares are [0, 0], remainders are | ||
| // [1, 2] (out of weightSum=3). Index 1 has the larger residual. | ||
| expect(allocateProportional(1n, [1n, 2n])).toEqual([0n, 1n]); | ||
| }); | ||
|
|
||
| it("weights determine proportion: 1:5 split of 6n exactly", () => { | ||
| expect(allocateProportional(6n, [1n, 5n])).toEqual([1n, 5n]); | ||
| }); | ||
|
|
||
| it("scales correctly to 1.5M credits across 1000 weighted subscribers", () => { | ||
| const weights: bigint[] = []; | ||
| for (let i = 0; i < 1000; i++) weights.push(BigInt(100 + (i % 50))); | ||
| const total = creditsToMicro(1_500_000); | ||
| const shares = allocateProportional(total, weights); | ||
| expect(shares).toHaveLength(1000); | ||
| const sum = shares.reduce((a, b) => a + b, 0n); | ||
| expect(sum).toBe(total); | ||
| }); | ||
|
|
||
| it("REGRESSION: subscriber A=$2, B=$10 gives B exactly 5x A's share", () => { | ||
| const total = creditsToMicro(0.6); // 600_000n | ||
| const shares = allocateProportional(total, [200n, 1000n]); | ||
| expect(shares[0] + shares[1]).toBe(total); | ||
| expect(shares[1]).toBe(shares[0] * 5n); | ||
| }); | ||
|
|
||
| it("ties on remainder break by index (lower index gets the +1)", () => { | ||
| // weights [1, 1] with total 1n: floor = [0, 0], remainders both 1. | ||
| // Tie broken by idx → idx 0 gets the leftover micro. | ||
| expect(allocateProportional(1n, [1n, 1n])).toEqual([1n, 0n]); | ||
| }); | ||
|
|
||
| it("never allocates more than total even with extreme weights", () => { | ||
| const total = creditsToMicro(1); | ||
| // One huge weight + many tiny — naïve float would round and over-allocate. | ||
| const weights = [10_000_000n, ...Array(100).fill(1n)]; | ||
| const shares = allocateProportional(total, weights); | ||
| expect(shares.reduce((a, b) => a + b, 0n)).toBe(total); | ||
| }); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| /** | ||
| * Proportional attribution helpers. | ||
| * | ||
| * The pool run distributes a fixed total of credits across many subscribers | ||
| * weighted by each subscriber's contribution. Doing this with float | ||
| * arithmetic (audit C1) means the per-subscriber shares may not sum to | ||
| * exactly the retired total — small drift, but compounds across many runs | ||
| * and undermines the on-chain invariant that retirements are exact. | ||
| * | ||
| * `allocateProportional` uses the largest-remainder method (Hamilton): | ||
| * 1. floor share for each weight: floor(total * weight / sumWeights) | ||
| * 2. distribute the remainder, one micro-unit at a time, to the weights | ||
| * with the largest fractional residual (ties broken by index) | ||
| * The returned bigint shares are guaranteed to sum to exactly `total`. | ||
| */ | ||
|
|
||
| /** Conversion scale used to ladder a JS-number credit count into bigint micro-credits. */ | ||
| export const ATTRIBUTION_SCALE = 1_000_000n; | ||
|
|
||
| /** | ||
| * Convert a JS number credit count (e.g. 1.5) to bigint micro-credits. | ||
| * The single float touch — bounded error of 0.5 micro per conversion. | ||
| */ | ||
| export function creditsToMicro(credits: number): bigint { | ||
| if (!Number.isFinite(credits) || credits < 0) return 0n; | ||
| return BigInt(Math.round(credits * Number(ATTRIBUTION_SCALE))); | ||
| } | ||
|
|
||
| /** Convert bigint micro-credits back to a JS number for REAL-column storage. */ | ||
| export function microToCredits(micro: bigint): number { | ||
| return Number(micro) / Number(ATTRIBUTION_SCALE); | ||
| } | ||
|
|
||
| /** | ||
| * Distribute `total` units across N buckets weighted by `weights`, returning | ||
| * an array of bigint shares whose sum is exactly `total`. | ||
| * | ||
| * Returns an array of zeros if total === 0n or all weights sum to 0 — caller | ||
| * should treat that as "nothing to allocate." | ||
| */ | ||
| export function allocateProportional(total: bigint, weights: readonly bigint[]): bigint[] { | ||
| if (weights.length === 0 || total <= 0n) { | ||
| return weights.map(() => 0n); | ||
| } | ||
| let weightSum = 0n; | ||
| for (const w of weights) { | ||
| if (w < 0n) throw new Error("Negative weight in allocateProportional"); | ||
| weightSum += w; | ||
| } | ||
| if (weightSum === 0n) { | ||
| return weights.map(() => 0n); | ||
| } | ||
|
|
||
| // Floor shares + fractional residuals. | ||
| const shares: bigint[] = new Array(weights.length).fill(0n); | ||
| const residuals: { idx: number; remainder: bigint }[] = []; | ||
| let allocated = 0n; | ||
| for (let i = 0; i < weights.length; i++) { | ||
| const numer = total * weights[i]; | ||
| const floor = numer / weightSum; | ||
| const remainder = numer - floor * weightSum; | ||
| shares[i] = floor; | ||
| allocated += floor; | ||
| residuals.push({ idx: i, remainder }); | ||
| } | ||
|
|
||
| // Distribute remainder to largest residuals; stable on idx for determinism. | ||
| let leftover = total - allocated; | ||
| if (leftover > 0n) { | ||
| residuals.sort((a, b) => { | ||
| if (a.remainder === b.remainder) return a.idx - b.idx; | ||
| return a.remainder < b.remainder ? 1 : -1; | ||
| }); | ||
| for (let i = 0; i < residuals.length && leftover > 0n; i++) { | ||
| shares[residuals[i].idx] += 1n; | ||
| leftover -= 1n; | ||
| } | ||
| } | ||
|
|
||
| return shares; | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ import { | |
| } from "../server/db.js"; | ||
| import { sendMonthlyEmails } from "./email.js"; | ||
| import { executeBurn, type BurnResult, formatBurnResult } from "./burn.js"; | ||
| import { allocateProportional, creditsToMicro, microToCredits } from "./attribution.js"; | ||
|
|
||
| export interface PoolRunResult { | ||
| poolRunId: number; | ||
|
|
@@ -547,16 +548,26 @@ function recordAttributions( | |
| biodiversity: CreditTypeResult, | ||
| uss: CreditTypeResult | ||
| ): void { | ||
| if (totalRevenueCents <= 0) return; | ||
| if (totalRevenueCents <= 0 || subscribers.length === 0) return; | ||
|
|
||
| // Audit C1: previously this used `sub.amount_cents / totalRevenueCents` then | ||
| // multiplied each retired total by that float fraction. Per-subscriber shares | ||
| // could drift such that their sum was not exactly the retired total. Now | ||
| // computed in bigint micro-credits via Hamilton's largest-remainder method, | ||
| // which guarantees the shares sum exactly to creditsToMicro(creditsRetired). | ||
| const weights = subscribers.map((s) => BigInt(s.amount_cents)); | ||
| const carbonShares = allocateProportional(creditsToMicro(carbon.creditsRetired), weights); | ||
| const biodiversityShares = allocateProportional(creditsToMicro(biodiversity.creditsRetired), weights); | ||
| const ussShares = allocateProportional(creditsToMicro(uss.creditsRetired), weights); | ||
|
|
||
| const txn = db.transaction(() => { | ||
| for (const sub of subscribers) { | ||
| const fraction = sub.amount_cents / totalRevenueCents; | ||
| for (let i = 0; i < subscribers.length; i++) { | ||
| const sub = subscribers[i]; | ||
|
Comment on lines
+564
to
+565
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| const attr = createAttribution(db, poolRunId, sub.id, sub.amount_cents); | ||
| updateAttribution(db, attr.id, { | ||
| carbon_credits: carbon.creditsRetired * fraction, | ||
| biodiversity_credits: biodiversity.creditsRetired * fraction, | ||
| uss_credits: uss.creditsRetired * fraction, | ||
| carbon_credits: microToCredits(carbonShares[i]), | ||
| biodiversity_credits: microToCredits(biodiversityShares[i]), | ||
| uss_credits: microToCredits(ussShares[i]), | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attribution logic is currently non-deterministic when multiple subscribers have equal fractional remainders (e.g., when they have the same
amount_cents). BecauseallocateProportionaluses the array index to break ties and thesubscribersarray is populated from a database query without an explicitORDER BYclause, the extra micro-credits may be assigned to different subscribers across different runs. Sorting the subscribers by a unique ID ensures that tie-breaking is consistent and deterministic, addressing the concern raised in the PR description.