diff --git a/CHANGELOG.md b/CHANGELOG.md index f01abe13dcf..56e13ab59b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## 22.7.7 + +### Additions and Improvements +- Tune EthScheduler thread pools to avoid recreating too many threads [#4529](https://github.com/hyperledger/besu/issues/4529) +- Reduce the number of runtime exceptions (SecurityModuleException) and unnecessary executions during ECIES handshake, by trying to decrypt EIP-8 formatted messages first [#4508](https://github.com/hyperledger/besu/pull/4508). +- The block variable was keeping too much memory while waiting for future to finish [#4489](https://github.com/hyperledger/besu/issues/4489) + +### Bug Fixes +- Corrects treating a block as bad on internal error [#4512](https://github.com/hyperledger/besu/issues/4512) +- update appache-commons-text to 1.10.0 to address CVE-2022-42889 [#4542](https://github.com/hyperledger/besu/pull/4542) +- In GraphQL update scalar parsing to be variable friendly [#4522](https://github.com/hyperledger/besu/pull/4522) + +### Download Links + ## 22.7.6 Hotfix release of the 22.7.x series to address [#4495](https://github.com/hyperledger/besu/issues/4495) which could result in failed block proposals on merge networks. diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java index ac396c3d5b0..bfaf092b3d7 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java @@ -15,7 +15,6 @@ package org.hyperledger.besu.ethereum.api.graphql.internal; import org.hyperledger.besu.datatypes.Address; -import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity; import graphql.language.IntValue; @@ -28,156 +27,239 @@ import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import org.apache.tuweni.units.bigints.UInt256; -import org.apache.tuweni.units.bigints.UInt256Value; public class Scalars { - private static final Coercing ADDRESS_COERCING = - new Coercing() { + private Scalars() {} + + private static final Coercing ADDRESS_COERCING = + new Coercing() { + Address convertImpl(final Object input) { + if (input instanceof Address) { + return (Address) input; + } else if (input instanceof Bytes) { + if (((Bytes) input).size() <= 20) { + return Address.wrap((Bytes) input); + } else { + return null; + } + } else if (input instanceof StringValue) { + return convertImpl(((StringValue) input).getValue()); + } else if (input instanceof String) { + try { + return Address.fromHexStringStrict((String) input); + } catch (IllegalArgumentException iae) { + return null; + } + } else { + return null; + } + } + @Override public String serialize(final Object input) throws CoercingSerializeException { - if (input instanceof Address) { - return input.toString(); + Address result = convertImpl(input); + if (result != null) { + return result.toHexString(); + } else { + throw new CoercingSerializeException("Unable to serialize " + input + " as an Address"); } - throw new CoercingSerializeException("Unable to serialize " + input + " as an Address"); } @Override - public String parseValue(final Object input) throws CoercingParseValueException { - if (input instanceof Address) { - return input.toString(); + public Address parseValue(final Object input) throws CoercingParseValueException { + Address result = convertImpl(input); + if (result != null) { + return result; + } else { + throw new CoercingParseValueException( + "Unable to parse variable value " + input + " as an Address"); } - throw new CoercingParseValueException( - "Unable to parse variable value " + input + " as an Address"); } @Override public Address parseLiteral(final Object input) throws CoercingParseLiteralException { - if (!(input instanceof StringValue)) { - throw new CoercingParseLiteralException("Value is not any Address : '" + input + "'"); - } - String inputValue = ((StringValue) input).getValue(); - try { - return Address.fromHexStringStrict(inputValue); - } catch (final IllegalArgumentException e) { + Address result = convertImpl(input); + if (result != null) { + return result; + } else { throw new CoercingParseLiteralException("Value is not any Address : '" + input + "'"); } } }; - private static final Coercing BIG_INT_COERCING = - new Coercing() { + private static final Coercing BIG_INT_COERCING = + new Coercing() { + + String convertImpl(final Object input) { + if (input instanceof String) { + try { + return Bytes.fromHexStringLenient((String) input).toShortHexString(); + } catch (IllegalArgumentException iae) { + return null; + } + } else if (input instanceof Bytes) { + return ((Bytes) input).toShortHexString(); + } else if (input instanceof StringValue) { + return convertImpl(((StringValue) input).getValue()); + } else if (input instanceof IntValue) { + return UInt256.valueOf(((IntValue) input).getValue()).toShortHexString(); + } else { + return null; + } + } + @Override public String serialize(final Object input) throws CoercingSerializeException { - if (input instanceof UInt256Value) { - return ((UInt256Value) input).toShortHexString(); + var result = convertImpl(input); + if (result != null) { + return result; + } else { + throw new CoercingSerializeException("Unable to serialize " + input + " as an BigInt"); } - throw new CoercingSerializeException("Unable to serialize " + input + " as an BigInt"); } @Override public String parseValue(final Object input) throws CoercingParseValueException { - if (input instanceof UInt256Value) { - return ((UInt256Value) input).toShortHexString(); + var result = convertImpl(input); + if (result != null) { + return result; + } else { + throw new CoercingParseValueException( + "Unable to parse variable value " + input + " as an BigInt"); } - throw new CoercingParseValueException( - "Unable to parse variable value " + input + " as an BigInt"); } @Override - public UInt256 parseLiteral(final Object input) throws CoercingParseLiteralException { - try { - if (input instanceof StringValue) { - return UInt256.fromHexString(((StringValue) input).getValue()); - } else if (input instanceof IntValue) { - return UInt256.valueOf(((IntValue) input).getValue()); - } - } catch (final IllegalArgumentException e) { - // fall through + public String parseLiteral(final Object input) throws CoercingParseLiteralException { + var result = convertImpl(input); + if (result != null) { + return result; + } else { + throw new CoercingParseLiteralException("Value is not any BigInt : '" + input + "'"); } - throw new CoercingParseLiteralException("Value is not any BigInt : '" + input + "'"); } }; - private static final Coercing BYTES_COERCING = - new Coercing() { + private static final Coercing BYTES_COERCING = + new Coercing() { + + Bytes convertImpl(final Object input) { + if (input instanceof Bytes) { + return (Bytes) input; + } else if (input instanceof StringValue) { + return convertImpl(((StringValue) input).getValue()); + } else if (input instanceof String) { + if (!Quantity.isValid((String) input)) { + throw new CoercingParseLiteralException( + "Bytes value '" + input + "' is not prefixed with 0x"); + } + try { + return Bytes.fromHexStringLenient((String) input); + } catch (IllegalArgumentException iae) { + return null; + } + } else { + return null; + } + } + @Override public String serialize(final Object input) throws CoercingSerializeException { - if (input instanceof Bytes) { - return input.toString(); + var result = convertImpl(input); + if (result != null) { + return result.toHexString(); + } else { + throw new CoercingSerializeException("Unable to serialize " + input + " as an Bytes"); } - throw new CoercingSerializeException("Unable to serialize " + input + " as an Bytes"); } @Override - public String parseValue(final Object input) throws CoercingParseValueException { - if (input instanceof Bytes) { - return input.toString(); + public Bytes parseValue(final Object input) throws CoercingParseValueException { + var result = convertImpl(input); + if (result != null) { + return result; + } else { + throw new CoercingParseValueException( + "Unable to parse variable value " + input + " as an Bytes"); } - throw new CoercingParseValueException( - "Unable to parse variable value " + input + " as an Bytes"); } @Override public Bytes parseLiteral(final Object input) throws CoercingParseLiteralException { - if (!(input instanceof StringValue)) { - throw new CoercingParseLiteralException("Value is not any Bytes : '" + input + "'"); - } - String inputValue = ((StringValue) input).getValue(); - if (!Quantity.isValid(inputValue)) { - throw new CoercingParseLiteralException( - "Bytes value '" + inputValue + "' is not prefixed with 0x"); - } - try { - return Bytes.fromHexStringLenient(inputValue); - } catch (final IllegalArgumentException e) { + var result = convertImpl(input); + if (result != null) { + return result; + } else { throw new CoercingParseLiteralException("Value is not any Bytes : '" + input + "'"); } } }; - private static final Coercing BYTES32_COERCING = - new Coercing() { + private static final Coercing BYTES32_COERCING = + new Coercing() { + + Bytes32 convertImpl(final Object input) { + if (input instanceof Bytes32) { + return (Bytes32) input; + } else if (input instanceof Bytes) { + if (((Bytes) input).size() <= 32) { + return Bytes32.leftPad((Bytes) input); + } else { + return null; + } + } else if (input instanceof StringValue) { + return convertImpl((((StringValue) input).getValue())); + } else if (input instanceof String) { + if (!Quantity.isValid((String) input)) { + throw new CoercingParseLiteralException( + "Bytes32 value '" + input + "' is not prefixed with 0x"); + } else { + try { + return Bytes32.fromHexStringLenient((String) input); + } catch (IllegalArgumentException iae) { + return null; + } + } + } else { + return null; + } + } + @Override public String serialize(final Object input) throws CoercingSerializeException { - if (input instanceof Hash) { - return ((Hash) input).toString(); - } - if (input instanceof Bytes32) { - return input.toString(); + var result = convertImpl(input); + if (result == null) { + throw new CoercingSerializeException("Unable to serialize " + input + " as an Bytes32"); + } else { + return result.toHexString(); } - throw new CoercingSerializeException("Unable to serialize " + input + " as an Bytes32"); } @Override - public String parseValue(final Object input) throws CoercingParseValueException { - if (input instanceof Bytes32) { - return input.toString(); + public Bytes32 parseValue(final Object input) throws CoercingParseValueException { + var result = convertImpl(input); + if (result == null) { + throw new CoercingParseValueException( + "Unable to parse variable value " + input + " as an Bytes32"); + } else { + return result; } - throw new CoercingParseValueException( - "Unable to parse variable value " + input + " as an Bytes32"); } @Override public Bytes32 parseLiteral(final Object input) throws CoercingParseLiteralException { - if (!(input instanceof StringValue)) { - throw new CoercingParseLiteralException("Value is not any Bytes32 : '" + input + "'"); - } - String inputValue = ((StringValue) input).getValue(); - if (!Quantity.isValid(inputValue)) { - throw new CoercingParseLiteralException( - "Bytes32 value '" + inputValue + "' is not prefixed with 0x"); - } - try { - return Bytes32.fromHexStringLenient(inputValue); - } catch (final IllegalArgumentException e) { + var result = convertImpl(input); + if (result == null) { throw new CoercingParseLiteralException("Value is not any Bytes32 : '" + input + "'"); + } else { + return result; } } }; - private static final Coercing LONG_COERCING = - new Coercing() { + private static final Coercing LONG_COERCING = + new Coercing() { @Override public Number serialize(final Object input) throws CoercingSerializeException { if (input instanceof Number) { @@ -210,7 +292,7 @@ public Number parseValue(final Object input) throws CoercingParseValueException } @Override - public Object parseLiteral(final Object input) throws CoercingParseLiteralException { + public Number parseLiteral(final Object input) throws CoercingParseLiteralException { try { if (input instanceof IntValue) { return ((IntValue) input).getValue().longValue(); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayload.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayload.java index 8b5aa3c75f4..98d80d0346d 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayload.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayload.java @@ -21,7 +21,6 @@ import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.VALID; import static org.hyperledger.besu.util.Slf4jLambdaHelper.debugLambda; import static org.hyperledger.besu.util.Slf4jLambdaHelper.traceLambda; -import static org.hyperledger.besu.util.Slf4jLambdaHelper.warnLambda; import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator; import org.hyperledger.besu.datatypes.Hash; @@ -108,6 +107,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) reqId, blockParam, mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(null), + INVALID, "Failed to decode transactions from block parameter"); } @@ -116,6 +116,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) reqId, blockParam, mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(null), + INVALID, "Field extraData must not be null"); } @@ -142,11 +143,12 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) // ensure the block hash matches the blockParam hash // this must be done before any other check if (!newBlockHeader.getHash().equals(blockParam.getBlockHash())) { - LOG.debug( + String errorMessage = String.format( "Computed block hash %s does not match block hash parameter %s", - newBlockHeader.getBlockHash(), blockParam.getBlockHash())); - return respondWith(reqId, blockParam, null, INVALID_BLOCK_HASH); + newBlockHeader.getBlockHash(), blockParam.getBlockHash()); + LOG.debug(errorMessage); + return respondWithInvalid(reqId, blockParam, null, INVALID_BLOCK_HASH, errorMessage); } // do we already have this payload if (protocolContext.getBlockchain().getBlockByHash(newBlockHeader.getBlockHash()).isPresent()) { @@ -154,13 +156,14 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) return respondWith(reqId, blockParam, blockParam.getBlockHash(), VALID); } if (mergeCoordinator.isBadBlock(blockParam.getBlockHash())) { - return respondWith( + return respondWithInvalid( reqId, blockParam, mergeCoordinator .getLatestValidHashOfBadBlock(blockParam.getBlockHash()) .orElse(Hash.ZERO), - INVALID); + INVALID, + "Block already present in bad block manager."); } Optional parentHeader = @@ -171,11 +174,13 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) reqId, blockParam, mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(null), + INVALID, "block timestamp not greater than parent"); } final var block = new Block(newBlockHeader, new BlockBody(transactions, Collections.emptyList())); + final String warningMessage = "Sync to block " + block.toLogString() + " failed"; if (mergeContext.get().isSyncing() || parentHeader.isEmpty()) { LOG.debug( @@ -187,8 +192,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) .appendNewPayloadToSync(block) .exceptionally( exception -> { - LOG.warn( - "Sync to block " + block.toLogString() + " failed", exception.getMessage()); + LOG.warn(warningMessage, exception.getMessage()); return null; }); return respondWith(reqId, blockParam, null, SYNCING); @@ -201,6 +205,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) reqId, blockParam, Hash.ZERO, + INVALID, newBlockHeader.getHash() + " did not descend from terminal block"); } @@ -229,7 +234,11 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } LOG.debug("New payload is invalid: {}", executionResult.errorMessage.get()); return respondWithInvalid( - reqId, blockParam, latestValidAncestor.get(), executionResult.errorMessage.get()); + reqId, + blockParam, + latestValidAncestor.get(), + INVALID, + executionResult.errorMessage.get()); } } @@ -238,6 +247,10 @@ JsonRpcResponse respondWith( final EnginePayloadParameter param, final Hash latestValidHash, final EngineStatus status) { + if (INVALID.equals(status) || INVALID_BLOCK_HASH.equals(status)) { + throw new IllegalArgumentException( + "Don't call respondWith() with invalid status of " + status.toString()); + } debugLambda( LOG, "New payload: number: {}, hash: {}, parentHash: {}, latestValidHash: {}, status: {}", @@ -257,22 +270,32 @@ JsonRpcResponse respondWithInvalid( final Object requestId, final EnginePayloadParameter param, final Hash latestValidHash, + final EngineStatus invalidStatus, final String validationError) { + if (!INVALID.equals(invalidStatus) && !INVALID_BLOCK_HASH.equals(invalidStatus)) { + throw new IllegalArgumentException( + "Don't call respondWithInvalid() with non-invalid status of " + invalidStatus.toString()); + } + final String invalidBlockLogMessage = + String.format( + "Invalid new payload: number: %s, hash: %s, parentHash: %s, latestValidHash: %s, status: %s, validationError: %s", + param.getBlockNumber(), + param.getBlockHash(), + param.getParentHash(), + latestValidHash == null ? null : latestValidHash.toHexString(), + invalidStatus.name(), + validationError); + // always log invalid at DEBUG + LOG.debug(invalidBlockLogMessage); + // periodically log at WARN if (lastInvalidWarn + ENGINE_API_LOGGING_THRESHOLD < System.currentTimeMillis()) { lastInvalidWarn = System.currentTimeMillis(); - warnLambda( - LOG, - "Invalid new payload: number: {}, hash: {}, parentHash: {}, latestValidHash: {}, status: {}, validationError: {}", - () -> param.getBlockNumber(), - () -> param.getBlockHash(), - () -> param.getParentHash(), - () -> latestValidHash == null ? null : latestValidHash.toHexString(), - INVALID::name, - () -> validationError); + LOG.warn(invalidBlockLogMessage); } return new JsonRpcSuccessResponse( requestId, - new EnginePayloadStatusResult(INVALID, latestValidHash, Optional.of(validationError))); + new EnginePayloadStatusResult( + invalidStatus, latestValidHash, Optional.of(validationError))); } private void logImportedBlockInfo(final Block block, final double timeInS) { diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/EthGraphQLHttpBySpecTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/EthGraphQLHttpBySpecTest.java index 6dedc04a36e..a2280923d9c 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/EthGraphQLHttpBySpecTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/EthGraphQLHttpBySpecTest.java @@ -113,6 +113,11 @@ public static Collection specs() { specs.add("graphql_tooComplex"); specs.add("graphql_tooComplexSchema"); + specs.add("graphql_variable_address"); + specs.add("graphql_variable_bytes"); + specs.add("graphql_variable_bytes32"); + specs.add("graphql_variable_long"); + return specs; } @@ -128,7 +133,13 @@ private void graphQLCall(final String name) throws IOException { EthGraphQLHttpBySpecTest.class.getResource(testSpecFile), Charsets.UTF_8); final JsonObject spec = new JsonObject(json); final String rawRequestBody = spec.getString("request"); - final RequestBody requestBody = RequestBody.create(rawRequestBody, GRAPHQL); + final String rawVariables = spec.getString("variables"); + final RequestBody requestBody = + rawVariables == null + ? RequestBody.create(rawRequestBody, GRAPHQL) + : RequestBody.create( + "{ \"query\":\"" + rawRequestBody + "\", \"variables\": " + rawVariables + "}", + JSON); final Request request = new Request.Builder().post(requestBody).url(baseUrl).build(); importBlocks(1, BLOCKS.size()); diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/AddressScalarTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/AddressScalarTest.java index 33a0e59289c..87153b49087 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/AddressScalarTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/AddressScalarTest.java @@ -27,10 +27,7 @@ import graphql.schema.GraphQLScalarType; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -@RunWith(MockitoJUnitRunner.class) public class AddressScalarTest { private GraphQLScalarType scalar; @@ -43,13 +40,13 @@ public class AddressScalarTest { @Test public void parseValueTest() { - final String result = (String) scalar.getCoercing().parseValue(addr); - assertThat(result).isEqualTo(addrStr); + final Address result = (Address) scalar.getCoercing().parseValue(addrStr); + assertThat(result).isEqualTo(addr); } @Test public void parseValueErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().parseValue(addrStr)) + assertThatThrownBy(() -> scalar.getCoercing().parseValue(3.4f)) .isInstanceOf(CoercingParseValueException.class); } @@ -61,7 +58,7 @@ public void serializeTest() { @Test public void serializeErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().serialize(addrStr)) + assertThatThrownBy(() -> scalar.getCoercing().serialize(3.4f)) .isInstanceOf(CoercingSerializeException.class); } @@ -73,7 +70,7 @@ public void parseLiteralTest() { @Test public void parseLiteralErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().parseLiteral(addrStr)) + assertThatThrownBy(() -> scalar.getCoercing().parseLiteral(3.4f)) .isInstanceOf(CoercingParseLiteralException.class); } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/BigIntScalarTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/BigIntScalarTest.java index 4f0ac678882..06fcfa42c2f 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/BigIntScalarTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/BigIntScalarTest.java @@ -27,17 +27,13 @@ import org.apache.tuweni.units.bigints.UInt256; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -@RunWith(MockitoJUnitRunner.class) public class BigIntScalarTest { private GraphQLScalarType scalar; private final String str = "0x10"; private final UInt256 value = UInt256.fromHexString(str); - private final StringValue strValue = StringValue.newStringValue(str).build(); private final StringValue invalidStrValue = StringValue.newStringValue("0xgh").build(); @Test @@ -48,7 +44,7 @@ public void parseValueTest() { @Test public void parseValueErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().parseValue(str)) + assertThatThrownBy(() -> scalar.getCoercing().parseValue(3.2)) .isInstanceOf(CoercingParseValueException.class); } @@ -60,19 +56,19 @@ public void serializeTest() { @Test public void serializeErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().serialize(str)) + assertThatThrownBy(() -> scalar.getCoercing().serialize(3.2)) .isInstanceOf(CoercingSerializeException.class); } @Test public void parseLiteralTest() { - final UInt256 result = (UInt256) scalar.getCoercing().parseLiteral(strValue); - assertThat(result).isEqualTo(value); + final String result = (String) scalar.getCoercing().parseLiteral(value); + assertThat(result).isEqualTo(str); } @Test public void parseLiteralErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().parseLiteral(str)) + assertThatThrownBy(() -> scalar.getCoercing().parseLiteral(3.2)) .isInstanceOf(CoercingParseLiteralException.class); } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/Bytes32ScalarTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/Bytes32ScalarTest.java index 56fc6c0e072..47e516c3b52 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/Bytes32ScalarTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/Bytes32ScalarTest.java @@ -27,10 +27,7 @@ import org.apache.tuweni.bytes.Bytes32; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -@RunWith(MockitoJUnitRunner.class) public class Bytes32ScalarTest { private GraphQLScalarType scalar; @@ -42,13 +39,13 @@ public class Bytes32ScalarTest { @Test public void pareValueTest() { - final String result = (String) scalar.getCoercing().parseValue(value); - assertThat(result).isEqualTo(str); + final var result = scalar.getCoercing().parseValue(str); + assertThat(result).isEqualTo(value); } @Test public void parseValueErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().parseValue(str)) + assertThatThrownBy(() -> scalar.getCoercing().parseValue(3.2f)) .isInstanceOf(CoercingParseValueException.class); } @@ -60,7 +57,7 @@ public void serializeTest() { @Test public void serializeErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().serialize(str)) + assertThatThrownBy(() -> scalar.getCoercing().serialize(3.2f)) .isInstanceOf(CoercingSerializeException.class); } @@ -72,7 +69,7 @@ public void parseLiteralTest() { @Test public void parseLiteralErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().parseLiteral(str)) + assertThatThrownBy(() -> scalar.getCoercing().parseLiteral(3.2f)) .isInstanceOf(CoercingParseLiteralException.class); } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/BytesScalarTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/BytesScalarTest.java index 493135d95f2..39ca321c3a5 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/BytesScalarTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/BytesScalarTest.java @@ -27,10 +27,7 @@ import org.apache.tuweni.bytes.Bytes; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -@RunWith(MockitoJUnitRunner.class) public class BytesScalarTest { private GraphQLScalarType scalar; @@ -42,13 +39,13 @@ public class BytesScalarTest { @Test public void parseValueTest() { - final String result = (String) scalar.getCoercing().parseValue(value); - assertThat(result).isEqualTo(str); + final var result = scalar.getCoercing().parseValue(str); + assertThat(result).isEqualTo(value); } @Test public void parseValueErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().parseValue(str)) + assertThatThrownBy(() -> scalar.getCoercing().parseValue(3.2f)) .isInstanceOf(CoercingParseValueException.class); } @@ -60,7 +57,7 @@ public void serializeTest() { @Test public void serializeErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().serialize(str)) + assertThatThrownBy(() -> scalar.getCoercing().serialize(3.2f)) .isInstanceOf(CoercingSerializeException.class); } @@ -72,7 +69,7 @@ public void parseLiteralTest() { @Test public void parseLiteralErrorTest() { - assertThatThrownBy(() -> scalar.getCoercing().parseLiteral(str)) + assertThatThrownBy(() -> scalar.getCoercing().parseLiteral(3.2f)) .isInstanceOf(CoercingParseLiteralException.class); } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/LongScalarTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/LongScalarTest.java index 5a1cfad7af4..d693d6e2c95 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/LongScalarTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/graphql/scalar/LongScalarTest.java @@ -26,10 +26,7 @@ import graphql.schema.GraphQLScalarType; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -@RunWith(MockitoJUnitRunner.class) public class LongScalarTest { private GraphQLScalarType scalar; diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadTest.java index f0a3896174f..ca13e736991 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadTest.java @@ -235,6 +235,8 @@ public void shouldNotReturnInvalidOnInternalException() { fromErrorResp(resp); verify(engineCallListener, times(1)).executionEngineCalled(); + verify(mergeCoordinator, times(0)).addBadBlock(any()); + // verify mainnetBlockValidator does not add to bad block manager } @Test @@ -362,7 +364,7 @@ public void shouldReturnInvalidWhenBadBlock() { EnginePayloadStatusResult res = fromSuccessResp(resp); assertThat(res.getLatestValidHash()).contains(Hash.ZERO); assertThat(res.getStatusAsString()).isEqualTo(INVALID.name()); - assertThat(res.getError()).isNull(); + assertThat(res.getError()).isEqualTo("Block already present in bad block manager."); verify(engineCallListener, times(1)).executionEngineCalled(); } diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_address.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_address.json new file mode 100644 index 00000000000..79a162dd1bb --- /dev/null +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_address.json @@ -0,0 +1,14 @@ +{ + "variables": "{ \"address\": \"0x6295ee1b4f6dd65047762f924ecd367c17eabf8f\"}", + "request": "query getAddressBalance($address :Address!) { pending { account(address:$address) { balance} } }", + "response": { + "data": { + "pending": { + "account": { + "balance": "0x140" + } + } + } + }, + "statusCode": 200 +} \ No newline at end of file diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_bytes.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_bytes.json new file mode 100644 index 00000000000..c65dc75e41c --- /dev/null +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_bytes.json @@ -0,0 +1,10 @@ +{ + "variables" : "{ \"data\": \"0xf86d0485174876e800830222e0945aae326516b4f8fe08074b7e972e40a713048d62880de0b6b3a7640000801ba05d4e7998757264daab67df2ce6f7e7a0ae36910778a406ca73898c9899a32b9ea0674700d5c3d1d27f2e6b4469957dfd1a1c49bf92383d80717afc84eb05695d5b\"}", + "request" : "mutation postTransaction($data: Bytes!) { sendRawTransaction(data: $data) }", + "response":{ + "data" : { + "sendRawTransaction" : "0xbaabcc1bd699e7378451e4ce5969edb9bdcae76cb79bdacae793525c31e423c7" + } + }, + "statusCode": 200 +} \ No newline at end of file diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_bytes32.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_bytes32.json new file mode 100644 index 00000000000..1512cbdc402 --- /dev/null +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_bytes32.json @@ -0,0 +1,12 @@ +{ + "variables": "{ \"hash\": \"0xc8df1f061abb4d0c107b2b1a794ade8780b3120e681f723fe55a7be586d95ba6\"}", + "request": "query getBlock($hash :Bytes32!) { block(hash:$hash) { number } }", + "response": { + "data": { + "block": { + "number": 30 + } + } + }, + "statusCode": 200 +} \ No newline at end of file diff --git a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_long.json b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_long.json new file mode 100644 index 00000000000..4f1b8c5e264 --- /dev/null +++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/graphql/graphql_variable_long.json @@ -0,0 +1,17 @@ +{ + "variables": "{ \"block1\": \"0x1d\", \"block2\": \"0x1e\"}", + "request": "query getBlockRange($block1 :Long, $block2: Long) { blocks(from: $block1, to: $block2) { hash } }", + "response": { + "data": { + "blocks": [ + { + "hash": "0xf8cfa377bd766cdf22edb388dd08cc149e85d24f2796678c835f3c54ab930803" + }, + { + "hash": "0xc8df1f061abb4d0c107b2b1a794ade8780b3120e681f723fe55a7be586d95ba6" + } + ] + } + }, + "statusCode": 200 +} \ No newline at end of file diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java index 3add3c53472..7cce534826e 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java @@ -149,10 +149,10 @@ private Result handleAndReportFailure(final Block invalidBlock, final String rea private Result handleAndReportFailure( final Block invalidBlock, final String reason, final BlockProcessor.Result result) { if (result.causedBy().isPresent()) { - LOG.error( - "{}. Block {}, caused by {}", reason, invalidBlock.toLogString(), result.causedBy()); - // TODO: if it's an internal error, don't add it - badBlockManager.addBadBlock(invalidBlock); + LOG.info("{}. Block {}, caused by {}", reason, invalidBlock.toLogString(), result.causedBy()); + if (!result.internalError()) { + badBlockManager.addBadBlock(invalidBlock); + } return new Result(reason, result.causedBy().get()); } else { LOG.error("{}. Block {}", reason, invalidBlock.toLogString()); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockProcessor.java index 22963082c61..008f71c473b 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/BlockProcessor.java @@ -22,6 +22,7 @@ import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.TransactionReceipt; import org.hyperledger.besu.ethereum.privacy.storage.PrivateMetadataUpdater; +import org.hyperledger.besu.plugin.services.exception.StorageException; import java.util.List; import java.util.Optional; @@ -65,6 +66,17 @@ default boolean isFailed() { default Optional causedBy() { return Optional.empty(); } + + default boolean internalError() { + if (causedBy().isPresent()) { + Throwable t = causedBy().get(); + // As new "internal only" types of exception are discovered, add them here. + if (t instanceof StorageException) { + return true; + } + } + return false; + } } /** diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java index 6f1233711c4..bc6ed4983bd 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java @@ -36,6 +36,7 @@ import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode; import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions; import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive; +import org.hyperledger.besu.plugin.services.exception.StorageException; import java.util.Collections; import java.util.List; @@ -257,6 +258,59 @@ public boolean isSuccessful() { assertThat(badBlockManager.getBadBlocks()).isEmpty(); } + @Test + public void shouldNotCacheWhenInternalError() { + when(blockchain.getBlockHeader(any(Hash.class))) + .thenReturn(Optional.of(new BlockHeaderTestFixture().buildHeader())); + when(blockHeaderValidator.validateHeader( + any(BlockHeader.class), + any(BlockHeader.class), + eq(protocolContext), + eq(HeaderValidationMode.DETACHED_ONLY))) + .thenReturn(true); + when(worldStateArchive.getMutable(any(Hash.class), any(Hash.class))) + .thenReturn(Optional.of(mock(MutableWorldState.class))); + when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(badBlock))) + .thenReturn( + new BlockProcessor.Result() { + @SuppressWarnings("unchecked") + @Override + public List getReceipts() { + return Collections.EMPTY_LIST; + } + + @SuppressWarnings("unchecked") + @Override + public List getPrivateReceipts() { + return Collections.EMPTY_LIST; + } + + @Override + public boolean isSuccessful() { + return false; + } + + @Override + public Optional causedBy() { + return Optional.of(new StorageException("database bedlam")); + } + }); + when(blockBodyValidator.validateBody( + eq(protocolContext), + eq(badBlock), + any(), + any(), + eq(HeaderValidationMode.DETACHED_ONLY))) + .thenReturn(true); + assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0); + mainnetBlockValidator.validateAndProcessBlock( + protocolContext, + badBlock, + HeaderValidationMode.DETACHED_ONLY, + HeaderValidationMode.DETACHED_ONLY); + assertThat(badBlockManager.getBadBlocks()).isEmpty(); + } + @Test public void shouldReturnBadBlockBasedOnTheHash() { when(blockchain.getBlockHeader(any(Hash.class))) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthScheduler.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthScheduler.java index df882c65d03..91818b9f5e2 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthScheduler.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthScheduler.java @@ -72,11 +72,12 @@ public EthScheduler( final MetricsSystem metricsSystem) { this( MonitoredExecutors.newFixedThreadPool( - EthScheduler.class.getSimpleName() + "-Workers", syncWorkerCount, metricsSystem), + EthScheduler.class.getSimpleName() + "-Workers", 1, syncWorkerCount, metricsSystem), MonitoredExecutors.newScheduledThreadPool( EthScheduler.class.getSimpleName() + "-Timer", 1, metricsSystem), MonitoredExecutors.newBoundedThreadPool( EthScheduler.class.getSimpleName() + "-Transactions", + 1, txWorkerCount, txWorkerQueueSize, metricsSystem), diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/MonitoredExecutors.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/MonitoredExecutors.java index ce6062de892..e56903e65e5 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/MonitoredExecutors.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/MonitoredExecutors.java @@ -38,8 +38,12 @@ public class MonitoredExecutors { public static ExecutorService newFixedThreadPool( - final String name, final int workerCount, final MetricsSystem metricsSystem) { - return newFixedThreadPool(name, 0, workerCount, new LinkedBlockingQueue<>(), metricsSystem); + final String name, + final int minWorkerCount, + final int workerCount, + final MetricsSystem metricsSystem) { + return newFixedThreadPool( + name, minWorkerCount, workerCount, new LinkedBlockingQueue<>(), metricsSystem); } public static ExecutorService newBoundedThreadPool( @@ -47,7 +51,7 @@ public static ExecutorService newBoundedThreadPool( final int workerCount, final int queueSize, final MetricsSystem metricsSystem) { - return newBoundedThreadPool(name, 0, workerCount, queueSize, metricsSystem); + return newBoundedThreadPool(name, 1, workerCount, queueSize, metricsSystem); } public static ExecutorService newBoundedThreadPool( @@ -77,8 +81,8 @@ private static ExecutorService newFixedThreadPool( new ThreadPoolExecutor( minWorkerCount, maxWorkerCount, - 0L, - TimeUnit.MILLISECONDS, + 60L, + TimeUnit.SECONDS, workingQueue, threadFactory, rejectedExecutionHandler)); diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/handshake/ecies/ECIESHandshaker.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/handshake/ecies/ECIESHandshaker.java index 5282ed62b07..d5620e9fe3c 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/handshake/ecies/ECIESHandshaker.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/handshake/ecies/ECIESHandshaker.java @@ -189,9 +189,6 @@ public Optional handleMessage(final ByteBuf buf) throws HandshakeExcept try { // Decrypt the message with our private key. try { - bytes = EncryptedMessage.decryptMsg(bytes, nodeKey); - version4 = false; - } catch (final Exception ex) { // Assume new format final int size = bufferedBytes.readUnsignedShort(); if (buf.writerIndex() >= size) { @@ -203,8 +200,11 @@ public Optional handleMessage(final ByteBuf buf) throws HandshakeExcept bytes = EncryptedMessage.decryptMsgEIP8(encryptedMsg, nodeKey); version4 = true; } else { - throw new HandshakeException("Failed to decrypt handshake message", ex); + throw new HandshakeException("Failed to decrypt handshake message"); } + } catch (final Exception ex) { + bytes = EncryptedMessage.decryptMsg(bytes, nodeKey); + version4 = false; } } catch (final InvalidCipherTextException e) { status.set(Handshaker.HandshakeStatus.FAILED); diff --git a/gradle.properties b/gradle.properties index fbfc091dee0..3884bb6cc81 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=22.7.7-SNAPSHOT +version=22.7.7 org.gradle.welcome=never org.gradle.jvmargs=-Xmx1g diff --git a/gradle/versions.gradle b/gradle/versions.gradle index bc4807f3532..9cf356a8860 100644 --- a/gradle/versions.gradle +++ b/gradle/versions.gradle @@ -117,7 +117,7 @@ dependencyManagement { dependency 'org.apache.commons:commons-compress:1.21' dependency 'org.apache.commons:commons-lang3:3.12.0' - dependency 'org.apache.commons:commons-text:1.9' + dependency 'org.apache.commons:commons-text:1.10.0' dependency 'org.apache.logging.log4j:log4j-api:2.17.2' dependency 'org.apache.logging.log4j:log4j-core:2.17.2'