feat(custom-currencies): add custom currencies support to ledger#4549
feat(custom-currencies): add custom currencies support to ledger#4549mark-vass-konghq wants to merge 7 commits into
Conversation
mark-vass-konghq
added
release-note/feature
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml 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 |
mark-vass-konghq
force-pushed
the
feat/add-custom-currencies-support-to-ledger
branch
from
4dbfd70 to
e82d034
Compare
| ALTER TABLE "billing_invoice_lines" ALTER COLUMN "currency" TYPE character varying(24); | ||
| -- modify "billing_invoice_split_line_groups" table | ||
| ALTER TABLE "billing_invoice_split_line_groups" ALTER COLUMN "currency" TYPE character varying(24); | ||
| -- modify "billing_invoices" table | ||
| ALTER TABLE "billing_invoices" ALTER COLUMN "currency" TYPE character varying(24); | ||
| -- modify "billing_standard_invoice_detailed_lines" table | ||
| ALTER TABLE "billing_standard_invoice_detailed_lines" ALTER COLUMN "currency" TYPE character varying(24); | ||
| -- modify "charge_credit_purchases" table | ||
| ALTER TABLE "charge_credit_purchases" ALTER COLUMN "currency" TYPE character varying(24); | ||
| -- modify "charge_flat_fee_run_detailed_lines" table | ||
| ALTER TABLE "charge_flat_fee_run_detailed_lines" ALTER COLUMN "currency" TYPE character varying(24); | ||
| -- modify "charge_flat_fees" table | ||
| ALTER TABLE "charge_flat_fees" ALTER COLUMN "currency" TYPE character varying(24); | ||
| -- modify "charge_usage_based" table | ||
| ALTER TABLE "charge_usage_based" ALTER COLUMN "currency" TYPE character varying(24); | ||
| -- modify "charge_usage_based_run_detailed_line" table | ||
| ALTER TABLE "charge_usage_based_run_detailed_line" ALTER COLUMN "currency" TYPE character varying(24); |
There was a problem hiding this comment.
let's only change those parts of the db where this makes sense for this change if possible.
e.g. billing will not use non-fiat currencies as currency.
mark-vass-konghq
force-pushed
the
feat/add-custom-currencies-support-to-ledger
branch
from
7ee9bcb to
7ecad00
Compare
Greptile SummaryThis PR extends the ledger to support custom (non-ISO) currency codes alongside standard fiat currencies. The main changes are: a new V3 routing key that adds a
Confidence Score: 4/5Safe to merge with minor cleanup; the routing, validation, and migration changes are structurally correct and well-tested, with one small API inconsistency in BuildRoutingKeyV3. The core routing versioning logic is correct: V3 keys are only auto-selected when Source is non-nil, V1/V2 guards reject Source, and the DB predicate and normalization paths handle the three-state filter consistently. The migration correctly drops and recreates the dependent view before widening columns. The main concern is that BuildRoutingKeyV3 — unlike BuildRoutingKeyV1 and BuildRoutingKeyV2 — does not reject a nil Source, so a caller who passes a sourceless route to the explicit V3 builder gets a key that diverges from what BuildRoutingKey would produce for the same route, which could create duplicate sub-accounts. The credit-purchase Intent.Validate() duplication is a maintainability risk rather than a present defect. openmeter/ledger/routing.go (BuildRoutingKeyV3 guard), openmeter/billing/charges/creditpurchase/charge.go (inline meta.Intent duplication) Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }) | ||
| } | ||
|
|
||
| allocations, err := currencyx.AllocateByAmount(calculator, currencyx.AmountAllocationInput[accruedBackfillBucketKey]{ | ||
| Amount: amount, | ||
| Items: items, | ||
| CompareKey: cmpx.Compare[accruedBackfillBucketKey], | ||
| }) | ||
| calculator, err := currency.Calculator() | ||
| if err == nil { | ||
| allocations, err := currencyx.AllocateByAmount(calculator, currencyx.AmountAllocationInput[accruedBackfillBucketKey]{ | ||
| Amount: amount, | ||
| Items: items, | ||
| CompareKey: cmpx.Compare[accruedBackfillBucketKey], | ||
| }) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("allocate accrued attribution: %w", err) | ||
| } | ||
|
|
||
| return allocations, nil | ||
| } | ||
|
|
||
| if err := ledger.ValidateCurrency(currency); err != nil { | ||
| return nil, fmt.Errorf("currency: %w", err) | ||
| } | ||
|
|
||
| allocations, err := allocateAccruedAttributionExactly(amount, items) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("allocate accrued attribution: %w", err) | ||
| } |
There was a problem hiding this comment.
allocateAccruedAttributionExactly does not round allocated amounts to the custom currency's precision. When the proportional split produces a repeating decimal (e.g., splitting 1 CREDIT among 3 equal buckets yields 0.333…), the ledger entries will carry arbitrary-precision amounts. The entry-amount precision guard in validateEntryAmountPrecision (validations.go) also skips custom currencies — when currency.Calculator() fails it falls through to ValidateCurrency, which only validates the code format. The combination means fractional amounts can be booked without any precision enforcement. Current tests use integer amounts so this doesn't surface yet, but any proportional split of an odd custom-currency amount will silently produce sub-unit entries.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openmeter/ledger/chargeadapter/creditpurchase.go
Line: 786-810
Comment:
**No currency-precision rounding in the custom-currency allocation path**
`allocateAccruedAttributionExactly` does not round allocated amounts to the custom currency's precision. When the proportional split produces a repeating decimal (e.g., splitting 1 CREDIT among 3 equal buckets yields 0.333…), the ledger entries will carry arbitrary-precision amounts. The entry-amount precision guard in `validateEntryAmountPrecision` (`validations.go`) also skips custom currencies — when `currency.Calculator()` fails it falls through to `ValidateCurrency`, which only validates the code format. The combination means fractional amounts can be booked without any precision enforcement. Current tests use integer amounts so this doesn't surface yet, but any proportional split of an odd custom-currency amount will silently produce sub-unit entries.
How can I resolve this? If you propose a fix, please make it concise.
2 participants
No description provided.