Skip to content

Commit

Permalink
RPC - default to internal error not invalid params (#5506)
Browse files Browse the repository at this point in the history
* added new error code

* fixed remaining privacy unit tests; added new error code

* internal error not invalid params

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>
Co-authored-by: Stefan Pingel <[email protected]>
  • Loading branch information
macfarla and pinges authored Jun 21, 2023
1 parent db173eb commit 251fded
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 12 deletions.
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);
new JsonRpcErrorResponse(null, JsonRpcError.GAS_PRICE_BELOW_CURRENT_BASE_FEE);

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.GAS_PRICE_BELOW_CURRENT_BASE_FEE);

final JsonRpcResponse response = method.response(request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,18 @@ public static JsonRpcError convertTransactionInvalidReason(
case TX_SENDER_NOT_AUTHORIZED:
return JsonRpcError.TX_SENDER_NOT_AUTHORIZED;
// Private Transaction Invalid Reasons
case PRIVATE_TRANSACTION_INVALID:
return JsonRpcError.PRIVATE_TRANSACTION_INVALID;
case PRIVATE_TRANSACTION_FAILED:
return JsonRpcError.PRIVATE_TRANSACTION_FAILED;
case PRIVATE_UNIMPLEMENTED_TRANSACTION_TYPE:
return JsonRpcError.UNSUPPORTED_PRIVATE_TRANSACTION_TYPE;
case CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE:
return JsonRpcError.CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE;
case GAS_PRICE_TOO_LOW:
return JsonRpcError.GAS_PRICE_TOO_LOW;
case GAS_PRICE_BELOW_CURRENT_BASE_FEE:
return JsonRpcError.GAS_PRICE_BELOW_CURRENT_BASE_FEE;
case TX_FEECAP_EXCEEDED:
return JsonRpcError.TX_FEECAP_EXCEEDED;
case MAX_PRIORITY_FEE_PER_GAS_EXCEEDS_MAX_FEE_PER_GAS:
Expand All @@ -68,7 +76,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 @@ -65,6 +65,7 @@ public enum JsonRpcError {
TX_SENDER_NOT_AUTHORIZED(-32007, "Sender account not authorized to send transactions"),
CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE(-32008, "Initial sync is still in progress"),
GAS_PRICE_TOO_LOW(-32009, "Gas price below configured minimum gas price"),
GAS_PRICE_BELOW_CURRENT_BASE_FEE(-32009, "Gas price below current base fee"),
WRONG_CHAIN_ID(-32000, "Wrong chainId"),
REPLAY_PROTECTED_SIGNATURES_NOT_SUPPORTED(-32000, "ChainId not supported"),
REPLAY_PROTECTED_SIGNATURE_REQUIRED(-32000, "ChainId is required"),
Expand Down Expand Up @@ -139,7 +140,7 @@ public enum JsonRpcError {

// Private transaction errors
ENCLAVE_ERROR(-50100, "Error communicating with enclave"),
UNIMPLEMENTED_PRIVATE_TRANSACTION_TYPE(-50100, "Unimplemented private transaction type"),
UNSUPPORTED_PRIVATE_TRANSACTION_TYPE(-50100, "Unsupported private transaction type"),
PRIVACY_NOT_ENABLED(-50100, "Privacy is not enabled"),
CREATE_PRIVACY_GROUP_ERROR(-50100, "Error creating privacy group"),
DECODE_ERROR(-50100, "Unable to decode the private signed raw transaction"),
Expand All @@ -160,6 +161,8 @@ 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_INVALID(-50100, "Private transaction invalid"),
PRIVATE_TRANSACTION_FAILED(-50100, "Private transaction failed"),

CANT_CONNECT_TO_LOCAL_PEER(-32100, "Cannot add local node as peer."),
CANT_RESOLVE_PEER_ENODE_DNS(-32100, "Cannot resolve enode DNS hostname"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.PRIVATE_TRANSACTION_FAILED;
import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.PRIVATE_TRANSACTION_INVALID;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.never;
Expand Down Expand Up @@ -139,11 +139,12 @@ public void invalidTransactionWithoutPrivateFromFieldFailsWithDecodeError() {
@Test
public void invalidTransactionIsNotSentToEnclaveAndIsNotAddedToTransactionPool() {
when(privacyController.validatePrivateTransaction(any(), anyString()))
.thenReturn(ValidationResult.invalid(PRIVATE_TRANSACTION_FAILED));
.thenReturn(ValidationResult.invalid(PRIVATE_TRANSACTION_INVALID));

final JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(
validPrivateForTransactionRequest.getRequest().getId(), JsonRpcError.INVALID_PARAMS);
validPrivateForTransactionRequest.getRequest().getId(),
JsonRpcError.PRIVATE_TRANSACTION_INVALID);

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.UNSUPPORTED_PRIVATE_TRANSACTION_TYPE);

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.UNSUPPORTED_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.UNSUPPORTED_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.UNSUPPORTED_PRIVATE_TRANSACTION_TYPE);

assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@ public enum TransactionInvalidReason {
TOTAL_DATA_GAS_TOO_HIGH,
GAS_PRICE_TOO_LOW,
GAS_PRICE_BELOW_CURRENT_BASE_FEE,
MAX_FEE_PER_GAS_BELOW_CURRENT_BASE_FEE,
TX_FEECAP_EXCEEDED,
INTERNAL_ERROR,

// Private Transaction Invalid Reasons
PRIVATE_TRANSACTION_INVALID,
PRIVATE_TRANSACTION_FAILED,
PRIVATE_NONCE_TOO_LOW,
OFFCHAIN_PRIVACY_GROUP_DOES_NOT_EXIST,
Expand Down

0 comments on commit 251fded

Please sign in to comment.