5098 branch 19 update more invalid params #7460

Matilda-Clerke · 2024-08-14T06:01:04Z

PR description

Update several more RpcErrorTypes

Fixed Issue(s)

#5098

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…ion, apply spotless Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…-branch-2-update-invalid-accounts-params

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…' into 5098-branch-3-update-invalid-address-hash-params

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…-branch-2-update-invalid-accounts-params

…' into 5098-branch-3-update-invalid-address-hash-params

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…-branch-2-update-invalid-accounts-params

…' into 5098-branch-3-update-invalid-address-hash-params

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…ounts-params' into 5098-branch-2-update-invalid-accounts-params

…' into 5098-branch-3-update-invalid-address-hash-params

…rams' into 5098-branch-4-update-invalid-address-params

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…' into 5098-branch-3-update-invalid-address-hash-params

…rams' into 5098-branch-4-update-invalid-address-params

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…ne-params Signed-off-by: Matilda-Clerke <matilda.clerke@consensys.net>

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…ss-blob-gas-params Signed-off-by: Matilda-Clerke <matilda.clerke@consensys.net>

…ss-blob-gas-params

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

…ss-blob-gas-params

…ss-blob-gas-params' into 5098-branch-17-update-extra-data-and-filter-params

…params' into 5098-branch-18-update-more-invalid-params

…nto 5098-branch-19-update-more-invalid-params

pinges

Comments for the files up to MinerSetMinGasPriceTest

pinges · 2024-08-15T06:30:58Z

...dger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetBlockByNumberIntegrationTest.java

