Skip to content

Commit 2499c7d

Browse files
bjcoombsbjcoombs
authored
feat: Update current-account service layer for dimension-agnostic Amount (#1256)
* feat: Update current-account service layer for dimension-agnostic Amount Replace NewMoney() calls and .Currency() method references in the service layer with dimension-agnostic Amount constructors and .InstrumentCode(): - Add protoMoneyToAmount() helper in grpc_mappers.go that converts a google.type.Money proto to domain.Amount using the account's instrument for precision-aware, dimension-agnostic conversion (supports CURRENCY, ENERGY, COMPUTE, CARBON, etc.) - grpc_deposit_endpoints.go: replace NewMoney() calls with protoMoneyToAmount() - grpc_withdrawal_execute.go: same replacement - grpc_withdrawal_manage.go: same replacement with minor-unit variable rename - lien_service.go: - Move account prefetch before amount parsing to provide instrument context - Add instrument code validation in legacy lien path before amount conversion - Replace NewMoney(account.Balance().InstrumentCode(), ...) with NewAmountFromInstrument(..., account.Balance().Dimension(), ...) at available-balance calculation sites - Remove unused protoToMoney() method and commonpb import * fix: address CodeRabbit review feedback on Amount conversion helpers - toMoneyAmount: make precision-aware (was hardcoded for 2-decimal currency) - use scale=10^precision and nanosPerMinor=10^(9-precision) for any instrument - protoMoneyToAmount: fix signed rounding and add post-rounding overflow guard - half-up rounding (+ half/nanosPerUnit) incorrectly rounded negative nanos toward zero - use half-away-from-zero branching on sign so negative amounts round correctly - add post-rounding overflow guard to catch unitsMinor+nanosMinor overflow - grpc_deposit_endpoints: add nil guard for req.Amount before account fetch - lien_service: fall back to account.AvailableBalance() if NewAmountFromInstrument fails * fix: replace NewMoney with NewAmountFromInstrument in valuation lien path The valuation path was hardcoding `* 100` (2-decimal assumption) and calling domain.NewMoney which only handles CURRENCY dimension. Replace with precision-aware decimal.Shift(precision) scaling and NewAmountFromInstrument so energy and other non-2-decimal instruments work correctly. * fix: address second-round CodeRabbit findings on Amount conversion - grpc_deposit_endpoints: use opStatusInvalidAmount instead of operationStatusInvalidCurrency when protoMoneyToAmount fails - grpc_mappers: add explicit precision bounds check [0..9] in protoMoneyToAmount; add ErrInvalidPrecision sentinel error --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent cc33aea commit 2499c7d

5 files changed

Lines changed: 169 additions & 138 deletions

File tree

‎services/current-account/service/grpc_deposit_endpoints.go‎

Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package service
33
import (
44
"context"
55
"errors"
6-
"math"
76
"time"
87

98
pb "github.com/meridianhub/meridian/api/proto/meridian/current_account/v1"
@@ -101,6 +100,12 @@ func (s *Service) ExecuteDeposit(ctx context.Context, req *pb.ExecuteDepositRequ
101100
}()
102101
}
103102

103+
// Validate amount is present before account fetch to fail fast on malformed requests.
104+
if req.Amount == nil || req.Amount.Amount == nil {
105+
operationStatus = opStatusMissingAmount
106+
return nil, status.Error(codes.InvalidArgument, "amount is required")
107+
}
108+
104109
// Retrieve account (context carries organization for multi-tenant routing)
105110
account, err := s.repo.FindByID(ctx, req.AccountId)
106111
if err != nil {
@@ -120,36 +125,13 @@ func (s *Service) ExecuteDeposit(ctx context.Context, req *pb.ExecuteDepositRequ
120125
account.Balance().InstrumentCode(), req.Amount.Amount.CurrencyCode)
121126
}
122127

123-
// Convert amount from proto (MoneyAmount wraps google.type.Money)
124-
// Validate overflow: Units*100 must not overflow int64
125-
if req.Amount.Amount.Units > math.MaxInt64/100 || req.Amount.Amount.Units < math.MinInt64/100 {
126-
operationStatus = opStatusAmountOverflow
127-
return nil, status.Errorf(codes.InvalidArgument,
128-
"amount too large: units %d would overflow", req.Amount.Amount.Units)
129-
}
130-
131-
// Convert to cents preserving precision
132-
unitsCents := req.Amount.Amount.Units * 100
133-
// Round nanos to nearest cent (0.5 rounds up)
134-
nanosCents := (req.Amount.Amount.Nanos + 5000000) / 10000000
135-
136-
// Use Money.Add to safely handle potential overflow from adding nanosCents
137-
centsMoney, err := domain.NewMoney(req.Amount.Amount.CurrencyCode, unitsCents)
138-
if err != nil {
139-
operationStatus = operationStatusInvalidCurrency
140-
return nil, status.Errorf(codes.InvalidArgument, "invalid currency: %v", err)
141-
}
142-
143-
nanosMoney, err := domain.NewMoney(req.Amount.Amount.CurrencyCode, int64(nanosCents))
128+
// Convert amount from proto (MoneyAmount wraps google.type.Money) using the account's instrument.
129+
// The account's instrument determines dimension and precision; the proto CurrencyCode is already
130+
// validated against account.Balance().InstrumentCode() above.
131+
amount, err := protoMoneyToAmount(req.Amount, account)
144132
if err != nil {
145-
operationStatus = operationStatusInvalidCurrency
146-
return nil, status.Errorf(codes.InvalidArgument, "invalid currency: %v", err)
147-
}
148-
149-
amount, err := centsMoney.Add(nanosMoney)
150-
if err != nil {
151-
operationStatus = operationStatusInvalidCurrency
152-
return nil, status.Errorf(codes.InvalidArgument, "invalid currency: %v", err)
133+
operationStatus = opStatusInvalidAmount
134+
return nil, status.Errorf(codes.InvalidArgument, "invalid amount: %v", err)
153135
}
154136

155137
// Validate amount is positive
@@ -162,7 +144,7 @@ func (s *Service) ExecuteDeposit(ctx context.Context, req *pb.ExecuteDepositRequ
162144
if amountCents <= 0 {
163145
operationStatus = opStatusInvalidAmount
164146
return nil, status.Errorf(codes.InvalidArgument,
165-
"deposit amount must be positive, got %d cents", amountCents)
147+
"deposit amount must be positive, got %d minor units", amountCents)
166148
}
167149

168150
// Generate transaction ID (full UUID required by position-keeping service)

‎services/current-account/service/grpc_mappers.go‎

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package service
22

33
import (
4+
"fmt"
45
"log/slog"
6+
"math"
57

68
commonpb "github.com/meridianhub/meridian/api/proto/meridian/common/v1"
79
pb "github.com/meridianhub/meridian/api/proto/meridian/current_account/v1"
@@ -50,17 +52,21 @@ func safeMinorUnits(m domain.Amount) int64 {
5052
}
5153

5254
func toMoneyAmount(m domain.Amount) *commonpb.MoneyAmount {
53-
amountCents := safeMinorUnits(m)
54-
units := amountCents / 100
55-
remainder := amountCents % 100
55+
amountMinor := safeMinorUnits(m)
56+
precision := m.Instrument().Precision
57+
// #nosec G115 - precision is bounded by instrument definition (0-9 in practice)
58+
scale := int64(math.Pow10(precision))
59+
nanosPerMinor := int64(math.Pow10(9 - precision))
5660

57-
// Convert remainder to nanos (9 digits, but we only use 8 for cents precision)
58-
// Per google.type.Money spec: nanos MUST share the sign of units
61+
units := amountMinor / scale
62+
remainder := amountMinor % scale
63+
64+
// Per google.type.Money spec: nanos MUST share the sign of units.
5965
// - Positive amounts: both units and nanos are positive or zero
6066
// - Negative amounts: both units and nanos are negative or zero
61-
// Example: -£1.23 = Units=-1, Nanos=-230000000
62-
// #nosec G115 - remainder is always -99 to 99, multiplication result fits in int32
63-
nanos := int32(remainder * 10000000)
67+
// remainder already shares the sign of amountMinor due to Go's truncating division.
68+
// #nosec G115 - remainder is always -(scale-1) to (scale-1); product fits in int32
69+
nanos := int32(remainder * nanosPerMinor)
6470

6571
return &commonpb.MoneyAmount{
6672
Amount: &money.Money{
@@ -114,6 +120,72 @@ func mapRegistryDimension(registryDimension string) string {
114120
return registryDimension
115121
}
116122

123+
// protoMoneyToAmount converts a proto MoneyAmount to a domain Amount using the account's
124+
// instrument for dimension-agnostic precision support. The proto CurrencyCode field carries
125+
// the instrument code (e.g. "GBP" or "KWH"); the account's dimension and precision are used
126+
// to construct the correct minor-unit Amount regardless of instrument type.
127+
//
128+
// The conversion formula is precision-aware:
129+
//
130+
// scale = 10^precision
131+
// nanosPerUnit = 10^(9-precision)
132+
// totalMinor = Units * scale + round(Nanos / nanosPerUnit)
133+
//
134+
// For 2-decimal CURRENCY (e.g. GBP): scale=100, nanosPerUnit=10_000_000
135+
// For 3-decimal ENERGY (e.g. KWH): scale=1000, nanosPerUnit=1_000_000
136+
func protoMoneyToAmount(amount *commonpb.MoneyAmount, account domain.CurrentAccount) (domain.Amount, error) {
137+
if amount == nil || amount.Amount == nil {
138+
return domain.Amount{}, ErrAmountRequired
139+
}
140+
141+
precision := account.Balance().Instrument().Precision
142+
// google.type.Money uses nanos (10^9), so precision must be in [0..9].
143+
// Values outside this range cannot be represented in nanos without losing precision.
144+
if precision < 0 || precision > 9 {
145+
return domain.Amount{}, fmt.Errorf("%w: got %d", ErrInvalidPrecision, precision)
146+
}
147+
// #nosec G115 - precision is bounded by instrument definition (0-9 in practice)
148+
scale := int64(math.Pow10(precision))
149+
nanosPerUnit := int64(math.Pow10(9 - precision))
150+
151+
// Overflow guard: reject units that would cause Units*scale to overflow int64.
152+
// Using the same boundary as math.MaxInt64/scale (integer division) which equals
153+
// the largest units value where units*scale does not overflow.
154+
if amount.Amount.Units > math.MaxInt64/scale || amount.Amount.Units < math.MinInt64/scale {
155+
return domain.Amount{}, fmt.Errorf("%w: units %d would overflow for precision %d",
156+
ErrAmountOverflow, amount.Amount.Units, precision)
157+
}
158+
159+
unitsMinor := amount.Amount.Units * scale
160+
161+
// Round nanos to nearest minor unit using half-away-from-zero (banker's-round alternative).
162+
// Simple half-up (+ half) rounds negative nanos toward zero, which is incorrect.
163+
// We must branch on sign so that -5_000_001 rounds to -1, not 0.
164+
nanos := int64(amount.Amount.Nanos)
165+
half := nanosPerUnit / 2
166+
var nanosMinor int64
167+
if nanos >= 0 {
168+
nanosMinor = (nanos + half) / nanosPerUnit
169+
} else {
170+
nanosMinor = (nanos - half) / nanosPerUnit
171+
}
172+
173+
// Post-rounding overflow guard: unitsMinor + nanosMinor must not overflow int64.
174+
if (nanosMinor > 0 && unitsMinor > math.MaxInt64-nanosMinor) ||
175+
(nanosMinor < 0 && unitsMinor < math.MinInt64-nanosMinor) {
176+
return domain.Amount{}, fmt.Errorf("%w: total minor units would overflow after nanos rounding",
177+
ErrAmountOverflow)
178+
}
179+
totalMinor := unitsMinor + nanosMinor
180+
181+
return domain.NewAmountFromInstrument(
182+
account.Balance().InstrumentCode(),
183+
account.Balance().Dimension(),
184+
precision,
185+
totalMinor,
186+
)
187+
}
188+
117189
func mapStatusToProto(status domain.AccountStatus) pb.AccountStatus {
118190
switch status {
119191
case domain.AccountStatusActive:

‎services/current-account/service/grpc_withdrawal_execute.go‎

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"math"
87
"time"
98

109
"github.com/google/uuid"
@@ -208,36 +207,13 @@ func (s *Service) ExecuteWithdrawal(ctx context.Context, req *pb.ExecuteWithdraw
208207
account.Balance().InstrumentCode(), reqAmount.Amount.CurrencyCode)
209208
}
210209

211-
// Convert amount from proto (MoneyAmount wraps google.type.Money)
212-
// Validate overflow: Units*100 must not overflow int64
213-
if reqAmount.Amount.Units > math.MaxInt64/100 || reqAmount.Amount.Units < math.MinInt64/100 {
214-
operationStatus = opStatusAmountOverflow
215-
return nil, status.Errorf(codes.InvalidArgument,
216-
"amount too large: units %d would overflow", reqAmount.Amount.Units)
217-
}
218-
219-
// Convert to cents preserving precision
220-
unitsCents := reqAmount.Amount.Units * 100
221-
// Round nanos to nearest cent (0.5 rounds up)
222-
nanosCents := (reqAmount.Amount.Nanos + 5000000) / 10000000
223-
224-
// Use Money.Add to safely handle potential overflow from adding nanosCents
225-
centsMoney, err := domain.NewMoney(reqAmount.Amount.CurrencyCode, unitsCents)
226-
if err != nil {
227-
operationStatus = operationStatusInvalidCurrency
228-
return nil, status.Errorf(codes.InvalidArgument, "invalid currency: %v", err)
229-
}
230-
231-
nanosMoney, err := domain.NewMoney(reqAmount.Amount.CurrencyCode, int64(nanosCents))
232-
if err != nil {
233-
operationStatus = operationStatusInvalidCurrency
234-
return nil, status.Errorf(codes.InvalidArgument, "invalid currency: %v", err)
235-
}
236-
237-
amount, err := centsMoney.Add(nanosMoney)
210+
// Convert amount from proto (MoneyAmount wraps google.type.Money) using the account's instrument.
211+
// The account's instrument determines dimension and precision; the proto CurrencyCode is already
212+
// validated against account.Balance().InstrumentCode() above.
213+
amount, err := protoMoneyToAmount(reqAmount, account)
238214
if err != nil {
239215
operationStatus = operationStatusInvalidCurrency
240-
return nil, status.Errorf(codes.InvalidArgument, "invalid currency: %v", err)
216+
return nil, status.Errorf(codes.InvalidArgument, "invalid amount: %v", err)
241217
}
242218

243219
// Validate amount is positive
@@ -250,7 +226,7 @@ func (s *Service) ExecuteWithdrawal(ctx context.Context, req *pb.ExecuteWithdraw
250226
if amountCents <= 0 {
251227
operationStatus = opStatusInvalidAmount
252228
return nil, status.Errorf(codes.InvalidArgument,
253-
"withdrawal amount must be positive, got %d cents", amountCents)
229+
"withdrawal amount must be positive, got %d minor units", amountCents)
254230
}
255231

256232
// Generate transaction ID (full UUID required by position-keeping service)

‎services/current-account/service/grpc_withdrawal_manage.go‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"math"
87
"time"
98

109
"github.com/google/uuid"
@@ -78,28 +77,29 @@ func (s *Service) InitiateWithdrawal(ctx context.Context, req *pb.InitiateWithdr
7877
account.Balance().InstrumentCode(), req.Amount.Amount.CurrencyCode)
7978
}
8079

81-
// Validate overflow: Units*100 must not overflow int64
82-
if req.Amount.Amount.Units > math.MaxInt64/100 || req.Amount.Amount.Units < math.MinInt64/100 {
83-
operationStatus = opStatusAmountOverflow
84-
return nil, status.Errorf(codes.InvalidArgument,
85-
"amount too large: units %d would overflow", req.Amount.Amount.Units)
80+
// Convert amount from proto using the account's instrument (supports any dimension/precision).
81+
// The proto CurrencyCode is already validated against the account's instrument code above.
82+
amount, err := protoMoneyToAmount(req.Amount, account)
83+
if err != nil {
84+
operationStatus = operationStatusInvalidCurrency
85+
return nil, status.Errorf(codes.InvalidArgument, "invalid amount: %v", err)
8686
}
8787

88-
// Convert and validate amount
89-
unitsCents := req.Amount.Amount.Units * 100
90-
nanosCents := (req.Amount.Amount.Nanos + 5000000) / 10000000
91-
amountCents := unitsCents + int64(nanosCents)
92-
93-
if amountCents <= 0 {
88+
amountMinor, err := amount.ToMinorUnits()
89+
if err != nil {
90+
operationStatus = opStatusAmountOverflow
91+
return nil, status.Errorf(codes.InvalidArgument, "amount too large: %v", err)
92+
}
93+
if amountMinor <= 0 {
9494
operationStatus = opStatusInvalidAmount
9595
return nil, status.Errorf(codes.InvalidArgument, "withdrawal amount must be positive")
9696
}
9797

9898
// Check available balance (warning only - balance could change before execution)
99-
availCents, _ := account.AvailableBalance().ToMinorUnits()
100-
if amountCents > availCents {
99+
availMinor, _ := account.AvailableBalance().ToMinorUnits()
100+
if amountMinor > availMinor {
101101
validationMessages = append(validationMessages,
102-
fmt.Sprintf("Warning: requested amount (%d cents) exceeds current available balance (%d cents)", amountCents, availCents))
102+
fmt.Sprintf("Warning: requested amount (%d minor units) exceeds current available balance (%d minor units)", amountMinor, availMinor))
103103
}
104104

105105
// Use provided reference if available, otherwise generate one
@@ -108,13 +108,6 @@ func (s *Service) InitiateWithdrawal(ctx context.Context, req *pb.InitiateWithdr
108108
reference = fmt.Sprintf("WTH-%s", uuid.New().String()[:8])
109109
}
110110

111-
// Create domain Money from amount cents
112-
amount, err := domain.NewMoney(req.Amount.Amount.CurrencyCode, amountCents)
113-
if err != nil {
114-
operationStatus = operationStatusInvalidCurrency
115-
return nil, status.Errorf(codes.InvalidArgument, "invalid currency: %v", err)
116-
}
117-
118111
// Create domain withdrawal
119112
domainWithdrawal, err := domain.NewWithdrawal(account.ID(), amount, reference)
120113
if err != nil {
@@ -138,7 +131,7 @@ func (s *Service) InitiateWithdrawal(ctx context.Context, req *pb.InitiateWithdr
138131
"withdrawal_id", domainWithdrawal.ID,
139132
"withdrawal_reference", reference,
140133
"account_id", req.AccountId,
141-
"amount_cents", amountCents)
134+
"amount_minor_units", amountMinor)
142135

143136
// Convert domain withdrawal to proto
144137
withdrawal := toProtoWithdrawal(domainWithdrawal, req.AccountId)

0 commit comments

Comments
 (0)