feat!: color of money — segregated balances per (account, asset, color)#1438
feat!: color of money — segregated balances per (account, asset, color)#1438gfyrag wants to merge 9 commits into
Conversation
Squashed from 30+ iterations. Color is a first-class dimension on every balance, posting, audit order, and read-side projection. - Posting.Color, AccountVolume keyed by (account, asset, color) - Numscript scripts default to "" color; explicit color via syntax - Audit orders carry color through transient/purged exclusion sets - check/ verifies color through excludedVolumesSet, partitionVolumes - REST/gRPC/JSON always emit color (including uncolored bucket) - analysis carries color through flow signatures, NUL-separated to prevent collisions - admission validates Posting.Color before preload extraction Will require manual rebase on top of main (post-master→main migration, metadata-immutability refactor, AppliedProposal envelopes, etc.).
…signedApplyRequest
- idempotency_failure_test.go used the old map-style Account.Volumes
lookup; switch to FindVolume("USD", ""). The e2e package fails
the build under `-tags e2e` otherwise.
- color_segregation_test.go / color_numscript_test.go: master replaced
the inline `&servicepb.ApplyRequest{Envelopes: UnsignedEnvelopes(...)}`
builder with the `servicepb.UnsignedApplyRequest("", ...)` helper.
Migrate every Apply call accordingly.
Addresses NumaryBot review on common.proto:148.
The sink JSON now always emits posting.color (commit 60aef1a18 dropped omitempty for the proto type, b858501db did the same for sinkPosting). The ClickHouse table DDL still declared posting columns as source/destination/amount/asset only, so colored postings would either be rejected by strict JSON typing or land in untyped/dynamic storage depending on ClickHouse settings. Add the color column so warehouse queries can reconstruct the segregated buckets exactly as the API surfaces them. Databricks is unaffected — its DDL uses STRING for the whole data column. Addresses NumaryBot review on sink_data_common.go:73.
…hape The antithesis workload module replaces ledger via go.mod, so it consumes our proto directly. The volume API changed from map<string, *> to repeated entries: - reads.go accountAssetVolumes: switch to acct.FindVolume(asset, ""). - validate.go postCommitVolume: scan VolumesByAssets.volumes for the (asset, color="") tuple. The workload only ever exercises uncolored postings, so both call sites match the uncolored bucket explicitly — colored buckets stay out of scope for this driver model. Addresses NumaryBot reviews on common.proto:109 and :148.
Master landed several major refactors that intersect color-of-money: - #573 (EN-1378): admission now emits Declare for absent volume keys and Scope.Volumes().Get returns ErrNotFound which readVolumeOrZero turns into a fresh zero balance. Drop our explicit ErrBalanceNotPreloaded branches around readVolumeOrZero; replace the no-longer-applicable TestGetBalances_VolumeNotFound_ReturnsError with the new contract test TreatedAsZero. Adapt the numscript adapter accordingly. - #569: Scope per-attribute trios collapsed into generic Accessor[K,V,R]. Switch buildPostCommitVolumes to s.Volumes().Get(...). - #587 (EN-868): business API now mounts under /v3 only. Update the collapseColors HTTP test URL accordingly. - #571 (mockgen helpers): pick up master's expectGetVolume / expectPutVolume helpers in tests. - Fan out the additional cfg/color parameters that processor and indexer signatures accumulated across master commits.
🛑 Changes requested — automated reviewThe PR introduces a breaking change to the persisted volume-key layout (adding a color segment to the Pebble key) without bumping the storage schema version or providing a migration path. Existing stores will silently pass boot validation yet produce incorrect or missing balance reads when the new |
NumaryBot
left a comment
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #1438 (comment)
| @@ -102,6 +123,8 @@ func (bk VolumeKey) AppendBytes(dst []byte) []byte { | |||
| dst = appendLedgerName(dst, bk.LedgerName) | |||
| dst = append(dst, bk.Account...) | |||
| dst = append(dst, dal.CanonicalKeySepVolume) | |||
There was a problem hiding this comment.
🔴 [blocker] Bump the storage schema for colored volume keys
When opening an existing schema-version-1 store, this new extra separator changes the Pebble volume key layout from [account]\0[asset][precision] to [account]\0[color]\0[asset][precision], but CurrentStorageSchemaVersion is still 1. Those stores will pass boot validation and then old volume keys either fail VolumeKey.Unmarshal or are scanned as malformed/missing, so existing balances and checker/index reads can be lost or fail instead of being rejected up front or migrated.
There was a problem hiding this comment.
Won't fix — same compat concern reposted 28 times on the previous PR (formancehq/ledger-v3-poc#234) before this branch was moved here. ledger-v3-poc convention is no compat shims and no rolling-upgrade contract on this branch; existing v1 stores must be wiped before the cut-over.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Two adjacent NumaryBot findings: 1. ctrl/store.go: buildAccountVolumes did a silent continue on a malformed canonical volume key, dropping balances from GetAccount without telling the caller. Every other Pebble scan path propagates unmarshal errors; this one was inconsistent. Surface a hard error instead — CLAUDE.md invariant #7 (no silent should-not-happen). assembleAccount/scanAccount now return the error up the chain; buildAccountVolumes too. New TestAssembleAccount_MalformedKeyReturnsError pins the contract with a too-short canonical key payload. 2. receipt/receipt.go: PostingClaim.Color had json:"color,omitempty" so the signed JWT dropped color:"" for uncolored postings, making v3 uncolored claims indistinguishable from pre-color claims. Drop omitempty to match the contract already enforced by commonpb.Posting / sinkPosting / AccountVolume. New TestPostingClaim_AlwaysEmitsColor pins the JSON shape.
NumaryBot
left a comment
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #1438 (comment)
| @@ -102,6 +123,8 @@ func (bk VolumeKey) AppendBytes(dst []byte) []byte { | |||
| dst = appendLedgerName(dst, bk.LedgerName) | |||
| dst = append(dst, bk.Account...) | |||
| dst = append(dst, dal.CanonicalKeySepVolume) | |||
There was a problem hiding this comment.
🔴 [blocker] Storage schema version not bumped for colored volume key layout change
reported by NumaryBot, codex
The volume key layout has changed from [account]\0[asset][precision] to [account]\0[color]\0[asset][precision], but CurrentStorageSchemaVersion remains at 1. Existing stores will pass boot validation and then either fail VolumeKey.Unmarshal or silently return incorrect/missing balances, as old keys are scanned as malformed rather than being rejected up front or migrated.
Suggestion: Bump CurrentStorageSchemaVersion to 2 (or the next appropriate value), add a migration path that re-encodes existing volume keys into the new colored format, and add a guard that refuses to open a store whose on-disk schema version does not match the expected version.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (76.83%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## release/v3.0 #1438 +/- ##
================================================
+ Coverage 73.89% 74.04% +0.15%
================================================
Files 382 383 +1
Lines 39032 39207 +175
================================================
+ Hits 28842 29032 +190
+ Misses 7725 7681 -44
- Partials 2465 2494 +29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Summary
Introduces strict balance segregation by color as a first-class
dimension of the ledger. A balance lives at
(account, asset, color);postings carry an explicit
colorfield. The empty color is theuncolored bucket and is itself fully segregated from every colored
bucket.
Companion PR upstream: formancehq/numscript#139 —
Posting.Colorexposed in numscript output,BalanceQueryrefactored to carry color as a separate dimension instead of an asset suffix. The ledger pins that branch via a localreplaceingo.modfor now; bump to a tagged release once #139 is merged.Invariants
changes it. Re-coloring is a deliberate composition (clearing
account + mint from
@world), not a primitive.Color == ""participates insegregation just like any colored bucket. A draw under color
Xcannot dip into uncolored funds, and vice versa.
^[A-Z]*$color charset, enforced both upstream (numscript) andat the ledger admission boundary. The empty string remains valid.
(asset, color), the sum ofall account balances is zero. Each bucket is its own conservation
universe; the
scenariotestinvariant check is updated accordingly.Wire shapes
Posting.colorexposed everywhere postings appear.Account.volumesbecomesrepeated AccountVolume{asset, color, volumes}sorted by
(asset, color)ascending — deterministic JSON.PostCommitVolumesinner shape becomes a sorted list ofVolumeEntry{asset, color, volumes}.AggregatedVolume.coloris set on every entry; the default responseis one entry per
(asset, color)tuple.For consumers that don't care about color, two opt-in flags let the
server collapse colors into a single per-asset entry:
GET /{ledger}/accounts/{address}?collapseColors=trueGET /{ledger}/volumes?collapseColors=true(orAggregateVolumesRequest.collapse_colors = true)Storage layout
domain.VolumeKeycarriesColor string. Canonical bytes:```
[ledgerID BE 4B] [account] \x00 [color] \x00 [asset_base] [precision 1B]
```
Color sits between account and asset so prefix scans behave naturally:
`[ledgerID][account]\x00` still returns every
(color, asset)for theaccount; `[ledgerID][account]\x00[color]\x00` is a fixed-prefix lookup
for a specific bucket. The precision byte stays trailing where it can
legitimately be `0x00` (e.g.
"EUR") without encoding ambiguity.v2 compat / mirror
Untouched by design. The v2 protocol has no color dimension, so every
mirrored posting lands in the uncolored bucket. The
MirrorIngestpreload branch explicitly passes
color = ""with a comment thatmakes the intent visible at the call site.
Numscript integration
The numscript interpreter (PR #139) emits postings with their
Colorfield set. The ledger's
numscriptStoreAdapterhonors the newAssetColoritems inBalanceQueryand returnsBalanceskeyed bymap[account][asset][color] -> *big.Int. No asset-suffix encodingremains in either repo.
Tests
tests/e2e/business/color_segregation_test.go— fullend-to-end gRPC test covering GetAccount segregation, collapse,
draw refusal across colors, draining a single bucket, color visible
on the emitted posting.
internal/application/ctrl/store_color_test.gocoveringassembleAccountsegregation by default +collapseColorsmode.internal/query/aggregate_test.go):segregation, collapse, collapse + max_precision, grouped aggregator.
layout in
internal/domain/keys_test.go.acct.GetVolumes()["USD"]map access toacct.FindVolume("USD", "").Test plan
tagged numscript release (drop the
replacedirective)and `ledgerctl accounts get alice` showing the COLOR column
🤖 Generated with Claude Code
Migrated from formancehq/ledger-v3-poc#234 on 2026-06-29.