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.

Sign up for GitHub

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
Merged

RPC - default to internal error not invalid params #5506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fb8c95c
default internal error
macfarla May 29, 2023
cb3b95f
fixed one test; added new error code
macfarla Jun 14, 2023
67d9c4d
Merge branch 'main' of github.com:hyperledger/besu into rpc-internal-…
macfarla Jun 14, 2023
5ef3c3d
fixed remaining privacy unit tests; added new error code
macfarla Jun 14, 2023
508dd1d
internal error not invalid params
macfarla Jun 14, 2023
25b7270
internal error not invalid params
macfarla Jun 14, 2023
eb920e4
Private transaction failed
macfarla Jun 14, 2023
b8cfd63
tuning error codes
macfarla Jun 16, 2023
29f3c3c
Merge branch 'main' of github.com:hyperledger/besu into rpc-internal-…
macfarla Jun 16, 2023
3997ae6
Merge branch 'main' into rpc-internal-error-default
pinges Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...s/src/test/java/org/hyperledger/besu/tests/acceptance/privacy/PrivCallAcceptanceTest.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions ...org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/london/EthCallIntegrationTest.java
View file
Open in desktop
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.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
10 changes: 9 additions & 1 deletion ...um/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java
View file
Open in desktop
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;
}
}
}
5 changes: 4 additions & 1 deletion ...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java
View file
Open in desktop
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
7 changes: 4 additions & 3 deletions ...ger/besu/ethereum/api/jsonrpc/internal/privacy/methods/eea/EeaSendRawTransactionTest.java
View file
Open in desktop
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
6 changes: 4 additions & 2 deletions ...api/jsonrpc/internal/privacy/methods/eea/RestrictedFlexibleEeaSendRawTransactionTest.java
View file
Open in desktop
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
6 changes: 4 additions & 2 deletions ...api/jsonrpc/internal/privacy/methods/eea/RestrictedOffchainEeaSendRawTransactionTest.java
View file
Open in desktop
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
2 changes: 2 additions & 0 deletions ...ore/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionInvalidReason.java
View file
Open in desktop
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