Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions blockchain-custody/blockchain-custody-api/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
plugins {
`java-library`
}

dependencies {
api("jakarta.validation:jakarta.validation-api")
api("com.fasterxml.jackson.core:jackson-annotations")

testImplementation("org.junit.jupiter:junit-jupiter")
testImplementation("org.assertj:assertj-core")
testRuntimeOnly("org.junit.platform:junit-platform-launcher")
}

tasks.withType<Test> {
useJUnitPlatform()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.stablecoin.payments.custody.api;

import java.util.List;
import java.util.Map;

public record ApiError(
String code,
String status,
String message,
Map<String, List<String>> errors
) {
public static ApiError of(String code, String status, String message) {
return new ApiError(code, status, message, Map.of());
}

public static ApiError withErrors(String code, String status, String message,
Map<String, List<String>> errors) {
return new ApiError(code, status, message, errors);
}
Comment on lines +16 to +19

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider defensive copy of errors map.

withErrors passes the map directly. If the caller retains a reference and mutates it, the ApiError instance becomes inconsistent. Wrap with Map.copyOf():

♻️ Suggested fix
     public static ApiError withErrors(String code, String status, String message,
                                       Map<String, List<String>> errors) {
-        return new ApiError(code, status, message, errors);
+        return new ApiError(code, status, message, errors == null ? Map.of() : Map.copyOf(errors));
     }
📝 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 static ApiError withErrors(String code, String status, String message,
Map<String, List<String>> errors) {
return new ApiError(code, status, message, errors);
}
public static ApiError withErrors(String code, String status, String message,
Map<String, List<String>> errors) {
return new ApiError(code, status, message, errors == null ? Map.of() : Map.copyOf(errors));
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody-api/src/main/java/com/stablecoin/payments/custody/api/ApiError.java`
around lines 16 - 19, withErrors currently stores the caller's Map reference
directly which can be mutated externally; update withErrors (and/or the ApiError
constructor it calls) to defensively copy the input by producing an unmodifiable
copy such that both the map and its value lists are immutable (e.g., create a
new map from errors where each entry's value is wrapped with List.copyOf(...)
and then wrap the resulting map with Map.copyOf(...)) and pass that copy into
the ApiError instance so external mutations do not affect the ApiError state.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.stablecoin.payments.custody.api;

import java.time.Instant;
import java.util.UUID;

public record TransferResponse(
UUID transferId,
UUID paymentId,
String status,
String chainId,
String stablecoin,
String amount,
String fromAddress,
String toAddress,
String txHash,
String blockNumber,
Integer confirmations,
String gasUsed,
String gasPriceGwei,
Integer estimatedConfirmationS,
Instant confirmedAt,
Instant createdAt
) {}
Comment on lines +6 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Clean DTO structure — consider typed status enum.

Using String for amount, gasUsed, and gasPriceGwei is appropriate for blockchain precision. Nullable confirmations/confirmedAt correctly models pending transfers.

Consider defining a TransferStatus enum instead of String status to provide compile-time safety and explicit state documentation for API consumers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody-api/src/main/java/com/stablecoin/payments/custody/api/TransferResponse.java`
around lines 6 - 23, The TransferResponse record currently uses a raw String for
status; define a new enum TransferStatus (e.g., PENDING, CONFIRMED, FAILED,
REJECTED, etc.) and change the TransferResponse signature to use TransferStatus
status instead of String status; update all places that construct or deserialize
TransferResponse (mappers, builders, controllers, tests) to use TransferStatus
values and, if using Jackson, ensure enum serialization/deserialization behavior
is correct (e.g., `@JsonCreator` or default name mapping) so API consumers and
code get compile-time safety and explicit state documentation.

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.stablecoin.payments.custody.api;

import java.time.Instant;
import java.util.List;
import java.util.UUID;

public record WalletBalanceResponse(
UUID walletId,
String address,
String chainId,
List<BalanceEntry> balances,
Instant updatedAt
) {
public record BalanceEntry(
String stablecoin,
String availableBalance,
String reservedBalance,
String blockchainBalance,
String lastIndexedBlock
) {}
Comment on lines +14 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider type consistency across API modules.

BalanceEntry uses String for monetary fields (availableBalance, etc.), while RefundResponse in fiat-on-ramp-api uses BigDecimal for refundAmount. Both approaches are valid—Strings avoid JSON floating-point precision loss, BigDecimal provides type safety.

For cross-module consistency, consider aligning on one approach for monetary values across both service APIs.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody-api/src/main/java/com/stablecoin/payments/custody/api/WalletBalanceResponse.java`
around lines 14 - 20, BalanceEntry currently models monetary fields as String
(availableBalance, reservedBalance, blockchainBalance) while
RefundResponse/refundAmount in another module uses BigDecimal; pick one
representation across APIs and make the DTOs consistent. Either change
BalanceEntry's monetary fields to BigDecimal (availableBalance, reservedBalance,
blockchainBalance) and ensure Jackson/serialization configuration accepts and
emits BigDecimal consistently, or change RefundResponse.refundAmount to String;
update any equals/hashCode/validation/tests and API consumers accordingly, and
run serialization/deserialization tests to confirm no precision or compatibility
regressions.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.stablecoin.payments.custody.api.events;

import java.time.Instant;
import java.util.UUID;

public record ChainReturnConfirmed(
String schemaVersion,
UUID eventId,
String eventType,
UUID transferId,
UUID parentTransferId,
UUID paymentId,
UUID correlationId,
String chainId,
String txHash,
Instant confirmedAt
) {
public static final String EVENT_TYPE = "chain.return.confirmed";
public static final String SCHEMA_VERSION = "1.0";
}
Comment on lines +6 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Lock down the event identity fields.

Line 7 and Line 9 duplicate metadata that is already fixed by Line 18 and Line 19. Leaving those fields unconstrained means ChainReturnConfirmed instances can carry inconsistent type/version values, which weakens the contract for downstream consumers. Enforce the constants in the constructor path instead of trusting callers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody-api/src/main/java/com/stablecoin/payments/custody/api/events/ChainReturnConfirmed.java`
around lines 6 - 20, The record currently allows callers to pass arbitrary
schemaVersion and eventType despite having fixed constants EVENT_TYPE and
SCHEMA_VERSION; add a compact record constructor in ChainReturnConfirmed that
validates schemaVersion.equals(SCHEMA_VERSION) and eventType.equals(EVENT_TYPE)
and throws an IllegalArgumentException (or NullPointerException for nulls) with
a clear message if they differ, so instances cannot carry inconsistent
type/version values; reference the record name ChainReturnConfirmed and the
component names schemaVersion and eventType when making this change.

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.stablecoin.payments.custody.api.events;

import java.time.Instant;
import java.util.UUID;

public record ChainTransferConfirmed(
String schemaVersion,
UUID eventId,
String eventType,
UUID transferId,
UUID paymentId,
UUID correlationId,
String chainId,
String txHash,
String blockNumber,
Integer confirmations,
String gasUsed,
Instant confirmedAt
) {
public static final String EVENT_TYPE = "chain.transfer.confirmed";
public static final String SCHEMA_VERSION = "1.0";
}
Comment on lines +6 to +22

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Protect the event contract from mismatched metadata.

Line 7 and Line 9 should not be freely settable when this record already defines SCHEMA_VERSION and EVENT_TYPE. A mismatched ChainTransferConfirmed payload is easy to construct here and hard to diagnose once it reaches Kafka consumers. Validate those fields in the canonical constructor or remove them from the caller-facing constructor path.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody-api/src/main/java/com/stablecoin/payments/custody/api/events/ChainTransferConfirmed.java`
around lines 6 - 22, The record ChainTransferConfirmed currently allows callers
to pass arbitrary schemaVersion and eventType which can diverge from the
declared constants; add a canonical constructor for the record (or a static
factory) that validates schemaVersion equals SCHEMA_VERSION and eventType equals
EVENT_TYPE and throw an IllegalArgumentException (or similar) on mismatch, or
alternatively remove those two parameters from the public construction path and
populate them from the constants internally; target the ChainTransferConfirmed
record and its constructor/creation sites to enforce this invariant.

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.stablecoin.payments.custody.api.events;

import java.time.Instant;
import java.util.UUID;

public record ChainTransferFailed(
String schemaVersion,
UUID eventId,
String eventType,
UUID transferId,
UUID paymentId,
UUID correlationId,
String chainId,
String reason,
String errorCode,
Instant failedAt
) {
public static final String EVENT_TYPE = "chain.transfer.failed";
public static final String SCHEMA_VERSION = "1.0";
}
Comment on lines +6 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Enforce fixed event metadata in the record API.

Line 7 and Line 9 make schemaVersion and eventType caller-controlled even though this type already declares fixed constants. That allows invalid ChainTransferFailed payloads to be created and can break consumer routing/deserialization. Prefer a compact constructor that validates both fields, or a secondary constructor/factory that hard-codes them.

Suggested fix
 public record ChainTransferFailed(
         String schemaVersion,
         UUID eventId,
         String eventType,
         UUID transferId,
         UUID paymentId,
         UUID correlationId,
         String chainId,
         String reason,
         String errorCode,
         Instant failedAt
 ) {
     public static final String EVENT_TYPE = "chain.transfer.failed";
     public static final String SCHEMA_VERSION = "1.0";
+
+    public ChainTransferFailed {
+        if (!SCHEMA_VERSION.equals(schemaVersion)) {
+            throw new IllegalArgumentException("Unsupported schemaVersion: " + schemaVersion);
+        }
+        if (!EVENT_TYPE.equals(eventType)) {
+            throw new IllegalArgumentException("Unexpected eventType: " + eventType);
+        }
+    }
 }
📝 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 ChainTransferFailed(
String schemaVersion,
UUID eventId,
String eventType,
UUID transferId,
UUID paymentId,
UUID correlationId,
String chainId,
String reason,
String errorCode,
Instant failedAt
) {
public static final String EVENT_TYPE = "chain.transfer.failed";
public static final String SCHEMA_VERSION = "1.0";
}
public record ChainTransferFailed(
String schemaVersion,
UUID eventId,
String eventType,
UUID transferId,
UUID paymentId,
UUID correlationId,
String chainId,
String reason,
String errorCode,
Instant failedAt
) {
public static final String EVENT_TYPE = "chain.transfer.failed";
public static final String SCHEMA_VERSION = "1.0";
public ChainTransferFailed {
if (!SCHEMA_VERSION.equals(schemaVersion)) {
throw new IllegalArgumentException("Unsupported schemaVersion: " + schemaVersion);
}
if (!EVENT_TYPE.equals(eventType)) {
throw new IllegalArgumentException("Unexpected eventType: " + eventType);
}
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody-api/src/main/java/com/stablecoin/payments/custody/api/events/ChainTransferFailed.java`
around lines 6 - 20, The ChainTransferFailed record exposes caller-controlled
schemaVersion and eventType which should be fixed to the constants EVENT_TYPE
and SCHEMA_VERSION; change ChainTransferFailed to enforce those values by adding
a compact constructor or factory method that validates schemaVersion equals
SCHEMA_VERSION and eventType equals EVENT_TYPE (or omits them from the public
API and sets them internally), and throw IllegalArgumentException on mismatch so
invalid payloads cannot be created; update usages to call the new
factory/constructor or pass only the remaining fields (transferId, paymentId,
correlationId, chainId, reason, errorCode, failedAt).

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.stablecoin.payments.custody.api.events;

import java.time.Instant;
import java.util.UUID;

public record ChainTransferSubmitted(
String schemaVersion,
UUID eventId,
String eventType,
UUID transferId,
UUID paymentId,
UUID correlationId,
String chainId,
String stablecoin,
String amount,
String txHash,
String fromAddress,
String toAddress,
Instant submittedAt
) {
public static final String EVENT_TYPE = "chain.transfer.submitted";
public static final String SCHEMA_VERSION = "1.0";
}
Comment on lines +6 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not let producers override eventType/schemaVersion.

Line 7 and Line 9 should be invariant contract metadata for this event, not arbitrary inputs. As written, a producer can instantiate ChainTransferSubmitted with values that disagree with SCHEMA_VERSION and EVENT_TYPE, which is risky for event dispatch and schema handling. Add constructor validation or expose a constructor/factory that always injects the constants.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody-api/src/main/java/com/stablecoin/payments/custody/api/events/ChainTransferSubmitted.java`
around lines 6 - 23, ChainTransferSubmitted currently allows producers to supply
schemaVersion and eventType; fix by preventing overrides: either remove those
fields from the record signature and always set SCHEMA_VERSION and EVENT_TYPE
internally, or add a compact constructor/validation that asserts the provided
schemaVersion equals SCHEMA_VERSION and eventType equals EVENT_TYPE and throw
IllegalArgumentException otherwise; alternatively add a static factory method
(e.g., ChainTransferSubmitted.create(...)) that omits these two parameters and
injects the constants. Ensure references to the constants SCHEMA_VERSION and
EVENT_TYPE and the record name ChainTransferSubmitted are used so no instance
can be created with mismatched metadata.

16 changes: 16 additions & 0 deletions blockchain-custody/blockchain-custody-client/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
plugins {
`java-library`
}

dependencies {
api(project(":blockchain-custody:blockchain-custody-api"))
implementation("org.springframework.cloud:spring-cloud-starter-openfeign")

testImplementation("org.junit.jupiter:junit-jupiter")
testImplementation("org.assertj:assertj-core")
testRuntimeOnly("org.junit.platform:junit-platform-launcher")
}

tasks.withType<Test> {
useJUnitPlatform()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.stablecoin.payments.custody.client;

import com.stablecoin.payments.custody.api.TransferResponse;
import com.stablecoin.payments.custody.api.WalletBalanceResponse;
import org.springframework.cloud.openfeign.FeignClient;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;

import java.util.UUID;

@FeignClient(name = "blockchain-custody-service", url = "${app.services.blockchain-custody.url}")
public interface BlockchainCustodyClient {

@GetMapping(value = "/v1/transfers/{transferId}", produces = "application/json")
TransferResponse getTransfer(@PathVariable("transferId") UUID transferId);

@GetMapping(value = "/v1/wallets/{walletId}/balance", produces = "application/json")
WalletBalanceResponse getWalletBalance(@PathVariable("walletId") UUID walletId);
}
Comment on lines +11 to +19

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding resilience configuration for production.

The Feign client scaffold is clean. For production readiness, consider adding:

  • fallback or fallbackFactory for graceful degradation
  • Custom ErrorDecoder for domain-specific error mapping
  • Circuit breaker integration (e.g., Resilience4j)

These can be added incrementally as the service matures.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@blockchain-custody/blockchain-custody-client/src/main/java/com/stablecoin/payments/custody/client/BlockchainCustodyClient.java`
around lines 11 - 19, Add production resilience to the BlockchainCustodyClient
by wiring in a Feign fallback/fallbackFactory and an ErrorDecoder and enabling a
circuit breaker: implement a fallback class (e.g.,
BlockchainCustodyClientFallback) that implements BlockchainCustodyClient and
returns safe defaults for getTransfer(UUID) and getWalletBalance(UUID), or
create a fallbackFactory for richer context; register it on the Feign client
annotation (use fallback=... or fallbackFactory=... on `@FeignClient`(name =
"blockchain-custody-service", ...)); implement a custom ErrorDecoder bean to
translate Feign errors into domain exceptions for getTransfer and
getWalletBalance; and integrate a circuit breaker (e.g., annotate
consumer/service methods that call BlockchainCustodyClient with Resilience4j
`@CircuitBreaker` or configure a Feign decorator) so calls degrade gracefully
under failure.

Loading
Loading