Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPC - default to internal error not invalid params #5506

Merged
merged 10 commits into from
Jun 21, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public void mustNotSucceedWithWronglyEncodedFunction() throws IOException {
privCall(privacyGroupId, eventEmitter, true, false, false);

final String errorMessage = priv_call.send().getError().getMessage();
assertThat(errorMessage).isEqualTo("Invalid params");
assertThat(errorMessage).isEqualTo("Private transaction failed");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public void shouldReturnErrorWithGasPriceLessThanCurrentBaseFee() {
null);
final JsonRpcRequestContext request = requestWithParams(callParameter, "latest");
final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.INVALID_PARAMS);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is still not giving our users good information about why it failed ...
Almost feels like invalid params in that case was better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exposed GAS_PRICE_BELOW_CURRENT_BASE_FEE

new JsonRpcErrorResponse(null, JsonRpcError.INTERNAL_ERROR);

final JsonRpcResponse response = method.response(request);

Expand Down Expand Up @@ -219,7 +219,7 @@ public void shouldReturnErrorWithValidMaxFeePerGasLessThanCurrentBaseFee() {
null);
final JsonRpcRequestContext request = requestWithParams(callParameter, "latest");
final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.INVALID_PARAMS);
new JsonRpcErrorResponse(null, JsonRpcError.INTERNAL_ERROR);
macfarla marked this conversation as resolved.
Show resolved Hide resolved

final JsonRpcResponse response = method.response(request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ public static JsonRpcError convertTransactionInvalidReason(
case TX_SENDER_NOT_AUTHORIZED:
return JsonRpcError.TX_SENDER_NOT_AUTHORIZED;
// Private Transaction Invalid Reasons
case PRIVATE_TRANSACTION_FAILED:
return JsonRpcError.PRIVATE_TRANSACTION_FAILED;
case PRIVATE_UNIMPLEMENTED_TRANSACTION_TYPE:
return JsonRpcError.UNIMPLEMENTED_PRIVATE_TRANSACTION_TYPE;
case CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE:
return JsonRpcError.CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE;
case GAS_PRICE_TOO_LOW:
Expand All @@ -68,7 +72,7 @@ public static JsonRpcError convertTransactionInvalidReason(
case TOTAL_DATA_GAS_TOO_HIGH:
return JsonRpcError.TOTAL_DATA_GAS_TOO_HIGH;
default:
return JsonRpcError.INVALID_PARAMS;
return JsonRpcError.INTERNAL_ERROR;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ public enum JsonRpcError {
PRIVATE_FROM_DOES_NOT_MATCH_ENCLAVE_PUBLIC_KEY(
-50100, "Private from does not match enclave public key"),
VALUE_NOT_ZERO(-50100, "We cannot transfer ether in a private transaction yet."),
PRIVATE_TRANSACTION_FAILED(-50100, "Private transaction failed"),

CANT_CONNECT_TO_LOCAL_PEER(-32100, "Cannot add local node as peer."),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ public void invalidTransactionIsNotSentToEnclaveAndIsNotAddedToTransactionPool()

final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(
validPrivateForTransactionRequest.getRequest().getId(), JsonRpcError.INVALID_PARAMS);
validPrivateForTransactionRequest.getRequest().getId(),
JsonRpcError.PRIVATE_TRANSACTION_FAILED);
macfarla marked this conversation as resolved.
Show resolved Hide resolved

final JsonRpcResponse actualResponse = method.response(validPrivateForTransactionRequest);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ public void flexiblePrivacyGroupTransactionFailsWhenGroupDoesNotExist() {

final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(
validPrivacyGroupTransactionRequest.getRequest().getId(), JsonRpcError.INVALID_PARAMS);
validPrivacyGroupTransactionRequest.getRequest().getId(),
JsonRpcError.UNIMPLEMENTED_PRIVATE_TRANSACTION_TYPE);
macfarla marked this conversation as resolved.
Show resolved Hide resolved

assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
}
Expand All @@ -152,7 +153,8 @@ public void flexiblePrivacyGroupTransactionFailsWhenGroupDoesNotExist() {

final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(
validPrivacyGroupTransactionRequest.getRequest().getId(), JsonRpcError.INVALID_PARAMS);
validPrivacyGroupTransactionRequest.getRequest().getId(),
JsonRpcError.UNIMPLEMENTED_PRIVATE_TRANSACTION_TYPE);

assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ public void validPantheonPrivacyGroupTransactionIsSentToTransactionPool() {

final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(
validPrivacyGroupTransactionRequest.getRequest().getId(), JsonRpcError.INVALID_PARAMS);
validPrivacyGroupTransactionRequest.getRequest().getId(),
JsonRpcError.UNIMPLEMENTED_PRIVATE_TRANSACTION_TYPE);

assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
}
Expand All @@ -120,7 +121,8 @@ public void validPantheonPrivacyGroupTransactionIsSentToTransactionPool() {

final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(
validPrivacyGroupTransactionRequest.getRequest().getId(), JsonRpcError.INVALID_PARAMS);
validPrivacyGroupTransactionRequest.getRequest().getId(),
JsonRpcError.UNIMPLEMENTED_PRIVATE_TRANSACTION_TYPE);

assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
}
Expand Down