From 6d05f8312cfc36cdf6ca6228e1fc0cdf1ea63583 Mon Sep 17 00:00:00 2001 From: Karim Taam Date: Tue, 21 Jan 2025 17:28:48 +0100 Subject: [PATCH 1/3] Improve error message for Invalid operation (#8141) Signed-off-by: Karim Taam --- .../tracing/flat/FlatTraceGenerator.java | 14 +++++++++- .../besu/evmtool/state-test/create-eof.json | 2 +- .../state-test/create-invalid-eof.json | 2 +- .../besu/evmtool/trace/badcode.json | 2 +- .../java/org/hyperledger/besu/evm/EVM.java | 4 +-- .../besu/evm/frame/ExceptionalHaltReason.java | 27 +++++++++++++++++++ .../besu/evm/operation/InvalidOperation.java | 16 +++++++---- 7 files changed, 56 insertions(+), 11 deletions(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/tracing/flat/FlatTraceGenerator.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/tracing/flat/FlatTraceGenerator.java index a253c6e323f..0a8166c032b 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/tracing/flat/FlatTraceGenerator.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/tracing/flat/FlatTraceGenerator.java @@ -492,7 +492,19 @@ private static FlatTrace.Context handleHalt( traceFrameBuilder = currentContext.getBuilder(); } traceFrameBuilder.error( - traceFrame.getExceptionalHaltReason().map(ExceptionalHaltReason::getDescription)); + traceFrame + .getExceptionalHaltReason() + .map( + exceptionalHaltReason -> { + if (exceptionalHaltReason + .name() + .equals(ExceptionalHaltReason.INVALID_OPERATION.name())) { + return ExceptionalHaltReason.INVALID_OPERATION.getDescription(); + } else { + return exceptionalHaltReason.getDescription(); + } + })); + if (currentContext != null) { final Action.Builder actionBuilder = traceFrameBuilder.getActionBuilder(); actionBuilder.value(Quantity.create(traceFrame.getValue())); diff --git a/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/state-test/create-eof.json b/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/state-test/create-eof.json index 72ada37da7b..156a690f6c1 100644 --- a/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/state-test/create-eof.json +++ b/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/state-test/create-eof.json @@ -80,7 +80,7 @@ {"pc":6,"section":0,"op":95,"gas":"0x793d6f","gasCost":"0x2","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"PUSH0"}, {"pc":7,"section":0,"op":238,"immediate":"0x00","gas":"0x793d6d","gasCost":"0x0","memSize":0,"stack":["0x0","0x0"],"depth":1,"refund":0,"opName":"RETURNCONTRACT"}, {"output":"","gasUsed":"0xe433","test":"create-eof","fork":"Osaka","d":0,"g":0,"v":0,"postHash":"0x1a8642a04dae90535f00f53d3a30284c4db051d508a653db89eb100ba9aecbf3","postLogsHash":"0xf48b954a6a6f4ce6b28e4950b7027413f4bdc8f459df6003b6e8d7a1567c8940","pass":true}, - {"pc":0,"op":239,"gas":"0x794068","gasCost":"0x0","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"INVALID","error":"Bad instruction"}, + {"pc":0,"op":239,"gas":"0x794068","gasCost":"0x0","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"INVALID","error":"Invalid opcode: 0xef"}, {"output":"","gasUsed":"0x7a1200","test":"create-eof","fork":"Cancun","d":0,"g":0,"v":0,"postHash":"0xaa80d89bc89f58da8de41d3894bd1a241896ff91f7a5964edaefb39e8e3a4a98","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true,"error":"INVALID_OPERATION"} ] } diff --git a/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/state-test/create-invalid-eof.json b/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/state-test/create-invalid-eof.json index f3448aa5a27..b6e539d7e07 100644 --- a/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/state-test/create-invalid-eof.json +++ b/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/state-test/create-invalid-eof.json @@ -72,7 +72,7 @@ }, "stdout": [ {"output":"","gasUsed":"0xd198","test":"create-eof","fork":"Osaka","d":0,"g":0,"v":0,"postHash":"0x2a9c58298ba5d4ec86ca682b9fcc9ff67c3fc44dbd39f85a2f9b74bfe4e5178e","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":false,"error":"Invalid EOF Layout: unexpected_header_kind expected 1 actual 17"}, - {"pc":0,"op":239,"gas":"0x794068","gasCost":"0x0","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"INVALID","error":"Bad instruction"}, + {"pc":0,"op":239,"gas":"0x794068","gasCost":"0x0","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"INVALID","error":"Invalid opcode: 0xef"}, {"output":"","gasUsed":"0x7a1200","test":"create-eof","fork":"Cancun","d":0,"g":0,"v":0,"postHash":"0xaa80d89bc89f58da8de41d3894bd1a241896ff91f7a5964edaefb39e8e3a4a98","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true,"error":"INVALID_OPERATION"} ] } diff --git a/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/trace/badcode.json b/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/trace/badcode.json index 78dc43c02e1..dd6616d7ac1 100644 --- a/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/trace/badcode.json +++ b/ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/trace/badcode.json @@ -8,7 +8,7 @@ "stdin": "", "stdout": [ {"pc":0,"op":96,"gas":"0x2540be400","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}, - {"pc":2,"op":239,"gas":"0x2540be3fd","gasCost":"0x0","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"INVALID","error":"Bad instruction"}, + {"pc":2,"op":239,"gas":"0x2540be3fd","gasCost":"0x0","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"INVALID","error":"Invalid opcode: 0xef"}, {"stateRoot":"0xfc9dc1be50c1b0a497afa545d770cc7064f0d71efbc4338f002dc2e086965d98","output":"0x","gasUsed":"0x2540be400","pass":false,"fork":"Cancun"} ] } \ No newline at end of file diff --git a/evm/src/main/java/org/hyperledger/besu/evm/EVM.java b/evm/src/main/java/org/hyperledger/besu/evm/EVM.java index 69575143923..4ce007dab46 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/EVM.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/EVM.java @@ -241,7 +241,7 @@ public void runToHalt(final MessageFrame frame, final OperationTracer tracing) { case 0x09 -> MulModOperation.staticOperation(frame); case 0x0a -> ExpOperation.staticOperation(frame, gasCalculator); case 0x0b -> SignExtendOperation.staticOperation(frame); - case 0x0c, 0x0d, 0x0e, 0x0f -> InvalidOperation.INVALID_RESULT; + case 0x0c, 0x0d, 0x0e, 0x0f -> InvalidOperation.invalidOperationResult(opcode); case 0x10 -> LtOperation.staticOperation(frame); case 0x11 -> GtOperation.staticOperation(frame); case 0x12 -> SLtOperation.staticOperation(frame); @@ -259,7 +259,7 @@ public void runToHalt(final MessageFrame frame, final OperationTracer tracing) { case 0x5f -> enableShanghai ? Push0Operation.staticOperation(frame) - : InvalidOperation.INVALID_RESULT; + : InvalidOperation.invalidOperationResult(opcode); case 0x60, // PUSH1-32 0x61, 0x62, diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/ExceptionalHaltReason.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/ExceptionalHaltReason.java index 729d10ffa50..1c72b1044eb 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/frame/ExceptionalHaltReason.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/frame/ExceptionalHaltReason.java @@ -86,6 +86,33 @@ public interface ExceptionalHaltReason { */ String getDescription(); + /** + * Creates an ExceptionalHaltReason for an invalid operation . + * + * @see org.hyperledger.besu.evm.operation.InvalidOperation + *

Includes the problematic opcode in the description. + * @param opcode The invalid opcode. + * @return An ExceptionalHaltReason instance with the opcode details. + */ + static ExceptionalHaltReason newInvalidOperation(final long opcode) { + return new ExceptionalHaltReason() { + @Override + public String name() { + return INVALID_OPERATION.name(); + } + + @Override + public String getDescription() { + return "Invalid opcode: 0x%02x".formatted(opcode); + } + + @Override + public String toString() { + return name(); + } + }; + } + /** The enum Default exceptional halt reason. */ enum DefaultExceptionalHaltReason implements ExceptionalHaltReason { /** None default exceptional halt reason. */ diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/InvalidOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/InvalidOperation.java index b7ca197d0d3..598fa95a900 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/operation/InvalidOperation.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/operation/InvalidOperation.java @@ -29,9 +29,6 @@ public class InvalidOperation extends AbstractOperation { public static final OperationResult INVALID_RESULT = new OperationResult(0, ExceptionalHaltReason.INVALID_OPERATION); - /** The Invalid operation result. */ - protected final OperationResult invalidResult; - /** * Instantiates a new Invalid operation. * @@ -49,11 +46,20 @@ public InvalidOperation(final GasCalculator gasCalculator) { */ public InvalidOperation(final int opcode, final GasCalculator gasCalculator) { super(opcode, "INVALID", -1, -1, gasCalculator); - invalidResult = new OperationResult(0L, ExceptionalHaltReason.INVALID_OPERATION); } @Override public OperationResult execute(final MessageFrame frame, final EVM evm) { - return invalidResult; + return invalidOperationResult(getOpcode()); + } + + /** + * Creates an {@link OperationResult} for an invalid opcode. + * + * @param opcode the invalid opcode encountered + * @return an {@link OperationResult} with zero gas cost and a description of the invalid opcode + */ + public static OperationResult invalidOperationResult(final int opcode) { + return new OperationResult(0, ExceptionalHaltReason.newInvalidOperation(opcode)); } } From e930c2dcb3001aea1dc8c131ab33ae6094bb58ce Mon Sep 17 00:00:00 2001 From: Matilda-Clerke Date: Wed, 22 Jan 2025 08:31:31 +1100 Subject: [PATCH 2/3] 8053: Add GetPooledTransactionsFromPeerTask and appropriate usage (#8058) * 8053: Add GetPooledTransactionsFromPeerTask and appropriate usage Signed-off-by: Matilda Clerke * 7582: Loosen restriction to allow fewer or equal results than requested, but not more Signed-off-by: Matilda Clerke * 8053: Protect against null message response in peer task executor Signed-off-by: Matilda Clerke * 8053: spotless Signed-off-by: Matilda Clerke --------- Signed-off-by: Matilda Clerke --- .../controller/BesuControllerBuilder.java | 3 +- .../besu/services/BesuEventsImplTest.java | 3 +- .../manager/peertask/PeerTaskExecutor.java | 4 + .../GetPooledTransactionsFromPeerTask.java | 79 ++++++++ ...dGetPooledTransactionsFromPeerFetcher.java | 76 +++++--- ...oledTransactionHashesMessageProcessor.java | 8 +- .../transactions/TransactionPoolFactory.java | 12 +- .../eth/manager/EthProtocolManagerTest.java | 3 +- .../ethtaskutils/AbstractMessageTaskTest.java | 3 +- ...GetPooledTransactionsFromPeerTaskTest.java | 101 ++++++++++ ...PooledTransactionsFromPeerFetcherTest.java | 3 +- ...tionsFromPeerFetcherUsingPeerTaskTest.java | 184 ++++++++++++++++++ ...TransactionHashesMessageProcessorTest.java | 3 +- .../ethereum/eth/transactions/TestNode.java | 3 +- .../TransactionPoolFactoryTest.java | 3 +- 15 files changed, 452 insertions(+), 36 deletions(-) create mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetPooledTransactionsFromPeerTask.java create mode 100644 ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetPooledTransactionsFromPeerTaskTest.java create mode 100644 ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcherUsingPeerTaskTest.java diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index d10ab04362c..97108f18070 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -717,7 +717,8 @@ public BesuController build() { syncState, transactionPoolConfiguration, besuComponent.map(BesuComponent::getBlobCache).orElse(new BlobCache()), - miningConfiguration); + miningConfiguration, + syncConfig.isPeerTaskSystemEnabled()); final List peerValidators = createPeerValidators(protocolSchedule, peerTaskExecutor); diff --git a/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java b/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java index 453e31378d2..83f6347a55e 100644 --- a/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java +++ b/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java @@ -173,7 +173,8 @@ public void setUp() { syncState, txPoolConfig, new BlobCache(), - MiningConfiguration.newDefault()); + MiningConfiguration.newDefault(), + false); serviceImpl = new BesuEventsImpl( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java index 8f5e411f6c5..4afbec6f418 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java @@ -136,6 +136,10 @@ public PeerTaskExecutorResult executeAgainstPeer( MessageData responseMessageData = requestSender.sendRequest(peerTask.getSubProtocol(), requestMessageData, peer); + if (responseMessageData == null) { + throw new InvalidPeerTaskResponseException(); + } + result = peerTask.processResponse(responseMessageData); } finally { inflightRequestCountForThisTaskClass.decrementAndGet(); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetPooledTransactionsFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetPooledTransactionsFromPeerTask.java new file mode 100644 index 00000000000..0b99a2676bb --- /dev/null +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetPooledTransactionsFromPeerTask.java @@ -0,0 +1,79 @@ +/* + * Copyright contributors to Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.eth.manager.peertask.task; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.core.Transaction; +import org.hyperledger.besu.ethereum.eth.EthProtocol; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.InvalidPeerTaskResponseException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTask; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskValidationResponse; +import org.hyperledger.besu.ethereum.eth.messages.GetPooledTransactionsMessage; +import org.hyperledger.besu.ethereum.eth.messages.PooledTransactionsMessage; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; + +import java.util.List; +import java.util.function.Predicate; + +public class GetPooledTransactionsFromPeerTask implements PeerTask> { + + private final List hashes; + + public GetPooledTransactionsFromPeerTask(final List hashes) { + this.hashes = hashes.stream().distinct().toList(); + } + + @Override + public SubProtocol getSubProtocol() { + return EthProtocol.get(); + } + + @Override + public MessageData getRequestMessage() { + return GetPooledTransactionsMessage.create(hashes); + } + + @Override + public List processResponse(final MessageData messageData) + throws InvalidPeerTaskResponseException { + final PooledTransactionsMessage pooledTransactionsMessage = + PooledTransactionsMessage.readFrom(messageData); + final List responseTransactions = pooledTransactionsMessage.transactions(); + if (responseTransactions.size() > hashes.size()) { + throw new InvalidPeerTaskResponseException( + "Response transaction count does not match request hash count"); + } + return responseTransactions; + } + + @Override + public Predicate getPeerRequirementFilter() { + return (peer) -> true; + } + + @Override + public PeerTaskValidationResponse validateResult(final List result) { + if (!result.stream().allMatch((t) -> hashes.contains(t.getHash()))) { + return PeerTaskValidationResponse.RESULTS_DO_NOT_MATCH_QUERY; + } + return PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD; + } + + public List getHashes() { + return hashes; + } +} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcher.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcher.java index 5bf2a364108..51c7cb924a9 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcher.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcher.java @@ -20,6 +20,8 @@ import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; import org.hyperledger.besu.ethereum.eth.transactions.PeerTransactionTracker; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolMetrics; @@ -28,6 +30,7 @@ import java.util.Collection; import java.util.List; import java.util.Queue; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ScheduledFuture; import com.google.common.collect.EvictingQueue; @@ -49,6 +52,7 @@ public class BufferedGetPooledTransactionsFromPeerFetcher { private final ScheduledFuture scheduledFuture; private final EthPeer peer; private final Queue txAnnounces; + private final boolean isPeerTaskSystemEnabled; public BufferedGetPooledTransactionsFromPeerFetcher( final EthContext ethContext, @@ -57,7 +61,8 @@ public BufferedGetPooledTransactionsFromPeerFetcher( final TransactionPool transactionPool, final PeerTransactionTracker transactionTracker, final TransactionPoolMetrics metrics, - final String metricLabel) { + final String metricLabel, + final boolean isPeerTaskSystemEnabled) { this.ethContext = ethContext; this.scheduledFuture = scheduledFuture; this.peer = peer; @@ -67,6 +72,7 @@ public BufferedGetPooledTransactionsFromPeerFetcher( this.metricLabel = metricLabel; this.txAnnounces = Queues.synchronizedQueue(EvictingQueue.create(DEFAULT_MAX_PENDING_TRANSACTIONS)); + this.isPeerTaskSystemEnabled = isPeerTaskSystemEnabled; } public ScheduledFuture getScheduledFuture() { @@ -76,27 +82,53 @@ public ScheduledFuture getScheduledFuture() { public void requestTransactions() { List txHashesAnnounced; while (!(txHashesAnnounced = getTxHashesAnnounced()).isEmpty()) { - final GetPooledTransactionsFromPeerTask task = - GetPooledTransactionsFromPeerTask.forHashes( - ethContext, txHashesAnnounced, metrics.getMetricsSystem()); - task.assignPeer(peer); - ethContext - .getScheduler() - .scheduleSyncWorkerTask(task) - .thenAccept( - result -> { - List retrievedTransactions = result.getResult(); - transactionTracker.markTransactionsAsSeen(peer, retrievedTransactions); - - LOG.atTrace() - .setMessage("Got {} transactions of {} hashes requested from peer {}") - .addArgument(retrievedTransactions::size) - .addArgument(task.getTransactionHashes()::size) - .addArgument(peer::getLoggableId) - .log(); - - transactionPool.addRemoteTransactions(retrievedTransactions); - }); + CompletableFuture> futureTransactions; + if (isPeerTaskSystemEnabled) { + final org.hyperledger.besu.ethereum.eth.manager.peertask.task + .GetPooledTransactionsFromPeerTask + task = + new org.hyperledger.besu.ethereum.eth.manager.peertask.task + .GetPooledTransactionsFromPeerTask(txHashesAnnounced); + futureTransactions = + ethContext + .getScheduler() + .scheduleSyncWorkerTask( + () -> { + PeerTaskExecutorResult> taskResult = + ethContext.getPeerTaskExecutor().executeAgainstPeer(task, peer); + if (taskResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + || taskResult.result().isEmpty()) { + return CompletableFuture.failedFuture( + new RuntimeException("Failed to retrieve transactions for hashes")); + } + return CompletableFuture.completedFuture(taskResult.result().get()); + }); + } else { + final GetPooledTransactionsFromPeerTask task = + GetPooledTransactionsFromPeerTask.forHashes( + ethContext, txHashesAnnounced, metrics.getMetricsSystem()); + task.assignPeer(peer); + futureTransactions = + ethContext + .getScheduler() + .scheduleSyncWorkerTask(task) + .thenCompose( + (peerTaskResult) -> + CompletableFuture.completedFuture(peerTaskResult.getResult())); + } + + futureTransactions.thenAccept( + retrievedTransactions -> { + transactionTracker.markTransactionsAsSeen(peer, retrievedTransactions); + + LOG.atTrace() + .setMessage("Got {} transactions requested from peer {}") + .addArgument(retrievedTransactions::size) + .addArgument(peer::getLoggableId) + .log(); + + transactionPool.addRemoteTransactions(retrievedTransactions); + }); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java index b4ae7ec87c0..a7875ac8f8e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java @@ -48,13 +48,15 @@ public class NewPooledTransactionHashesMessageProcessor { private final TransactionPoolConfiguration transactionPoolConfiguration; private final EthContext ethContext; private final TransactionPoolMetrics metrics; + private final boolean isPeerTaskSystemEnabled; public NewPooledTransactionHashesMessageProcessor( final PeerTransactionTracker transactionTracker, final TransactionPool transactionPool, final TransactionPoolConfiguration transactionPoolConfiguration, final EthContext ethContext, - final TransactionPoolMetrics metrics) { + final TransactionPoolMetrics metrics, + final boolean isPeerTaskSystemEnabled) { this.transactionTracker = transactionTracker; this.transactionPool = transactionPool; this.transactionPoolConfiguration = transactionPoolConfiguration; @@ -62,6 +64,7 @@ public NewPooledTransactionHashesMessageProcessor( this.metrics = metrics; metrics.initExpiredMessagesCounter(METRIC_LABEL); this.scheduledTasks = new ConcurrentHashMap<>(); + this.isPeerTaskSystemEnabled = isPeerTaskSystemEnabled; } void processNewPooledTransactionHashesMessage( @@ -114,7 +117,8 @@ private void processNewPooledTransactionHashesMessage( transactionPool, transactionTracker, metrics, - METRIC_LABEL); + METRIC_LABEL, + isPeerTaskSystemEnabled); }); bufferedTask.addHashes( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java index 9f133236302..65c45d098d4 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java @@ -56,7 +56,8 @@ public static TransactionPool createTransactionPool( final SyncState syncState, final TransactionPoolConfiguration transactionPoolConfiguration, final BlobCache blobCache, - final MiningConfiguration miningConfiguration) { + final MiningConfiguration miningConfiguration, + final boolean isPeerTaskSystemEnabled) { final TransactionPoolMetrics metrics = new TransactionPoolMetrics(metricsSystem); @@ -80,7 +81,8 @@ public static TransactionPool createTransactionPool( transactionsMessageSender, newPooledTransactionHashesMessageSender, blobCache, - miningConfiguration); + miningConfiguration, + isPeerTaskSystemEnabled); } static TransactionPool createTransactionPool( @@ -95,7 +97,8 @@ static TransactionPool createTransactionPool( final TransactionsMessageSender transactionsMessageSender, final NewPooledTransactionHashesMessageSender newPooledTransactionHashesMessageSender, final BlobCache blobCache, - final MiningConfiguration miningConfiguration) { + final MiningConfiguration miningConfiguration, + final boolean isPeerTaskSystemEnabled) { final TransactionPool transactionPool = new TransactionPool( @@ -135,7 +138,8 @@ static TransactionPool createTransactionPool( transactionPool, transactionPoolConfiguration, ethContext, - metrics), + metrics, + isPeerTaskSystemEnabled), transactionPoolConfiguration.getUnstable().getTxMessageKeepAliveSeconds()); subscribeTransactionHandlers( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java index 52fcf322c52..f90ec06e13b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java @@ -1149,7 +1149,8 @@ public void transactionMessagesGoToTheCorrectExecutor() { new SyncState(blockchain, ethManager.ethContext().getEthPeers()), TransactionPoolConfiguration.DEFAULT, new BlobCache(), - MiningConfiguration.newDefault()) + MiningConfiguration.newDefault(), + false) .setEnabled(); // Send just a transaction message. diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java index 79638fee41e..bd8589520e7 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java @@ -147,7 +147,8 @@ public void setupTest() { syncState, TransactionPoolConfiguration.DEFAULT, new BlobCache(), - MiningConfiguration.newDefault()); + MiningConfiguration.newDefault(), + false); transactionPool.setEnabled(); ethProtocolManager = diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetPooledTransactionsFromPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetPooledTransactionsFromPeerTaskTest.java new file mode 100644 index 00000000000..1058413e974 --- /dev/null +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/task/GetPooledTransactionsFromPeerTaskTest.java @@ -0,0 +1,101 @@ +/* + * Copyright contributors to Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.eth.manager.peertask.task; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.core.BlockDataGenerator; +import org.hyperledger.besu.ethereum.core.Transaction; +import org.hyperledger.besu.ethereum.eth.manager.peertask.InvalidPeerTaskResponseException; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskValidationResponse; +import org.hyperledger.besu.ethereum.eth.messages.PooledTransactionsMessage; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; + +import java.util.List; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +public class GetPooledTransactionsFromPeerTaskTest { + private static final BlockDataGenerator GENERATOR = new BlockDataGenerator(); + + @Test + public void testGetRequestMessage() { + List hashes = List.of(Hash.EMPTY); + GetPooledTransactionsFromPeerTask task = new GetPooledTransactionsFromPeerTask(hashes); + + MessageData result = task.getRequestMessage(); + + Assertions.assertEquals( + "0xe1a0c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", + result.getData().toHexString()); + } + + @Test + public void testProcessResponse() throws InvalidPeerTaskResponseException { + List hashes = List.of(Hash.EMPTY); + GetPooledTransactionsFromPeerTask task = new GetPooledTransactionsFromPeerTask(hashes); + + Transaction transaction = GENERATOR.transaction(); + PooledTransactionsMessage pooledTransactionsMessage = + PooledTransactionsMessage.create(List.of(transaction)); + + List result = task.processResponse(pooledTransactionsMessage); + + Assertions.assertEquals(List.of(transaction), result); + } + + @Test + public void testProcessResponseWithIncorrectTransactionCount() { + List hashes = List.of(Hash.EMPTY); + GetPooledTransactionsFromPeerTask task = new GetPooledTransactionsFromPeerTask(hashes); + + PooledTransactionsMessage pooledTransactionsMessage = + PooledTransactionsMessage.create(List.of(GENERATOR.transaction(), GENERATOR.transaction())); + + InvalidPeerTaskResponseException exception = + Assertions.assertThrows( + InvalidPeerTaskResponseException.class, + () -> task.processResponse(pooledTransactionsMessage)); + + Assertions.assertEquals( + "Response transaction count does not match request hash count", exception.getMessage()); + } + + @Test + public void testValidateResult() { + List hashes = List.of(Hash.EMPTY); + GetPooledTransactionsFromPeerTask task = new GetPooledTransactionsFromPeerTask(hashes); + + Transaction transaction = Mockito.mock(Transaction.class); + Mockito.when(transaction.getHash()).thenReturn(Hash.EMPTY); + + PeerTaskValidationResponse validationResponse = task.validateResult(List.of(transaction)); + Assertions.assertEquals(PeerTaskValidationResponse.RESULTS_VALID_AND_GOOD, validationResponse); + } + + @Test + public void testValidateResultWithMismatchedResults() { + List hashes = List.of(Hash.EMPTY); + GetPooledTransactionsFromPeerTask task = new GetPooledTransactionsFromPeerTask(hashes); + + Transaction transaction = Mockito.mock(Transaction.class); + Mockito.when(transaction.getHash()).thenReturn(Hash.EMPTY_TRIE_HASH); + + PeerTaskValidationResponse validationResponse = task.validateResult(List.of(transaction)); + Assertions.assertEquals( + PeerTaskValidationResponse.RESULTS_DO_NOT_MATCH_QUERY, validationResponse); + } +} diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcherTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcherTest.java index 232ccc0aae6..b9f2d03a9f1 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcherTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcherTest.java @@ -81,7 +81,8 @@ public void setup() { transactionPool, transactionTracker, new TransactionPoolMetrics(metricsSystem), - "new_pooled_transaction_hashes"); + "new_pooled_transaction_hashes", + false); } @Test diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcherUsingPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcherUsingPeerTaskTest.java new file mode 100644 index 00000000000..f45f8067c32 --- /dev/null +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/BufferedGetPooledTransactionsFromPeerFetcherUsingPeerTaskTest.java @@ -0,0 +1,184 @@ +/* + * Copyright contributors to Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.eth.manager.task; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.core.BlockDataGenerator; +import org.hyperledger.besu.ethereum.core.Transaction; +import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.EthPeers; +import org.hyperledger.besu.ethereum.eth.manager.EthScheduler; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutor; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResponseCode; +import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskExecutorResult; +import org.hyperledger.besu.ethereum.eth.transactions.PeerTransactionTracker; +import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; +import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolMetrics; +import org.hyperledger.besu.metrics.StubMetricsSystem; +import org.hyperledger.besu.testutil.DeterministicEthScheduler; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import io.netty.util.concurrent.ScheduledFuture; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; + +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class BufferedGetPooledTransactionsFromPeerFetcherUsingPeerTaskTest { + + private @Mock EthPeer ethPeer; + private @Mock TransactionPool transactionPool; + private @Mock EthContext ethContext; + private @Mock EthPeers ethPeers; + private @Mock PeerTaskExecutor peerTaskExecutor; + + private final BlockDataGenerator generator = new BlockDataGenerator(); + private final EthScheduler ethScheduler = new DeterministicEthScheduler(); + + private BufferedGetPooledTransactionsFromPeerFetcher fetcher; + private StubMetricsSystem metricsSystem; + private PeerTransactionTracker transactionTracker; + + @BeforeEach + public void setup() { + metricsSystem = new StubMetricsSystem(); + when(ethContext.getEthPeers()).thenReturn(ethPeers); + transactionTracker = new PeerTransactionTracker(ethPeers); + when(ethContext.getScheduler()).thenReturn(ethScheduler); + when(ethContext.getPeerTaskExecutor()).thenReturn(peerTaskExecutor); + ScheduledFuture mock = mock(ScheduledFuture.class); + fetcher = + new BufferedGetPooledTransactionsFromPeerFetcher( + ethContext, + mock, + ethPeer, + transactionPool, + transactionTracker, + new TransactionPoolMetrics(metricsSystem), + "new_pooled_transaction_hashes", + true); + } + + @Test + public void requestTransactionShouldStartTaskWhenUnknownTransaction() { + final Transaction transaction = generator.transaction(); + final List taskResult = List.of(transaction); + final PeerTaskExecutorResult> peerTaskResult = + new PeerTaskExecutorResult>( + Optional.of(taskResult), PeerTaskExecutorResponseCode.SUCCESS, Optional.of(ethPeer)); + + when(peerTaskExecutor.executeAgainstPeer( + any( + org.hyperledger.besu.ethereum.eth.manager.peertask.task + .GetPooledTransactionsFromPeerTask.class), + eq(ethPeer))) + .thenReturn(peerTaskResult); + + fetcher.addHashes(List.of(transaction.getHash())); + fetcher.requestTransactions(); + + verify(peerTaskExecutor) + .executeAgainstPeer( + any( + org.hyperledger.besu.ethereum.eth.manager.peertask.task + .GetPooledTransactionsFromPeerTask.class), + eq(ethPeer)); + verifyNoMoreInteractions(peerTaskExecutor); + verify(transactionPool, times(1)).addRemoteTransactions(taskResult); + + assertThat(transactionTracker.hasSeenTransaction(transaction.getHash())).isTrue(); + } + + @Test + public void requestTransactionShouldSplitRequestIntoSeveralTasks() { + final Map transactionsByHash = + IntStream.range(0, 257) + .mapToObj(unused -> generator.transaction()) + .collect(Collectors.toMap((t) -> t.getHash(), (t) -> t)); + fetcher.addHashes(transactionsByHash.keySet()); + + when(peerTaskExecutor.executeAgainstPeer( + any( + org.hyperledger.besu.ethereum.eth.manager.peertask.task + .GetPooledTransactionsFromPeerTask.class), + eq(ethPeer))) + .thenAnswer( + (invocationOnMock) -> { + org.hyperledger.besu.ethereum.eth.manager.peertask.task + .GetPooledTransactionsFromPeerTask + task = + invocationOnMock.getArgument( + 0, + org.hyperledger.besu.ethereum.eth.manager.peertask.task + .GetPooledTransactionsFromPeerTask.class); + List resultTransactions = + task.getHashes().stream().map(transactionsByHash::get).toList(); + return new PeerTaskExecutorResult>( + Optional.of(resultTransactions), + PeerTaskExecutorResponseCode.SUCCESS, + Optional.of(ethPeer)); + }); + + fetcher.requestTransactions(); + + verify(peerTaskExecutor, times(2)) + .executeAgainstPeer( + any( + org.hyperledger.besu.ethereum.eth.manager.peertask.task + .GetPooledTransactionsFromPeerTask.class), + eq(ethPeer)); + verifyNoMoreInteractions(peerTaskExecutor); + } + + @Test + public void requestTransactionShouldNotStartTaskWhenTransactionAlreadySeen() { + + final Transaction transaction = generator.transaction(); + final Hash hash = transaction.getHash(); + transactionTracker.markTransactionHashesAsSeen(ethPeer, List.of(hash)); + + fetcher.addHashes(List.of(hash)); + fetcher.requestTransactions(); + + verifyNoInteractions(peerTaskExecutor); + verify(transactionPool, never()).addRemoteTransactions(List.of(transaction)); + assertThat( + metricsSystem.getCounterValue( + "remote_transactions_already_seen_total", "new_pooled_transaction_hashes")) + .isEqualTo(1); + } +} diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java index 556ccc677d2..0b7277b536e 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessorTest.java @@ -100,7 +100,8 @@ public void setup() { transactionPool, transactionPoolConfiguration, ethContext, - new TransactionPoolMetrics(metricsSystem)); + new TransactionPoolMetrics(metricsSystem), + false); when(ethContext.getScheduler()).thenReturn(ethScheduler); } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java index e29b8285c9c..1ba5d304bf9 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java @@ -182,7 +182,8 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod syncState, TransactionPoolConfiguration.DEFAULT, new BlobCache(), - MiningConfiguration.newDefault()); + MiningConfiguration.newDefault(), + false); final EthProtocolManager ethProtocolManager = new EthProtocolManager( diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java index 11b0f59f690..463cbe2610f 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java @@ -412,7 +412,8 @@ private TransactionPool createTransactionPool( transactionsMessageSender, newPooledTransactionHashesMessageSender, new BlobCache(), - MiningConfiguration.newDefault()); + MiningConfiguration.newDefault(), + false); } private TransactionPool createAndEnableTransactionPool( From 519323f77e95cfefe71586a0a736b935c26e61ba Mon Sep 17 00:00:00 2001 From: Iryoung Jeong Date: Wed, 22 Jan 2025 07:04:42 +0900 Subject: [PATCH 3/3] Fix eth_getBlockByNumber with empty params returns (#8134) * fix testcase Signed-off-by: Iryoung Jeong * fix EthGetBlockByNumber logic to pass testcase Signed-off-by: Iryoung Jeong * fix missed testcase - JsonRpcHttpServiceTest Signed-off-by: Iryoung Jeong --------- Signed-off-by: Iryoung Jeong Co-authored-by: Matilda-Clerke --- .../jsonrpc/internal/methods/EthGetBlockByNumber.java | 2 +- .../ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java | 2 +- .../internal/methods/EthGetBlockByNumberTest.java | 10 +++++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumber.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumber.java index 0a26a24ee38..761870ddd12 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumber.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumber.java @@ -69,7 +69,7 @@ protected BlockParameter blockParameter(final JsonRpcRequestContext request) { return request.getRequiredParameter(0, BlockParameter.class); } catch (JsonRpcParameterException e) { throw new InvalidJsonRpcParameters( - "Invalid block parameter (index 0)", RpcErrorType.INVALID_BLOCK_PARAMS, e); + "Invalid block parameter (index 0)", RpcErrorType.INVALID_BLOCK_NUMBER_PARAMS, e); } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java index 52f2ee050cd..f919a5cf359 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTest.java @@ -980,7 +980,7 @@ public void getBlockByNumberForInvalidBlockParameter() throws Exception { // Check general format of result final String respBody = resp.body().string(); final JsonObject json = new JsonObject(respBody); - final RpcErrorType expectedError = RpcErrorType.INVALID_BLOCK_PARAMS; + final RpcErrorType expectedError = RpcErrorType.INVALID_BLOCK_NUMBER_PARAMS; testHelper.assertValidJsonRpcError( json, id, expectedError.getCode(), expectedError.getMessage()); } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumberTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumberTest.java index fd92b55ba48..619d0a82f4b 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumberTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetBlockByNumberTest.java @@ -106,14 +106,17 @@ public void returnsCorrectMethodName() { @Test public void exceptionWhenNoParamsSupplied() { assertThatThrownBy(() -> method.response(requestWithParams())) - .isInstanceOf(InvalidJsonRpcParameters.class); + .isInstanceOf(InvalidJsonRpcParameters.class) + .hasFieldOrPropertyWithValue("rpcErrorType", RpcErrorType.INVALID_BLOCK_NUMBER_PARAMS); verifyNoMoreInteractions(blockchainQueries); } @Test public void exceptionWhenNoNumberSupplied() { assertThatThrownBy(() -> method.response(requestWithParams("false"))) - .isInstanceOf(InvalidJsonRpcParameters.class); + .isInstanceOf(InvalidJsonRpcParameters.class) + .hasFieldOrPropertyWithValue("rpcErrorType", RpcErrorType.INVALID_BLOCK_NUMBER_PARAMS); + verifyNoMoreInteractions(blockchainQueries); } @@ -129,7 +132,8 @@ public void exceptionWhenNoBoolSupplied() { public void exceptionWhenNumberParamInvalid() { assertThatThrownBy(() -> method.response(requestWithParams("invalid", "true"))) .isInstanceOf(InvalidJsonRpcParameters.class) - .hasMessage("Invalid block parameter (index 0)"); + .hasMessage("Invalid block parameter (index 0)") + .hasFieldOrPropertyWithValue("rpcErrorType", RpcErrorType.INVALID_BLOCK_NUMBER_PARAMS); verifyNoMoreInteractions(blockchainQueries); }