feat(infra): add Resilience4j retry to all external adapters (STA-117)#186
Merged
Puneethkumarck merged 3 commits intoMar 17, 2026
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (35)
WalkthroughAdds Resilience4j retry across multiple modules: dependency and YAML retry configs added, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter
participant RetryAspect as "Resilience4j Retry"
participant CircuitBreaker as "Resilience4j CircuitBreaker"
participant ExternalAPI
Client->>Adapter: call adapter.method(...)
Adapter->>RetryAspect: intercepted (retry policy)
RetryAspect->>CircuitBreaker: forward call (circuit check)
CircuitBreaker->>ExternalAPI: HTTP request
ExternalAPI-->>CircuitBreaker: 503 / 200 / 4xx
CircuitBreaker-->>RetryAspect: propagate result or exception
alt success
RetryAspect-->>Client: return result
else transient failure then success
RetryAspect->>ExternalAPI: retry attempts
RetryAspect-->>Client: eventual result
else retries exhausted or 4xx
RetryAspect-->>Adapter: invoke fallback(Exception)
Adapter-->>Client: throw fallback exception
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
📝 Coding Plan
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java (1)
91-95:⚠️ Potential issue | 🟠 MajorFallback incorrectly wraps client errors in
IllegalStateException.Line 91 accepts broad
Exception, and the fallback always wraps all exceptions inIllegalStateException. Since the retry config ignoresHttpClientErrorException, 4xx errors bypass retry but still reach the circuit breaker's fallback. This collapses the distinction between validation/client errors (4xx) and service unavailability (5xx/timeout), preventing callers from differentiating error types.Proposed fix
+import org.springframework.web.client.HttpClientErrorException; @@ - private RedemptionResult redeemFallback(RedemptionRequest request, Exception ex) { - log.error("[CIRCLE] Circuit breaker open — redemption failed payoutId={}", - request.payoutId(), ex); + private RedemptionResult redeemFallback(RedemptionRequest request, Throwable ex) { + if (ex instanceof HttpClientErrorException clientError) { + throw clientError; + } + log.error("[CIRCLE] Redemption failed after resilience policies payoutId={}", + request.payoutId(), ex); throw new IllegalStateException("Circle redemption unavailable", ex); }🤖 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/infrastructure/provider/circle/CircleRedemptionAdapter.java` around lines 91 - 95, The fallback redeemFallback in CircleRedemptionAdapter currently catches all Exception and always wraps it in IllegalStateException, collapsing 4xx client errors with service/unavailability errors; update redeemFallback(Request, Exception) to detect client-side HTTP errors (e.g., instance-of org.springframework.web.client.HttpClientErrorException or reactor.netty/WebClient equivalents like WebClientResponseException) and rethrow the original exception unchanged for those cases so callers can handle validation/4xx errors, while continuing to log and wrap non-client errors (5xx/timeouts) in IllegalStateException; reference the redeemFallback method and the Exception parameter when making this change.blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java (1)
67-90:⚠️ Potential issue | 🟠 MajorSelf-invocation bypasses Retry/CircuitBreaker proxy on
getLatestBlockNumber.Line 83 calls
getLatestBlockNumber(chainId)directly from withingetTransactionReceipt. Spring AOP proxies don't intercept self-calls, so the@Retryand@CircuitBreakerannotations ongetLatestBlockNumberwon't apply—failures propagate immediately without the configured resilience safety net.Consider extracting block number fetching to a separate collaborator bean or injecting
ChainRpcProviderto call through the proxy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java` around lines 67 - 90, getTransactionReceipt currently calls getLatestBlockNumber(chainId) directly which bypasses Spring AOP proxies and therefore ignores the `@Retry/`@CircuitBreaker on getLatestBlockNumber; fix by invoking the method through a proxied collaborator instead of a self-call — either extract the logic that fetches the latest block into a separate bean (e.g., ChainRpcProvider/LatestBlockFetcher) and inject and call that bean from getTransactionReceipt, or obtain the proxied instance of this class (via an injected proxy or ApplicationContext.getBean) and call getLatestBlockNumber on the proxy so the `@Retry/`@CircuitBreaker annotations on getLatestBlockNumber are honored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.java`:
- Around line 72-73: Replace the specific "Circuit breaker open" wording in the
fallback logging with a neutral, generic failure message and include contextual
details (e.g., request id, tx id) where available; update the
signAndSubmitFallback method (referenced by the `@Retry` and `@CircuitBreaker`
annotations) and any other fallback handlers shown around lines 114-115 and
136-145 to log a neutral message such as "Custody operation failed; executing
fallback" plus relevant identifiers and the exception, ensuring the fallback
remains the same behavior but uses the neutral text for both retry and
circuit-breaker paths.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapterRetryTest.java`:
- Around line 129-130: Replace the broad Exception assertion in
FireblocksCustodyAdapterRetryTest with concrete checks: for the
custodyEngine.signAndSubmit(aSignRequest()) call assert the specific exception
type thrown (e.g., HttpClientErrorException with 400 status or your custom
BadRequestException) and also assert the exception message or cause indicates
retries were exhausted (the adapter's retry fallback message); do the same
update for the second occurrence around the other test (the later assertion at
lines referenced in the comment) so both assertions verify the HTTP 400 contract
and the exhausted-retry fallback text or specific retry-exhausted exception type
instead of Exception.class.
- Around line 87-145: Add a new test mirroring the POST retry/fallback test but
for the GET transaction status path: stub GET /v1/transactions/{txId} to always
return 503 (use urlPathEqualTo or urlPathMatching("/v1/transactions/[^/]+")),
call the adapter's getTransactionStatus(txId) (or the public method that polls
status, e.g., getTransactionStatus / custodyEngine.getTransactionStatus) and
assert it throws, then verify WireMock received 3 GET requests to that path;
follow the same retry/fallback expectations and use the same
scenario/verification style as the existing POST tests (e.g., the
shouldExhaustRetriesAndInvokeFallback test and wireMock.verify(3, ...)).
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.java`:
- Around line 57-65: The test currently mocks Redis (redisTemplate, valueOps) in
startWireMock within OnfidoKycAdapterRetryTest which prevents exercising the
real cache; remove the Mockito stubbing and instead run a real Redis instance
(e.g., use Testcontainers' RedisContainer or a Spring-managed embedded/test
Redis) and wire a real StringRedisTemplate into the test (or let `@SpringBootTest`
autoconfigure it) so the adapter's retry path uses the real cache; update
startWireMock to start the Redis container and ensure tests obtain the real
StringRedisTemplate bean (or configure its connection factory) rather than
mocking redisTemplate/valueOps.
- Around line 141-142: The test OnfidoKycAdapterRetryTest currently asserts any
Exception from kycProvider.verify(SENDER_ID, RECIPIENT_ID), which is too broad;
update the assertions to specifically assert the upstream 4xx error type/message
and the exhausted-retry fallback (e.g., a RetryExhaustedException or the
adapter's specific fallback exception/message). Replace the generic
isInstanceOf(Exception.class) with assertions on the exact exception class or
HTTP 4xx indicator thrown by the Onfido client and then assert the outer/final
exception (or its message/cause) that indicates retries were exhausted; apply
the same tightening to the second occurrence around lines 153-154 so both
assertions verify meaningful, specific failure values.
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapterRetryTest.java`:
- Around line 135-136: In WorldCheckSanctionsAdapterRetryTest, replace the broad
assertThatThrownBy(...).isInstanceOf(Exception.class) checks (calls to
sanctionsProvider.screen in this class) with assertions that verify the concrete
sanctions-failure exception and message: assert the thrown type is the specific
sanctions screening exception used by the module (e.g.,
SanctionsScreeningException or a dedicated RetryExhaustedException) and assert
the exception message or error code contains the expected "400" path text for
the bad-request scenario and the "retry exhausted" text for the retry-exhausted
scenario; update both occurrences (the current assert at
sanctionsProvider.screen and the other at lines 151-152) to use
isInstanceOf(expectedClass) and hasMessageContaining(expectedSubstring) so the
test enforces the fail-closed sanctions contract.
In
`@fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.java`:
- Around line 139-140: Replace the broad isInstanceOf(Exception.class)
assertions with assertions that check the concrete expected exception types for
each scenario: for the 400 (bad request) path assert that
redemptionGateway.redeem(aRedemptionRequest()) throws the specific
client/bad-request exception type used by your code (e.g.,
CircleBadRequestException or CircleClientException), and for the retry-exhausted
path assert the specific retry failure exception (e.g., RetryExhaustedException
or PaymentRetryExhaustedException); update the two assertions in
CircleRedemptionAdapterRetryTest that call
redemptionGateway.redeem(aRedemptionRequest()) so they assert the exact
exception class and, when applicable, validate meaningful properties/messages on
that exception rather than using Exception.class.
In
`@fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.java`:
- Around line 149-150: Replace the overly-broad assertions in
StripePspAdapterRetryTest that currently do assertThatThrownBy(() ->
pspGateway.initiatePayment(aPaymentRequest())).isInstanceOf(Exception.class)
with assertions for the concrete expected exception types: use the specific
exception class that represents a 402/declined payment for the test covering the
402 path and the specific retry-exhausted exception class for the test covering
exhausted retries (both located around the
pspGateway.initiatePayment(aPaymentRequest()) calls); assert the concrete
exception class with isInstanceOf(ExpectedConcreteException.class) and
optionally assert a meaningful property (error code/message) to make the
negative-path contract explicit.
In
`@fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapter.java`:
- Around line 46-47: The fallback method getRateFallback in RefinitivRateAdapter
is logging “Circuit breaker open” which is misleading now that `@Retry` and
`@CircuitBreaker` are both applied; change the log to a neutral resilience message
(e.g., “Resilience fallback triggered”) and include the exception type and
message from the passed-in Throwable/Exception parameter (use
ex.getClass().getName() or getSimpleName() plus ex.getMessage()) so the log
reads something like “Resilience fallback triggered: <ExceptionType>:
<message>”. Ensure the getRateFallback signature accepts the thrown
Throwable/Exception and use that variable when composing the new log entry.
In
`@fx-liquidity-engine/fx-liquidity-engine/src/test/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapterRetryTest.java`:
- Around line 106-118: The test in RefinitivRateAdapterRetryTest builds an
expected CorridorRate with magic numbers spreadBps=30 and feeBps=30; extract
those into named constants or reference the adapter/config defaults to improve
clarity and maintainability. Update the test to use constants (e.g.,
EXPECTED_SPREAD_BPS and EXPECTED_FEE_BPS) or a static accessor from the
adapter/config and replace the inline 30 values in CorridorRate.builder() so
readers can see the source of these defaults when inspecting spreadBps and
feeBps.
---
Outside diff comments:
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java`:
- Around line 67-90: getTransactionReceipt currently calls
getLatestBlockNumber(chainId) directly which bypasses Spring AOP proxies and
therefore ignores the `@Retry/`@CircuitBreaker on getLatestBlockNumber; fix by
invoking the method through a proxied collaborator instead of a self-call —
either extract the logic that fetches the latest block into a separate bean
(e.g., ChainRpcProvider/LatestBlockFetcher) and inject and call that bean from
getTransactionReceipt, or obtain the proxied instance of this class (via an
injected proxy or ApplicationContext.getBean) and call getLatestBlockNumber on
the proxy so the `@Retry/`@CircuitBreaker annotations on getLatestBlockNumber are
honored.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java`:
- Around line 91-95: The fallback redeemFallback in CircleRedemptionAdapter
currently catches all Exception and always wraps it in IllegalStateException,
collapsing 4xx client errors with service/unavailability errors; update
redeemFallback(Request, Exception) to detect client-side HTTP errors (e.g.,
instance-of org.springframework.web.client.HttpClientErrorException or
reactor.netty/WebClient equivalents like WebClientResponseException) and rethrow
the original exception unchanged for those cases so callers can handle
validation/4xx errors, while continuing to log and wrap non-client errors
(5xx/timeouts) in IllegalStateException; reference the redeemFallback method and
the Exception parameter when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64185cfa-2bb9-4fb0-a288-f97d71e6365c
📒 Files selected for processing (35)
blockchain-custody/blockchain-custody/build.gradle.ktsblockchain-custody/blockchain-custody/src/integration-test/resources/application-integration-test.ymlblockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.javablockchain-custody/blockchain-custody/src/main/resources/application.ymlblockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapterRetryTest.javablockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapterRetryTest.javablockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/build.gradle.ktscompliance-travel-rule/compliance-travel-rule/src/integration-test/resources/application-integration-test.ymlcompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.javacompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapter.javacompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.javacompliance-travel-rule/compliance-travel-rule/src/main/resources/application.ymlcompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapterRetryTest.javafiat-off-ramp/fiat-off-ramp/build.gradle.ktsfiat-off-ramp/fiat-off-ramp/src/integration-test/resources/application-integration-test.ymlfiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/resources/application.ymlfiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.javafiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.javafiat-on-ramp/fiat-on-ramp/build.gradle.ktsfiat-on-ramp/fiat-on-ramp/src/integration-test/resources/application-integration-test.ymlfiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapter.javafiat-on-ramp/fiat-on-ramp/src/main/resources/application.ymlfiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.javafx-liquidity-engine/fx-liquidity-engine/build.gradle.ktsfx-liquidity-engine/fx-liquidity-engine/src/integration-test/resources/application-integration-test.ymlfx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapter.javafx-liquidity-engine/fx-liquidity-engine/src/main/resources/application.ymlfx-liquidity-engine/fx-liquidity-engine/src/test/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapterRetryTest.java
Comment on lines
+57
to
+65
| @BeforeAll | ||
| @SuppressWarnings("unchecked") | ||
| static void startWireMock() { | ||
| wireMock = new WireMockServer(wireMockConfig().dynamicPort()); | ||
| wireMock.start(); | ||
| redisTemplate = mock(StringRedisTemplate.class); | ||
| valueOps = mock(ValueOperations.class); | ||
| given(redisTemplate.opsForValue()).willReturn(valueOps); | ||
| } |
There was a problem hiding this comment.
Don't mock Redis in this SpringBootTest.
This suite replaces StringRedisTemplate with Mockito, so the retry path never exercises the real cache integration used in production. Use Testcontainers Redis, or move the cache behind a port and keep this test purely at the adapter boundary.
As per coding guidelines, **/*Test*.java: "Integration tests must use @SpringBootTest or Testcontainers — no mocking of DB".
Also applies to: 164-167, 180-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.java`
around lines 57 - 65, The test currently mocks Redis (redisTemplate, valueOps)
in startWireMock within OnfidoKycAdapterRetryTest which prevents exercising the
real cache; remove the Mockito stubbing and instead run a real Redis instance
(e.g., use Testcontainers' RedisContainer or a Spring-managed embedded/test
Redis) and wire a real StringRedisTemplate into the test (or let `@SpringBootTest`
autoconfigure it) so the adapter's retry path uses the real cache; update
startWireMock to start the Redis container and ensure tests obtain the real
StringRedisTemplate bean (or configure its connection factory) rather than
mocking redisTemplate/valueOps.
Comment on lines
+141
to
+142
| assertThatThrownBy(() -> kycProvider.verify(SENDER_ID, RECIPIENT_ID)) | ||
| .isInstanceOf(Exception.class); |
There was a problem hiding this comment.
Tighten the failure assertions.
Exception.class is too broad for a retry contract test; any unrelated runtime failure will satisfy it. Assert the expected 4xx exception and the exhausted-retry fallback exception/message.
As per coding guidelines, **/*Test*.java: "Assert on meaningful values — avoid assertTrue(result != null)".
Also applies to: 153-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.java`
around lines 141 - 142, The test OnfidoKycAdapterRetryTest currently asserts any
Exception from kycProvider.verify(SENDER_ID, RECIPIENT_ID), which is too broad;
update the assertions to specifically assert the upstream 4xx error type/message
and the exhausted-retry fallback (e.g., a RetryExhaustedException or the
adapter's specific fallback exception/message). Replace the generic
isInstanceOf(Exception.class) with assertions on the exact exception class or
HTTP 4xx indicator thrown by the Onfido client and then assert the outer/final
exception (or its message/cause) that indicates retries were exhausted; apply
the same tightening to the second occurrence around lines 153-154 so both
assertions verify meaningful, specific failure values.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.java (1)
49-55:⚠️ Potential issue | 🔴 CriticalRemove
@Retryfrom the aggregatescreen()method—retrying replays both sender and recipient screenings, creating duplicate external cases and corrupting the audit trail.Retry must scope to individual outbound call boundaries (e.g., dedicated client bean method per
screenEntitycall). When recipient screening fails, the current decorator re-executes the entire method and resubmits sender screening, causing duplicate case creation in WorldCheck with inconsistent provider refs/case IDs. This violates auditability requirements and weakens compliance decision traceability in an AML context.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java`:
- Around line 65-67: getTransactionReceipt currently calls getLatestBlockNumber
directly which bypasses the Resilience4j proxy on that inner method
(self-invocation). Make getLatestBlockNumber be invoked through the Spring proxy
so its `@Retry/`@CircuitBreaker take effect: either move getLatestBlockNumber into
a separate `@Component` (e.g., EvmRpcClient) with its annotations and inject that
component into EvmRpcAdapter, or keep it in this class but make it public and
inject the EvmRpcAdapter bean (self) and call self.getLatestBlockNumber(...)
instead of calling the method directly; update references accordingly and keep
the existing fallback getTransactionReceiptFallback unchanged.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java`:
- Around line 59-61: The getTransactionReceipt method in SolanaRpcAdapter calls
getLatestBlockNumber/getSlot directly which bypasses Resilience4j proxies so
`@Retry` on getLatestBlockNumber/getSlot won't run; to fix, move the RPC helper
methods (getLatestBlockNumber and getSlot) into a separate Spring bean (e.g.,
SolanaRpcClient) or obtain a proxied reference to this bean and call them
through that proxy from getTransactionReceipt, ensuring the calls go through the
proxy so their `@Retry/`@CircuitBreaker annotations are applied; update references
in SolanaRpcAdapter to call the new bean methods instead of the private/internal
methods.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapterRetryTest.java`:
- Around line 78-126: Add parallel retry tests for getTransactionReceipt and
getTokenBalance mirroring the existing getLatestBlockNumber scenarios: for each
method create (1) a transient 503 then 200 success case that verifies correct
parsing of the hex response, (2) a 400 client error case that asserts
IllegalStateException with message "EVM RPC unavailable for <methodName>" and
that only one request is made, and (3) a persistent 503 case that asserts the
exception and verifies the retry count (3 requests) and fallback behavior;
locate the tests by referencing the existing test method names
shouldRetryOnTransientFailureThenSucceed, shouldNotRetryOnClientError,
shouldExhaustRetriesAndInvokeFallback and the adapter methods
getTransactionReceipt and getTokenBalance to implement analogous WireMock stubs
and assertions.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapterRetryTest.java`:
- Around line 76-124: Add parallel retry tests for getTransactionReceipt and
getTokenBalance in SolanaRpcAdapterRetryTest: for each method create a
transient-503-then-200 scenario (use inScenario + state transitions) that
returns realistic JSON-RPC responses matching the method's expected nested/array
structure and assert the parsed result equals the expected value; add a
client-error (400) test that asserts IllegalStateException with the appropriate
fallback message for that method; and add a persistent-503 test that verifies
the retry count (same number as getLatestBlockNumber) and that the fallback
handler (getTransactionReceiptFallback / getTokenBalanceFallback) behavior is
invoked (via thrown exception or asserted result), using the same wireMock
patterns (post(urlEqualTo("/")), postRequestedFor) and the existing
chainRpcProvider.getTransactionReceipt(...) and
chainRpcProvider.getTokenBalance(...) calls to locate where to add tests.
In
`@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.java`:
- Around line 53-54: The Retry annotation on analyze() causes both
registerAndAnalyzeTransfer() calls to be retried together, producing duplicate
sender POSTs; update ChainalysisAmlAdapter to avoid this by either moving
`@Retry/`@CircuitBreaker from analyze() onto the individual
registerAndAnalyzeTransfer(...) method so each transfer call is retried
independently, or add 409 Conflict handling inside registerAndAnalyzeTransfer()
to treat a duplicate-registration 409 as a successful idempotent outcome (catch
the HTTP client exception for 409 and return success), and ensure
analyzeFallback remains compatible with the new retry scope; validate with
ChainalysisAmlAdapterRetryTest.shouldExhaustRetriesAndInvokeFallback().
In
`@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.java`:
- Around line 165-166: The fallback log in WorldCheckSanctionsAdapter is passing
the throwable (ex) into the logger which may leak PII; change the log call to
only emit sanitized metadata (e.g., senderId, recipientId, and
ex.getClass().getSimpleName()) and remove the throwable argument so no
stacktrace/message is recorded. Locate the log.error(...) in
WorldCheckSanctionsAdapter and replace the current call that passes ex as the
final parameter with a call that formats and logs only the IDs and exception
class name (do not log ex.getMessage() or the throwable).
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapterRetryTest.java`:
- Around line 161-168: The bean method chainalysisProperties() currently reads a
static wireMock variable which relies on JUnit `@BeforeAll` ordering; change it to
explicitly depend on the WireMockServer by adding it as a method parameter
(e.g., chainalysisProperties(WireMockServer wireMockServer)) and use
wireMockServer.baseUrl() instead of the static wireMock, or alternatively inject
the WireMockServer via `@Autowired` into the test configuration; this removes the
implicit lifecycle coupling and makes the dependency explicit when constructing
the ChainalysisProperties instance.
- Around line 142-154: The test method should also assert that the thrown
IllegalStateException from shouldExhaustRetriesAndInvokeFallback has a root
cause of HttpServerErrorException; update the assertion on
amlProvider.analyze(SENDER_ID, RECIPIENT_ID) to check the cause (or cause chain)
is an instance of org.springframework.web.client.HttpServerErrorException so the
contract explicitly verifies the wrapped server-error type while keeping the
existing message and retry count verification in
shouldExhaustRetriesAndInvokeFallback.
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.java`:
- Around line 121-132: The test OnfidoKycAdapterRetryTest currently asserts the
KycResult but doesn't verify WireMock request counts, so add a WireMock
verification after calling kycProvider.verify(SENDER_ID, RECIPIENT_ID) to assert
three requests were made to the Onfido endpoint (total: 3 — two for sender due
to a 503 then retry and one for recipient 200). Locate the test method that
calls kycProvider.verify and append a wireMock.verify(...) call targeting the
same request URL/matching stub used for the sender/recipient checks (use the
same request matcher used in the other tests) with count 3 to ensure the retry
actually occurred. Ensure you reference the same WireMock server instance used
in this test class when adding the verification.
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapterRetryTest.java`:
- Around line 62-63: The test currently hardcodes credentials via
registry.add("app.sanctions.world-check.api-key", () -> "test-key") and
registry.add("app.sanctions.world-check.api-secret", () -> "test-secret");
replace those literal strings with non-secret placeholders or generated values
(e.g., "placeholder-key-<random>" / UUID-based string) so no hardcoded secret
appears in the test fixture; update the WorldCheckSanctionsAdapterRetryTest
setup where registry.add is invoked to produce and inject the placeholder values
instead of the embedded "test-key"/"test-secret".
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java`:
- Around line 53-54: The fallback method redeemFallback in
CircleRedemptionAdapter currently maps all exceptions to
IllegalStateException("Circle redemption unavailable"), losing HTTP 4xx
semantics; update redeemFallback to detect and re-throw instances of
org.springframework.web.client.HttpClientErrorException (or
HttpStatusCodeException) so client errors (4xx) propagate unchanged, and only
map non-client errors to the current IllegalStateException; locate the method
redeemFallback and add an instanceof check for HttpClientErrorException (or
HttpStatusCodeException) to rethrow the original exception before creating the
generic unavailable error (this change applies to error handling invoked by the
`@Retry` and `@CircuitBreaker` on the redeem method).
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapter.java`:
- Around line 60-62: The fallback initiatePayoutFallback in ModulrPayoutAdapter
is catching Exception broadly and treating all errors as transient
unavailability; change it to detect and differentiate HTTP 4xx client errors
(e.g., WebClientResponseException or HttpClientErrorException /
ResponseStatusException) inside initiatePayoutFallback and rethrow or propagate
a specific client-error exception for 4xx (so caller can handle invalid IBAN/bad
request), while preserving the existing "unavailable" fallback behavior for
non-4xx errors (network/timeouts/5xx). Update initiatePayoutFallback to inspect
exception.getStatusCode().is4xxClientError() (or use instanceof checks) and
handle accordingly, referencing the methods initiatePayout and
initiatePayoutFallback in ModulrPayoutAdapter.
In
`@fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java`:
- Around line 1-11: The file ModulrPayoutAdapterRetryTest.java is failing
Spotless formatting; run the project's formatter (e.g. mvn spotless:apply or
./gradlew spotlessApply) and reformat this test class so imports, spacing, and
ordering conform to Spotless rules; ensure the import block and class-level
spacing around package, imports and the class declaration for
ModulrPayoutAdapterRetryTest match the project's style and re-run spotlessCheck
before committing.
In
`@fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.java`:
- Around line 54-58: The WireMock server is being started in `@BeforeAll`
(startWireMock) which runs after Spring initializes TestConfig and invokes
stripeProperties(), causing wireMock to be null; move the WireMockServer
initialization into a static initializer so wireMock is created and started
before the Spring context loads (initialize the static wireMock field in a
static { ... } block and start it there), leaving stripeProperties() and other
test methods unchanged so calls to wireMock.baseUrl() succeed during bean
creation.
In
`@fx-liquidity-engine/fx-liquidity-engine/src/test/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapterRetryTest.java`:
- Around line 81-124: In the test method
shouldRetryOnTransientFailureThenSucceed(), add an explicit WireMock
request-count assertion to confirm the retry occurred: after the existing
assertions for result equality, call wireMock.verify(2,
getRequestedFor(urlEqualTo("/data/pricing/v1/rates/USDEUR"))) (or equivalent
verify(2, ...) using the same urlEqualTo used in the stubs) to assert exactly
two requests were made (one 503 then one 200).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d42e2bd3-d331-470d-be61-db0b392b5b06
📒 Files selected for processing (20)
blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.javablockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapterRetryTest.javablockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapterRetryTest.javablockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.javacompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapter.javacompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.javacompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapterRetryTest.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapter.javafiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.javafiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.javafiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapter.javafiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.javafx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapter.javafx-liquidity-engine/fx-liquidity-engine/src/test/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapterRetryTest.java
Comment on lines
+65
to
67
| @Retry(name = "evmRpc", fallbackMethod = "getTransactionReceiptFallback") | ||
| @CircuitBreaker(name = "evmRpc", fallbackMethod = "getTransactionReceiptFallback") | ||
| public TransactionReceipt getTransactionReceipt(ChainId chainId, String txHash) { |
There was a problem hiding this comment.
Same nested call proxy bypass as Solana adapter.
getTransactionReceipt internally calls getLatestBlockNumber at line 83. This bypasses the retry aspect on the inner method since it's a self-invocation within the same proxy. The outer method's retry will re-execute the entire flow if it fails.
Also applies to: 83-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java`
around lines 65 - 67, getTransactionReceipt currently calls getLatestBlockNumber
directly which bypasses the Resilience4j proxy on that inner method
(self-invocation). Make getLatestBlockNumber be invoked through the Spring proxy
so its `@Retry/`@CircuitBreaker take effect: either move getLatestBlockNumber into
a separate `@Component` (e.g., EvmRpcClient) with its annotations and inject that
component into EvmRpcAdapter, or keep it in this class but make it public and
inject the EvmRpcAdapter bean (self) and call self.getLatestBlockNumber(...)
instead of calling the method directly; update references accordingly and keep
the existing fallback getTransactionReceiptFallback unchanged.
Comment on lines
+59
to
61
| @Retry(name = "solanaRpc", fallbackMethod = "getTransactionReceiptFallback") | ||
| @CircuitBreaker(name = "solanaRpc", fallbackMethod = "getTransactionReceiptFallback") | ||
| public TransactionReceipt getTransactionReceipt(ChainId chainId, String txHash) { |
There was a problem hiding this comment.
Nested RPC call bypasses retry for inner method.
getTransactionReceipt at line 81 invokes getLatestBlockNumber directly. Since both methods are on the same bean, this internal call bypasses the proxy—@Retry on getLatestBlockNumber won't apply. If getSlot fails, no retry occurs for that inner call.
If independent retry per RPC call is desired, extract to a separate bean or accept the current behavior where the outer method's retry will re-execute the entire flow including the inner call.
Also applies to: 81-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java`
around lines 59 - 61, The getTransactionReceipt method in SolanaRpcAdapter calls
getLatestBlockNumber/getSlot directly which bypasses Resilience4j proxies so
`@Retry` on getLatestBlockNumber/getSlot won't run; to fix, move the RPC helper
methods (getLatestBlockNumber and getSlot) into a separate Spring bean (e.g.,
SolanaRpcClient) or obtain a proxied reference to this bean and call them
through that proxy from getTransactionReceipt, ensuring the calls go through the
proxy so their `@Retry/`@CircuitBreaker annotations are applied; update references
in SolanaRpcAdapter to call the new bean methods instead of the private/internal
methods.
Comment on lines
+78
to
+126
| @Test | ||
| @DisplayName("should retry on transient 503 failure then succeed") | ||
| void shouldRetryOnTransientFailureThenSucceed() { | ||
| wireMock.stubFor(post(urlEqualTo("/")) | ||
| .inScenario("retry-then-success") | ||
| .whenScenarioStateIs(Scenario.STARTED) | ||
| .willReturn(aResponse().withStatus(503)) | ||
| .willSetStateTo("second-attempt")); | ||
|
|
||
| wireMock.stubFor(post(urlEqualTo("/")) | ||
| .inScenario("retry-then-success") | ||
| .whenScenarioStateIs("second-attempt") | ||
| .willReturn(aResponse() | ||
| .withStatus(200) | ||
| .withHeader("Content-Type", "application/json") | ||
| .withBody(""" | ||
| {"jsonrpc":"2.0","id":1,"result":"0x1a4"} | ||
| """))); | ||
|
|
||
| var result = chainRpcProvider.getLatestBlockNumber(BASE_CHAIN); | ||
|
|
||
| assertThat(result).isEqualTo(420L); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("should not retry on 400 client error") | ||
| void shouldNotRetryOnClientError() { | ||
| wireMock.stubFor(post(urlEqualTo("/")) | ||
| .willReturn(aResponse().withStatus(400))); | ||
|
|
||
| assertThatThrownBy(() -> chainRpcProvider.getLatestBlockNumber(BASE_CHAIN)) | ||
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessage("EVM RPC unavailable for getLatestBlockNumber"); | ||
|
|
||
| wireMock.verify(1, postRequestedFor(urlEqualTo("/"))); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("should exhaust retries on persistent 503 and invoke fallback") | ||
| void shouldExhaustRetriesAndInvokeFallback() { | ||
| wireMock.stubFor(post(urlEqualTo("/")) | ||
| .willReturn(aResponse().withStatus(503))); | ||
|
|
||
| assertThatThrownBy(() -> chainRpcProvider.getLatestBlockNumber(BASE_CHAIN)) | ||
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessage("EVM RPC unavailable for getLatestBlockNumber"); | ||
|
|
||
| wireMock.verify(3, postRequestedFor(urlEqualTo("/"))); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same coverage suggestion: add tests for getTransactionReceipt and getTokenBalance.
Consistent with the Solana test feedback—all three public methods have @Retry annotations but only getLatestBlockNumber is exercised. The EVM adapter's hex parsing paths in getTransactionReceipt and getTokenBalance would benefit from retry scenario coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapterRetryTest.java`
around lines 78 - 126, Add parallel retry tests for getTransactionReceipt and
getTokenBalance mirroring the existing getLatestBlockNumber scenarios: for each
method create (1) a transient 503 then 200 success case that verifies correct
parsing of the hex response, (2) a 400 client error case that asserts
IllegalStateException with message "EVM RPC unavailable for <methodName>" and
that only one request is made, and (3) a persistent 503 case that asserts the
exception and verifies the retry count (3 requests) and fallback behavior;
locate the tests by referencing the existing test method names
shouldRetryOnTransientFailureThenSucceed, shouldNotRetryOnClientError,
shouldExhaustRetriesAndInvokeFallback and the adapter methods
getTransactionReceipt and getTokenBalance to implement analogous WireMock stubs
and assertions.
Comment on lines
+76
to
+124
| @Test | ||
| @DisplayName("should retry on transient 503 failure then succeed") | ||
| void shouldRetryOnTransientFailureThenSucceed() { | ||
| wireMock.stubFor(post(urlEqualTo("/")) | ||
| .inScenario("retry-then-success") | ||
| .whenScenarioStateIs(Scenario.STARTED) | ||
| .willReturn(aResponse().withStatus(503)) | ||
| .willSetStateTo("second-attempt")); | ||
|
|
||
| wireMock.stubFor(post(urlEqualTo("/")) | ||
| .inScenario("retry-then-success") | ||
| .whenScenarioStateIs("second-attempt") | ||
| .willReturn(aResponse() | ||
| .withStatus(200) | ||
| .withHeader("Content-Type", "application/json") | ||
| .withBody(""" | ||
| {"jsonrpc":"2.0","id":1,"result":285630456} | ||
| """))); | ||
|
|
||
| var result = chainRpcProvider.getLatestBlockNumber(SOLANA_CHAIN); | ||
|
|
||
| assertThat(result).isEqualTo(285630456L); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("should not retry on 400 client error") | ||
| void shouldNotRetryOnClientError() { | ||
| wireMock.stubFor(post(urlEqualTo("/")) | ||
| .willReturn(aResponse().withStatus(400))); | ||
|
|
||
| assertThatThrownBy(() -> chainRpcProvider.getLatestBlockNumber(SOLANA_CHAIN)) | ||
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessage("Solana RPC unavailable for getSlot"); | ||
|
|
||
| wireMock.verify(1, postRequestedFor(urlEqualTo("/"))); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("should exhaust retries on persistent 503 and invoke fallback") | ||
| void shouldExhaustRetriesAndInvokeFallback() { | ||
| wireMock.stubFor(post(urlEqualTo("/")) | ||
| .willReturn(aResponse().withStatus(503))); | ||
|
|
||
| assertThatThrownBy(() -> chainRpcProvider.getLatestBlockNumber(SOLANA_CHAIN)) | ||
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessage("Solana RPC unavailable for getSlot"); | ||
|
|
||
| wireMock.verify(3, postRequestedFor(urlEqualTo("/"))); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which Solana adapter methods have retry test coverage
rg -n 'getTransactionReceipt|getTokenBalance|getLatestBlockNumber' \
blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/ \
--type=javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 3907
🏁 Script executed:
#!/bin/bash
# Find SolanaRpcAdapter implementation and check for `@Retry` annotations
fd -e java 'SolanaRpcAdapter\.java' blockchain-custody/ | head -20Repository: Puneethkumarck/stablebridge-platform
Length of output: 216
🏁 Script executed:
#!/bin/bash
# Check the SolanaRpcAdapter implementation for `@Retry` annotations
cat blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java | head -200Repository: Puneethkumarck/stablebridge-platform
Length of output: 9219
Add retry tests for getTransactionReceipt and getTokenBalance methods.
Both methods carry @Retry and @CircuitBreaker annotations with distinct fallback handlers (getTransactionReceiptFallback, getTokenBalanceFallback), yet only getLatestBlockNumber is covered by SolanaRpcAdapterRetryTest. Adding transient failure scenarios for both would validate resilience behavior and JSON-RPC parsing paths under retry conditions—particularly important given their complex nested and array-based response parsing logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapterRetryTest.java`
around lines 76 - 124, Add parallel retry tests for getTransactionReceipt and
getTokenBalance in SolanaRpcAdapterRetryTest: for each method create a
transient-503-then-200 scenario (use inScenario + state transitions) that
returns realistic JSON-RPC responses matching the method's expected nested/array
structure and assert the parsed result equals the expected value; add a
client-error (400) test that asserts IllegalStateException with the appropriate
fallback message for that method; and add a persistent-503 test that verifies
the retry count (same number as getLatestBlockNumber) and that the fallback
handler (getTransactionReceiptFallback / getTokenBalanceFallback) behavior is
invoked (via thrown exception or asserted result), using the same wireMock
patterns (post(urlEqualTo("/")), postRequestedFor) and the existing
chainRpcProvider.getTransactionReceipt(...) and
chainRpcProvider.getTokenBalance(...) calls to locate where to add tests.
Comment on lines
+53
to
54
| @Retry(name = "aml", fallbackMethod = "analyzeFallback") | ||
| @CircuitBreaker(name = "aml", fallbackMethod = "analyzeFallback") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if RestClient calls are retried elsewhere or if there's API documentation
rg -n "registerTransfer|idempotent" --type java -C2Repository: Puneethkumarck/stablebridge-platform
Length of output: 38842
🏁 Script executed:
# Check the full ChainalysisAmlAdapterRetryTest to understand the test scenario
sed -n '140,160p' compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 888
🏁 Script executed:
# Check how transferReference is set in registerTransfer request
sed -n '91,110p' compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 769
🏁 Script executed:
# Check if there's any 409 Conflict or idempotency handling in the adapter
rg -n "409|Conflict|transferReference" compliance-travel-rule/ --type javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 2303
🏁 Script executed:
# Check the full ChainalysisAmlAdapter to see if there's any 409 handling or error handling strategy
cat -n compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.java | head -120Repository: Puneethkumarck/stablebridge-platform
Length of output: 5996
🏁 Script executed:
# Check if there's a RestClient configuration that handles 409 responses
rg -n "RestClient|RestClientBuilderCustomizer|onStatus|HttpStatusCodeException" compliance-travel-rule/compliance-travel-rule/src/main/java/ --type java -B2 -A2Repository: Puneethkumarck/stablebridge-platform
Length of output: 20328
🏁 Script executed:
# Check what happens when POST returns 409 - see if there are any conflict handlers in the codebase
rg -n "FeignException|HttpClientErrorException|409" compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ --type java -B1 -A1Repository: Puneethkumarck/stablebridge-platform
Length of output: 62
Retry scope causes duplicate POSTs on partial failure — tighten idempotency.
The @Retry on analyze() will re-execute both registerAndAnalyzeTransfer() calls if the second fails after the first succeeds. Since registerTransfer() POSTs to /users/{userId}/transfers with transferReference=userId and there is no 409 Conflict handling or idempotency guard, the Chainalysis API will receive duplicate transfer registrations for the sender.
Either:
- Narrow
@Retryto individualregisterAndAnalyzeTransfer()calls, or - Add 409 Conflict exception handling to treat duplicate registrations as success (idempotent).
Test ChainalysisAmlAdapterRetryTest.shouldExhaustRetriesAndInvokeFallback() confirms the current scope retries the sender POST multiple times.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.java`
around lines 53 - 54, The Retry annotation on analyze() causes both
registerAndAnalyzeTransfer() calls to be retried together, producing duplicate
sender POSTs; update ChainalysisAmlAdapter to avoid this by either moving
`@Retry/`@CircuitBreaker from analyze() onto the individual
registerAndAnalyzeTransfer(...) method so each transfer call is retried
independently, or add 409 Conflict handling inside registerAndAnalyzeTransfer()
to treat a duplicate-registration 409 as a successful idempotent outcome (catch
the HTTP client exception for 409 and return success), and ensure
analyzeFallback remains compatible with the new retry scope; validate with
ChainalysisAmlAdapterRetryTest.shouldExhaustRetriesAndInvokeFallback().
Comment on lines
+53
to
54
| @Retry(name = "circle", fallbackMethod = "redeemFallback") | ||
| @CircuitBreaker(name = "circle", fallbackMethod = "redeemFallback") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify broad fallback + CB config does not explicitly ignore 4xx for Circle.
rg -n '@Retry\(name = "circle"|@CircuitBreaker\(name = "circle"|redeemFallback\(.*Exception ex\)' --type=java
rg -n 'resilience4j\.circuitbreaker\.instances\.circle\.(ignore-exceptions|record-exceptions)' -g '*.yml' -g '*.yaml'Repository: Puneethkumarck/stablebridge-platform
Length of output: 712
🏁 Script executed:
sed -n '85,100p' fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 569
🏁 Script executed:
# Check if HttpClientErrorException or similar is already used elsewhere in this file
rg -n 'HttpClientErrorException|HttpServerErrorException|RestClientException' fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 62
🏁 Script executed:
# Verify the redeem method's actual call site and whether it catches exceptions before fallback
sed -n '45,95p' fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 2500
🏁 Script executed:
# Check CircleRedemptionAdapter for any explicit HTTP error handling
rg -n 'catch|throw|HttpStatus|4[0-9]{2}|5[0-9]{2}' fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java -A 2Repository: Puneethkumarck/stablebridge-platform
Length of output: 292
🏁 Script executed:
sed -n '1,50p' fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 2316
🏁 Script executed:
# Check what Spring version/client is being used
rg -n 'import.*RestClient|import.*RestTemplate|import.*WebClient' fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 132
🏁 Script executed:
# Verify if there's any error handling wrapper around the HTTP call
rg -B5 -A5 'retrieve\(\)' fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 547
Re-throw HTTP 4xx errors instead of mapping to generic "unavailable" message.
The fallback at line 91 converts all exceptions to IllegalStateException("Circle redemption unavailable"). However, Spring's RestClient.retrieve() throws HttpClientErrorException for 4xx responses (e.g., invalid destination), which should be distinguished from transient failures. Currently, all errors—whether client validation or provider outage—signal the same "unavailable" status to callers, losing critical error fidelity.
Preserve 4xx semantics by re-throwing HttpClientErrorException before the generic conversion:
Proposed fix
+import org.springframework.web.client.HttpClientErrorException;
@@
`@SuppressWarnings`("unused")
private RedemptionResult redeemFallback(RedemptionRequest request, Exception ex) {
+ if (ex instanceof HttpClientErrorException) {
+ throw ex;
+ }
log.error("[CIRCLE] Resilience fallback — redemption failed payoutId={} due to {}",
request.payoutId(), ex.getClass().getSimpleName(), ex);
throw new IllegalStateException("Circle redemption unavailable", ex);
}Also applies to: 91-94
🤖 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/infrastructure/provider/circle/CircleRedemptionAdapter.java`
around lines 53 - 54, The fallback method redeemFallback in
CircleRedemptionAdapter currently maps all exceptions to
IllegalStateException("Circle redemption unavailable"), losing HTTP 4xx
semantics; update redeemFallback to detect and re-throw instances of
org.springframework.web.client.HttpClientErrorException (or
HttpStatusCodeException) so client errors (4xx) propagate unchanged, and only
map non-client errors to the current IllegalStateException; locate the method
redeemFallback and add an instanceof check for HttpClientErrorException (or
HttpStatusCodeException) to rethrow the original exception before creating the
generic unavailable error (this change applies to error handling invoked by the
`@Retry` and `@CircuitBreaker` on the redeem method).
Comment on lines
+60
to
62
| @Retry(name = "modulr", fallbackMethod = "initiatePayoutFallback") | ||
| @CircuitBreaker(name = "modulr", fallbackMethod = "initiatePayoutFallback") | ||
| public PayoutResult initiatePayout(PayoutRequest request) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "ModulrPayoutAdapter.java" | head -1Repository: Puneethkumarck/stablebridge-platform
Length of output: 211
🏁 Script executed:
cat -n ./fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapter.java | head -120Repository: Puneethkumarck/stablebridge-platform
Length of output: 5834
Differentiate client errors (4xx) from transient unavailability in fallback.
The fallback at line 112 catches Exception broadly, masking HTTP 4xx errors (e.g., invalid IBAN, malformed request) as "Modulr payout unavailable". This conflates caller-fixable failures with transient outages, preventing proper error handling upstream.
💡 Proposed fix
+import org.springframework.web.client.HttpClientErrorException;
@@
`@SuppressWarnings`("unused")
private PayoutResult initiatePayoutFallback(PayoutRequest request, Exception ex) {
+ if (ex instanceof HttpClientErrorException clientError) {
+ throw new PayoutPartnerException(
+ request.payoutId(), "modulr",
+ "Modulr rejected payout request: " + clientError.getStatusCode(),
+ clientError
+ );
+ }
log.error("[MODULR] Resilience fallback — payout failed payoutId={} due to {}",
request.payoutId(), ex.getClass().getSimpleName(), ex);
throw new PayoutPartnerException(
request.payoutId(), "modulr", "Modulr payout unavailable", ex);
}🤖 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/infrastructure/provider/modulr/ModulrPayoutAdapter.java`
around lines 60 - 62, The fallback initiatePayoutFallback in ModulrPayoutAdapter
is catching Exception broadly and treating all errors as transient
unavailability; change it to detect and differentiate HTTP 4xx client errors
(e.g., WebClientResponseException or HttpClientErrorException /
ResponseStatusException) inside initiatePayoutFallback and rethrow or propagate
a specific client-error exception for 4xx (so caller can handle invalid IBAN/bad
request), while preserving the existing "unavailable" fallback behavior for
non-4xx errors (network/timeouts/5xx). Update initiatePayoutFallback to inspect
exception.getStatusCode().is4xxClientError() (or use instanceof checks) and
handle accordingly, referencing the methods initiatePayout and
initiatePayoutFallback in ModulrPayoutAdapter.
Comment on lines
+54
to
+58
| @BeforeAll | ||
| static void startWireMock() { | ||
| wireMock = new WireMockServer(wireMockConfig().dynamicPort()); | ||
| wireMock.start(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JUnit 5 with SpringExtension and @SpringBootTest, does ApplicationContext/bean creation occur before user-defined @BeforeAll methods?
💡 Result:
Yes. With @SpringBootTest (meta-annotated with @ExtendWith(SpringExtension.class)), the Spring ApplicationContext is prepared before any user-defined @BeforeAll methods run.
- JUnit Jupiter executes extension
BeforeAllCallbackcode before user@BeforeAlllifecycle methods. [1] SpringExtensionimplementsBeforeAllCallback, and itsbeforeAll()delegates toTestContextManager.beforeTestClass(), which is where the Spring TestContext framework performs its “before test class” work (including preparing/loading the testApplicationContext). [2]- Spring explicitly supports injecting dependencies from the test
ApplicationContextinto JUnit@BeforeAllmethods, which requires the context to exist by then. [3]
Sources: [1] [2] [3]
🏁 Script executed:
cd fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe && wc -l StripePspAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 114
🏁 Script executed:
cd fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe && sed -n '40,60p' StripePspAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 674
🏁 Script executed:
cd fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe && sed -n '170,185p' StripePspAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 530
Move WireMock initialization to static block to ensure server is running before Spring context loads TestConfig.
Spring's ApplicationContext is initialized before @BeforeAll executes. Since stripeProperties() at line 176 calls wireMock.baseUrl() during bean creation, wireMock will be null, causing NullPointerException during test context bootstrap.
Use a static initializer to guarantee the server is available before Spring loads:
Suggested fix
- private static WireMockServer wireMock;
-
- `@Autowired`
- private PspGateway pspGateway;
+ private static final WireMockServer wireMock = new WireMockServer(wireMockConfig().dynamicPort());
+
+ static {
+ wireMock.start();
+ }
+
+ `@Autowired`
+ private PspGateway pspGateway;Also applies to: 175-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.java`
around lines 54 - 58, The WireMock server is being started in `@BeforeAll`
(startWireMock) which runs after Spring initializes TestConfig and invokes
stripeProperties(), causing wireMock to be null; move the WireMockServer
initialization into a static initializer so wireMock is created and started
before the Spring context loads (initialize the static wireMock field in a
static { ... } block and start it there), leaving stripeProperties() and other
test methods unchanged so calls to wireMock.baseUrl() succeed during bean
creation.
Comment on lines
+81
to
+124
| @Test | ||
| @DisplayName("should retry on transient 503 failure then succeed") | ||
| void shouldRetryOnTransientFailureThenSucceed() { | ||
| long nowMs = System.currentTimeMillis(); | ||
|
|
||
| wireMock.stubFor(get(urlEqualTo("/data/pricing/v1/rates/USDEUR")) | ||
| .inScenario("retry-then-success") | ||
| .whenScenarioStateIs(Scenario.STARTED) | ||
| .willReturn(aResponse().withStatus(503)) | ||
| .willSetStateTo("second-attempt")); | ||
|
|
||
| wireMock.stubFor(get(urlEqualTo("/data/pricing/v1/rates/USDEUR")) | ||
| .inScenario("retry-then-success") | ||
| .whenScenarioStateIs("second-attempt") | ||
| .willReturn(aResponse() | ||
| .withStatus(200) | ||
| .withHeader("Content-Type", "application/json") | ||
| .withBody(""" | ||
| { | ||
| "currencyPair": "USDEUR", | ||
| "mid": 0.9200000000, | ||
| "bid": 0.9195000000, | ||
| "ask": 0.9205000000, | ||
| "timestamp": %d | ||
| } | ||
| """.formatted(nowMs)))); | ||
|
|
||
| var result = rateProvider.getRate("USD", "EUR"); | ||
|
|
||
| assertThat(result).isPresent(); | ||
| var expected = CorridorRate.builder() | ||
| .fromCurrency("USD") | ||
| .toCurrency("EUR") | ||
| .rate(new BigDecimal("0.92")) | ||
| .spreadBps(EXPECTED_SPREAD_BPS) | ||
| .feeBps(EXPECTED_FEE_BPS) | ||
| .provider("refinitiv") | ||
| .build(); | ||
| assertThat(result.get()) | ||
| .usingRecursiveComparison() | ||
| .withComparatorForType(BigDecimal::compareTo, BigDecimal.class) | ||
| .ignoringFields("ageMs") | ||
| .isEqualTo(expected); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add request count verification for completeness.
The other two tests verify exact request counts (verify(1, ...) and verify(3, ...)). For consistency and to confirm retry actually triggered, assert that exactly 2 requests were made (1 failure + 1 success).
🔧 Proposed fix
Add after line 123:
assertThat(result.get())
.usingRecursiveComparison()
.withComparatorForType(BigDecimal::compareTo, BigDecimal.class)
.ignoringFields("ageMs")
.isEqualTo(expected);
+
+ wireMock.verify(2, getRequestedFor(urlEqualTo("/data/pricing/v1/rates/USDEUR")));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fx-liquidity-engine/fx-liquidity-engine/src/test/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapterRetryTest.java`
around lines 81 - 124, In the test method
shouldRetryOnTransientFailureThenSucceed(), add an explicit WireMock
request-count assertion to confirm the retry occurred: after the existing
assertions for result equality, call wireMock.verify(2,
getRequestedFor(urlEqualTo("/data/pricing/v1/rates/USDEUR"))) (or equivalent
verify(2, ...) using the same urlEqualTo used in the stubs) to assert exactly
two requests were made (one 503 then one 200).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java`:
- Around line 45-58: The WireMock server is started in `@BeforeAll`
(startWireMock) which runs after Spring initializes the test context; move its
startup into a static initializer so wireMock is running before any beans (e.g.,
PayoutPartnerGateway) are instantiated and before calls to wireMock.baseUrl()
occur. Replace the `@BeforeAll` startWireMock logic by initializing and starting
the static WireMockServer wireMock inside a static { ... } block (ensuring
wireMockConfig().dynamicPort() is used) so all test beans and
CircuitBreakerRegistry load with the mock server available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3f3e0d07-ef40-4f2d-8add-c4c158507512
📒 Files selected for processing (1)
fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java
Comment on lines
+45
to
+58
| private static WireMockServer wireMock; | ||
| private static final UUID PAYOUT_ID = UUID.fromString("887adb57-1d2e-4f3a-b5c6-d7e8f9a0b1c2"); | ||
|
|
||
| @Autowired | ||
| private PayoutPartnerGateway payoutPartnerGateway; | ||
|
|
||
| @Autowired | ||
| private CircuitBreakerRegistry circuitBreakerRegistry; | ||
|
|
||
| @BeforeAll | ||
| static void startWireMock() { | ||
| wireMock = new WireMockServer(wireMockConfig().dynamicPort()); | ||
| wireMock.start(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if the test passes or fails with NPE during context initialization
# Check for similar patterns in other retry tests that might have the same issue
echo "=== Checking other retry tests for WireMock initialization patterns ==="
rg -n "private static.*WireMockServer" --type java -A 10 | head -60
echo ""
echo "=== Looking for static initializer blocks in retry tests ==="
rg -nP "static\s*\{" --type java -g '*RetryTest*.java' -A 5Repository: Puneethkumarck/stablebridge-platform
Length of output: 9778
🏁 Script executed:
# Read the test file to check TestConfig bean and Spring context setup
head -n 180 fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java | tail -n 100Repository: Puneethkumarck/stablebridge-platform
Length of output: 4151
🏁 Script executed:
head -n 50 fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 2700
🏁 Script executed:
grep -n "@DynamicPropertySource" fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java -A 10Repository: Puneethkumarck/stablebridge-platform
Length of output: 938
🏁 Script executed:
# Check the exact line where modulrProperties is defined and if there's `@Lazy`
sed -n '162,180p' fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 617
🏁 Script executed:
# Verify if there's a `@Lazy` annotation and check Spring Boot lifecycle
# Also check if the test file has been successfully compiled/run
cd fiat-off-ramp/fiat-off-ramp && \
find . -name "*.class" -path "*ModulrPayoutAdapterRetryTest*" 2>/dev/null | head -5
# Check if there are any test execution logs or build artifacts
ls -la target/surefire-reports/ 2>/dev/null | grep -i modulr || echo "No test reports found"
# Verify Spring context initialization: check if TestConfig bean has `@Lazy`
grep -n "@Lazy\|@Bean" fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 276
🏁 Script executed:
# Search for similar patterns in the codebase to understand if they're working
# Look at the RefinitivRateAdapterRetryTest which has the same pattern
grep -n "wireMock.baseUrl()" --include="*.java" -r .Repository: Puneethkumarck/stablebridge-platform
Length of output: 5876
🏁 Script executed:
# Check CircleRedemptionAdapterRetryTest to see how it initializes WireMock
sed -n '1,100p' fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.java | tail -60Repository: Puneethkumarck/stablebridge-platform
Length of output: 2577
🏁 Script executed:
# Check CircleRedemptionAdapterRetryTest's TestConfig to see how it handles wireMock.baseUrl()
grep -n "@Configuration\|@Bean\|wireMock.baseUrl()" fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.java | tail -20Repository: Puneethkumarck/stablebridge-platform
Length of output: 225
🏁 Script executed:
# Check if modulrPayoutAdapter bean is actually instantiated or if there's `@Lazy`
# Also check if the TestConfig bean itself has `@Lazy` or deferred instantiation
grep -B5 -A10 "class TestConfig" fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 606
🏁 Script executed:
# Let's verify the Spring/JUnit5 initialization order by checking Spring documentation
# First, let's see if similar tests have any special configuration or if they're working as-is
# Check if there's any `@RegisterExtension` or other mechanisms in similar tests
grep -B5 -A15 "@SpringBootTest" fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.java | head -30Repository: Puneethkumarck/stablebridge-platform
Length of output: 945
🏁 Script executed:
# Final verification: Check if ModulrPayoutAdapterTest (non-retry version) has the same pattern
grep -B5 -A10 "@SpringBootTest\|@BeforeAll" fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterTest.java | head -30Repository: Puneethkumarck/stablebridge-platform
Length of output: 544
🌐 Web query:
JUnit 5 @BeforeAllexecution order relative to Spring Boot@SpringBootTest context initialization
💡 Result:
With @SpringBootTest, the Spring ApplicationContext is initialized before your test class’s @BeforeAll method runs.
Execution order (high level):
- JUnit Jupiter runs
BeforeAllCallbackextensions first.SpringExtensionis one of these extensions. [1] SpringExtension.beforeAll()delegates toTestContextManager.beforeTestClass(), which is explicitly designed to run prior to JUnit’s@BeforeAll. This is where Spring’s TestContext framework performs its “before test class” work (including preparing/loading the context as needed). [2][3]- Only after that does JUnit invoke your
@BeforeAllmethod. [1]
Implication: in a @SpringBootTest, you should assume the Spring test context is already available by the time @BeforeAll executes (and you can typically use Spring injection there, subject to JUnit/Spring rules).
If you need code to run before the Spring context is created, you generally need to do it via:
- a JUnit extension that runs before
SpringExtension(ordering can be subtle with@SpringBootTest’s meta-registration), or - Spring’s context customization hooks (e.g.,
ApplicationContextInitializer/ context customizers), depending on what you’re trying to do. [4]
Sources
[1] JUnit “Relative Execution Order of User Code and Extensions” (BeforeAllCallback runs before @BeforeAll) (docs.junit.org)
[2] Spring SpringExtension Javadoc (beforeAll delegates to TestContextManager.beforeTestClass()) (docs.spring.io)
[3] Spring TestContextManager Javadoc (beforeTestClass is called prior to framework-specific @BeforeAll) (docs.enterprise.spring.io)
[4] Discussion of ordering pitfalls (e.g., PER_CLASS and extension ordering relative to SpringExtension) (stackoverflow.com)
Citations:
- 1: https://docs.junit.org/current/extensions/relative-execution-order-of-user-code-and-extensions.html
- 2: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/test/context/junit/jupiter/SpringExtension.html
- 3: https://docs.enterprise.spring.io/spring-framework/docs/6.1.22/javadoc-api/org/springframework/test/context/TestContextManager.html
- 4: https://stackoverflow.com/questions/51554973/executing-extension-before-springextension?utm_source=openai
WireMock initialization order: @BeforeAll runs after Spring context creation.
@BeforeAll executes after Spring's TestContextManager initializes the context. While wireMock.baseUrl() is called during bean instantiation, the pattern works throughout the codebase (12+ similar tests), suggesting beans are lazily instantiated or PayoutPartnerGateway is deferred.
However, follow Spring/JUnit 5 best practices: move WireMock initialization to a static initializer block to guarantee it's ready before context creation.
Recommended fix: static initializer
class ModulrPayoutAdapterRetryTest {
- private static WireMockServer wireMock;
+ private static final WireMockServer wireMock;
private static final UUID PAYOUT_ID = UUID.fromString("887adb57-1d2e-4f3a-b5c6-d7e8f9a0b1c2");
+ static {
+ wireMock = new WireMockServer(wireMockConfig().dynamicPort());
+ wireMock.start();
+ }
+
`@Autowired`
private PayoutPartnerGateway payoutPartnerGateway;
`@Autowired`
private CircuitBreakerRegistry circuitBreakerRegistry;
- `@BeforeAll`
- static void startWireMock() {
- wireMock = new WireMockServer(wireMockConfig().dynamicPort());
- wireMock.start();
- }
-
`@AfterAll`
static void stopWireMock() {
wireMock.stop();
}Also applies to: 167-176
🤖 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/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java`
around lines 45 - 58, The WireMock server is started in `@BeforeAll`
(startWireMock) which runs after Spring initializes the test context; move its
startup into a static initializer so wireMock is running before any beans (e.g.,
PayoutPartnerGateway) are instantiated and before calls to wireMock.baseUrl()
occur. Replace the `@BeforeAll` startWireMock logic by initializing and starting
the static WireMockServer wireMock inside a static { ... } block (ensuring
wireMockConfig().dynamicPort() is used) so all test beans and
CircuitBreakerRegistry load with the mock server available.
Puneethkumarck
force-pushed
the
feature/STA-117-resilience4j-retry-adapters
branch
from
12d6d85 to
483957c
Compare
Add @Retry annotation alongside existing @CIRCUITBREAKER on all 10 external service adapters across 5 services. Decorator ordering ensures Retry (outer, order=3) wraps CircuitBreaker (inner, order=1). Changes per service: - S2 Compliance: WorldCheck, Onfido, Chainalysis adapters - S6 FX: Refinitiv rate adapter (exponential backoff) - S3 On-Ramp: Stripe PSP adapter (payment + refund methods) - S4 Blockchain: Fireblocks, EVM RPC, Solana RPC adapters - S5 Off-Ramp: Circle, Modulr adapters (fallback params widened from CallNotPermittedException to Exception) Each service includes: - YAML retry config with retry-exceptions/ignore-exceptions - resilience4j-retry Gradle dependency - 3 retry behavior tests per adapter (30 total) - IT YAML overrides with wait-duration=10ms Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…r assertions (STA-117) - Replace Circuit breaker open with Resilience fallback in all 10 adapter fallback log messages since fallbacks now serve both Retry and CircuitBreaker - Tighten all retry test exception assertions from Exception.class to concrete types (IllegalStateException/PayoutPartnerException) with exact message checks - Add 3 getTransactionStatus retry tests for Fireblocks (GET endpoint coverage) - Extract magic spreadBps/feeBps numbers to named constants in Refinitiv test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…117) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Puneethkumarck
force-pushed
the
feature/STA-117-resilience4j-retry-adapters
branch
from
483957c to
4947d3a
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.java (1)
49-55:⚠️ Potential issue | 🟠 MajorScope retry per external call, not the entire
screen()method.Method-level
@Retryat line 49 wraps both sender and recipient screenings. When recipientscreenEntity()fails with a retryable exception (network timeout, HttpServerErrorException), the entire method retries—replaying the sender POST even though it already succeeded. This creates duplicate requests to the WorldCheck provider and scatters compliance decisions across multiple audit entries, complicating the audit trail for a single business transaction.The retry config filters non-retryable exceptions (HttpClientErrorException is ignored), but transient failures still trigger this behavior. Apply retry at the
screenEntity()call level instead to isolate failures and preserve audit clarity per compliance decision.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.java` around lines 49 - 55, Remove the method-level `@Retry` on screen(UUID, UUID) so retries don't replay both calls; instead annotate the lower-level external call method screenEntity(String, String) with `@Retry`(name = "sanctions", fallbackMethod = "screenFallback") so each WorldCheck POST is retried independently. Leave the `@CircuitBreaker` on screen(...) if you want an overall circuit for the composite operation, and ensure screenEntity(...) retains appropriate exception handling/ignored exceptions as configured by the "sanctions" retry policy.blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java (1)
81-83:⚠️ Potential issue | 🟠 MajorGuard confirmation math for Solana reorg/stale-slot cases.
confirmations = latestSlot - slotcan go negative under reorg or RPC lag. Return a bounded non-negative value and flag the inconsistency in logs/metrics.As per coding guidelines,
blockchain-custody/**: “Block confirmation polling must handle reorgs”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java` around lines 81 - 83, The confirmation calculation in SolanaRpcAdapter (where getLatestBlockNumber(chainId) sets latestSlot and confirmations is computed as (int)(latestSlot - slot)) can produce negative values during reorgs or RPC lag; change the logic to clamp confirmations to a non-negative value (e.g., Math.max(0, latestSlot - slot)) and emit a log/metric when latestSlot < slot to flag the inconsistency (include chainId, slot, latestSlot, and any request id/context) so callers of getLatestBlockNumber/confirmation logic can handle reorgs correctly.blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java (1)
83-85:⚠️ Potential issue | 🟠 MajorHandle negative confirmations for reorg/node-lag scenarios.
confirmations = latestBlock - blockNumbercan become negative during reorgs or stale-node reads. Clamp to0and emit a reorg/consistency warning signal before returning receipt data.As per coding guidelines,
blockchain-custody/**: “Block confirmation polling must handle reorgs”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java` around lines 83 - 85, In EvmRpcAdapter, when computing confirmations after calling getLatestBlockNumber(chainId) (variables latestBlock and blockNumber), clamp confirmations to a minimum of 0 instead of allowing a negative value and emit a reorg/consistency warning signal (e.g., via existing logging/metrics/alerting mechanism) before returning receipt data so callers can observe node-lag or chain reorgs; update the logic around the confirmations calculation to set confirmations = Math.max(0, latestBlock - blockNumber) (or equivalent) and add a conditional that logs/records a reorg warning when latestBlock < blockNumber.blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.java (1)
72-110:⚠️ Potential issue | 🟠 MajorHandle 400/1438 idempotency error from Fireblocks instead of treating it as a fatal failure.
Fireblocks prevents duplicate transactions via
externalTxId(stored permanently), and the code correctly setsexternalTxIdtorequest.transferId().toString(). However, if the first request succeeds but response delivery fails and a retry occurs, Fireblocks will return HTTP 400 with code 1438 ("external tx id already exists"). The fallback method currently treats this as a fatal error and throwsIllegalStateException, breaking the idempotency contract.Implement the Fireblocks-recommended pattern: on HTTP 400 with code 1438, query the existing transaction via
GET /v1/transactions/external_tx_id/{externalTxId}and return its result rather than propagating the error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.java` around lines 72 - 110, The signAndSubmit method currently treats Fireblocks 400/code 1438 as a fatal error; instead catch the 400 response with error code 1438 (from the POST to /v1/transactions) and implement the recommended idempotency flow: call GET /v1/transactions/external_tx_id/{externalTxId} (use the same restClient, buildJwt for the GET URI via buildJwt("/v1/transactions/external_tx_id/" + request.transferId(), null) or appropriate overload), parse the response into FireblocksCreateTransactionResponse and return a SignResult built from that existing transaction (instead of throwing IllegalStateException or letting signAndSubmitFallback treat it as fatal). Update signAndSubmit (or its error handling) to detect the 400+1438 case, perform the GET lookup, log the lookup result, and return the existing custodyTxId using the FireblocksCreateTransactionResponse.id(); leave other errors to the existing retry/fallback behavior.
♻️ Duplicate comments (13)
compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.java (1)
52-54:⚠️ Potential issue | 🟠 MajorRetry on
analyze()can cause duplicate transfer registrations on partial failure.This was flagged in a previous review. The
@Retrywraps the entire method, so ifregisterAndAnalyzeTransfer(recipientId)fails afterregisterAndAnalyzeTransfer(senderId)succeeds, the retry re-executes the sender POST.Options:
- Move
@RetrytoregisterAndAnalyzeTransfer()for per-call isolation- Handle 409 Conflict in
registerTransfer()as idempotent success🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.java` around lines 52 - 54, The analyze() method is currently annotated with `@Retry` which can cause duplicate transfer registrations if one registerAndAnalyzeTransfer(recipientId) call fails after another succeeded; move the `@Retry` and `@CircuitBreaker` annotations off analyze() and instead apply `@Retry` (and optionally `@CircuitBreaker`) to registerAndAnalyzeTransfer(String transferParticipantId) so each POST is retried/isolated per participant, or alternatively modify registerTransfer(...) to treat HTTP 409 Conflict as an idempotent success (returning the existing/accepted response) so retries from analyze() won’t create duplicates; update the code paths in analyze(), registerAndAnalyzeTransfer(), and registerTransfer() accordingly to implement the chosen approach.compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.java (2)
62-64:⚠️ Potential issue | 🟠 MajorMocked Redis violates integration test guidelines.
This was flagged in a previous review. The
StringRedisTemplateis mocked, so the retry path never exercises the real cache integration.Use Testcontainers Redis or isolate cache behavior behind a port interface:
`@Container` static GenericContainer<?> redis = new GenericContainer<>("redis:7-alpine") .withExposedPorts(6379);As per coding guidelines,
**/*Test*.java: "Integration tests must use@SpringBootTestor Testcontainers — no mocking of DB".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.java` around lines 62 - 64, The test OnfidoKycAdapterRetryTest currently mocks StringRedisTemplate (redisTemplate) and ValueOperations (valueOps), which violates integration-test guidelines; replace the mocks with a real Redis instance using Testcontainers (e.g., a GenericContainer for redis:7-alpine with exposed port 6379) and wire a real StringRedisTemplate to that container in the test setup (or refactor cache access behind an interface and inject a test implementation) so the retry path exercises the real cache; ensure OnfidoKycAdapterRetryTest starts the container before Spring context initialization and configure the StringRedisTemplate to point to the container's mapped port (or swap to the cache interface implementation) and remove the given(redisTemplate.opsForValue()) mock setup.
121-132:⚠️ Potential issue | 🟡 MinorAdd request count verification to confirm retry occurred.
This was flagged in a previous review. Without
wireMock.verify(), the test passes even if retry logic is broken.Proposed fix
assertThat(result) .usingRecursiveComparison() .ignoringFields("kycResultId", "checkId", "providerRef", "checkedAt") .isEqualTo(expected); + + // Expect 3 requests: sender (503 → retry → 200 = 2) + recipient (200 = 1) + wireMock.verify(3, getRequestedFor(urlPathEqualTo("/v3.6/checks")));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.java` around lines 121 - 132, Add a WireMock verification after the assert to ensure the retry actually occurred: call WireMock.verify with the expected repeat count (e.g., 3) and a request matcher for the Onfido endpoint used by OnfidoKycAdapter (e.g., postRequestedFor(urlPathEqualTo("/v3/checks")) or the exact path your adapter posts to) so the test not only compares KycResult but also asserts the number of outbound requests to confirm retry logic executed.compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapterRetryTest.java (1)
62-63:⚠️ Potential issue | 🟡 MinorHardcoded test credentials should use generated placeholders.
This was flagged in a previous review. Use UUID-based placeholders instead of literal strings:
registry.add("app.sanctions.world-check.api-key", () -> "wc-it-" + UUID.randomUUID()); registry.add("app.sanctions.world-check.api-secret", () -> UUID.randomUUID().toString());Same applies to lines 169-170 in
TestConfig.As per coding guidelines,
**/*Test*.java: "No hardcoded secrets or real PII in test fixtures".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapterRetryTest.java` around lines 62 - 63, Replace the hardcoded test credentials added to the EnvironmentRegistry in WorldCheckSanctionsAdapterRetryTest (the two registry.add calls for "app.sanctions.world-check.api-key" and "app.sanctions.world-check.api-secret") with UUID-based placeholders (e.g., prefix the api-key with "wc-it-" plus UUID.randomUUID() and use UUID.randomUUID().toString() for the secret); apply the same change for the corresponding hardcoded entries in TestConfig (the two entries around lines 169-170) so both tests use generated UUID placeholders instead of literal strings.compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapterRetryTest.java (1)
142-154: 🧹 Nitpick | 🔵 TrivialConsider asserting the root cause for stronger contract validation.
This was suggested in a previous review. Adding cause verification strengthens the test contract:
Optional: assert cause chain
assertThatThrownBy(() -> amlProvider.analyze(SENDER_ID, RECIPIENT_ID)) .isInstanceOf(IllegalStateException.class) - .hasMessage("AML screening unavailable"); + .hasMessage("AML screening unavailable") + .hasCauseInstanceOf(org.springframework.web.client.HttpServerErrorException.class);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapterRetryTest.java` around lines 142 - 154, The test should also assert the root cause of the thrown IllegalStateException to strengthen the contract: after invoking amlProvider.analyze in shouldExhaustRetriesAndInvokeFallback, add an assertion on the exception's cause (using AssertJ's hasCauseInstanceOf or hasRootCauseInstanceOf) that matches the actual underlying exception produced by the retry/fallback logic (inspect the real cause type thrown by the HTTP/client layer used by amlProvider). This ensures the test not only checks the message but also validates the underlying failure that led to IllegalStateException.compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.java (1)
165-166:⚠️ Potential issue | 🟠 MajorRemove throwable from compliance fallback log.
At Line 166, passing
exlogs stacktrace/message content; keep only sanitized metadata.Proposed fix
- log.error("[WORLD-CHECK] Resilience fallback — screening failed sender={} recipient={} due to {}", - senderId, recipientId, ex.getClass().getSimpleName(), ex); + log.error("[WORLD-CHECK] Resilience fallback — screening failed sender={} recipient={} due to {}", + senderId, recipientId, ex.getClass().getSimpleName());As per coding guidelines,
compliance-travel-rule/**: "PII data (names, addresses) must never appear in logs — use IDs only".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.java` around lines 165 - 166, The log in WorldCheckSanctionsAdapter currently passes the throwable `ex` to log.error which emits stacktraces/PII; update the resilience fallback log in the method that logs senderId/recipientId so it only includes sanitized metadata and the exception type (e.g., ex.getClass().getSimpleName()) and does NOT pass the throwable as the last parameter—remove `ex` from the varargs so no stacktrace/message is logged. Locate the log.error call in WorldCheckSanctionsAdapter and adjust the arguments accordingly to avoid emitting the throwable.blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapterRetryTest.java (1)
78-126:⚠️ Potential issue | 🟡 MinorRetry scenarios should also cover EVM receipt and balance methods.
Current tests exercise only
getLatestBlockNumber; retry behavior ongetTransactionReceiptandgetTokenBalanceis unverified in this suite. Add equivalent transient/4xx/persistent failure cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapterRetryTest.java` around lines 78 - 126, Tests only exercise retry behavior for getLatestBlockNumber; add equivalent test cases to cover getTransactionReceipt and getTokenBalance: for each method create three tests mirroring the existing patterns in EvmRpcAdapterRetryTest — (1) transient 503 then success scenario using a WireMock scenario that returns 503 then 200 and assert correct successful return, (2) 400 client error that asserts IllegalStateException with "EVM RPC unavailable for <methodName>" and verify only one request, and (3) persistent 503 that asserts IllegalStateException and verifies the retry count (3) — reuse the same WireMock stubbing style and assertions as in the existing methods but call chainRpcProvider.getTransactionReceipt(...) and chainRpcProvider.getTokenBalance(...) instead of getLatestBlockNumber.blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java (1)
83-83:⚠️ Potential issue | 🟡 Minor
getLatestBlockNumberresilience annotations are bypassed on internal call.Line 83 invokes
getLatestBlockNumbervia self-invocation, so the inner@Retry/@CircuitBreakeradvice is skipped. If independent retry semantics are intended for that inner RPC, route the call through a proxied collaborator bean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java` at line 83, The call to getLatestBlockNumber inside EvmRpcAdapter (the var latestBlock = getLatestBlockNumber(chainId) line) bypasses the `@Retry/`@CircuitBreaker advice because it’s a self-invocation; change the call to go through a proxied bean so Spring AOP can apply resilience annotations. Refactor by extracting the RPC method(s) into a separate Spring component (e.g., EvmRpcClient or EvmRpcAdapterClient) or obtain the proxied self via injection/ApplicationContext and call that bean’s getLatestBlockNumber(chainId) instead of the private/self method; ensure the method keeping `@Retry/`@CircuitBreaker remains public/proxied on the new bean so retry/circuit-breaker semantics are executed.blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapterRetryTest.java (1)
76-124:⚠️ Potential issue | 🟡 MinorRetry test coverage is still partial for Solana adapter.
This class validates only
getLatestBlockNumber, while retry was also added ongetTransactionReceiptandgetTokenBalance. Add the same 503→200, 400 no-retry, and persistent-503 fallback scenarios for those methods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapterRetryTest.java` around lines 76 - 124, Add the same retry test scenarios currently present for getLatestBlockNumber to cover getTransactionReceipt and getTokenBalance: for each method (chainRpcProvider.getTransactionReceipt and chainRpcProvider.getTokenBalance) add a test that stubs a 503 then a 200 (verify success and expected returned value), a test that returns 400 and asserts the IllegalStateException with message "Solana RPC unavailable for <methodName>" and verifies exactly 1 request, and a test that returns persistent 503 that asserts the same IllegalStateException and verifies the retry count (3) was made; use the same WireMock scenario and verification patterns (post(urlEqualTo("/")), inScenario/whenScenarioStateIs/willSetStateTo, willReturn with status/body) as in the existing getLatestBlockNumber tests.blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java (1)
81-81:⚠️ Potential issue | 🟡 MinorInner
getLatestBlockNumberretry/circuit-breaker advice is bypassed.Line 81 calls
getLatestBlockNumberwithin the same bean, bypassing proxy interception. If you need per-call resilience semantics on that method, invoke through a proxied collaborator.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java` at line 81, The call to getLatestBlockNumber(...) on the same SolanaRpcAdapter instance bypasses Spring proxy interception so its retry/circuit-breaker annotations are not applied; instead invoke the method via a proxied collaborator: either extract getLatestBlockNumber into a separate `@Component/`@Service (e.g., SolanaRpcClient or SolanaBlockService) and inject that bean into SolanaRpcAdapter, or obtain the proxied self bean (ApplicationContext.getBean(SolanaRpcAdapter.class) or `@Autowired` proxy) and call getLatestBlockNumber through that proxy; update the call site where latestSlot is assigned to use the proxied bean to ensure the resilience annotations are honored.fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java (1)
91-94:⚠️ Potential issue | 🟠 MajorRe-throw
HttpClientErrorExceptionto preserve 4xx semantics.The broadened
Exceptionfallback now catches 4xx client errors (e.g., invalid destination). These should propagate to callers rather than being masked as "Circle redemption unavailable"—losing error fidelity.Proposed fix
+import org.springframework.web.client.HttpClientErrorException; `@SuppressWarnings`("unused") private RedemptionResult redeemFallback(RedemptionRequest request, Exception ex) { + if (ex instanceof HttpClientErrorException) { + throw (HttpClientErrorException) ex; + } log.error("[CIRCLE] Resilience fallback — redemption failed payoutId={} due to {}", request.payoutId(), ex.getClass().getSimpleName(), ex); throw new IllegalStateException("Circle redemption unavailable", ex); }🤖 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/infrastructure/provider/circle/CircleRedemptionAdapter.java` around lines 91 - 94, The fallback method redeemFallback currently catches all Exception and wraps everything as IllegalStateException, which masks 4xx client errors; update redeemFallback(RedemptionRequest request, Exception ex) to detect if ex is an instance of org.springframework.web.client.HttpClientErrorException (or its superclass used in the project) and re-throw that exception unchanged so 4xx semantics propagate to callers, otherwise keep the current behavior of logging and throwing new IllegalStateException("Circle redemption unavailable", ex); ensure the log still records ex and request.payoutId() as before.fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java (1)
54-58:⚠️ Potential issue | 🟡 MinorWireMock initialization should use static initializer to avoid NPE.
TestConfig.modulrProperties()callswireMock.baseUrl()during Spring context creation, which happens before@BeforeAll. If the context instantiatesModulrPayoutAdaptereagerly,wireMockwill benull.Recommended fix
class ModulrPayoutAdapterRetryTest { - private static WireMockServer wireMock; + private static final WireMockServer wireMock; private static final UUID PAYOUT_ID = UUID.fromString("887adb57-1d2e-4f3a-b5c6-d7e8f9a0b1c2"); + static { + wireMock = new WireMockServer(wireMockConfig().dynamicPort()); + wireMock.start(); + } + `@Autowired` private PayoutPartnerGateway payoutPartnerGateway; `@Autowired` private CircuitBreakerRegistry circuitBreakerRegistry; - `@BeforeAll` - static void startWireMock() { - wireMock = new WireMockServer(wireMockConfig().dynamicPort()); - wireMock.start(); - } - `@AfterAll` static void stopWireMock() { wireMock.stop(); }Also applies to: 167-171
🤖 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/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java` around lines 54 - 58, The test's WireMockServer is initialized in `@BeforeAll` which can be too late because TestConfig.modulrProperties() calls wireMock.baseUrl() during Spring context creation, causing a NPE; fix by initializing the static WireMockServer earlier (e.g., in a static initializer or inline static field assignment) so the static variable is non-null before the Spring context is built—update the static wireMock declaration in ModulrPayoutAdapterRetryTest (and the similar occurrence around the later block) to create the WireMockServer with wireMockConfig().dynamicPort() at class-load time instead of waiting for `@BeforeAll`, keeping the start() call in `@BeforeAll` if you still need to start it there.fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.java (1)
54-58:⚠️ Potential issue | 🔴 CriticalWireMock still starts too late for
stripeProperties().Line 176 still calls
wireMock.baseUrl()while the Spring test context is being created. With@SpringBootTest,SpringExtensionis active; JUnit executes extensionBeforeAllCallbacks before user@BeforeAll, and Spring performs its test-class bootstrap from that callback. This can still fail beforestartWireMock()initializes the server. (docs.spring.io)Also applies to: 174-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.java` around lines 54 - 58, The WireMock server is being started too late for stripeProperties() which is invoked during Spring test bootstrap; to fix it initialize and start the WireMockServer earlier (before any JUnit extensions run) by creating and starting the server in a static initializer (e.g., a static block that constructs wireMock = new WireMockServer(wireMockConfig().dynamicPort()) and calls wireMock.start()) or by switching to a JUnit `@RegisterExtension` WireMockExtension so the server is available when stripeProperties() calls wireMock.baseUrl(); update or remove the existing startWireMock() method accordingly and ensure references to wireMock, startWireMock(), and stripeProperties() reflect the early-start change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@blockchain-custody/blockchain-custody/build.gradle.kts`:
- Line 103: The new dependency
implementation("io.github.resilience4j:resilience4j-retry:$resilience4jVersion")
appears added without confirmation—verify whether resilience4j-retry is actually
required by this module (vs available transitively) and either remove it if
unnecessary or keep it but align it to the project version-management approach:
if you use a version catalog (libs.versions.toml) reference the catalog entry
(e.g., libs.resilience4j.retry) instead of a raw $resilience4jVersion, or if
project-wide variables are used ensure $resilience4jVersion is defined
centrally; update the build.gradle.kts dependency declaration accordingly and
add a brief justification in the PR comment.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.java`:
- Around line 72-74: The CircuitBreaker is short-circuiting Retry because both
`@CircuitBreaker` and `@Retry` point to the same fallback that accepts broad
Exception; update the fallbacks so the CircuitBreaker fallback only handles
CallNotPermittedException (e.g., create
signAndSubmitCircuitBreakerFallback(CallNotPermittedException ex, SignRequest
request) and
getTransactionStatusCircuitBreakerFallback(CallNotPermittedException ex, String
txId)) while keeping the Retry fallback to accept the broader Exception
(signAndSubmitFallback(Exception ex, SignRequest request) and
getTransactionStatusFallback(Exception ex, String txId)), or alternatively set
explicit aspect order via retryAspectOrder/circuitBreakerAspectOrder so Retry
runs before CircuitBreaker; ensure the new fallback signatures match the methods
signAndSubmit and getTransactionStatus to let Retry perform retries for
transient errors.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapterRetryTest.java`:
- Around line 91-117: The test and adapter must include and assert an
idempotency token on POST /v1/transactions: update the FireblocksCustodyAdapter
(the method that builds and sends the POST in signAndSubmit flow) to add a
unique Idempotency-Key header for each logical request, then update
FireblocksCustodyAdapterRetryTest.shouldRetryOnTransientFailureThenSucceed to
stub/expect that header on the WireMock POST stub (check header value or
presence) and verify the exact POST invocation count for the scenario; reference
the POST to "/v1/transactions" and the signAndSubmit/aSignRequest() path so the
test ensures retries include the same Idempotency-Key and that duplicate
submissions are prevented.
In
`@compliance-travel-rule/compliance-travel-rule/src/main/resources/application.yml`:
- Around line 137-149: Update the sanctions retry config under
instances.sanctions to enable exponential backoff and jitter for production
traffic: add enable-exponential-backoff: true, exponential-backoff-multiplier:
2, exponential-max-wait-duration: 5s (and consider adding jitter if supported by
the retry library) so the retry strategy for the sanctions configuration
(instances.sanctions) uses exponential growth instead of the fixed wait-duration
of 500ms.
In
`@fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.java`:
- Around line 50-58: The test fails because wireMock is null during Spring
context bootstrap; move its startup into a static initializer so it exists
before beans like circleProperties() are created: replace the `@BeforeAll`
startWireMock method with a static initializer block that instantiates and
starts the WireMockServer assigned to the static wireMock field in
CircleRedemptionAdapterRetryTest, keep or retain the `@AfterAll` stopWireMock to
stop the server (or register a JVM shutdown hook) so tear-down still occurs, and
remove the `@BeforeAll` annotation/method to avoid late initialization.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapter.java`:
- Around line 50-52: The circuit-breaker and retry are sharing the same fallback
(initiatePaymentFallback) which prevents Retry from seeing exceptions; change
the CircuitBreaker annotation to use a dedicated fallback (e.g.,
initiatePaymentCircuitOpenFallback) and implement that new method to accept
CallNotPermittedException plus the original parameters/return type, while
leaving initiatePaymentFallback as the retry fallback that accepts broad
Exception; update the `@CircuitBreaker`(name = "stripe", fallbackMethod =
"initiatePaymentCircuitOpenFallback") on initiatePayment and add the new
initiatePaymentCircuitOpenFallback(CallNotPermittedException, PspPaymentRequest)
method to handle circuit-open cases only.
In `@fx-liquidity-engine/fx-liquidity-engine/src/main/resources/application.yml`:
- Around line 129-131: The aspect ordering is inverted: swap the values of
circuit-breaker-aspect-order and retry-aspect-order so Retry executes as the
outer aspect; specifically change the properties named
circuit-breaker-aspect-order and retry-aspect-order so retry-aspect-order is set
to 1 and circuit-breaker-aspect-order is set to 3 to ensure Retry wraps
CircuitBreaker.
In
`@fx-liquidity-engine/fx-liquidity-engine/src/test/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapterRetryTest.java`:
- Around line 50-53: RefinitivRateAdapterRetryTest fails because wireMock is
started in `@BeforeAll` (startWireMock) which runs after Spring context creation
while RefinitivProperties calls wireMock.baseUrl() during bean instantiation;
fix by starting the WireMockServer earlier (e.g., initialize and start the
static wireMock in a static initializer or as a static field that calls new
WireMockServer(...).start()) so wireMock is non-null and running before Spring
bootstraps the context; update the startWireMock method or remove it in favor of
the static initializer and ensure the static wireMock variable remains the
single server used by the test.
---
Outside diff comments:
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java`:
- Around line 83-85: In EvmRpcAdapter, when computing confirmations after
calling getLatestBlockNumber(chainId) (variables latestBlock and blockNumber),
clamp confirmations to a minimum of 0 instead of allowing a negative value and
emit a reorg/consistency warning signal (e.g., via existing
logging/metrics/alerting mechanism) before returning receipt data so callers can
observe node-lag or chain reorgs; update the logic around the confirmations
calculation to set confirmations = Math.max(0, latestBlock - blockNumber) (or
equivalent) and add a conditional that logs/records a reorg warning when
latestBlock < blockNumber.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.java`:
- Around line 72-110: The signAndSubmit method currently treats Fireblocks
400/code 1438 as a fatal error; instead catch the 400 response with error code
1438 (from the POST to /v1/transactions) and implement the recommended
idempotency flow: call GET /v1/transactions/external_tx_id/{externalTxId} (use
the same restClient, buildJwt for the GET URI via
buildJwt("/v1/transactions/external_tx_id/" + request.transferId(), null) or
appropriate overload), parse the response into
FireblocksCreateTransactionResponse and return a SignResult built from that
existing transaction (instead of throwing IllegalStateException or letting
signAndSubmitFallback treat it as fatal). Update signAndSubmit (or its error
handling) to detect the 400+1438 case, perform the GET lookup, log the lookup
result, and return the existing custodyTxId using the
FireblocksCreateTransactionResponse.id(); leave other errors to the existing
retry/fallback behavior.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java`:
- Around line 81-83: The confirmation calculation in SolanaRpcAdapter (where
getLatestBlockNumber(chainId) sets latestSlot and confirmations is computed as
(int)(latestSlot - slot)) can produce negative values during reorgs or RPC lag;
change the logic to clamp confirmations to a non-negative value (e.g.,
Math.max(0, latestSlot - slot)) and emit a log/metric when latestSlot < slot to
flag the inconsistency (include chainId, slot, latestSlot, and any request
id/context) so callers of getLatestBlockNumber/confirmation logic can handle
reorgs correctly.
In
`@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.java`:
- Around line 49-55: Remove the method-level `@Retry` on screen(UUID, UUID) so
retries don't replay both calls; instead annotate the lower-level external call
method screenEntity(String, String) with `@Retry`(name = "sanctions",
fallbackMethod = "screenFallback") so each WorldCheck POST is retried
independently. Leave the `@CircuitBreaker` on screen(...) if you want an overall
circuit for the composite operation, and ensure screenEntity(...) retains
appropriate exception handling/ignored exceptions as configured by the
"sanctions" retry policy.
---
Duplicate comments:
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.java`:
- Line 83: The call to getLatestBlockNumber inside EvmRpcAdapter (the var
latestBlock = getLatestBlockNumber(chainId) line) bypasses the
`@Retry/`@CircuitBreaker advice because it’s a self-invocation; change the call to
go through a proxied bean so Spring AOP can apply resilience annotations.
Refactor by extracting the RPC method(s) into a separate Spring component (e.g.,
EvmRpcClient or EvmRpcAdapterClient) or obtain the proxied self via
injection/ApplicationContext and call that bean’s getLatestBlockNumber(chainId)
instead of the private/self method; ensure the method keeping
`@Retry/`@CircuitBreaker remains public/proxied on the new bean so
retry/circuit-breaker semantics are executed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.java`:
- Line 81: The call to getLatestBlockNumber(...) on the same SolanaRpcAdapter
instance bypasses Spring proxy interception so its retry/circuit-breaker
annotations are not applied; instead invoke the method via a proxied
collaborator: either extract getLatestBlockNumber into a separate
`@Component/`@Service (e.g., SolanaRpcClient or SolanaBlockService) and inject
that bean into SolanaRpcAdapter, or obtain the proxied self bean
(ApplicationContext.getBean(SolanaRpcAdapter.class) or `@Autowired` proxy) and
call getLatestBlockNumber through that proxy; update the call site where
latestSlot is assigned to use the proxied bean to ensure the resilience
annotations are honored.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapterRetryTest.java`:
- Around line 78-126: Tests only exercise retry behavior for
getLatestBlockNumber; add equivalent test cases to cover getTransactionReceipt
and getTokenBalance: for each method create three tests mirroring the existing
patterns in EvmRpcAdapterRetryTest — (1) transient 503 then success scenario
using a WireMock scenario that returns 503 then 200 and assert correct
successful return, (2) 400 client error that asserts IllegalStateException with
"EVM RPC unavailable for <methodName>" and verify only one request, and (3)
persistent 503 that asserts IllegalStateException and verifies the retry count
(3) — reuse the same WireMock stubbing style and assertions as in the existing
methods but call chainRpcProvider.getTransactionReceipt(...) and
chainRpcProvider.getTokenBalance(...) instead of getLatestBlockNumber.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapterRetryTest.java`:
- Around line 76-124: Add the same retry test scenarios currently present for
getLatestBlockNumber to cover getTransactionReceipt and getTokenBalance: for
each method (chainRpcProvider.getTransactionReceipt and
chainRpcProvider.getTokenBalance) add a test that stubs a 503 then a 200 (verify
success and expected returned value), a test that returns 400 and asserts the
IllegalStateException with message "Solana RPC unavailable for <methodName>" and
verifies exactly 1 request, and a test that returns persistent 503 that asserts
the same IllegalStateException and verifies the retry count (3) was made; use
the same WireMock scenario and verification patterns (post(urlEqualTo("/")),
inScenario/whenScenarioStateIs/willSetStateTo, willReturn with status/body) as
in the existing getLatestBlockNumber tests.
In
`@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.java`:
- Around line 52-54: The analyze() method is currently annotated with `@Retry`
which can cause duplicate transfer registrations if one
registerAndAnalyzeTransfer(recipientId) call fails after another succeeded; move
the `@Retry` and `@CircuitBreaker` annotations off analyze() and instead apply
`@Retry` (and optionally `@CircuitBreaker`) to registerAndAnalyzeTransfer(String
transferParticipantId) so each POST is retried/isolated per participant, or
alternatively modify registerTransfer(...) to treat HTTP 409 Conflict as an
idempotent success (returning the existing/accepted response) so retries from
analyze() won’t create duplicates; update the code paths in analyze(),
registerAndAnalyzeTransfer(), and registerTransfer() accordingly to implement
the chosen approach.
In
`@compliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.java`:
- Around line 165-166: The log in WorldCheckSanctionsAdapter currently passes
the throwable `ex` to log.error which emits stacktraces/PII; update the
resilience fallback log in the method that logs senderId/recipientId so it only
includes sanitized metadata and the exception type (e.g.,
ex.getClass().getSimpleName()) and does NOT pass the throwable as the last
parameter—remove `ex` from the varargs so no stacktrace/message is logged.
Locate the log.error call in WorldCheckSanctionsAdapter and adjust the arguments
accordingly to avoid emitting the throwable.
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapterRetryTest.java`:
- Around line 142-154: The test should also assert the root cause of the thrown
IllegalStateException to strengthen the contract: after invoking
amlProvider.analyze in shouldExhaustRetriesAndInvokeFallback, add an assertion
on the exception's cause (using AssertJ's hasCauseInstanceOf or
hasRootCauseInstanceOf) that matches the actual underlying exception produced by
the retry/fallback logic (inspect the real cause type thrown by the HTTP/client
layer used by amlProvider). This ensures the test not only checks the message
but also validates the underlying failure that led to IllegalStateException.
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.java`:
- Around line 62-64: The test OnfidoKycAdapterRetryTest currently mocks
StringRedisTemplate (redisTemplate) and ValueOperations (valueOps), which
violates integration-test guidelines; replace the mocks with a real Redis
instance using Testcontainers (e.g., a GenericContainer for redis:7-alpine with
exposed port 6379) and wire a real StringRedisTemplate to that container in the
test setup (or refactor cache access behind an interface and inject a test
implementation) so the retry path exercises the real cache; ensure
OnfidoKycAdapterRetryTest starts the container before Spring context
initialization and configure the StringRedisTemplate to point to the container's
mapped port (or swap to the cache interface implementation) and remove the
given(redisTemplate.opsForValue()) mock setup.
- Around line 121-132: Add a WireMock verification after the assert to ensure
the retry actually occurred: call WireMock.verify with the expected repeat count
(e.g., 3) and a request matcher for the Onfido endpoint used by OnfidoKycAdapter
(e.g., postRequestedFor(urlPathEqualTo("/v3/checks")) or the exact path your
adapter posts to) so the test not only compares KycResult but also asserts the
number of outbound requests to confirm retry logic executed.
In
`@compliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapterRetryTest.java`:
- Around line 62-63: Replace the hardcoded test credentials added to the
EnvironmentRegistry in WorldCheckSanctionsAdapterRetryTest (the two registry.add
calls for "app.sanctions.world-check.api-key" and
"app.sanctions.world-check.api-secret") with UUID-based placeholders (e.g.,
prefix the api-key with "wc-it-" plus UUID.randomUUID() and use
UUID.randomUUID().toString() for the secret); apply the same change for the
corresponding hardcoded entries in TestConfig (the two entries around lines
169-170) so both tests use generated UUID placeholders instead of literal
strings.
In
`@fiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.java`:
- Around line 91-94: The fallback method redeemFallback currently catches all
Exception and wraps everything as IllegalStateException, which masks 4xx client
errors; update redeemFallback(RedemptionRequest request, Exception ex) to detect
if ex is an instance of org.springframework.web.client.HttpClientErrorException
(or its superclass used in the project) and re-throw that exception unchanged so
4xx semantics propagate to callers, otherwise keep the current behavior of
logging and throwing new IllegalStateException("Circle redemption unavailable",
ex); ensure the log still records ex and request.payoutId() as before.
In
`@fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.java`:
- Around line 54-58: The test's WireMockServer is initialized in `@BeforeAll`
which can be too late because TestConfig.modulrProperties() calls
wireMock.baseUrl() during Spring context creation, causing a NPE; fix by
initializing the static WireMockServer earlier (e.g., in a static initializer or
inline static field assignment) so the static variable is non-null before the
Spring context is built—update the static wireMock declaration in
ModulrPayoutAdapterRetryTest (and the similar occurrence around the later block)
to create the WireMockServer with wireMockConfig().dynamicPort() at class-load
time instead of waiting for `@BeforeAll`, keeping the start() call in `@BeforeAll`
if you still need to start it there.
In
`@fiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.java`:
- Around line 54-58: The WireMock server is being started too late for
stripeProperties() which is invoked during Spring test bootstrap; to fix it
initialize and start the WireMockServer earlier (before any JUnit extensions
run) by creating and starting the server in a static initializer (e.g., a static
block that constructs wireMock = new
WireMockServer(wireMockConfig().dynamicPort()) and calls wireMock.start()) or by
switching to a JUnit `@RegisterExtension` WireMockExtension so the server is
available when stripeProperties() calls wireMock.baseUrl(); update or remove the
existing startWireMock() method accordingly and ensure references to wireMock,
startWireMock(), and stripeProperties() reflect the early-start change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98c9db3e-3e25-4bd0-b166-5f8f6d21437a
📒 Files selected for processing (35)
blockchain-custody/blockchain-custody/build.gradle.ktsblockchain-custody/blockchain-custody/src/integration-test/resources/application-integration-test.ymlblockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.javablockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapter.javablockchain-custody/blockchain-custody/src/main/resources/application.ymlblockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/evm/EvmRpcAdapterRetryTest.javablockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapterRetryTest.javablockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/solana/SolanaRpcAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/build.gradle.ktscompliance-travel-rule/compliance-travel-rule/src/integration-test/resources/application-integration-test.ymlcompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapter.javacompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapter.javacompliance-travel-rule/compliance-travel-rule/src/main/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapter.javacompliance-travel-rule/compliance-travel-rule/src/main/resources/application.ymlcompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/chainalysis/ChainalysisAmlAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/onfido/OnfidoKycAdapterRetryTest.javacompliance-travel-rule/compliance-travel-rule/src/test/java/com/stablecoin/payments/compliance/infrastructure/provider/worldcheck/WorldCheckSanctionsAdapterRetryTest.javafiat-off-ramp/fiat-off-ramp/build.gradle.ktsfiat-off-ramp/fiat-off-ramp/src/integration-test/resources/application-integration-test.ymlfiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapter.javafiat-off-ramp/fiat-off-ramp/src/main/resources/application.ymlfiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.javafiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/modulr/ModulrPayoutAdapterRetryTest.javafiat-on-ramp/fiat-on-ramp/build.gradle.ktsfiat-on-ramp/fiat-on-ramp/src/integration-test/resources/application-integration-test.ymlfiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapter.javafiat-on-ramp/fiat-on-ramp/src/main/resources/application.ymlfiat-on-ramp/fiat-on-ramp/src/test/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapterRetryTest.javafx-liquidity-engine/fx-liquidity-engine/build.gradle.ktsfx-liquidity-engine/fx-liquidity-engine/src/integration-test/resources/application-integration-test.ymlfx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapter.javafx-liquidity-engine/fx-liquidity-engine/src/main/resources/application.ymlfx-liquidity-engine/fx-liquidity-engine/src/test/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapterRetryTest.java
| // Resilience4j | ||
| implementation("io.github.resilience4j:resilience4j-spring-boot3:$resilience4jVersion") | ||
| implementation("io.github.resilience4j:resilience4j-circuitbreaker:$resilience4jVersion") | ||
| implementation("io.github.resilience4j:resilience4j-retry:$resilience4jVersion") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Confirm this new dependency is intentionally required in this module.
resilience4j-retry is newly added in main scope. Please confirm this is required (vs transitive availability) and remains aligned with your version management approach to avoid dependency drift.
As per coding guidelines, **/*.gradle.kts: “Flag any newly added dependencies and ask if they are necessary” and “Check for version catalog alignment (all versions should come from libs.versions.toml if present)”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@blockchain-custody/blockchain-custody/build.gradle.kts` at line 103, The new
dependency
implementation("io.github.resilience4j:resilience4j-retry:$resilience4jVersion")
appears added without confirmation—verify whether resilience4j-retry is actually
required by this module (vs available transitively) and either remove it if
unnecessary or keep it but align it to the project version-management approach:
if you use a version catalog (libs.versions.toml) reference the catalog entry
(e.g., libs.resilience4j.retry) instead of a raw $resilience4jVersion, or if
project-wide variables are used ensure $resilience4jVersion is defined
centrally; update the build.gradle.kts dependency declaration accordingly and
add a brief justification in the PR comment.
Comment on lines
+72
to
74
| @Retry(name = "fireblocks", fallbackMethod = "signAndSubmitFallback") | ||
| @CircuitBreaker(name = "fireblocks", fallbackMethod = "signAndSubmitFallback") | ||
| public SignResult signAndSubmit(SignRequest request) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Resilience4j Spring annotations, when both @Retryand@CIRCUITBREAKER define fallbackMethod, does a broad CircuitBreaker fallback(Exception) prevent retry attempts? Please cite official docs/issues.
💡 Result:
Yes. With Resilience4j Spring annotations, the aspects are nested by default as:
Retry ( CircuitBreaker ( … ( Function ) ) ) — i.e., Retry wraps CircuitBreaker. If the CircuitBreaker executes a fallbackMethod that matches the thrown exception (e.g., fallback(..., Exception) / RuntimeException), it handles the failure and returns a normal value, so no exception escapes outward to the Retry aspect, and therefore Retry won’t perform any retry attempts. This exact “retry never called when CircuitBreaker specifies a fallback” behavior is documented in Resilience4j issue #558. [1][2]
Why a broad fallback(Exception) is especially “blocking”:
- Resilience4j chooses the fallback based on the closest matching exception parameter; a broad
Exception/RuntimeExceptionfallback will match many failures, so it will be used and will swallow the exception before Retry can see it. [1]
How to avoid suppressing retries (per the same issue/docs):
- Don’t use a broad CircuitBreaker fallback; instead, make the CircuitBreaker fallback only handle the “open circuit” case (e.g.,
CallNotPermittedException), so other exceptions propagate to Retry. [2] - Or change aspect order via properties so CircuitBreaker runs after Retry (so Retry gets a chance to retry before CB fallback handles), using
retryAspectOrder/circuitBreakerAspectOrder. [1]
Sources (official):
- Resilience4j “Getting Started” docs: fallback matching rules + default aspect order + aspect-order properties. [1]
- Resilience4j GitHub issue
#558: confirms CircuitBreaker fallback short-circuits Retry; suggests usingCallNotPermittedException-typed CB fallback to avoid it. [2]
🏁 Script executed:
# Get the file size and relevant sections
wc -l blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 232
🏁 Script executed:
# Read the critical sections: 70-150 to see method signatures and fallback implementation
sed -n '70,150p' blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 3816
Replace broad Exception fallback to prevent CircuitBreaker from short-circuiting Retry logic.
Both @Retry and @CircuitBreaker reference the same fallback method accepting Exception, causing the CircuitBreaker aspect (which wraps Retry) to intercept and handle exceptions before Retry can attempt retries. Even though the fallback re-throws, the retry mechanism never executes. Define separate fallback strategies: CircuitBreaker fallback should handle only CallNotPermittedException, allowing other exceptions to propagate to Retry. Alternatively, configure aspect order via retryAspectOrder / circuitBreakerAspectOrder properties so Retry executes first.
Applies to:
signAndSubmitFallback(lines 72–74, 136–140)getTransactionStatusFallback(lines 114–115, 142–147)
Note: Fireblocks requests include transfer ID as an idempotency key, but retry logic should still be explicitly ordered to handle transient failures correctly without silent suppression.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/main/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapter.java`
around lines 72 - 74, The CircuitBreaker is short-circuiting Retry because both
`@CircuitBreaker` and `@Retry` point to the same fallback that accepts broad
Exception; update the fallbacks so the CircuitBreaker fallback only handles
CallNotPermittedException (e.g., create
signAndSubmitCircuitBreakerFallback(CallNotPermittedException ex, SignRequest
request) and
getTransactionStatusCircuitBreakerFallback(CallNotPermittedException ex, String
txId)) while keeping the Retry fallback to accept the broader Exception
(signAndSubmitFallback(Exception ex, SignRequest request) and
getTransactionStatusFallback(Exception ex, String txId)), or alternatively set
explicit aspect order via retryAspectOrder/circuitBreakerAspectOrder so Retry
runs before CircuitBreaker; ensure the new fallback signatures match the methods
signAndSubmit and getTransactionStatus to let Retry perform retries for
transient errors.
Comment on lines
+91
to
+117
| @Test | ||
| @DisplayName("should retry on transient 503 failure then succeed") | ||
| void shouldRetryOnTransientFailureThenSucceed() { | ||
| wireMock.stubFor(post(urlEqualTo("/v1/transactions")) | ||
| .inScenario("retry-then-success") | ||
| .whenScenarioStateIs(Scenario.STARTED) | ||
| .willReturn(aResponse().withStatus(503)) | ||
| .willSetStateTo("second-attempt")); | ||
|
|
||
| wireMock.stubFor(post(urlEqualTo("/v1/transactions")) | ||
| .inScenario("retry-then-success") | ||
| .whenScenarioStateIs("second-attempt") | ||
| .willReturn(aResponse() | ||
| .withStatus(200) | ||
| .withHeader("Content-Type", "application/json") | ||
| .withBody(""" | ||
| { | ||
| "id": "fb-tx-001", | ||
| "status": "SUBMITTED" | ||
| } | ||
| """))); | ||
|
|
||
| var result = custodyEngine.signAndSubmit(aSignRequest()); | ||
|
|
||
| var expected = new SignResult(null, "fb-tx-001"); | ||
| assertThat(result).usingRecursiveComparison().isEqualTo(expected); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the adapter implementation.
ADAPTER_FILE="$(fd '^FireblocksCustodyAdapter\.java$' | head -n1)"
if [ -z "${ADAPTER_FILE:-}" ]; then
echo "FireblocksCustodyAdapter.java not found"
exit 1
fi
echo "=== Adapter file: $ADAPTER_FILE ==="
sed -n '1,260p' "$ADAPTER_FILE"
echo
echo "=== Search for Fireblocks transaction submission + idempotency handling ==="
rg -n --type=java -C4 'class FireblocksCustodyAdapter|signAndSubmit\s*\(|/v1/transactions|Idempotency|idempotency|HttpHeaders|setHeader|header' .
echo
echo "=== Search this retry test for request-header assertions and exact POST-count verification ==="
rg -n --type=java -C3 'postRequestedFor|withHeader|Idempotency|idempotency|/v1/transactions' \
blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapterRetryTest.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 50393
Attach idempotency token to POST /v1/transactions to guard against duplicate submissions on retry.
The adapter sends no idempotency key to Fireblocks on POST /v1/transactions (line ~115 of adapter). Transient 503 retries without provider-side deduplication can duplicate custody submissions. Fireblocks likely expects an idempotency token header per their API spec. Add one to the adapter request, then assert it in the test alongside an exact POST-count verification for the retry-then-success scenario.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/test/java/com/stablecoin/payments/custody/infrastructure/provider/fireblocks/FireblocksCustodyAdapterRetryTest.java`
around lines 91 - 117, The test and adapter must include and assert an
idempotency token on POST /v1/transactions: update the FireblocksCustodyAdapter
(the method that builds and sends the POST in signAndSubmit flow) to add a
unique Idempotency-Key header for each logical request, then update
FireblocksCustodyAdapterRetryTest.shouldRetryOnTransientFailureThenSucceed to
stub/expect that header on the WireMock POST stub (check header value or
presence) and verify the exact POST invocation count for the scenario; reference
the POST to "/v1/transactions" and the signAndSubmit/aSignRequest() path so the
test ensures retries include the same Idempotency-Key and that duplicate
submissions are prevented.
Comment on lines
+137
to
+149
| instances: | ||
| sanctions: | ||
| max-attempts: 3 | ||
| wait-duration: 500ms | ||
| retry-exceptions: | ||
| - java.io.IOException | ||
| - java.net.http.HttpTimeoutException | ||
| - java.net.http.HttpConnectTimeoutException | ||
| - org.springframework.web.client.ResourceAccessException | ||
| - org.springframework.web.client.HttpServerErrorException | ||
| ignore-exceptions: | ||
| - org.springframework.web.client.HttpClientErrorException | ||
| - io.github.resilience4j.circuitbreaker.CallNotPermittedException |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding exponential backoff for production traffic.
The retry configuration uses a fixed 500ms wait. For external providers under load, exponential backoff with jitter reduces thundering herd effects and improves recovery.
sanctions:
max-attempts: 3
wait-duration: 500ms
enable-exponential-backoff: true
exponential-backoff-multiplier: 2
exponential-max-wait-duration: 5s🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@compliance-travel-rule/compliance-travel-rule/src/main/resources/application.yml`
around lines 137 - 149, Update the sanctions retry config under
instances.sanctions to enable exponential backoff and jitter for production
traffic: add enable-exponential-backoff: true, exponential-backoff-multiplier:
2, exponential-max-wait-duration: 5s (and consider adding jitter if supported by
the retry library) so the retry strategy for the sanctions configuration
(instances.sanctions) uses exponential growth instead of the fixed wait-duration
of 500ms.
Comment on lines
+50
to
+58
| @BeforeAll | ||
| static void startWireMock() { | ||
| wireMock = new WireMockServer(wireMockConfig().dynamicPort()); | ||
| wireMock.start(); | ||
| } | ||
|
|
||
| @AfterAll | ||
| static void stopWireMock() { | ||
| wireMock.stop(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CircleRedemptionAdapterRetryTest.java" -type fRepository: Puneethkumarck/stablebridge-platform
Length of output: 224
🏁 Script executed:
cat -n fiat-off-ramp/fiat-off-ramp/src/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.java | head -200Repository: Puneethkumarck/stablebridge-platform
Length of output: 8877
Fix Spring context bootstrap order: initialize WireMock in static block.
During @SpringBootTest context startup, circleProperties() bean (line 167) dereferences wireMock.baseUrl(), but wireMock remains null because @BeforeAll runs after SpringExtension has already begun Spring initialization. Move WireMock startup to a static initializer block to ensure it starts before the test context is built.
Fix
- private static WireMockServer wireMock;
+ private static final WireMockServer wireMock = new WireMockServer(wireMockConfig().dynamicPort());
+
+ static {
+ wireMock.start();
+ }
- `@BeforeAll`
- static void startWireMock() {
- wireMock = new WireMockServer(wireMockConfig().dynamicPort());
- wireMock.start();
- }
-
`@AfterAll`
static void stopWireMock() {
wireMock.stop();
}📝 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
| @BeforeAll | |
| static void startWireMock() { | |
| wireMock = new WireMockServer(wireMockConfig().dynamicPort()); | |
| wireMock.start(); | |
| } | |
| @AfterAll | |
| static void stopWireMock() { | |
| wireMock.stop(); | |
| private static final WireMockServer wireMock = new WireMockServer(wireMockConfig().dynamicPort()); | |
| static { | |
| wireMock.start(); | |
| } | |
| `@AfterAll` | |
| static void stopWireMock() { | |
| wireMock.stop(); | |
| } |
🤖 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/test/java/com/stablecoin/payments/offramp/infrastructure/provider/circle/CircleRedemptionAdapterRetryTest.java`
around lines 50 - 58, The test fails because wireMock is null during Spring
context bootstrap; move its startup into a static initializer so it exists
before beans like circleProperties() are created: replace the `@BeforeAll`
startWireMock method with a static initializer block that instantiates and
starts the WireMockServer assigned to the static wireMock field in
CircleRedemptionAdapterRetryTest, keep or retain the `@AfterAll` stopWireMock to
stop the server (or register a JVM shutdown hook) so tear-down still occurs, and
remove the `@BeforeAll` annotation/method to avoid late initialization.
Comment on lines
+50
to
52
| @Retry(name = "stripe", fallbackMethod = "initiatePaymentFallback") | ||
| @CircuitBreaker(name = "stripe", fallbackMethod = "initiatePaymentFallback") | ||
| public PspPaymentResult initiatePayment(PspPaymentRequest request) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "StripePspAdapter.java" --type fRepository: Puneethkumarck/stablebridge-platform
Length of output: 203
🏁 Script executed:
cat -n fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapter.javaRepository: Puneethkumarck/stablebridge-platform
Length of output: 6991
🌐 Web query:
Resilience4j @Retry @CIRCUITBREAKER stacking fallback method annotation behavior
💡 Result:
When you “stack” @Retry and @CircuitBreaker (Spring AOP annotations), fallbacks run at the aspect that catches/handles the exception first, and that can prevent the other aspect from ever seeing the failure.
1) Default aspect order (and why your retries may “never happen”)
Resilience4j’s documented Spring aspect wrapping order is:
Retry ( CircuitBreaker ( RateLimiter ( TimeLimiter ( Bulkhead ( Function ) ) ) ) ) and it notes that Retry is applied at the end. It also documents that you can change precedence via *AspectOrder properties. [1]
In practice, if the CircuitBreaker aspect handles the exception with its fallbackMethod, the call becomes “successful” from the outside, so Retry has nothing to retry—this exact “CB fallback called immediately, retry never called” behavior is reported and closed as “not a bug”. [2]
2) What happens if both annotations declare fallbackMethod
If both have fallbacks, the one belonging to the aspect that handles the exception first will run, and the other fallback will typically not run (because the exception doesn’t propagate further). This is the root of the “stacked fallbacks short-circuit retries” effect. [2]
3) The common fix: make the CircuitBreaker fallback only handle “open circuit”
If your intent is:
- retry on “real” downstream failures, and only
- fallback fast when the circuit is OPEN,
then define the CircuitBreaker fallback to accept CallNotPermittedException (the exception thrown when the circuit is open). That way:
- while CLOSED: exceptions propagate → Retry can retry (and eventually its fallback can run)
- while OPEN: CB throws
CallNotPermittedException→ CB fallback runs immediately (no retries)
This workaround is described in the GitHub issue. [2]
4) How Resilience4j chooses which fallback overload to call
Resilience4j requires fallback methods to be in the same class with the same params plus one extra exception parameter, and when multiple overloads exist, it picks the most specific exception match. [1]
5) If you want different stacking semantics
You can adjust aspect precedence using:
resilience4j.retry.retryAspectOrderresilience4j.circuitbreaker.circuitBreakerAspectOrder
…(higher value = higher priority), per the docs. [1]
Sources
[1] Resilience4j “Getting Started” (Spring Boot) – fallback signature rules + aspect order + *AspectOrder properties (resilience4j.readme.io)
[2] resilience4j/resilience4j GitHub issue #558 – “Retry logic is never called when CircuitBreaker specifies a fallback” + CallNotPermittedException workaround (github.com)
Citations:
- 1: https://resilience4j.readme.io/docs/getting-started-3
- 2: Spring Boot: Retry logic is never called when CircuitBreaker specifies a fallback resilience4j/resilience4j#558
Separate circuit-breaker fallback from retry fallback to preserve retry semantics.
Lines 50–51 and 81–82 bind both @Retry and @CircuitBreaker to the same fallback method. Resilience4j wraps aspects in order: Retry ( CircuitBreaker ( ... ) ). When CircuitBreaker fallback accepts broad Exception (lines 107–117), it catches and handles exceptions before Retry ever sees them, making retries impossible on transient failures. This is documented in resilience4j issue #558.
Define the circuit-breaker fallback to accept only CallNotPermittedException (exception thrown when circuit opens):
- Circuit CLOSED: Exceptions propagate through →
Retryintercepts and retries - Circuit OPEN:
CallNotPermittedException→ Circuit-breaker fallback handles immediately
Suggested fix
+import io.github.resilience4j.circuitbreaker.CallNotPermittedException;
import io.github.resilience4j.circuitbreaker.annotation.CircuitBreaker;
import io.github.resilience4j.retry.annotation.Retry;
@@
- `@Retry`(name = "stripe", fallbackMethod = "initiatePaymentFallback")
- `@CircuitBreaker`(name = "stripe", fallbackMethod = "initiatePaymentFallback")
+ `@Retry`(name = "stripe", fallbackMethod = "initiatePaymentFallback")
+ `@CircuitBreaker`(name = "stripe", fallbackMethod = "initiatePaymentCircuitBreakerFallback")
@@
- `@Retry`(name = "stripe", fallbackMethod = "initiateRefundFallback")
- `@CircuitBreaker`(name = "stripe", fallbackMethod = "initiateRefundFallback")
+ `@Retry`(name = "stripe", fallbackMethod = "initiateRefundFallback")
+ `@CircuitBreaker`(name = "stripe", fallbackMethod = "initiateRefundCircuitBreakerFallback")
@@
private PspPaymentResult initiatePaymentFallback(PspPaymentRequest request, Exception ex) {
log.error("[STRIPE] Resilience fallback — payment initiation failed collectionId={} due to {}",
request.collectionId(), ex.getClass().getSimpleName(), ex);
throw new IllegalStateException("Stripe payment initiation unavailable", ex);
}
+
+ `@SuppressWarnings`("unused")
+ private PspPaymentResult initiatePaymentCircuitBreakerFallback(
+ PspPaymentRequest request, CallNotPermittedException ex) {
+ log.error("[STRIPE] Circuit breaker open — payment initiation blocked collectionId={}",
+ request.collectionId(), ex);
+ throw new IllegalStateException("Stripe payment initiation unavailable", ex);
+ }
@@
private PspRefundResult initiateRefundFallback(PspRefundRequest request, Exception ex) {
log.error("[STRIPE] Resilience fallback — refund initiation failed collectionId={} due to {}",
request.collectionId(), ex.getClass().getSimpleName(), ex);
throw new IllegalStateException("Stripe refund initiation unavailable", ex);
}
+
+ `@SuppressWarnings`("unused")
+ private PspRefundResult initiateRefundCircuitBreakerFallback(
+ PspRefundRequest request, CallNotPermittedException ex) {
+ log.error("[STRIPE] Circuit breaker open — refund initiation blocked collectionId={}",
+ request.collectionId(), ex);
+ throw new IllegalStateException("Stripe refund initiation unavailable", ex);
+ }Also applies to: 81–82, 107–117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fiat-on-ramp/fiat-on-ramp/src/main/java/com/stablecoin/payments/onramp/infrastructure/provider/stripe/StripePspAdapter.java`
around lines 50 - 52, The circuit-breaker and retry are sharing the same
fallback (initiatePaymentFallback) which prevents Retry from seeing exceptions;
change the CircuitBreaker annotation to use a dedicated fallback (e.g.,
initiatePaymentCircuitOpenFallback) and implement that new method to accept
CallNotPermittedException plus the original parameters/return type, while
leaving initiatePaymentFallback as the retry fallback that accepts broad
Exception; update the `@CircuitBreaker`(name = "stripe", fallbackMethod =
"initiatePaymentCircuitOpenFallback") on initiatePayment and add the new
initiatePaymentCircuitOpenFallback(CallNotPermittedException, PspPaymentRequest)
method to handle circuit-open cases only.
Comment on lines
+129
to
+131
| circuit-breaker-aspect-order: 1 | ||
| retry: | ||
| retry-aspect-order: 3 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Resilience4j Spring Boot, with Spring AOP ordering, does a lower *-aspect-ordervalue execute as the outer aspect? Specifically, forretry-aspect-orderandcircuit-breaker-aspect-order, which one should be lower to make Retry wrap CircuitBreaker?
💡 Result:
Yes. In Spring AOP, the aspect/advice with the lower Ordered / @Order value has higher precedence (i.e., it runs “first” on entry and therefore acts as the outer wrapper for @Around advice). [1][2]
So, to make Retry wrap CircuitBreaker (Retry outer, CircuitBreaker inner), set:
retry-aspect-orderlower thancircuit-breaker-aspect-order(e.g., Retry =1, CircuitBreaker =2).
Resilience4j’s default nesting is Retry ( CircuitBreaker ( ... ) ), i.e. Retry outer by default. [3]
Sources:
[1] Spring Framework AOP reference: lower Ordered.getValue() ⇒ higher precedence
[2] Spring @Order javadoc: lower values have higher priority
[3] Resilience4j “Aspect order” default nesting documentation
Aspect order inverted; CircuitBreaker executing outside Retry
Lines 129–131: circuit-breaker-aspect-order: 1 and retry-aspect-order: 3 are reversed. In Spring AOP, lower order values execute as the outer aspect. This arrangement places the circuit breaker outside the retry wrapper, preventing proper failure accounting across retry attempts and undermining the breaker's effectiveness during retry storms.
Swap the values so Retry (order 1) wraps CircuitBreaker (order 3):
Config correction
resilience4j:
circuitbreaker:
- circuit-breaker-aspect-order: 1
+ circuit-breaker-aspect-order: 3
retry:
- retry-aspect-order: 3
+ retry-aspect-order: 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fx-liquidity-engine/fx-liquidity-engine/src/main/resources/application.yml`
around lines 129 - 131, The aspect ordering is inverted: swap the values of
circuit-breaker-aspect-order and retry-aspect-order so Retry executes as the
outer aspect; specifically change the properties named
circuit-breaker-aspect-order and retry-aspect-order so retry-aspect-order is set
to 1 and circuit-breaker-aspect-order is set to 3 to ensure Retry wraps
CircuitBreaker.
Comment on lines
+50
to
+53
| @BeforeAll | ||
| static void startWireMock() { | ||
| wireMock = new WireMockServer(wireMockConfig().dynamicPort()); | ||
| wireMock.start(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Spring Boot tests using @SpringBootTestwith JUnit 5, is the ApplicationContext (including@beanmethod execution) initialized before@BeforeAll methods run?
💡 Result:
Yes — with @SpringBootTest (which registers SpringExtension), the Spring ApplicationContext is created/refreshed (therefore @Configuration/@Bean methods run and singleton beans are instantiated) before JUnit Jupiter executes your test class’s @BeforeAll.
Reason: JUnit runs extension lifecycle callbacks first; SpringExtension.beforeAll() delegates to TestContextManager.beforeTestClass() [1], and beforeTestClass() is explicitly defined to run prior to any framework “before all” methods such as JUnit Jupiter’s @BeforeAll [2].
Sources: SpringExtension Javadoc [1], TestContextManager Javadoc [2].
Initialize WireMock before Spring context bootstrap.
wireMock.baseUrl() is invoked during RefinitivProperties bean creation, but WireMock is not started until @BeforeAll. With @SpringBootTest, Spring context initialization (including @Configuration beans) runs before JUnit's @BeforeAll callbacks, leaving wireMock null during bean instantiation and causing initialization failure.
Suggested fix
- private static WireMockServer wireMock;
+ private static final WireMockServer wireMock = new WireMockServer(wireMockConfig().dynamicPort());
+ static {
+ wireMock.start();
+ }
- `@BeforeAll`
- static void startWireMock() {
- wireMock = new WireMockServer(wireMockConfig().dynamicPort());
- wireMock.start();
- }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fx-liquidity-engine/fx-liquidity-engine/src/test/java/com/stablecoin/payments/fx/infrastructure/provider/refinitiv/RefinitivRateAdapterRetryTest.java`
around lines 50 - 53, RefinitivRateAdapterRetryTest fails because wireMock is
started in `@BeforeAll` (startWireMock) which runs after Spring context creation
while RefinitivProperties calls wireMock.baseUrl() during bean instantiation;
fix by starting the WireMockServer earlier (e.g., initialize and start the
static wireMock in a static initializer or as a static field that calls new
WireMockServer(...).start()) so wireMock is non-null and running before Spring
bootstraps the context; update the startWireMock method or remove it in favor of
the static initializer and ensure the static wireMock variable remains the
single server used by the test.
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
@Retryalongside@CircuitBreakeron all 10 external adapter methods across 5 services@SpringBootTest+ WireMock scenariosCircleRedemptionAdapterandModulrPayoutAdapterfromCallNotPermittedExceptiontoExceptionRetry Configuration
Closes STA-117
Test plan
wait-duration: 10msfor fast test execution🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Tests
Style