@@ -437,7 +437,7 @@ public void missingHashesOrTransactionParameter() {

    assertThat(thrown)
        .isInstanceOf(InvalidJsonRpcParameters.class)
-        .hasMessage("Missing required json rpc parameter at index 1");
+        .hasMessage("Invalid is transaction complete parameter (index 1)");


Suggested change

.hasMessage("Invalid is transaction complete parameter (index 1)");

.hasMessage("Invalid return complete transactions parameter (index 1)");

pinges · 2024-08-15T06:59:22Z

.../main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugAccountRange.java

@@ -75,7 +75,13 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
      throw new InvalidJsonRpcParameters(


Looks like the param at index 1, transaction index, is ignored. Should we document that?

I'm not sure where these Debug* methods would be documented. I searched in besu-docs for DebugAccountRange and couldn't find anything.

pinges · 2024-08-15T07:02:58Z

...ain/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugStorageRangeAt.java

@@ -85,8 +85,13 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
      throw new InvalidJsonRpcParameters(
          "Invalid account address parameter (index 2)", RpcErrorType.INVALID_ADDRESS_PARAMS, e);
    }
-    final Hash startKey =
-        Hash.fromHexStringLenient(requestContext.getRequiredParameter(3, String.class));
+    final Hash startKey;


Why do we not try catch for the two Integer.class required parameters? Lines 80 and 95?

They'll be covered in later PRs

pinges · 2024-08-15T07:14:57Z

.../main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByHash.java

+      return requestContext.getRequiredParameter(1, Boolean.class);
+    } catch (Exception e) { // TODO:replace with JsonRpcParameter.JsonRpcParameterException
+      throw new InvalidJsonRpcParameters(
+          "Invalid is complete transaction parameter (index 1)",


Suggested change

"Invalid is complete transaction parameter (index 1)",

"Invalid return complete transactions parameter (index 1)",

This is definitely worth another PR to update. I wasn't really sure what to call it.

pinges · 2024-08-15T07:18:12Z

...ain/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumber.java

+      return request.getRequiredParameter(1, Boolean.class);
+    } catch (Exception e) { // TODO:replace with JsonRpcParameter.JsonRpcParameterException
+      throw new InvalidJsonRpcParameters(
+          "Invalid is transaction complete parameter (index 1)",


Suggested change

"Invalid is transaction complete parameter (index 1)",

"Invalid return complete transactions parameter (index 1)",

pinges · 2024-08-15T13:15:20Z

...va/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/PrivGetFilterLogs.java

@@ -52,7 +53,13 @@ public String getName() {
  @Override
  public JsonRpcResponse response(final JsonRpcRequestContext request) {
    final String privacyGroupId = request.getRequiredParameter(0, String.class);
-    final String filterId = request.getRequiredParameter(1, String.class);
+    final String filterId;


Line 55 no try/catch

Handled in a later PR

pinges · 2024-08-15T13:15:50Z

.../org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/PrivUninstallFilter.java

@@ -46,7 +48,13 @@ public String getName() {
  @Override
  public JsonRpcResponse response(final JsonRpcRequestContext request) {
    final String privacyGroupId = request.getRequiredParameter(0, String.class);
-    final String filterId = request.getRequiredParameter(1, String.class);
+    final String filterId;


pinges · 2024-08-15T13:16:21Z

...ava/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivGetLogs.java

@@ -62,11 +63,17 @@ public String getName() {
  @Override
  public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
    final String privacyGroupId = requestContext.getRequiredParameter(0, String.class);
-    final FilterParameter filter = requestContext.getRequiredParameter(1, FilterParameter.class);
+    final FilterParameter filter;


pinges · 2024-08-15T13:16:45Z

...a/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivNewFilter.java

@@ -50,7 +51,13 @@ public String getName() {
  @Override
  public JsonRpcResponse response(final JsonRpcRequestContext request) {
    final String privacyGroupId = request.getRequiredParameter(0, String.class);
-    final FilterParameter filter = request.getRequiredParameter(1, FilterParameter.class);
+    final FilterParameter filter;


pinges · 2024-08-15T13:22:12Z

...reum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java

@@ -784,7 +784,7 @@ public void getBlockByHashWithMissingBooleanParameter() throws Exception {
      assertThat(resp.code()).isEqualTo(200);
      // Check general format of result
      final JsonObject json = new JsonObject(resp.body().string());
-      final RpcErrorType expectedError = RpcErrorType.INVALID_PARAMS;
+      final RpcErrorType expectedError = RpcErrorType.INVALID_IS_TRANSACTION_COMPLETE_PARAMS;


We should probably rename this. If this parameter is true besu returns the complete transaction objects, if false only the tx hashes.

Agreed, INVALID_RETURN_COMPLETE_TRANSACTION_PARAMS sounds much better

* 5098: Add RpcErrorTypes Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net> --------- Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net> Signed-off-by: Matilda-Clerke <matilda.clerke@consensys.net> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: gconnect <agatevureglory@gmail.com>

Matilda-Clerke added 30 commits July 30, 2024 08:54

5098: Add RpcErrorTypes

e41cf31

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Modify InvalidJsonRpcParameters and InvalidJsonRpcRequestExcept…

147d580

…ion, apply spotless Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Modify InvalidJsonRpcParameters and InvalidJsonRpcRequestExcept…

8da4815

…ion, apply spotless Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Add JsonRpcParameterException for later use

8ca34d7

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Update locations for RpcErrorType.INVALID_ACCOUNTS_PARAMS

5b5631a

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Address review comments, apply spotless

950117b

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'refs/heads/5098-branch-1-add-rpc-error-types' into 5098…

0004fa2

…-branch-2-update-invalid-accounts-params

5098: Update with changes from branch 1

7fc2c2b

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'main' into 5098-branch-1-add-rpc-error-types

b70da06

5098: Update code to use RpcErrorType.INVALID_ADDRESS_HASH_PARAMS

a54a5aa

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: apply spotless

5acb2d7

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'refs/heads/5098-branch-2-update-invalid-accounts-params…

aba28d4

…' into 5098-branch-3-update-invalid-address-hash-params

5098: apply spotless

afb8860

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Update plugin-api gradle hash

0d4a9b3

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'refs/heads/5098-branch-1-add-rpc-error-types' into 5098…

ad7d7b3

…-branch-2-update-invalid-accounts-params

Merge branch 'refs/heads/5098-branch-2-update-invalid-accounts-params…

8aac343

…' into 5098-branch-3-update-invalid-address-hash-params

5098: Add comment on INVALID_PARAMS_ERROR_CODE

9ba006b

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Apply spotless on latest changes

531c530

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'main' into 5098-branch-1-add-rpc-error-types

e946b06

Merge branch 'refs/heads/5098-branch-1-add-rpc-error-types' into 5098…

b0dc1a8

…-branch-2-update-invalid-accounts-params

Merge branch 'refs/heads/5098-branch-2-update-invalid-accounts-params…

cc6bdf3

…' into 5098-branch-3-update-invalid-address-hash-params

Merge branch 'main' into 5098-branch-2-update-invalid-accounts-params

983ac77

5098: Update code to use RpcErrorType.INVALID_ADDRESS_PARAMS

6fc67ee

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Fix broken unit test

728ab2c

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge remote-tracking branch 'origin/5098-branch-2-update-invalid-acc…

6480308

…ounts-params' into 5098-branch-2-update-invalid-accounts-params

Merge branch 'refs/heads/5098-branch-2-update-invalid-accounts-params…

8b72fbc

…' into 5098-branch-3-update-invalid-address-hash-params

Merge branch 'refs/heads/5098-branch-3-update-invalid-address-hash-pa…

a60966c

…rams' into 5098-branch-4-update-invalid-address-params

5098: Fix broken unit test

0fb6919

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'refs/heads/5098-branch-2-update-invalid-accounts-params…

e967e32

…' into 5098-branch-3-update-invalid-address-hash-params

Merge branch 'refs/heads/5098-branch-3-update-invalid-address-hash-pa…

32db32b

…rams' into 5098-branch-4-update-invalid-address-params

Matilda-Clerke added 23 commits August 12, 2024 16:09

5098: Update RpcErrorType.INVALID_ENGINE_PREPARE_PAYLOAD_PARAMS

7e873e7

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Apply spotless

3d6824e

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'main' into 5098-branch-15-update-remaining-invalid-engi…

20b5a08

…ne-params Signed-off-by: Matilda-Clerke <matilda.clerke@consensys.net>

5098: Update RpcErrorType.INVALID_ENODE_PARAMS

a70a150

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Update RpcErrorType.INVALID_EXCESS_BLOB_GAS_PARAMS

2db0f4b

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Apply spotless

d15a65e

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Update RpcErrorType.INVALID_EXTRA_DATA_PARAMS

36a0819

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Update RpcErrorType.INVALID_FILTER_PARAMS

509c15e

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Apply spotless

086a1b9

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Fix broken tests

a6efed6

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Update several RpcErrorTypes

597670e

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Apply spotless

fd16783

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Hopefully fix broken integration test

9f4c7cf

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Hopefully fix broken integration test

d2ed77d

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'main' into 5098-branch-16-update-invalid-enode-and-exce…

667b03a

…ss-blob-gas-params Signed-off-by: Matilda-Clerke <matilda.clerke@consensys.net>

Merge branch 'main' into 5098-branch-16-update-invalid-enode-and-exce…

7f04e66

…ss-blob-gas-params

5098: Update several more RpcErrorTypes

eca3323

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

5098: Apply spotless

df64162

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>

Merge branch 'main' into 5098-branch-16-update-invalid-enode-and-exce…

e87ebe6

…ss-blob-gas-params

Merge branch 'refs/heads/5098-branch-16-update-invalid-enode-and-exce…

ae5b11f

…ss-blob-gas-params' into 5098-branch-17-update-extra-data-and-filter-params

Merge branch 'refs/heads/5098-branch-17-update-extra-data-and-filter-…

70218ef

…params' into 5098-branch-18-update-more-invalid-params

Merge branch 'refs/heads/5098-branch-18-update-more-invalid-params' i…

8da7f70

…nto 5098-branch-19-update-more-invalid-params

Merge branch 'main' into 5098-branch-19-update-more-invalid-params

3ce42ce

macfarla approved these changes Aug 15, 2024

View reviewed changes

macfarla enabled auto-merge (squash) August 15, 2024 23:53

Matilda-Clerke added 2 commits August 16, 2024 11:19

Merge branch 'main' into 5098-branch-19-update-more-invalid-params

44e1153

Merge branch 'main' into 5098-branch-19-update-more-invalid-params

a6e28a1

macfarla merged commit 68d7bd0 into hyperledger:main Aug 16, 2024
40 checks passed

pinges reviewed Aug 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

5098 branch 19 update more invalid params #7460

5098 branch 19 update more invalid params #7460

Matilda-Clerke commented Aug 14, 2024

pinges left a comment

pinges Aug 15, 2024

pinges Aug 15, 2024

Matilda-Clerke Aug 16, 2024

pinges Aug 15, 2024

Matilda-Clerke Aug 16, 2024

pinges Aug 15, 2024

Matilda-Clerke Aug 16, 2024

pinges Aug 15, 2024

pinges Aug 15, 2024

Matilda-Clerke Aug 16, 2024

pinges Aug 15, 2024

pinges Aug 15, 2024

pinges Aug 15, 2024

pinges Aug 15, 2024

Matilda-Clerke Aug 16, 2024

	.hasMessage("Invalid is transaction complete parameter (index 1)");
	.hasMessage("Invalid return complete transactions parameter (index 1)");

		@@ -75,7 +75,13 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
		throw new InvalidJsonRpcParameters(

	"Invalid is complete transaction parameter (index 1)",
	"Invalid return complete transactions parameter (index 1)",

	"Invalid is transaction complete parameter (index 1)",
	"Invalid return complete transactions parameter (index 1)",

5098 branch 19 update more invalid params #7460

5098 branch 19 update more invalid params #7460

Conversation

Matilda-Clerke commented Aug 14, 2024

PR description

Fixed Issue(s)

pinges left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment