fix(billing): enforce currency match on wallet credit/debit

[marcelo-maciel]

Problem

Wallet.Credit and Wallet.Debit took a raw decimal and stamped the wallet's own currency onto the resulting Money. BillingService.MarkInvoicePaidAsync credited a top-up by passing invoice.SubtotalAmount.Amount, dropping invoice.Currency on the way in.

The effect: a top-up invoice in one currency credited to a wallet in another currency was applied at face value with the wallet's currency label. For example, a 100 EUR invoice paid against a pre-existing USD wallet credited 100 USD. The Money.Add/Money.Subtract currency guard never fired, because by the time it ran both operands had already been forced to the wallet's currency.

This is latent while the product is single-currency (a freshly created wallet at line 255 inherits invoice.Currency, so it always matches). It becomes a real, silent balance error the moment a pre-existing wallet's currency diverges from a later invoice.

Fix

Now that both sides are Money (following the recent Money value-object adoption in Billing):

• Wallet.Credit/Wallet.Debit take a Money instead of a decimal and route the balance through Money.Add/Money.Subtract, which throw on a currency mismatch. The hole closes at the domain boundary.
• BillingService passes invoice.SubtotalAmount through unchanged and guards the top-up credit: if a pre-existing wallet's currency differs from the invoice, it throws a CustomException with HttpStatusCode.Conflict and a clear message, so the mismatch surfaces as a meaningful error rather than a raw 500 bubbling out of Money.

Testing

• dotnet build clean, 0 warnings (TreatWarningsAsErrors).
• Full Billing.Tests suite: 124 passed, 0 failed. Adds Credit_rejects_currency_mismatch as a regression guard for the closed hole; existing Wallet tests migrated to the Money signature.
• Integration tests (WalletTopupServiceTests) were not run in this environment (require Docker/Testcontainers); the change to the credit path is value-preserving on the happy path and the new guard only fires on a currency mismatch those tests do not set up.

Notes

• Wallet.Credit/Debit signature changes decimal -> Money. Wallet is internal domain, so no public HTTP surface changes; no endpoint, config, or docs change is required. Flagging the signature change for reviewer awareness.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[iammukeshm]

Good catch and the right fix at the right layer — routing the balance through Money.Add/Subtract closes the silent-relabel hole at the domain boundary, and the caller sweep checks out (the invoice-paid credit is the only production call site, and the pre-guard gives it a proper 409 instead of a raw 500). One real defect and a test gap before this merges:

1. Credit() mutates the aggregate before the currency check fires (Wallet.cs:39)

WalletTransaction.Create + _transactions.Add(tx) run before Balance = Balance.Add(amount), and Money.Add is where the mismatch throws. On a mismatched credit the wallet is left holding a phantom ledger row (wrong currency, no balance movement) in the tracked aggregate — if any caller catches the exception and the same DbContext later saves, the transaction row persists and the ledger no longer sums to the balance. You got the ordering right in Debit() (compute remaining first); please mirror it:

var newBalance = Balance.Add(amount);   // validates currency before any mutation
var tx = WalletTransaction.Create(Id, TenantId, amount, kind, description, referenceId);
_transactions.Add(tx);
Balance = newBalance;

2. Strengthen the regression test (WalletTests.cs:97)

Credit_rejects_currency_mismatch only asserts the throw — it currently passes while the aggregate ends the test corrupted (one EUR tx on a USD wallet, balance 0). Add after the Should.Throw:

w.Transactions.ShouldBeEmpty();
w.Balance.Amount.ShouldBe(0m);

That will fail until fix 1 lands, which is exactly the coverage this PR exists to add. A matching Debit currency-mismatch test would be welcome too (nothing pins that behavior on the Wallet API today).

Nits (take or leave):

• (IEnumerable<string>?)null → the codebase convention is a named argument: new CustomException("...", errors: null, HttpStatusCode.Conflict).
• amount with { Amount = -amount.Amount } → amount.Multiply(-1m) keeps the negation inside Money's own API.

With 1 + 2 in, this is a merge.