feat(s5): domain model — PayoutOrder aggregate, state machine, events, ports (STA-147)#152
Merged
Merged
Conversation
…, ports (STA-147) 35 new files implementing the S5 Fiat Off-Ramp domain model: - PayoutOrder aggregate root with 12-transition state machine (PENDING → REDEEMING → REDEEMED → PAYOUT_INITIATED → PAYOUT_PROCESSING → COMPLETED) plus HOLD_STABLECOIN path (PENDING → STABLECOIN_HELD → COMPLETED) and failure/escalation paths to REDEMPTION_FAILED, PAYOUT_FAILED, MANUAL_REVIEW - StablecoinRedemption entity (child of PayoutOrder) - OffRampTransaction entity (audit trail) - Value objects: Money, BankAccount, MobileMoneyAccount, PartnerIdentifier, StablecoinTicker, PaymentRail enum - 4 domain events: StablecoinRedeemedEvent, FiatPayoutInitiatedEvent, FiatPayoutCompletedEvent, FiatPayoutFailedEvent - 6 ports: PayoutOrderRepository, StablecoinRedemptionRepository, OffRampTransactionRepository, RedemptionGateway (Circle USDC), PayoutPartnerGateway (Modulr SEPA), PayoutEventPublisher - 4 port DTOs: RedemptionRequest/Result, PayoutRequest/Result - 4 domain exceptions: FR-1003, FR-2001, FR-2002, FR-2003 - Reusable StateMachine<S,T> with StateTransition and StateMachineException - Invariants: fiat_amount tolerance ±0.01, COMPLETED terminal, PAYOUT_INITIATED requires REDEEMED, HOLD_STABLECOIN type guard All 8 existing ArchUnit tests pass. Zero Spring/JPA imports in domain layer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughThis PR introduces the foundational domain layer for a fiat off-ramp payment system. It defines domain models, value objects, aggregates, port abstractions, event types, custom exceptions, and a generic state machine infrastructure to support stablecoin redemption and fiat payout workflows. Changes
Sequence DiagramsequenceDiagram
participant Client
participant PayoutOrder as PayoutOrder<br/>(Aggregate)
participant RedemptionGW as RedemptionGateway
participant PayoutGW as PayoutPartnerGateway
participant EventPublisher
Client->>PayoutOrder: create(paymentId, correlationId, ...)
PayoutOrder->>PayoutOrder: validate inputs, init PENDING
Client->>PayoutOrder: startRedemption()
PayoutOrder->>PayoutOrder: transition to REDEEMING
Client->>RedemptionGW: redeem(RedemptionRequest)
RedemptionGW-->>Client: RedemptionResult
Client->>PayoutOrder: completeRedemption(fiatAmount)
PayoutOrder->>PayoutOrder: validate amount tolerance<br/>transition to REDEEMED
PayoutOrder->>EventPublisher: publish(StablecoinRedeemedEvent)
Client->>PayoutOrder: initiatePayout(partnerRef)
PayoutOrder->>PayoutOrder: transition to PAYOUT_INITIATED
Client->>PayoutGW: initiatePayout(PayoutRequest)
PayoutGW-->>Client: PayoutResult
Client->>PayoutOrder: markPayoutProcessing()
PayoutOrder->>PayoutOrder: transition to PAYOUT_PROCESSING
Client->>PayoutOrder: completePayout(partnerRef, settledAt)
PayoutOrder->>PayoutOrder: transition to COMPLETED
PayoutOrder->>EventPublisher: publish(FiatPayoutCompletedEvent)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The PayoutOrder aggregate (364 lines) requires careful validation of state transition logic, invariant enforcement (FX rate tolerance checks), and immutability preservation via builder pattern. Multiple domain-driven value objects with compact constructor validation and port abstraction boundaries demand verification of contracts. While most supporting types are straightforward records, the heterogeneity of 30+ new types and interconnected validation rules elevates complexity. Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/event/FiatPayoutInitiatedEvent.java`:
- Around line 7-16: The event currently types the payment rail as String; change
the FiatPayoutInitiatedEvent record to use the existing PaymentRail enum for the
paymentRail component so callers get compile-time safety (update the record
signature in FiatPayoutInitiatedEvent to accept PaymentRail instead of String
and adjust any creators/consumers to pass/serialize PaymentRail values at
boundaries); if external serialization requires a string, convert PaymentRail
to/from its string form only at the boundary code that serializes/deserializes
the event.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/exception/PayoutNotRefundableException.java`:
- Around line 9-19: Create a shared ErrorCodeException interface with a single
method getErrorCode() and have the FR domain exceptions implement it;
specifically add public interface ErrorCodeException { String getErrorCode(); }
and update PayoutNotRefundableException to implement ErrorCodeException (replace
or add the existing errorCode() accessor with getErrorCode() returning the
ERROR_CODE constant), and apply the same change to PayoutPartnerException,
PayoutNotFoundException, and RedemptionFailedException so upstream handlers can
uniformly call getErrorCode() on any of these exception types.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/BankAccount.java`:
- Around line 3-23: The compact constructor in record BankAccount currently
checks for null/blank country but doesn't enforce ISO 3166-1 alpha-2
length/format; update the BankAccount compact constructor to additionally
validate country matches two uppercase letters (e.g., using a regex like
"^[A-Z]{2}$" or by normalizing to upper case then testing length and letters)
and throw an IllegalArgumentException with a clear message if it fails; ensure
validation occurs after the existing null/blank check and reference the
BankAccount record and its compact constructor when making the change.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/MobileMoneyAccount.java`:
- Around line 9-19: Add phone number format validation inside the
MobileMoneyAccount constructor: beyond the existing null/blank checks for
provider, phoneNumber, and country, validate phoneNumber conforms to E.164
(e.g., starts with '+' followed by 10–15 digits) or at minimum enforce a
sensible length and digit-only rules; if validation fails, throw an
IllegalArgumentException with a clear message. Keep validation logic localized
to the MobileMoneyAccount constructor (or a small private helper called from it)
so malformed numbers are rejected early while retaining flexibility to
move/adjust the rule if the application layer needs regional exceptions.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/Money.java`:
- Around line 7-10: The compact constructor for Money currently rejects zero
because it checks amount.compareTo(BigDecimal.ZERO) <= 0; decide whether zero
should be allowed and implement one of two fixes: 1) if zero must be valid
(e.g., for fee waivers or deltas), change the validation in the Money compact
constructor from <= 0 to < 0 so only negatives are rejected, or 2) if zero must
remain invalid, make the invariant explicit by renaming the type (e.g.,
PositiveMoney) and/or add a clear Javadoc on the Money record/constructor
documenting that zero is not permitted; update all usages accordingly to reflect
the chosen semantic.
- Around line 11-13: The current currency check in Money (the constructor/setter
that contains the null/isBlank check) is too permissive; replace it with an
ISO-4217 validation by either using java.util.Currency.getInstance(currency)
inside a try/catch to validate the 3-letter uppercase code or validate with a
regex like "^[A-Z]{3}$" before accepting it, and on failure throw an
IllegalArgumentException with a clear message (e.g., "Currency must be a valid
ISO-4217 3-letter code"); alternatively convert the field to use
java.util.Currency type so invalid codes are rejected at assignment.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/OffRampTransaction.java`:
- Line 64: The call to Instant.now() in OffRampTransaction (builder invocation
.receivedAt(Instant.now())) reduces testability; modify the factory/creation
path (the OffRampTransaction builder usage) to accept an explicit Instant or a
Clock so tests can supply a deterministic time—either add an overloaded
factory/constructor that takes an Instant timestamp (e.g.,
create/withReceivedAt(Instant)) or inject a Clock into the component that builds
OffRampTransaction and replace Instant.now() with Instant.now(clock); update all
call sites that currently use .receivedAt(Instant.now()) to pass the provided
Instant or use Instant.now(clock).
- Line 25: The rawResponse field on OffRampTransaction stores unfiltered partner
API responses and must be sanitized in OffRampTransaction.create() before
persistence; update OffRampTransaction.create() to pass rawResponse through a
PII sanitizer (e.g., a new utility method sanitizePartnerResponse or a Sanitizer
service) that masks account numbers, IBANs, card numbers, personal names, and
other identifiers (retain non-PII fields for auditing), then assign the
sanitized string to the rawResponse field; document or enforce that adapter
implementations should not bypass this sanitizer and add unit tests for
OffRampTransaction.create() to verify masking behavior for representative
Modulr/CurrencyCloud/Safaricom responses.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/PayoutOrder.java`:
- Around line 107-175: PayoutOrder.create currently calls Instant.now() and
UUID.randomUUID() directly which hinders testability; change the factory to
accept the createdAt/updatedAt Instant and payoutId (or accept a Clock and a
Supplier<UUID>) as additional parameters so tests can supply deterministic
values, updating PayoutOrder.create (and any callers/tests) to pass those values
instead of relying on Instant.now()/UUID.randomUUID(); keep names
PayoutOrder.create, Instant.now(), and UUID.randomUUID() in mind when locating
the code to modify.
- Around line 354-363: validateFiatAmountTolerance uses
redeemedAmount.multiply(appliedFxRate) without a MathContext/scale causing
non-deterministic precision; change the multiplication to use an explicit
rounding strategy (e.g., use BigDecimal.multiply(BigDecimal, MathContext) or
setScale after multiplication with a defined RoundingMode) so expectedFiat has a
fixed scale/precision before the subtraction and comparison to
FIAT_AMOUNT_TOLERANCE; update references in validateFiatAmountTolerance
(expectedFiat, redeemedAmount, appliedFxRate) accordingly to ensure all
financial math uses the same MathContext or scale/rounding.
- Around line 275-283: The failPayout method in PayoutOrder lacks the defensive
guard present in failRedemption; add a call to assertNotTerminal() at the start
of failPayout (before invoking STATE_MACHINE.transition and building the new
object) so terminal orders cannot be transitioned, mirroring the behavior in
failRedemption and keeping PayoutOrder consistent and safe.
- Around line 215-223: Add the same terminal-state guard used elsewhere: call
assertNotTerminal(status, "failRedemption") at the start of failRedemption() to
prevent mutating terminal orders; then proceed with the
STATE_MACHINE.transition(status, FAIL_REDEMPTION) and the toBuilder() update.
This ensures failRedemption() checks for terminal states (e.g.,
REDEMPTION_FAILED or other TERMINAL_STATES) and yields a clearer error before
attempting the transition.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/PayoutStatus.java`:
- Around line 3-14: Add state-query helper methods to the PayoutStatus enum to
centralize domain checks: implement isTerminal(), isFailed(), and
requiresManualIntervention() on PayoutStatus so callers can use
status.isTerminal(), status.isFailed(), and status.requiresManualIntervention()
instead of scattered equality checks; ensure isTerminal returns true for
COMPLETED and STABLECOIN_HELD, isFailed returns true for REDEMPTION_FAILED and
PAYOUT_FAILED, and requiresManualIntervention returns true for MANUAL_REVIEW.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/StablecoinRedemption.java`:
- Line 23: The fiatCurrency field in StablecoinRedemption should not be a raw
String; change the field type to java.util.Currency (or a dedicated Currency
value object) and update the StablecoinRedemption constructor and any
getters/setters to accept/return Currency instead of String; validate incoming
codes at construction (e.g., Currency.getInstance or value object factory) and
throw an IllegalArgumentException for invalid codes so only ISO 4217 currencies
are allowed (update any usages of StablecoinRedemption, factory methods,
builders, and serialization logic accordingly).
- Around line 32-69: The factory method StablecoinRedemption.create embeds
system calls Instant.now() and UUID.randomUUID(), making tests hard to control;
refactor by injecting a Clock and a Supplier<UUID> (or overloaded create
accepting redemptionId and redeemedAt) so callers/tests can provide
deterministic values—update the create signature to accept a Clock and
Supplier<UUID> (or optional redemptionId/Instant parameters), replace
Instant.now() with clock.instant() and UUID.randomUUID() with idSupplier.get(),
and keep the existing validations (payoutId, stablecoin, redeemedAmount,
fiatReceived, fiatCurrency, partner, partnerReference) unchanged so production
callers can pass Clock.systemUTC()/UUID::randomUUID while tests pass mocks.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/StablecoinTicker.java`:
- Around line 7-13: The SUPPORTED static Map in StablecoinTicker currently
hardcodes assets and should be externalized to configuration so new stablecoins
can be added without redeploying; replace the private static final Map<String,
StablecoinInfo> SUPPORTED with a configurable bean (e.g., bind a Map<String,
StablecoinInfo> via `@ConfigurationProperties` or load from application.yml) and
inject that bean into StablecoinTicker (or provide a constructor/factory that
accepts the map), or alternatively load the registry from a JSON/YAML file at
startup and populate the map; if the current static approach is intentional for
compliance, add a clear comment and documentation stating that each change
requires code review and redeployment.
- Line 12: The StablecoinTicker mapping entry for "RLUSD" currently uses new
StablecoinInfo("ripple", 6) which is the wrong decimal precision; update that
entry to use the correct precision (set to 18 for ERC-20 usage, or 15 if you
intend XRPL-specific behavior) so amount math is correct, or make decimals
configurable per-network; modify the "RLUSD" value in the StablecoinTicker
mapping to new StablecoinInfo("ripple", 18) (or implement a network-aware
configuration in the StablecoinInfo/StablecoinTicker logic if you need XRPL
support).
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutEventPublisher.java`:
- Around line 3-5: The public API PayoutEventPublisher.publish(Object) is too
permissive; change the contract to accept a domain event supertype by
introducing a sealed/interface type (e.g., PayoutDomainEvent) and updating
PayoutEventPublisher.publish to void publish(PayoutDomainEvent event); then make
existing event records (FiatPayoutCompletedEvent, FiatPayoutFailedEvent, etc.)
implement/extend PayoutDomainEvent; update any callers to pass the typed events
and adjust implementations of PayoutEventPublisher accordingly to accept the new
interface.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutOrderRepository.java`:
- Around line 20-22: The repository methods findByStatus and findByRecipientId
currently return unbounded List<PayoutOrder>; change their signatures to support
pagination by accepting a Pageable parameter or by returning
Page<PayoutOrder>/Slice<PayoutOrder> (e.g., findByStatus(PayoutStatus status,
Pageable pageable) or Page<PayoutOrder> findByStatus(...)), and similarly update
findByRecipientId(UUID recipientId, Pageable pageable) or return
Page<PayoutOrder>; update any callers and tests to pass Pageable and handle
Page/Slice results accordingly.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutPartnerGateway.java`:
- Around line 7-10: The PayoutPartnerGateway interface's synchronous method
initiatePayout can propagate runtime/network exceptions; update the application
service(s) that call PayoutPartnerGateway.initiatePayout to catch
RuntimeException (and timeouts) and map failures to domain states like
PAYOUT_FAILED or MANUAL_REVIEW, ensuring the failure path persists/retries as
needed; alternatively, change the port signature to return a failure wrapper
(e.g., Either<PayoutError, PayoutResult>) and adapt callers to handle the error
case consistently.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutRequest.java`:
- Around line 17-25: Add validation to the PayoutRequest record to enforce that
exactly the account type required by paymentRail is provided: in the
PayoutRequest compact constructor (or a static factory) check paymentRail and
assert that when paymentRail indicates a bank rail (e.g.,
paymentRail.requiresBankAccount() or paymentRail.isBank()) bankAccount is
non-null and mobileMoneyAccount is null, and conversely when paymentRail
indicates mobile money the mobileMoneyAccount is non-null and bankAccount is
null; throw an IllegalArgumentException with a clear message if the invariant is
violated so adapters cannot NPE when accessing the wrong account field.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutResult.java`:
- Around line 5-9: PayoutResult currently allows a null partnerReference; add a
null-check for partnerReference in the record's compact constructor (i.e., add a
compact ctor in PayoutResult that throws NPE or IllegalArgumentException when
partnerReference is null/blank) to guarantee non-null reconciliation IDs, and
leave a note in the Javadoc for the status field (or convert mapping in the
application layer) to document expected values or map status Strings to a domain
enum during ingestion/translation.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/RedemptionRequest.java`:
- Around line 6-10: RedemptionRequest currently accepts nullable/invalid fields;
add a compact-record constructor on RedemptionRequest to validate inputs:
requireNonNull(payoutId) and requireNonNull(amount), requireNonNull(stablecoin)
(or better, change the stablecoin field type from String to the Strongly-typed
StablecoinTicker), and enforce amount.signum() > 0 (throw
IllegalArgumentException for non-positive amounts). Use Objects.requireNonNull
for null checks and clear IllegalArgumentException messages for business
invariants so invalid DTOs are rejected at construction time; update any call
sites to pass a StablecoinTicker if you change the type.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/RedemptionResult.java`:
- Around line 6-11: The RedemptionResult record lacks defensive null-checks for
required fields; add a compact constructor on RedemptionResult that validates
partnerReference and fiatReceived are non-null (and optionally non-empty for
partnerReference), throwing a clear NPE or IllegalArgumentException if violated
so malformed data from RedemptionGateway cannot propagate into domain logic;
ensure the validations reference the record components partnerReference and
fiatReceived (and optionally fiatCurrency/redeemedAt if you want them validated
too).
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/statemachine/StateMachine.java`:
- Around line 13-19: The transition(S currentState, T trigger) method doesn't
guard against null inputs and calling t.from().equals(currentState) or
t.trigger().equals(trigger) can throw NPEs; at the top of transition add
defensive null checks for currentState and trigger and fail fast (e.g., throw
StateMachineException.invalidTransition(currentState, trigger) or a clear
IllegalArgumentException) before streaming transitions so comparisons like
StateTransition::from and StateTransition::trigger never call equals on a null;
update any tests that expect null handling accordingly.
- Around line 21-24: The canTransition(S currentState, T trigger) method needs
the same null-safety as elsewhere: avoid calling equals on possibly-null values
by using null-safe comparisons (e.g., Objects.equals) or explicit null guards
when comparing currentState to t.from() and trigger to t.trigger(); update the
stream predicate in canTransition to use Objects.equals(currentState, t.from())
&& Objects.equals(trigger, t.trigger()) (or equivalent null checks) so no NPE
occurs when any side is null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 15743598-2f8d-4d26-809a-f99915394ae9
📒 Files selected for processing (35)
fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/event/FiatPayoutCompletedEvent.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/event/FiatPayoutFailedEvent.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/event/FiatPayoutInitiatedEvent.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/event/StablecoinRedeemedEvent.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/exception/PayoutNotFoundException.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/exception/PayoutNotRefundableException.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/exception/PayoutPartnerException.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/exception/RedemptionFailedException.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/AccountType.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/BankAccount.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/MobileMoneyAccount.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/MobileMoneyProvider.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/Money.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/OffRampTransaction.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/PartnerIdentifier.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/PaymentRail.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/PayoutOrder.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/PayoutStatus.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/PayoutTrigger.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/PayoutType.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/StablecoinRedemption.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/StablecoinTicker.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/OffRampTransactionRepository.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutEventPublisher.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutOrderRepository.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutPartnerGateway.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutRequest.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutResult.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/RedemptionGateway.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/RedemptionRequest.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/RedemptionResult.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/StablecoinRedemptionRepository.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/statemachine/StateMachine.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/statemachine/StateMachineException.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/statemachine/StateTransition.java
Comment on lines
+7
to
+16
| public record FiatPayoutInitiatedEvent( | ||
| UUID payoutId, | ||
| UUID paymentId, | ||
| UUID correlationId, | ||
| BigDecimal fiatAmount, | ||
| String targetCurrency, | ||
| String paymentRail, | ||
| String partner, | ||
| Instant initiatedAt | ||
| ) {} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using PaymentRail enum instead of String.
Per the PR, a PaymentRail enum exists. Using it directly provides compile-time safety and prevents invalid values from propagating through events.
♻️ Proposed change
+import com.stablecoin.payments.offramp.domain.model.PaymentRail;
+import com.stablecoin.payments.offramp.domain.model.PartnerIdentifier;
+
public record FiatPayoutInitiatedEvent(
UUID payoutId,
UUID paymentId,
UUID correlationId,
BigDecimal fiatAmount,
String targetCurrency,
- String paymentRail,
- String partner,
+ PaymentRail paymentRail,
+ PartnerIdentifier partner,
Instant initiatedAt
) {}If serialization compatibility with external systems requires strings, convert at the boundary instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/event/FiatPayoutInitiatedEvent.java`
around lines 7 - 16, The event currently types the payment rail as String;
change the FiatPayoutInitiatedEvent record to use the existing PaymentRail enum
for the paymentRail component so callers get compile-time safety (update the
record signature in FiatPayoutInitiatedEvent to accept PaymentRail instead of
String and adjust any creators/consumers to pass/serialize PaymentRail values at
boundaries); if external serialization requires a string, convert PaymentRail
to/from its string form only at the boundary code that serializes/deserializes
the event.
Comment on lines
+9
to
+19
| public class PayoutNotRefundableException extends RuntimeException { | ||
|
|
||
| public static final String ERROR_CODE = "FR-2001"; | ||
|
|
||
| public PayoutNotRefundableException(UUID payoutId, String reason) { | ||
| super("Payout order %s is not refundable: %s".formatted(payoutId, reason)); | ||
| } | ||
|
|
||
| public String errorCode() { | ||
| return ERROR_CODE; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Off-ramp exceptions ==="
rg -n --type=java -C2 'class .*Exception extends RuntimeException|String errorCode\s*\(' fiat-off-rampRepository: Puneethkumarck/stablebridge-platform
Length of output: 6912
Extract error code contract to a shared interface.
All FR domain exceptions replicate the same ERROR_CODE constant and errorCode() method without abstraction. Consumers will need to catch and type-cast each exception individually to extract the error code, defeating the point of a standardized error model.
Define a common ErrorCodeException interface (or abstract base):
public interface ErrorCodeException {
String getErrorCode();
}Then have PayoutNotRefundableException, PayoutPartnerException, PayoutNotFoundException, and RedemptionFailedException implement it. This allows upstream handlers to treat all FR exceptions uniformly and extract codes via polymorphism.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/exception/PayoutNotRefundableException.java`
around lines 9 - 19, Create a shared ErrorCodeException interface with a single
method getErrorCode() and have the FR domain exceptions implement it;
specifically add public interface ErrorCodeException { String getErrorCode(); }
and update PayoutNotRefundableException to implement ErrorCodeException (replace
or add the existing errorCode() accessor with getErrorCode() returning the
ERROR_CODE constant), and apply the same change to PayoutPartnerException,
PayoutNotFoundException, and RedemptionFailedException so upstream handlers can
uniformly call getErrorCode() on any of these exception types.
Comment on lines
+3
to
+23
| public record BankAccount( | ||
| String accountNumber, | ||
| String bankCode, | ||
| AccountType accountType, | ||
| String country | ||
| ) { | ||
|
|
||
| public BankAccount { | ||
| if (accountNumber == null || accountNumber.isBlank()) { | ||
| throw new IllegalArgumentException("Account number is required"); | ||
| } | ||
| if (bankCode == null || bankCode.isBlank()) { | ||
| throw new IllegalArgumentException("Bank code is required"); | ||
| } | ||
| if (accountType == null) { | ||
| throw new IllegalArgumentException("Account type is required"); | ||
| } | ||
| if (country == null || country.isBlank()) { | ||
| throw new IllegalArgumentException("Country is required"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify AccountType enum exists in the codebase
fd -e java AccountTypeRepository: Puneethkumarck/stablebridge-platform
Length of output: 284
🏁 Script executed:
cat fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/AccountType.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 199
🏁 Script executed:
# Search for existing country validation or ISO code patterns
rg -i "iso.*3166|country.*validat|[a-z]{2}.*country" --type javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 50393
🏁 Script executed:
# Check for country-related validation in tests or existing code
rg "country" fiat-off-ramp/fiat-off-ramp/src --type java -A 3 -B 1Repository: Puneethkumarck/stablebridge-platform
Length of output: 2710
AccountType enum confirmed; country code validation is optional.
The AccountType enum exists in the same package. For the country field, adding ISO 3166-1 alpha-2 format validation is optional but recommended, given the database schema constrains this to 2 characters and downstream payment partners expect valid codes.
Optional country code validation
if (country == null || country.isBlank()) {
throw new IllegalArgumentException("Country is required");
}
+ if (!country.matches("^[A-Z]{2}$")) {
+ throw new IllegalArgumentException("Country must be ISO 3166-1 alpha-2 code");
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/BankAccount.java`
around lines 3 - 23, The compact constructor in record BankAccount currently
checks for null/blank country but doesn't enforce ISO 3166-1 alpha-2
length/format; update the BankAccount compact constructor to additionally
validate country matches two uppercase letters (e.g., using a regex like
"^[A-Z]{2}$" or by normalizing to upper case then testing length and letters)
and throw an IllegalArgumentException with a clear message if it fails; ensure
validation occurs after the existing null/blank check and reference the
BankAccount record and its compact constructor when making the change.
Comment on lines
+9
to
+19
| public MobileMoneyAccount { | ||
| if (provider == null) { | ||
| throw new IllegalArgumentException("Provider is required"); | ||
| } | ||
| if (phoneNumber == null || phoneNumber.isBlank()) { | ||
| throw new IllegalArgumentException("Phone number is required"); | ||
| } | ||
| if (country == null || country.isBlank()) { | ||
| throw new IllegalArgumentException("Country is required"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider phone number format validation.
Mobile money transactions rely on accurate phone numbers. A malformed number could cause payout failures at the partner gateway level. Consider validating E.164 format or at minimum length constraints.
♻️ Example E.164 validation
public MobileMoneyAccount {
if (provider == null) {
throw new IllegalArgumentException("Provider is required");
}
if (phoneNumber == null || phoneNumber.isBlank()) {
throw new IllegalArgumentException("Phone number is required");
}
+ if (!phoneNumber.matches("^\\+[1-9]\\d{6,14}$")) {
+ throw new IllegalArgumentException("Phone number must be in E.164 format");
+ }
if (country == null || country.isBlank()) {
throw new IllegalArgumentException("Country is required");
}
}Alternatively, defer format validation to the application layer if flexibility is needed for regional variations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public MobileMoneyAccount { | |
| if (provider == null) { | |
| throw new IllegalArgumentException("Provider is required"); | |
| } | |
| if (phoneNumber == null || phoneNumber.isBlank()) { | |
| throw new IllegalArgumentException("Phone number is required"); | |
| } | |
| if (country == null || country.isBlank()) { | |
| throw new IllegalArgumentException("Country is required"); | |
| } | |
| } | |
| public MobileMoneyAccount { | |
| if (provider == null) { | |
| throw new IllegalArgumentException("Provider is required"); | |
| } | |
| if (phoneNumber == null || phoneNumber.isBlank()) { | |
| throw new IllegalArgumentException("Phone number is required"); | |
| } | |
| if (!phoneNumber.matches("^\\+[1-9]\\d{6,14}$")) { | |
| throw new IllegalArgumentException("Phone number must be in E.164 format"); | |
| } | |
| if (country == null || country.isBlank()) { | |
| throw new IllegalArgumentException("Country is required"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/MobileMoneyAccount.java`
around lines 9 - 19, Add phone number format validation inside the
MobileMoneyAccount constructor: beyond the existing null/blank checks for
provider, phoneNumber, and country, validate phoneNumber conforms to E.164
(e.g., starts with '+' followed by 10–15 digits) or at minimum enforce a
sensible length and digit-only rules; if validation fails, throw an
IllegalArgumentException with a clear message. Keep validation logic localized
to the MobileMoneyAccount constructor (or a small private helper called from it)
so malformed numbers are rejected early while retaining flexibility to
move/adjust the rule if the application layer needs regional exceptions.
Comment on lines
+7
to
+10
| public Money { | ||
| if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { | ||
| throw new IllegalArgumentException("Amount must be positive"); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Zero amount is rejected — verify this is intentional.
The validation amount.compareTo(BigDecimal.ZERO) <= 0 rejects zero-amount Money instances. This is likely correct for payout amounts but could cause issues if Money is reused for fee waivers, balance deltas, or adjustment scenarios.
If zero should remain invalid, consider renaming to PositiveMoney or adding a Javadoc clarifying the invariant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/model/Money.java`
around lines 7 - 10, The compact constructor for Money currently rejects zero
because it checks amount.compareTo(BigDecimal.ZERO) <= 0; decide whether zero
should be allowed and implement one of two fixes: 1) if zero must be valid
(e.g., for fee waivers or deltas), change the validation in the Money compact
constructor from <= 0 to < 0 so only negatives are rejected, or 2) if zero must
remain invalid, make the invariant explicit by renaming the type (e.g.,
PositiveMoney) and/or add a clear Javadoc on the Money record/constructor
documenting that zero is not permitted; update all usages accordingly to reflect
the chosen semantic.
Comment on lines
+5
to
+9
| public record PayoutResult( | ||
| String partnerReference, | ||
| String status, | ||
| Instant settledAt | ||
| ) {} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validation for partnerReference.
partnerReference is critical for reconciliation with external partner. Ensure it's non-null when returned. status as String is flexible for varying partner responses but loses type safety — document expected values or map to domain enum in application layer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/PayoutResult.java`
around lines 5 - 9, PayoutResult currently allows a null partnerReference; add a
null-check for partnerReference in the record's compact constructor (i.e., add a
compact ctor in PayoutResult that throws NPE or IllegalArgumentException when
partnerReference is null/blank) to guarantee non-null reconciliation IDs, and
leave a note in the Javadoc for the status field (or convert mapping in the
application layer) to document expected values or map status Strings to a domain
enum during ingestion/translation.
Comment on lines
+6
to
+10
| public record RedemptionRequest( | ||
| UUID payoutId, | ||
| String stablecoin, | ||
| BigDecimal amount | ||
| ) {} |
There was a problem hiding this comment.
Add input validation and consider stronger typing.
This port DTO lacks null/invariant checks:
payoutId— should be non-nullamount— should be non-null and positivestablecoin— PR introducesStablecoinTickervalue object; consider using it instead of rawStringfor type safety
🛡️ Proposed validation
public record RedemptionRequest(
UUID payoutId,
- String stablecoin,
+ StablecoinTicker stablecoin,
BigDecimal amount
-) {}
+) {
+ public RedemptionRequest {
+ if (payoutId == null) {
+ throw new IllegalArgumentException("Payout ID is required");
+ }
+ if (stablecoin == null) {
+ throw new IllegalArgumentException("Stablecoin is required");
+ }
+ if (amount == null || amount.signum() <= 0) {
+ throw new IllegalArgumentException("Amount must be positive");
+ }
+ }
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/RedemptionRequest.java`
around lines 6 - 10, RedemptionRequest currently accepts nullable/invalid
fields; add a compact-record constructor on RedemptionRequest to validate
inputs: requireNonNull(payoutId) and requireNonNull(amount),
requireNonNull(stablecoin) (or better, change the stablecoin field type from
String to the Strongly-typed StablecoinTicker), and enforce amount.signum() > 0
(throw IllegalArgumentException for non-positive amounts). Use
Objects.requireNonNull for null checks and clear IllegalArgumentException
messages for business invariants so invalid DTOs are rejected at construction
time; update any call sites to pass a StablecoinTicker if you change the type.
Comment on lines
+6
to
+11
| public record RedemptionResult( | ||
| String partnerReference, | ||
| BigDecimal fiatReceived, | ||
| String fiatCurrency, | ||
| Instant redeemedAt | ||
| ) {} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding null-checks for required fields.
As a port DTO returned from the RedemptionGateway, defensive validation on partnerReference and fiatReceived would prevent NPEs from propagating into domain logic if an adapter implementation returns malformed data.
🛡️ Optional: Add compact constructor validation
public record RedemptionResult(
String partnerReference,
BigDecimal fiatReceived,
String fiatCurrency,
Instant redeemedAt
-) {}
+) {
+ public RedemptionResult {
+ Objects.requireNonNull(partnerReference, "partnerReference must not be null");
+ Objects.requireNonNull(fiatReceived, "fiatReceived must not be null");
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public record RedemptionResult( | |
| String partnerReference, | |
| BigDecimal fiatReceived, | |
| String fiatCurrency, | |
| Instant redeemedAt | |
| ) {} | |
| public record RedemptionResult( | |
| String partnerReference, | |
| BigDecimal fiatReceived, | |
| String fiatCurrency, | |
| Instant redeemedAt | |
| ) { | |
| public RedemptionResult { | |
| Objects.requireNonNull(partnerReference, "partnerReference must not be null"); | |
| Objects.requireNonNull(fiatReceived, "fiatReceived must not be null"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/port/RedemptionResult.java`
around lines 6 - 11, The RedemptionResult record lacks defensive null-checks for
required fields; add a compact constructor on RedemptionResult that validates
partnerReference and fiatReceived are non-null (and optionally non-empty for
partnerReference), throwing a clear NPE or IllegalArgumentException if violated
so malformed data from RedemptionGateway cannot propagate into domain logic;
ensure the validations reference the record components partnerReference and
fiatReceived (and optionally fiatCurrency/redeemedAt if you want them validated
too).
Comment on lines
+13
to
+19
| public S transition(S currentState, T trigger) { | ||
| return transitions.stream() | ||
| .filter(t -> t.from().equals(currentState) && t.trigger().equals(trigger)) | ||
| .map(StateTransition::to) | ||
| .findFirst() | ||
| .orElseThrow(() -> StateMachineException.invalidTransition(currentState, trigger)); | ||
| } |
There was a problem hiding this comment.
Null parameters will cause NPE in equals() comparison.
If currentState or trigger is null, the t.from().equals(currentState) call succeeds but comparing with a null trigger could produce unexpected behavior depending on enum/object implementation.
🛡️ Defensive null check
public S transition(S currentState, T trigger) {
+ if (currentState == null || trigger == null) {
+ throw new IllegalArgumentException("currentState and trigger must not be null");
+ }
return transitions.stream()
.filter(t -> t.from().equals(currentState) && t.trigger().equals(trigger))
.map(StateTransition::to)
.findFirst()
.orElseThrow(() -> StateMachineException.invalidTransition(currentState, trigger));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public S transition(S currentState, T trigger) { | |
| return transitions.stream() | |
| .filter(t -> t.from().equals(currentState) && t.trigger().equals(trigger)) | |
| .map(StateTransition::to) | |
| .findFirst() | |
| .orElseThrow(() -> StateMachineException.invalidTransition(currentState, trigger)); | |
| } | |
| public S transition(S currentState, T trigger) { | |
| if (currentState == null || trigger == null) { | |
| throw new IllegalArgumentException("currentState and trigger must not be null"); | |
| } | |
| return transitions.stream() | |
| .filter(t -> t.from().equals(currentState) && t.trigger().equals(trigger)) | |
| .map(StateTransition::to) | |
| .findFirst() | |
| .orElseThrow(() -> StateMachineException.invalidTransition(currentState, trigger)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/statemachine/StateMachine.java`
around lines 13 - 19, The transition(S currentState, T trigger) method doesn't
guard against null inputs and calling t.from().equals(currentState) or
t.trigger().equals(trigger) can throw NPEs; at the top of transition add
defensive null checks for currentState and trigger and fail fast (e.g., throw
StateMachineException.invalidTransition(currentState, trigger) or a clear
IllegalArgumentException) before streaming transitions so comparisons like
StateTransition::from and StateTransition::trigger never call equals on a null;
update any tests that expect null handling accordingly.
Comment on lines
+21
to
+24
| public boolean canTransition(S currentState, T trigger) { | ||
| return transitions.stream() | ||
| .anyMatch(t -> t.from().equals(currentState) && t.trigger().equals(trigger)); | ||
| } |
There was a problem hiding this comment.
Same null-safety gap in canTransition().
Apply consistent defensive checks here as well.
🛡️ Proposed fix
public boolean canTransition(S currentState, T trigger) {
+ if (currentState == null || trigger == null) {
+ return false;
+ }
return transitions.stream()
.anyMatch(t -> t.from().equals(currentState) && t.trigger().equals(trigger));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public boolean canTransition(S currentState, T trigger) { | |
| return transitions.stream() | |
| .anyMatch(t -> t.from().equals(currentState) && t.trigger().equals(trigger)); | |
| } | |
| public boolean canTransition(S currentState, T trigger) { | |
| if (currentState == null || trigger == null) { | |
| return false; | |
| } | |
| return transitions.stream() | |
| .anyMatch(t -> t.from().equals(currentState) && t.trigger().equals(trigger)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/domain/statemachine/StateMachine.java`
around lines 21 - 24, The canTransition(S currentState, T trigger) method needs
the same null-safety as elsewhere: avoid calling equals on possibly-null values
by using null-safe comparisons (e.g., Objects.equals) or explicit null guards
when comparing currentState to t.from() and trigger to t.trigger(); update the
stream predicate in canTransition to use Objects.equals(currentState, t.from())
&& Objects.equals(trigger, t.trigger()) (or equivalent null checks) so no NPE
occurs when any side is null.
This was referenced Mar 9, 2026
Merged
feat(s5): infrastructure persistence — JPA entities, repos, adapters, 24 ITs (STA-149, STA-150)
#154
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
State Machine
Test plan
Closes STA-147
🤖 Generated with Claude Code
Summary by CodeRabbit