Skip to content

Commit

Permalink
[ANCHOR-833] Improve RestRateIntegration fee handling (#1518)
Browse files Browse the repository at this point in the history 
### Description

- Fix the the inaccurate validation error message.
- Populate the fee object with total=0 if the business server does not
provide one. This is to avoid SEP-6 and SEP-31 validation errors when
the business server does not provide a `rate.fee` object.

### Context

Bug fixes.

### Testing

- `./gradlew test`

### Documentation

N/A

### Known limitations

N/A
  • Loading branch information
@lijamie98
lijamie98 authored Sep 25, 2024
1 parent ee09da6 commit 02b84f8
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 23 deletions.
3 changes: 2 additions & 1 deletion platform/src/main/java/org/stellar/anchor/platform/callback/PlatformIntegrationHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public static Request.Builder getRequestBuilder(AuthHelper authHelper)
Request.Builder requestBuilder =
new Request.Builder().header("Content-Type", "application/json");

AuthHeader<String, String> authHeader = authHelper.createCallbackAuthHeader();
AuthHeader<String, String> authHeader =
(authHelper == null) ? null : authHelper.createCallbackAuthHeader();
return authHeader == null
? requestBuilder
: requestBuilder.header(authHeader.getName(), authHeader.getValue());
Expand Down
56 changes: 40 additions & 16 deletions platform/src/main/java/org/stellar/anchor/platform/callback/RestRateIntegration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.stellar.anchor.api.callback.GetRateResponse;
import org.stellar.anchor.api.callback.RateIntegration;
import org.stellar.anchor.api.exception.AnchorException;
import org.stellar.anchor.api.exception.InvalidConfigException;
import org.stellar.anchor.api.exception.ServerErrorException;
import org.stellar.anchor.api.sep.AssetInfo;
import org.stellar.anchor.api.shared.FeeDescription;
Expand Down Expand Up @@ -65,20 +66,8 @@ public RestRateIntegration(

@Override
public GetRateResponse getRate(GetRateRequest request) throws AnchorException {
Builder urlBuilder = get(anchorEndpoint).newBuilder().addPathSegment("rate");
Type type = new TypeToken<Map<String, ?>>() {}.getType();
Map<String, String> paramsMap = gson.fromJson(gson.toJson(request), type);
paramsMap.forEach(
(key, value) -> {
if (value != null) {
urlBuilder.addQueryParameter(key, value);
}
});
HttpUrl url = urlBuilder.build();

Request httpRequest =
PlatformIntegrationHelper.getRequestBuilder(authHelper).url(url).get().build();
try (Response response = PlatformIntegrationHelper.call(httpClient, httpRequest)) {
try (Response response = invokeGetRateRequest(request, authHelper)) {
String responseContent = PlatformIntegrationHelper.getContent(response);

if (response.code() != HttpStatus.OK.value()) {
Expand All @@ -94,6 +83,12 @@ public GetRateResponse getRate(GetRateRequest request) throws AnchorException {
}

validateRateResponse(request, getRateResponse);

// If the rate.fee is not present, we need to set it to 0 so that the fee always exists
if (getRateResponse.getRate().getFee() == null) {
getRateResponse.getRate().setFee(new FeeDetails("0", request.getSellAsset()));
}

return getRateResponse;
}
}
Expand Down Expand Up @@ -160,6 +155,16 @@ void validateRateResponse(GetRateRequest request, GetRateResponse getRateRespons
"'rate.fee.total' is missing or a negative number in the GET /rate response",
ServerErrorException.class);
}

// if fee.total is zero, fee.details must be empty or non-existent
if ((new BigDecimal(fee.getTotal()).compareTo(BigDecimal.ZERO) == 0)
&& fee.getDetails() != null
&& !fee.getDetails().isEmpty()) {
logErrorAndThrow(
"'rate.fee.details' must be empty or not-existent when 'rate.fee.total' is zero in the GET /rate response",
ServerErrorException.class);
}

// fee.asset is a valid asset
AssetInfo feeAsset = assetService.getAssetByName(fee.getAsset());
if (fee.getAsset() == null || feeAsset == null) {
Expand Down Expand Up @@ -221,18 +226,18 @@ void validateRateResponse(GetRateRequest request, GetRateResponse getRateRespons
for (FeeDescription feeDescription : fee.getDetails()) {
if (!isPositiveNumber(feeDescription.getAmount())) {
logErrorAndThrow(
"'rate.fee.details[?].description.amount' is missing or not a positive number in the GET /rate response",
"'rate.fee.details[?].amount' is missing or not a positive number in the GET /rate response",
ServerErrorException.class);
}
if (!hasProperSignificantDecimals(
feeDescription.getAmount(), feeAsset.getSignificantDecimals())) {
logErrorAndThrow(
"'rate.fee.details[?].description.amount' has incorrect number of significant decimals in the GET /rate response",
"'rate.fee.details[?].amount' has incorrect number of significant decimals in the GET /rate response",
ServerErrorException.class);
}
if (feeDescription.getName() == null) {
logErrorAndThrow(
"'rate.fee.details.description[?].name' is missing in the GET /rate response",
"'rate.fee.details[?].name' is missing in the GET /rate response",
ServerErrorException.class);
}
totalFee = totalFee.add(new BigDecimal(feeDescription.getAmount()));
Expand Down Expand Up @@ -262,6 +267,25 @@ void validateRateResponse(GetRateRequest request, GetRateResponse getRateRespons
}
}

Response invokeGetRateRequest(GetRateRequest request, AuthHelper authHelper)
throws InvalidConfigException, ServerErrorException {
Builder urlBuilder = get(anchorEndpoint).newBuilder().addPathSegment("rate");
Type type = new TypeToken<Map<String, ?>>() {}.getType();
Map<String, String> paramsMap = gson.fromJson(gson.toJson(request), type);
paramsMap.forEach(
(key, value) -> {
if (value != null) {
urlBuilder.addQueryParameter(key, value);
}
});

HttpUrl url = urlBuilder.build();

Request httpRequest =
PlatformIntegrationHelper.getRequestBuilder(authHelper).url(url).get().build();
return PlatformIntegrationHelper.call(httpClient, httpRequest);
}

/**
* Check the amount is within rounding error of the expected amount.
*
Expand Down
47 changes: 41 additions & 6 deletions platform/src/test/kotlin/org/stellar/anchor/platform/callback/RestRateIntegrationTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package org.stellar.anchor.platform.callback
import com.google.gson.Gson
import io.mockk.every
import io.mockk.mockk
import io.mockk.spyk
import java.math.BigDecimal
import java.time.Instant
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertTrue
import okhttp3.Response
import okhttp3.ResponseBody
import org.junit.jupiter.api.Assertions.*
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
Expand All @@ -29,7 +31,8 @@ class RestRateIntegrationTest {
private val gson: Gson = GsonUtils.getInstance()
private var usdAssetInfo = AssetInfo()
private var usdcAssetInfo = AssetInfo()
private val rateIntegration = RestRateIntegration("/", null, null, gson, assetService)
private val rateIntegration =
RestRateIntegration("http://localhost/callback", null, null, gson, assetService)
private var request: GetRateRequest = GetRateRequest()
private var rateResponseWithFee: GetRateResponse = GetRateResponse()
private var rateResponseWithoutFee: GetRateResponse = GetRateResponse()
Expand Down Expand Up @@ -112,6 +115,22 @@ class RestRateIntegrationTest {
)
}

@Test
fun `test getRate when the callback response does not provide a fee`() {
val mockResponse = mockk<Response>()
val mockResponseBody = mockk<ResponseBody>()
val spyRateIntegration = spyk(rateIntegration)
every { mockResponse.body } returns mockResponseBody
every { mockResponse.code } returns 200
every { mockResponse.close() } returns Unit
every { mockResponseBody.string() } returns gson.toJson(rateResponseWithoutFee)
every { spyRateIntegration.invokeGetRateRequest(any(), any()) } returns mockResponse

val rate = spyRateIntegration.getRate(request)
assertNotNull(rate.rate.fee)
assertEquals(BigDecimal(rate.rate.fee.total).compareTo(BigDecimal.ZERO), 0)
}

@ParameterizedTest
@ValueSource(strings = ["indicative", "firm"])
fun `test INDICATIVE and FIRM validateRateResponse with fee`(type: String) {
Expand Down Expand Up @@ -211,6 +230,7 @@ class RestRateIntegrationTest {
)
fun `test fee total`(total: String?, hasError: Boolean, errorMessage: String) {
rateResponseWithFee.rate.fee.total = total
rateResponseWithFee.rate.fee.details = null
if (hasError) {
val ex =
assertThrows<ServerErrorException> {
Expand Down Expand Up @@ -266,7 +286,7 @@ class RestRateIntegrationTest {
rateIntegration.validateRateResponse(request, rateResponseWithFee)
}
assertEquals(
"'rate.fee.details.description[?].name' is missing in the GET /rate response",
"'rate.fee.details[?].name' is missing in the GET /rate response",
ex.message,
)

Expand All @@ -277,7 +297,7 @@ class RestRateIntegrationTest {
rateIntegration.validateRateResponse(request, rateResponseWithFee)
}
assertEquals(
"'rate.fee.details[?].description.amount' is missing or not a positive number in the GET /rate response",
"'rate.fee.details[?].amount' is missing or not a positive number in the GET /rate response",
ex.message,
)

Expand Down Expand Up @@ -327,7 +347,7 @@ class RestRateIntegrationTest {
rateIntegration.validateRateResponse(request, rateResponseWithFee)
}
assertEquals(
"'rate.fee.details[?].description.amount' has incorrect number of significant decimals in the GET /rate response",
"'rate.fee.details[?].amount' has incorrect number of significant decimals in the GET /rate response",
ex.message,
)
}
Expand Down Expand Up @@ -380,4 +400,19 @@ class RestRateIntegrationTest {
withinRoundingError(BigDecimal(amount), BigDecimal(expected), scale.toInt()),
)
}

@Test
fun `test 0 fee with details`() {
rateResponseWithFee.rate.fee.total = "0.00"
rateResponseWithFee.rate.fee.details[0].amount = "0.00"
rateResponseWithFee.rate.fee.details[1].amount = "0.00"
val ex =
assertThrows<ServerErrorException> {
rateIntegration.validateRateResponse(request, rateResponseWithFee)
}
assertEquals(
"'rate.fee.details' must be empty or not-existent when 'rate.fee.total' is zero in the GET /rate response",
ex.message
)
}
}

0 comments on commit 02b84f8

Please sign in to comment.