diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2f0661e3cfb..7c9b70891d1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,8 +14,9 @@ jobs: steps: - name: Pre-process Release Name id: pre_process_release_name + env: + RELEASE_NAME: "${{ github.event.release.name }}" run: | - RELEASE_NAME="${{ github.event.release.name }}" # strip all whitespace RELEASE_NAME="${RELEASE_NAME//[[:space:]]/}" if [[ ! "$RELEASE_NAME" =~ ^[0-9]+\.[0-9]+(\.[0-9]+)?(-.*)?$ ]]; then diff --git a/CHANGELOG.md b/CHANGELOG.md index e0e64087c04..dce16b3f9d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,6 @@ # Changelog ## [Unreleased] -- Add configuration of Consolidation Request Contract Address via genesis configuration [#7647](https://github.com/hyperledger/besu/pull/7647) - ### Upcoming Breaking Changes - k8s (KUBERNETES) Nat method is now deprecated and will be removed in a future release @@ -14,7 +12,9 @@ - Remove privacy test classes support [#7569](https://github.com/hyperledger/besu/pull/7569) - Add Blob Transaction Metrics [#7622](https://github.com/hyperledger/besu/pull/7622) - Implemented support for emptyBlockPeriodSeconds in QBFT [#6965](https://github.com/hyperledger/besu/pull/6965) - +- LUKSO Cancun Hardfork [#7686](https://github.com/hyperledger/besu/pull/7686) +- Add configuration of Consolidation Request Contract Address via genesis configuration [#7647](https://github.com/hyperledger/besu/pull/7647) +- Interrupt pending transaction processing on block creation timeout [#7673](https://github.com/hyperledger/besu/pull/7673) ### Bug fixes - Fix mounted data path directory permissions for besu user [#7575](https://github.com/hyperledger/besu/pull/7575) diff --git a/MAINTAINERS.md b/MAINTAINERS.md index f699321efbd..17958bdbd43 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -82,7 +82,7 @@ The following steps must occur for a contributor to be "upgraded" as a maintaine - The proposed maintainer accepts the nomination and expresses a willingness to be a long-term (more than 6 month) committer by adding a comment in the proposal PR. - The PR will be communicated in all appropriate communication channels - including at least [besu-contributors channel on Hyperledger Discord](https://discord.gg/hyperledger), + including at least [besu-contributors channel on Discord](https://discord.gg/hyperledger), the [mailing list](https://lists.hyperledger.org/g/besu) and any maintainer/community call. - Approval by at least 3 current maintainers within two weeks of the proposal or diff --git a/README.md b/README.md index cd4930fc76b..00aad005ac7 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Besu is an Apache 2.0 licensed, MainNet compatible, Ethereum client written in J * [Besu User Documentation] * [Besu Issues] -* [Besu Wiki](https://wiki.hyperledger.org/display/BESU/Hyperledger+Besu) +* [Besu Wiki](https://wiki.hyperledger.org/display/BESU/Besu) * [How to Contribute to Besu](https://wiki.hyperledger.org/display/BESU/How+to+Contribute) * [Besu Roadmap & Planning](https://wiki.hyperledger.org/pages/viewpage.action?pageId=24781786) diff --git a/SUPPORT.md b/SUPPORT.md index a9eb54acb86..9e4a7b7b42b 100644 --- a/SUPPORT.md +++ b/SUPPORT.md @@ -1,4 +1,4 @@ -# Hyperledger Besu Support +# Besu Support Welcome to the Besu repository! The following links are a set of guidelines for contributing to this repo and its packages. These are mostly guidelines, not rules. Use your best judgement, and feel free to propose changes to this document in a pull request. Contributions come in the form of code submissions, writing documentation, raising issues, helping others in chat, and any other actions that help develop Besu. @@ -6,13 +6,13 @@ Welcome to the Besu repository! The following links are a set of guidelines for Having Github, Discord, and Linux Foundation accounts is necessary for obtaining support for Besu through the community channels, wiki and issue management. * If you want to raise an issue, you can do so [on the github issue tab](https://github.com/hyperledger/besu/issues). -* Hyperledger Discord requires a [Discord account]. -* The Hyperledger wiki also requires a [Linux Foundation (LF) account] in order to edit pages. +* Discord requires a [Discord account]. +* The Besu wiki also requires a [Linux Foundation (LF) account] in order to edit pages. ### Useful support links * [Besu User Documentation] -* [Besu channel on Hyperledger Discord] +* [Besu channel on Discord] * [I just have a quick question](https://wiki.hyperledger.org/display/BESU/I+just+have+a+quick+question) * [Did you find a bug?](https://wiki.hyperledger.org/display/BESU/Reporting+Bugs) * [Issues](https://wiki.hyperledger.org/display/BESU/Issues) @@ -20,5 +20,5 @@ Having Github, Discord, and Linux Foundation accounts is necessary for obtaining [Besu User Documentation]: https://besu.hyperledger.org -[Besu channel on Hyperledger Discord]: https://discord.gg/hyperledger +[Besu channel on Discord]: https://discord.gg/hyperledger [Contributing Guidelines]: CONTRIBUTING.md diff --git a/besu/src/test/java/org/hyperledger/besu/cli/subcommands/operator/OperatorSubCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/subcommands/operator/OperatorSubCommandTest.java index 7e9a36719d7..ca3e6d95df3 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/subcommands/operator/OperatorSubCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/subcommands/operator/OperatorSubCommandTest.java @@ -282,8 +282,10 @@ public void shouldCreateIbft2ExtraData() throws IOException { false, singletonList("key.pub"), Optional.empty(), - Optional.of( - "0xf853a00000000000000000000000000000000000000000000000000000000000000000ea94d5feb0fc5a54a89f97aeb34c3df15397c19f6dd294d6a9a4c886eb008ac307abdc1f38745c1dd13a88808400000000c0")); + List.of( + new Field( + "extraData", + "0xf853a00000000000000000000000000000000000000000000000000000000000000000ea94d5feb0fc5a54a89f97aeb34c3df15397c19f6dd294d6a9a4c886eb008ac307abdc1f38745c1dd13a88808400000000c0"))); } @Test @@ -296,8 +298,49 @@ public void shouldCreateQbftExtraData() throws IOException { false, singletonList("key.pub"), Optional.empty(), - Optional.of( - "0xf84fa00000000000000000000000000000000000000000000000000000000000000000ea94d5feb0fc5a54a89f97aeb34c3df15397c19f6dd294d6a9a4c886eb008ac307abdc1f38745c1dd13a88c080c0")); + List.of( + new Field( + "extraData", + "0xf84fa00000000000000000000000000000000000000000000000000000000000000000ea94d5feb0fc5a54a89f97aeb34c3df15397c19f6dd294d6a9a4c886eb008ac307abdc1f38745c1dd13a88c080c0"))); + } + + @Test + public void generatedGenesisFileShouldContainAllOriginalFieldsExcludingExtraData() + throws IOException { + final JsonObject alloc = + new JsonObject( + """ + { + "24defc2d149861d3d245749b81fe0e6b28e04f31": { + "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" + }, + "2a813d7db3de19b07f92268b6d4125ed295cbe00": { + "balance": "0x446c3b15f9926687d2c40534fdb542000000000000" + } + }"""); + final List fields = + List.of( + new Field("nonce", "0x0"), + new Field("timestamp", "0x5b3c3d18"), + new Field("gasUsed", "0x0"), + new Field( + "parentHash", "0x0000000000000000000000000000000000000000000000000000000000000000"), + new Field("gasLimit", "0x47b760"), + new Field("difficulty", "0x1"), + new Field( + "mixHash", "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365"), + new Field("coinbase", "0x0000000000000000000000000000000000000000"), + new Field("alloc", alloc.getMap().toString())); + + runCmdAndCheckOutput( + cmd(), + "/operator/config_generate_keys.json", + tmpOutputDirectoryPath, + "genesis.json", + false, + singletonList("key.pub"), + Optional.empty(), + fields); } private void runCmdAndCheckOutput( @@ -316,7 +359,7 @@ private void runCmdAndCheckOutput( generate, expectedKeyFiles, Optional.empty(), - Optional.empty()); + List.of()); } private void runCmdAndCheckOutput( @@ -336,9 +379,11 @@ private void runCmdAndCheckOutput( generate, expectedKeyFiles, signatureAlgorithm, - Optional.empty()); + List.of()); } + private record Field(String key, String value) {} + private void runCmdAndCheckOutput( final Cmd cmd, final String configFile, @@ -347,7 +392,7 @@ private void runCmdAndCheckOutput( final boolean generate, final Collection expectedKeyFiles, final Optional signatureAlgorithm, - final Optional expectedExtraData) + final List expectedFields) throws IOException { final URL configFilePath = this.getClass().getResource(configFile); parseCommand( @@ -368,8 +413,9 @@ private void runCmdAndCheckOutput( final String genesisString = contentOf(outputGenesisFile, UTF_8); final JsonObject genesisContent = new JsonObject(genesisString); assertThat(genesisContent.containsKey("extraData")).isTrue(); - expectedExtraData.ifPresent( - extraData -> assertThat(genesisContent.getString("extraData")).isEqualTo(extraData)); + + expectedFields.forEach( + field -> assertThat(genesisContent.getString(field.key)).isEqualTo(field.value)); final Path expectedKeysPath = outputDirectoryPath.resolve("keys"); final File keysDirectory = new File(expectedKeysPath.toUri()); diff --git a/besu/src/test/resources/operator/config_generate_keys.json b/besu/src/test/resources/operator/config_generate_keys.json index 3723fdc3bce..212bb943eac 100644 --- a/besu/src/test/resources/operator/config_generate_keys.json +++ b/besu/src/test/resources/operator/config_generate_keys.json @@ -4,7 +4,9 @@ "chainId": 2017, "eip150Block": 0, "ibft2": { - + "blockperiodseconds": 2, + "epochlength": 30000, + "requesttimeoutseconds": 10 } }, "nonce": "0x0", @@ -16,11 +18,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/besu/src/test/resources/operator/config_generate_keys_ec_invalid.json b/besu/src/test/resources/operator/config_generate_keys_ec_invalid.json index 1483c45f381..3699740aba2 100644 --- a/besu/src/test/resources/operator/config_generate_keys_ec_invalid.json +++ b/besu/src/test/resources/operator/config_generate_keys_ec_invalid.json @@ -5,7 +5,9 @@ "eip150Block": 0, "ecCurve": "abcd", "ibft2": { - + "blockperiodseconds": 2, + "epochlength": 30000, + "requesttimeoutseconds": 10 } }, "nonce": "0x0", @@ -17,11 +19,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/besu/src/test/resources/operator/config_generate_keys_secp256r1.json b/besu/src/test/resources/operator/config_generate_keys_secp256r1.json index f66f588ffe8..bbfe9f0c6ab 100644 --- a/besu/src/test/resources/operator/config_generate_keys_secp256r1.json +++ b/besu/src/test/resources/operator/config_generate_keys_secp256r1.json @@ -5,7 +5,9 @@ "eip150Block": 0, "ecCurve": "secp256r1", "ibft2": { - + "blockperiodseconds": 2, + "epochlength": 30000, + "requesttimeoutseconds": 10 } }, "nonce": "0x0", @@ -17,11 +19,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/besu/src/test/resources/operator/config_import_keys.json b/besu/src/test/resources/operator/config_import_keys.json index db4e57b5455..78e4cd3343e 100644 --- a/besu/src/test/resources/operator/config_import_keys.json +++ b/besu/src/test/resources/operator/config_import_keys.json @@ -4,7 +4,9 @@ "chainId": 2017, "petersburgBlock": 0, "ibft2": { - + "blockperiodseconds": 2, + "epochlength": 30000, + "requesttimeoutseconds": 10 } }, "nonce": "0x0", @@ -16,11 +18,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/besu/src/test/resources/operator/config_import_keys_invalid_keys.json b/besu/src/test/resources/operator/config_import_keys_invalid_keys.json index 3fc4561e3a1..b9c20be753f 100644 --- a/besu/src/test/resources/operator/config_import_keys_invalid_keys.json +++ b/besu/src/test/resources/operator/config_import_keys_invalid_keys.json @@ -4,7 +4,9 @@ "chainId": 2017, "petersburgBlock": 0, "ibft2": { - + "blockperiodseconds": 2, + "epochlength": 30000, + "requesttimeoutseconds": 10 } }, "nonce": "0x0", @@ -16,11 +18,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/besu/src/test/resources/operator/config_import_keys_qbft.json b/besu/src/test/resources/operator/config_import_keys_qbft.json index fe757326b40..449ba343ee5 100644 --- a/besu/src/test/resources/operator/config_import_keys_qbft.json +++ b/besu/src/test/resources/operator/config_import_keys_qbft.json @@ -4,7 +4,9 @@ "chainId": 2017, "petersburgBlock": 0, "qbft": { - + "blockperiodseconds": 2, + "epochlength": 30000, + "requesttimeoutseconds": 10 } }, "nonce": "0x0", @@ -16,11 +18,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/besu/src/test/resources/operator/config_import_keys_secp256r1.json b/besu/src/test/resources/operator/config_import_keys_secp256r1.json index bd189d0235c..b56654b1363 100644 --- a/besu/src/test/resources/operator/config_import_keys_secp256r1.json +++ b/besu/src/test/resources/operator/config_import_keys_secp256r1.json @@ -5,7 +5,9 @@ "petersburgBlock": 0, "ecCurve": "secp256r1", "ibft2": { - + "blockperiodseconds": 2, + "epochlength": 30000, + "requesttimeoutseconds": 10 } }, "nonce": "0x0", @@ -17,11 +19,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/besu/src/test/resources/operator/config_import_keys_secp256r1_invalid_keys.json b/besu/src/test/resources/operator/config_import_keys_secp256r1_invalid_keys.json index 4f59b9ebd7f..0870f9e872b 100644 --- a/besu/src/test/resources/operator/config_import_keys_secp256r1_invalid_keys.json +++ b/besu/src/test/resources/operator/config_import_keys_secp256r1_invalid_keys.json @@ -5,7 +5,9 @@ "petersburgBlock": 0, "ecCurve": "secp256r1", "ibft2": { - + "blockperiodseconds": 2, + "epochlength": 30000, + "requesttimeoutseconds": 10 } }, "nonce": "0x0", @@ -17,11 +19,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/besu/src/test/resources/operator/config_no_config_section.json b/besu/src/test/resources/operator/config_no_config_section.json index 2841aea9c47..5dfa4547974 100644 --- a/besu/src/test/resources/operator/config_no_config_section.json +++ b/besu/src/test/resources/operator/config_no_config_section.json @@ -9,11 +9,6 @@ "difficulty": "0x1", "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", "coinbase": "0x0000000000000000000000000000000000000000", - "ibft2": { - "blockperiodseconds": 2, - "epochlength": 30000, - "requesttimeoutseconds": 10 - }, "alloc": { "24defc2d149861d3d245749b81fe0e6b28e04f31": { "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" diff --git a/config/src/main/java/org/hyperledger/besu/config/GenesisReader.java b/config/src/main/java/org/hyperledger/besu/config/GenesisReader.java index 316aa73d043..6b42c3c2edf 100644 --- a/config/src/main/java/org/hyperledger/besu/config/GenesisReader.java +++ b/config/src/main/java/org/hyperledger/besu/config/GenesisReader.java @@ -53,12 +53,12 @@ class FromObjectNode implements GenesisReader { private final ObjectNode rootWithoutAllocations; public FromObjectNode(final ObjectNode root) { - final var removedAllocations = root.remove(ALLOCATION_FIELD); this.allocations = - removedAllocations != null - ? (ObjectNode) removedAllocations + root.get(ALLOCATION_FIELD) != null + ? (ObjectNode) root.get(ALLOCATION_FIELD) : JsonUtil.createEmptyObjectNode(); - this.rootWithoutAllocations = normalizeKeys(root); + this.rootWithoutAllocations = + normalizeKeys(root, field -> !field.getKey().equals(ALLOCATION_FIELD)); } @Override diff --git a/config/src/main/java/org/hyperledger/besu/config/JsonUtil.java b/config/src/main/java/org/hyperledger/besu/config/JsonUtil.java index bcb89c64edb..430c74efb94 100644 --- a/config/src/main/java/org/hyperledger/besu/config/JsonUtil.java +++ b/config/src/main/java/org/hyperledger/besu/config/JsonUtil.java @@ -25,6 +25,7 @@ import java.util.OptionalLong; import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonParser; @@ -37,6 +38,7 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.JsonNodeType; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.base.Predicates; import org.apache.tuweni.bytes.Bytes; /** The Json util class. */ @@ -59,11 +61,29 @@ private JsonUtil() {} * @return a copy of the json object with all keys in lower case. */ public static ObjectNode normalizeKeys(final ObjectNode objectNode) { + return normalizeKeys(objectNode, Predicates.alwaysTrue()); + } + + /** + * Converts all the object keys (but none of the string values) to lowercase for easier lookup. + * This is useful in cases such as the 'genesis.json' file where all keys are assumed to be case + * insensitive. + * + * @param objectNode The ObjectNode to be normalized + * @param fieldPredicate The predicate to filter the fields to normalize + * @return a copy of the json object with all keys in lower case. + */ + public static ObjectNode normalizeKeys( + final ObjectNode objectNode, final Predicate> fieldPredicate) { final ObjectNode normalized = JsonUtil.createEmptyObjectNode(); objectNode .fields() .forEachRemaining( entry -> { + if (!fieldPredicate.test(entry)) { + return; + } + final String key = entry.getKey(); final JsonNode value = entry.getValue(); final String normalizedKey = normalizeKey(key); diff --git a/config/src/main/resources/lukso.json b/config/src/main/resources/lukso.json index ff4296eae84..68bf32d35ce 100644 --- a/config/src/main/resources/lukso.json +++ b/config/src/main/resources/lukso.json @@ -17,7 +17,7 @@ "terminalTotalDifficulty": 0, "terminalTotalDifficultyPassed": true, "shanghaiTime": 1687969198, - "cancunTime": 1767182400, + "cancunTime": 1732119595, "discovery": { "bootnodes": [ "enode://c2bb19ce658cfdf1fecb45da599ee6c7bf36e5292efb3fb61303a0b2cd07f96c20ac9b376a464d687ac456675a2e4a44aec39a0509bcb4b6d8221eedec25aca2@34.147.73.193:30303", diff --git a/config/src/test/java/org/hyperledger/besu/config/GenesisReaderTest.java b/config/src/test/java/org/hyperledger/besu/config/GenesisReaderTest.java index 5d73a2829f5..f8327174a33 100644 --- a/config/src/test/java/org/hyperledger/besu/config/GenesisReaderTest.java +++ b/config/src/test/java/org/hyperledger/besu/config/GenesisReaderTest.java @@ -53,6 +53,22 @@ public void readGenesisFromObjectNode() { .containsExactly(new GenesisAccount(Address.BLS12_G2MUL, 0, Wei.ONE, null, Map.of(), null)); } + @Test + public void readGenesisFromObjectDoesNotModifyObjectNodeArg() { + final var configNode = mapper.createObjectNode(); + configNode.put("londonBlock", 1); + final var allocNode = mapper.createObjectNode(); + allocNode.put(Address.BLS12_G2MUL.toUnprefixedHexString(), generateAllocation(Wei.ONE)); + final var rootNode = mapper.createObjectNode(); + rootNode.put("chainId", 12); + rootNode.put(CONFIG_FIELD, configNode); + rootNode.put(ALLOCATION_FIELD, allocNode); + var rootNodeCopy = rootNode.deepCopy(); + new GenesisReader.FromObjectNode(rootNode); + + assertThat(rootNode).isEqualTo(rootNodeCopy); + } + @Test public void readGenesisFromURL(@TempDir final Path folder) throws IOException { final String jsonStr = diff --git a/config/src/test/java/org/hyperledger/besu/config/JsonUtilTest.java b/config/src/test/java/org/hyperledger/besu/config/JsonUtilTest.java index 1e1df382e71..fa6b4ec1467 100644 --- a/config/src/test/java/org/hyperledger/besu/config/JsonUtilTest.java +++ b/config/src/test/java/org/hyperledger/besu/config/JsonUtilTest.java @@ -147,6 +147,36 @@ public void normalizeKeys_arrayNode_withString() { assertThat(normalizedObj).isEqualTo(expectedObj); } + @Test + public void normalizeKeys_predicate() { + final ObjectNode originalObj = + mapper + .createObjectNode() + .put("Ant", "Tiny") + .put("Ape", "Smart") + .put("Armadillo", "Armored") + .put("Cat", "Meow") + .put("Bat", "Flying") + .put("Cow", "Moo") + .put("Crocodile", "Snap") + .put("Bear", "Strong") + .put("Cheetah", "Fast") + .put("Beaver", "Builder"); + + final ObjectNode expectedObj = + mapper + .createObjectNode() + .put("cat", "Meow") + .put("cow", "Moo") + .put("cheetah", "Fast") + .put("crocodile", "Snap"); + + final ObjectNode normalizedObj = + JsonUtil.normalizeKeys(originalObj, s -> s.getKey().startsWith("C")); + + assertThat(normalizedObj).isEqualTo(expectedObj); + } + @Test public void getLong_nonExistentKey() { final ObjectNode node = mapper.createObjectNode(); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetClientVersionV1.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetClientVersionV1.java index 689cb44e6a3..149728186bc 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetClientVersionV1.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetClientVersionV1.java @@ -49,9 +49,11 @@ public String getName() { @Override public JsonRpcResponse syncResponse(final JsonRpcRequestContext request) { + String safeCommit = + (commit != null && commit.length() >= 8) ? commit.substring(0, 8) : "unknown"; return new JsonRpcSuccessResponse( request.getRequest().getId(), new EngineGetClientVersionResultV1( - ENGINE_CLIENT_CODE, ENGINE_CLIENT_NAME, clientVersion, commit.substring(0, 8))); + ENGINE_CLIENT_CODE, ENGINE_CLIENT_NAME, clientVersion, safeCommit)); } } diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java index e07b43b9904..5370d2ec46c 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.ethereum.blockcreation.txselection; import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.BLOCK_SELECTION_TIMEOUT; +import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.BLOCK_SELECTION_TIMEOUT_INVALID_TX; import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.INVALID_TX_EVALUATION_TOO_LONG; import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.SELECTED; import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.TX_EVALUATION_TOO_LONG; @@ -52,9 +53,11 @@ import org.hyperledger.besu.plugin.services.tracer.BlockAwareOperationTracer; import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelector; +import java.time.Duration; import java.util.List; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; +import java.util.concurrent.FutureTask; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; @@ -97,11 +100,12 @@ public class BlockTransactionSelector { new TransactionSelectionResults(); private final List transactionSelectors; private final PluginTransactionSelector pluginTransactionSelector; - private final BlockAwareOperationTracer pluginOperationTracer; + private final BlockAwareOperationTracer operationTracer; private final EthScheduler ethScheduler; private final AtomicBoolean isTimeout = new AtomicBoolean(false); private final long blockTxsSelectionMaxTime; private WorldUpdater blockWorldStateUpdater; + private volatile TransactionEvaluationContext currTxEvaluationContext; public BlockTransactionSelector( final MiningParameters miningParameters, @@ -139,7 +143,8 @@ public BlockTransactionSelector( transactionPool); transactionSelectors = createTransactionSelectors(blockSelectionContext); this.pluginTransactionSelector = pluginTransactionSelector; - this.pluginOperationTracer = pluginTransactionSelector.getOperationTracer(); + this.operationTracer = + new InterruptibleOperationTracer(pluginTransactionSelector.getOperationTracer()); blockWorldStateUpdater = worldState.updater(); blockTxsSelectionMaxTime = miningParameters.getBlockTxsSelectionMaxTime(); } @@ -178,15 +183,17 @@ public TransactionSelectionResults buildTransactionListForBlock() { } private void timeLimitedSelection() { - final var txSelection = - ethScheduler.scheduleBlockCreationTask( + final var txSelectionTask = + new FutureTask( () -> blockSelectionContext .transactionPool() - .selectTransactions(this::evaluateTransaction)); + .selectTransactions(this::evaluateTransaction), + null); + ethScheduler.scheduleBlockCreationTask(txSelectionTask); try { - txSelection.get(blockTxsSelectionMaxTime, TimeUnit.MILLISECONDS); + txSelectionTask.get(blockTxsSelectionMaxTime, TimeUnit.MILLISECONDS); } catch (InterruptedException | ExecutionException e) { if (isCancelled.get()) { throw new CancellationException("Cancelled during transaction selection"); @@ -197,6 +204,9 @@ private void timeLimitedSelection() { synchronized (isTimeout) { isTimeout.set(true); } + + cancelEvaluatingTxWithGraceTime(txSelectionTask); + LOG.warn( "Interrupting the selection of transactions for block inclusion as it exceeds the maximum configured duration of " + blockTxsSelectionMaxTime @@ -205,6 +215,40 @@ private void timeLimitedSelection() { } } + private void cancelEvaluatingTxWithGraceTime(final FutureTask txSelectionTask) { + final long elapsedTime = + currTxEvaluationContext.getEvaluationTimer().elapsed(TimeUnit.MILLISECONDS); + // adding 100ms so we are sure it take strictly more than the block selection max time + final long txRemainingTime = (blockTxsSelectionMaxTime - elapsedTime) + 100; + + LOG.atDebug() + .setMessage( + "Transaction {} is processing for {}ms, giving it {}ms grace time, before considering it taking too much time to execute") + .addArgument(currTxEvaluationContext.getPendingTransaction()::toTraceLog) + .addArgument(elapsedTime) + .addArgument(txRemainingTime) + .log(); + + ethScheduler.scheduleFutureTask( + () -> { + if (!txSelectionTask.isDone()) { + LOG.atDebug() + .setMessage( + "Transaction {} is still processing after the grace time, total processing time {}ms," + + " greater than max block selection time of {}ms, forcing an interrupt") + .addArgument(currTxEvaluationContext.getPendingTransaction()::toTraceLog) + .addArgument( + () -> + currTxEvaluationContext.getEvaluationTimer().elapsed(TimeUnit.MILLISECONDS)) + .addArgument(blockTxsSelectionMaxTime) + .log(); + + txSelectionTask.cancel(true); + } + }, + Duration.ofMillis(txRemainingTime)); + } + /** * Evaluates a list of transactions and updates the selection results accordingly. If a * transaction is not selected during the evaluation, it is updated as not selected in the @@ -236,6 +280,7 @@ private TransactionSelectionResult evaluateTransaction( final TransactionEvaluationContext evaluationContext = createTransactionEvaluationContext(pendingTransaction); + currTxEvaluationContext = evaluationContext; TransactionSelectionResult selectionResult = evaluatePreProcessing(evaluationContext); if (!selectionResult.selected()) { @@ -337,7 +382,7 @@ private TransactionProcessingResult processTransaction( blockSelectionContext.pendingBlockHeader(), pendingTransaction.getTransaction(), blockSelectionContext.miningBeneficiary(), - pluginOperationTracer, + operationTracer, blockHashLookup, false, TransactionValidationParams.mining(), @@ -422,14 +467,10 @@ private TransactionSelectionResult handleTransactionNotSelected( final var pendingTransaction = evaluationContext.getPendingTransaction(); // check if this tx took too much to evaluate, and in case it was invalid remove it from the - // pool, otherwise penalize it. + // pool, otherwise penalize it. Not synchronized since there is no state change here. final TransactionSelectionResult actualResult = isTimeout.get() - ? transactionTookTooLong(evaluationContext, selectionResult) - ? selectionResult.discard() - ? INVALID_TX_EVALUATION_TOO_LONG - : TX_EVALUATION_TOO_LONG - : BLOCK_SELECTION_TIMEOUT + ? rewriteSelectionResultForTimeout(evaluationContext, selectionResult) : selectionResult; transactionSelectionResults.updateNotSelected(evaluationContext.getTransaction(), actualResult); @@ -446,6 +487,34 @@ private TransactionSelectionResult handleTransactionNotSelected( return actualResult; } + /** + * In case of a block creation timeout, we rewrite the selection result, so we can easily spot + * what happened looking at the transaction selection results. + * + * @param evaluationContext The current selection session data. + * @param selectionResult The result of the transaction selection process. + * @return the rewritten selection result + */ + private TransactionSelectionResult rewriteSelectionResultForTimeout( + final TransactionEvaluationContext evaluationContext, + final TransactionSelectionResult selectionResult) { + + if (transactionTookTooLong(evaluationContext, selectionResult)) { + return selectionResult.discard() ? INVALID_TX_EVALUATION_TOO_LONG : TX_EVALUATION_TOO_LONG; + } + + return selectionResult.discard() ? BLOCK_SELECTION_TIMEOUT_INVALID_TX : BLOCK_SELECTION_TIMEOUT; + } + + /** + * Check if the evaluation of this tx took more than the block creation max time, because if true + * we want to penalize it. We penalize it, instead of directly removing, because it could happen + * that the tx will evaluate in time next time. Invalid txs are always removed. + * + * @param evaluationContext The current selection session data. + * @param selectionResult The result of the transaction selection process. + * @return true if the evaluation of this tx took more than the block creation max time + */ private boolean transactionTookTooLong( final TransactionEvaluationContext evaluationContext, final TransactionSelectionResult selectionResult) { diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/InterruptibleOperationTracer.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/InterruptibleOperationTracer.java new file mode 100644 index 00000000000..eb7f34bd9a0 --- /dev/null +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/InterruptibleOperationTracer.java @@ -0,0 +1,143 @@ +/* + * Copyright contributors to Hyperledger 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.blockcreation.txselection; + +import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.datatypes.Transaction; +import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; +import org.hyperledger.besu.evm.frame.MessageFrame; +import org.hyperledger.besu.evm.log.Log; +import org.hyperledger.besu.evm.operation.Operation; +import org.hyperledger.besu.evm.worldstate.WorldView; +import org.hyperledger.besu.plugin.data.BlockBody; +import org.hyperledger.besu.plugin.data.BlockHeader; +import org.hyperledger.besu.plugin.data.ProcessableBlockHeader; +import org.hyperledger.besu.plugin.services.tracer.BlockAwareOperationTracer; + +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import org.apache.tuweni.bytes.Bytes; + +public class InterruptibleOperationTracer implements BlockAwareOperationTracer { + private final BlockAwareOperationTracer delegate; + + public InterruptibleOperationTracer(final BlockAwareOperationTracer delegate) { + this.delegate = delegate; + } + + @Override + public void traceStartBlock(final BlockHeader blockHeader, final BlockBody blockBody) { + delegate.traceStartBlock(blockHeader, blockBody); + } + + @Override + public void traceEndBlock(final BlockHeader blockHeader, final BlockBody blockBody) { + delegate.traceEndBlock(blockHeader, blockBody); + } + + @Override + public void traceStartBlock(final ProcessableBlockHeader processableBlockHeader) { + delegate.traceStartBlock(processableBlockHeader); + } + + @Override + public boolean isExtendedTracing() { + return delegate.isExtendedTracing(); + } + + @Override + public void tracePreExecution(final MessageFrame frame) { + checkInterrupt(); + delegate.tracePreExecution(frame); + } + + @Override + public void tracePostExecution( + final MessageFrame frame, final Operation.OperationResult operationResult) { + checkInterrupt(); + delegate.tracePostExecution(frame, operationResult); + } + + @Override + public void tracePrecompileCall( + final MessageFrame frame, final long gasRequirement, final Bytes output) { + checkInterrupt(); + delegate.tracePrecompileCall(frame, gasRequirement, output); + } + + @Override + public void traceAccountCreationResult( + final MessageFrame frame, final Optional haltReason) { + checkInterrupt(); + delegate.traceAccountCreationResult(frame, haltReason); + } + + @Override + public void tracePrepareTransaction(final WorldView worldView, final Transaction transaction) { + delegate.tracePrepareTransaction(worldView, transaction); + } + + @Override + public void traceStartTransaction(final WorldView worldView, final Transaction transaction) { + delegate.traceStartTransaction(worldView, transaction); + } + + @Override + public void traceBeforeRewardTransaction( + final WorldView worldView, final Transaction tx, final Wei miningReward) { + delegate.traceBeforeRewardTransaction(worldView, tx, miningReward); + } + + @Override + public void traceEndTransaction( + final WorldView worldView, + final Transaction tx, + final boolean status, + final Bytes output, + final List logs, + final long gasUsed, + final Set
selfDestructs, + final long timeNs) { + delegate.traceEndTransaction( + worldView, tx, status, output, logs, gasUsed, selfDestructs, timeNs); + } + + @Override + public void traceContextEnter(final MessageFrame frame) { + checkInterrupt(); + delegate.traceContextEnter(frame); + } + + @Override + public void traceContextReEnter(final MessageFrame frame) { + checkInterrupt(); + delegate.traceContextReEnter(frame); + } + + @Override + public void traceContextExit(final MessageFrame frame) { + checkInterrupt(); + delegate.traceContextExit(frame); + } + + private void checkInterrupt() { + if (Thread.interrupted()) { + throw new RuntimeException(new InterruptedException("Transaction execution interrupted")); + } + } +} diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/ProcessingResultTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/ProcessingResultTransactionSelector.java index 9eed2bff09e..5a9fd6030cd 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/ProcessingResultTransactionSelector.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/ProcessingResultTransactionSelector.java @@ -112,6 +112,7 @@ private TransactionSelectionResult transactionSelectionResultForInvalidResult( private boolean isTransientValidationError(final TransactionInvalidReason invalidReason) { return invalidReason.equals(TransactionInvalidReason.UPFRONT_COST_EXCEEDS_BALANCE) || invalidReason.equals(TransactionInvalidReason.GAS_PRICE_BELOW_CURRENT_BASE_FEE) - || invalidReason.equals(TransactionInvalidReason.NONCE_TOO_HIGH); + || invalidReason.equals(TransactionInvalidReason.NONCE_TOO_HIGH) + || invalidReason.equals(TransactionInvalidReason.EXECUTION_INTERRUPTED); } } diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java index adcc3ee1e27..6eb03ece947 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java @@ -14,13 +14,15 @@ */ package org.hyperledger.besu.ethereum.blockcreation; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; import static org.awaitility.Awaitility.await; import static org.hyperledger.besu.ethereum.core.MiningParameters.DEFAULT_NON_POA_BLOCK_TXS_SELECTION_MAX_TIME; +import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.EXECUTION_INTERRUPTED; import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.NONCE_TOO_LOW; import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.BLOCK_SELECTION_TIMEOUT; -import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.INVALID_TX_EVALUATION_TOO_LONG; +import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.BLOCK_SELECTION_TIMEOUT_INVALID_TX; import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.PRIORITY_FEE_PER_GAS_BELOW_CURRENT_MIN; import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.SELECTED; import static org.hyperledger.besu.plugin.data.TransactionSelectionResult.TX_EVALUATION_TOO_LONG; @@ -90,6 +92,7 @@ import org.hyperledger.besu.util.number.PositiveNumber; import java.math.BigInteger; +import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; @@ -185,6 +188,14 @@ public void setup() { when(ethContext.getEthPeers().subscribeConnect(any())).thenReturn(1L); when(ethScheduler.scheduleBlockCreationTask(any(Runnable.class))) .thenAnswer(invocation -> CompletableFuture.runAsync(invocation.getArgument(0))); + when(ethScheduler.scheduleFutureTask(any(Runnable.class), any(Duration.class))) + .thenAnswer( + invocation -> { + final Duration delay = invocation.getArgument(1); + CompletableFuture.delayedExecutor(delay.toMillis(), MILLISECONDS) + .execute(invocation.getArgument(0)); + return null; + }); } protected abstract GenesisConfigFile getGenesisConfigFile(); @@ -982,9 +993,17 @@ private void internalBlockSelectionTimeoutSimulation( .TransactionEvaluationContext ctx = invocation.getArgument(0); if (ctx.getTransaction().equals(p)) { - Thread.sleep(t); + try { + Thread.sleep(t); + } catch (final InterruptedException e) { + return TransactionSelectionResult.invalidTransient(EXECUTION_INTERRUPTED.name()); + } } else { - Thread.sleep(fastProcessingTxTime); + try { + Thread.sleep(fastProcessingTxTime); + } catch (final InterruptedException e) { + return TransactionSelectionResult.invalidTransient(EXECUTION_INTERRUPTED.name()); + } } return SELECTED; }; @@ -1081,18 +1100,19 @@ public void subsetOfInvalidPendingTransactionsIncludedWhenTxSelectionMaxTimeIsOv processingTooLate, postProcessingTooLate, 500, - BLOCK_SELECTION_TIMEOUT, - false, + BLOCK_SELECTION_TIMEOUT_INVALID_TX, + true, NONCE_TOO_LOW); } @ParameterizedTest @MethodSource("subsetOfPendingTransactionsIncludedWhenTxSelectionMaxTimeIsOver") - public void invalidPendingTransactionsThatTakesTooLongToEvaluateIsDroppedFromThePool( - final boolean isPoa, - final boolean preProcessingTooLate, - final boolean processingTooLate, - final boolean postProcessingTooLate) { + public void + evaluationOfInvalidPendingTransactionThatTakesTooLongToEvaluateIsInterruptedAndPenalized( + final boolean isPoa, + final boolean preProcessingTooLate, + final boolean processingTooLate, + final boolean postProcessingTooLate) { internalBlockSelectionTimeoutSimulationInvalidTxs( isPoa, @@ -1100,8 +1120,8 @@ public void invalidPendingTransactionsThatTakesTooLongToEvaluateIsDroppedFromThe processingTooLate, postProcessingTooLate, 900, - INVALID_TX_EVALUATION_TOO_LONG, - true, + TX_EVALUATION_TOO_LONG, + false, NONCE_TOO_LOW); } @@ -1128,9 +1148,17 @@ private void internalBlockSelectionTimeoutSimulationInvalidTxs( .TransactionEvaluationContext ctx = invocation.getArgument(0); if (ctx.getTransaction().equals(p)) { - Thread.sleep(t); + try { + Thread.sleep(t); + } catch (final InterruptedException e) { + return TransactionSelectionResult.invalidTransient(EXECUTION_INTERRUPTED.name()); + } } else { - Thread.sleep(fastProcessingTxTime); + try { + Thread.sleep(fastProcessingTxTime); + } catch (final InterruptedException e) { + return TransactionSelectionResult.invalidTransient(EXECUTION_INTERRUPTED.name()); + } } return invalidSelectionResult; }; @@ -1199,7 +1227,7 @@ private void internalBlockSelectionTimeoutSimulationInvalidTxs( final TransactionSelectionResults results = selector.buildTransactionListForBlock(); - // no tx is selected since all are invalid + // no tx is selected since all are invalid or late assertThat(results.getSelectedTransactions()).isEmpty(); // all txs are not selected so wait until all are evaluated @@ -1350,7 +1378,12 @@ protected void ensureTransactionIsValid( .thenAnswer( invocation -> { if (processingTime > 0) { - Thread.sleep(processingTime); + try { + Thread.sleep(processingTime); + } catch (final InterruptedException e) { + return TransactionProcessingResult.invalid( + ValidationResult.invalid(EXECUTION_INTERRUPTED)); + } } return TransactionProcessingResult.successful( new ArrayList<>(), @@ -1375,7 +1408,12 @@ protected void ensureTransactionIsInvalid( .thenAnswer( invocation -> { if (processingTime > 0) { - Thread.sleep(processingTime); + try { + Thread.sleep(processingTime); + } catch (final InterruptedException e) { + return TransactionProcessingResult.invalid( + ValidationResult.invalid(EXECUTION_INTERRUPTED)); + } } return TransactionProcessingResult.invalid(ValidationResult.invalid(invalidReason)); }); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java index 91c964525e0..30a1ea8e762 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java @@ -599,6 +599,12 @@ public TransactionProcessingResult processTransaction( EMPTY_ADDRESS_SET, 0L); + final var cause = re.getCause(); + if (cause != null && cause instanceof InterruptedException) { + return TransactionProcessingResult.invalid( + ValidationResult.invalid(TransactionInvalidReason.EXECUTION_INTERRUPTED)); + } + LOG.error("Critical Exception Processing Transaction", re); return TransactionProcessingResult.invalid( ValidationResult.invalid( diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionInvalidReason.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionInvalidReason.java index 8273c556d91..f20cc8fce79 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionInvalidReason.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionInvalidReason.java @@ -45,6 +45,7 @@ public enum TransactionInvalidReason { MAX_FEE_PER_GAS_BELOW_CURRENT_BASE_FEE, TX_FEECAP_EXCEEDED, INTERNAL_ERROR, + EXECUTION_INTERRUPTED, TX_POOL_DISABLED, INVALID_BLOBS, PLUGIN_TX_POOL_VALIDATOR, diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java index 070c1dcf771..29459ba3da4 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java @@ -236,7 +236,9 @@ public Optional processWithWorldUpdater( : blockHeaderToProcess.getGasLimit(); if (rpcGasCap > 0) { gasLimit = rpcGasCap; - LOG.info("Capping gasLimit to " + rpcGasCap); + LOG.trace( + "Gas limit capped at {} for transaction simulation due to provided RPC gas cap.", + rpcGasCap); } final Wei value = callParams.getValue() != null ? callParams.getValue() : Wei.ZERO; final Bytes payload = callParams.getPayload() != null ? callParams.getPayload() : Bytes.EMPTY; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelector.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelector.java index c334773d168..41d2e9b700b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelector.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelector.java @@ -15,13 +15,17 @@ package org.hyperledger.besu.ethereum.eth.manager.peertask; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; import org.hyperledger.besu.ethereum.p2p.peers.PeerId; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; +import java.util.Collection; import java.util.Comparator; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; +import java.util.function.Supplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,8 +37,13 @@ public class DefaultPeerSelector implements PeerSelector { private static final Logger LOG = LoggerFactory.getLogger(DefaultPeerSelector.class); + private final Supplier protocolSpecSupplier; private final Map ethPeersByPeerId = new ConcurrentHashMap<>(); + public DefaultPeerSelector(final Supplier protocolSpecSupplier) { + this.protocolSpecSupplier = protocolSpecSupplier; + } + /** * Gets the highest reputation peer matching the supplied filter * @@ -42,8 +51,7 @@ public class DefaultPeerSelector implements PeerSelector { * @return the highest reputation peer matching the supplies filter * @throws NoAvailablePeerException If there are no suitable peers */ - @Override - public EthPeer getPeer(final Predicate filter) throws NoAvailablePeerException { + private EthPeer getPeer(final Predicate filter) throws NoAvailablePeerException { LOG.trace("Finding peer from pool of {} peers", ethPeersByPeerId.size()); return ethPeersByPeerId.values().stream() .filter(filter) @@ -51,6 +59,20 @@ public EthPeer getPeer(final Predicate filter) throws NoAvailablePeerEx .orElseThrow(NoAvailablePeerException::new); } + @Override + public EthPeer getPeer( + final Collection usedEthPeers, + final long requiredPeerHeight, + final SubProtocol requiredSubProtocol) + throws NoAvailablePeerException { + return getPeer( + (candidatePeer) -> + isPeerUnused(candidatePeer, usedEthPeers) + && (protocolSpecSupplier.get().isPoS() + || isPeerHeightHighEnough(candidatePeer, requiredPeerHeight)) + && isPeerProtocolSuitable(candidatePeer, requiredSubProtocol)); + } + @Override public Optional getPeerByPeerId(final PeerId peerId) { return Optional.ofNullable(ethPeersByPeerId.get(peerId)); @@ -65,4 +87,16 @@ public void addPeer(final EthPeer ethPeer) { public void removePeer(final PeerId peerId) { ethPeersByPeerId.remove(peerId); } + + private boolean isPeerUnused(final EthPeer ethPeer, final Collection usedEthPeers) { + return !usedEthPeers.contains(ethPeer); + } + + private boolean isPeerHeightHighEnough(final EthPeer ethPeer, final long requiredHeight) { + return ethPeer.chainState().getEstimatedHeight() >= requiredHeight; + } + + private boolean isPeerProtocolSuitable(final EthPeer ethPeer, final SubProtocol protocol) { + return ethPeer.getProtocolName().equals(protocol.getName()); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerSelector.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerSelector.java index 3f5589f93b2..93d98a193b1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerSelector.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerSelector.java @@ -16,21 +16,28 @@ import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.p2p.peers.PeerId; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; +import java.util.Collection; import java.util.Optional; -import java.util.function.Predicate; /** Selects the EthPeers for the PeerTaskExecutor */ public interface PeerSelector { /** - * Gets a peer matching the supplied filter + * Gets a peer with the requiredPeerHeight (if not PoS), and with the requiredSubProtocol, and + * which is not in the supplied collection of usedEthPeers * - * @param filter a filter to match prospective peers with - * @return a peer matching the supplied filter + * @param usedEthPeers a collection of EthPeers to be excluded from selection because they have + * already been used + * @param requiredPeerHeight the minimum peer height required of the selected peer + * @param requiredSubProtocol the SubProtocol required of the peer + * @return a peer matching the supplied conditions * @throws NoAvailablePeerException If there are no suitable peers */ - EthPeer getPeer(final Predicate filter) throws NoAvailablePeerException; + EthPeer getPeer( + Collection usedEthPeers, long requiredPeerHeight, SubProtocol requiredSubProtocol) + throws NoAvailablePeerException; /** * Attempts to get the EthPeer identified by peerId @@ -39,19 +46,19 @@ public interface PeerSelector { * @return An Optional\ containing the EthPeer identified by peerId if present in the * PeerSelector, or empty otherwise */ - Optional getPeerByPeerId(final PeerId peerId); + Optional getPeerByPeerId(PeerId peerId); /** * Add the supplied EthPeer to the PeerSelector * * @param ethPeer the EthPeer to be added to the PeerSelector */ - void addPeer(final EthPeer ethPeer); + void addPeer(EthPeer ethPeer); /** * Remove the EthPeer identified by peerId from the PeerSelector * * @param peerId the PeerId of the EthPeer to be removed from the PeerSelector */ - void removePeer(final PeerId peerId); + void removePeer(PeerId peerId); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java index 244908c9216..36bd03531bd 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.ethereum.eth.manager.peertask; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; import java.util.Collection; @@ -29,7 +30,7 @@ public interface PeerTask { * * @return the SubProtocol used for this PeerTask */ - String getSubProtocol(); + SubProtocol getSubProtocol(); /** * Gets the minimum required block number for a peer to have to successfully execute this task @@ -59,5 +60,5 @@ public interface PeerTask { * * @return the Collection of behaviors this task is expected to exhibit in the PeetTaskExecutor */ - Collection getPeerTaskBehaviors(); + Collection getPeerTaskBehaviors(); } 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 bd6445226b6..10c882e7e5a 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 @@ -15,7 +15,6 @@ package org.hyperledger.besu.ethereum.eth.manager.peertask; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; import org.hyperledger.besu.metrics.BesuMetricCategory; @@ -23,61 +22,60 @@ import org.hyperledger.besu.plugin.services.metrics.LabelledMetric; import org.hyperledger.besu.plugin.services.metrics.OperationTimer; -import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; -import java.util.function.Supplier; -/** Manages the execution of PeerTasks, respecting their PeerTaskBehavior */ +/** Manages the execution of PeerTasks, respecting their PeerTaskRetryBehavior */ public class PeerTaskExecutor { + public static final int RETRIES_WITH_SAME_PEER = 3; + public static final int RETRIES_WITH_OTHER_PEER = 3; + public static final int NO_RETRIES = 1; private final PeerSelector peerSelector; private final PeerTaskRequestSender requestSender; - private final Supplier protocolSpecSupplier; + private final LabelledMetric requestTimer; public PeerTaskExecutor( final PeerSelector peerSelector, final PeerTaskRequestSender requestSender, - final Supplier protocolSpecSupplier, final MetricsSystem metricsSystem) { this.peerSelector = peerSelector; this.requestSender = requestSender; - this.protocolSpecSupplier = protocolSpecSupplier; requestTimer = metricsSystem.createLabelledTimer( BesuMetricCategory.PEERS, "PeerTaskExecutor:RequestTime", - "Time taken to send a request", + "Time taken to send a request and receive a response", "className"); } public PeerTaskExecutorResult execute(final PeerTask peerTask) { PeerTaskExecutorResult executorResult; int triesRemaining = - peerTask.getPeerTaskBehaviors().contains(PeerTaskBehavior.RETRY_WITH_OTHER_PEERS) ? 3 : 1; - final Collection usedEthPeers = new ArrayList<>(); + peerTask.getPeerTaskBehaviors().contains(PeerTaskRetryBehavior.RETRY_WITH_OTHER_PEERS) + ? RETRIES_WITH_OTHER_PEER + : NO_RETRIES; + final Collection usedEthPeers = new HashSet<>(); do { EthPeer peer; try { peer = peerSelector.getPeer( - (candidatePeer) -> - isPeerUnused(candidatePeer, usedEthPeers) - && (protocolSpecSupplier.get().isPoS() - || isPeerHeightHighEnough( - candidatePeer, peerTask.getRequiredBlockNumber())) - && isPeerProtocolSuitable(candidatePeer, peerTask.getSubProtocol())); + usedEthPeers, peerTask.getRequiredBlockNumber(), peerTask.getSubProtocol()); usedEthPeers.add(peer); executorResult = executeAgainstPeer(peerTask, peer); } catch (NoAvailablePeerException e) { executorResult = - new PeerTaskExecutorResult<>(null, PeerTaskExecutorResponseCode.NO_PEER_AVAILABLE); + new PeerTaskExecutorResult<>( + Optional.empty(), PeerTaskExecutorResponseCode.NO_PEER_AVAILABLE); } } while (--triesRemaining > 0 - && executorResult.getResponseCode() != PeerTaskExecutorResponseCode.SUCCESS); + && executorResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS); return executorResult; } @@ -91,40 +89,48 @@ public PeerTaskExecutorResult executeAgainstPeer( MessageData requestMessageData = peerTask.getRequestMessage(); PeerTaskExecutorResult executorResult; int triesRemaining = - peerTask.getPeerTaskBehaviors().contains(PeerTaskBehavior.RETRY_WITH_SAME_PEER) ? 3 : 1; + peerTask.getPeerTaskBehaviors().contains(PeerTaskRetryBehavior.RETRY_WITH_SAME_PEER) + ? RETRIES_WITH_SAME_PEER + : NO_RETRIES; do { try { - - MessageData responseMessageData; - try (final OperationTimer.TimingContext timingContext = + T result; + try (final OperationTimer.TimingContext ignored = requestTimer.labels(peerTask.getClass().getSimpleName()).startTimer()) { - responseMessageData = + MessageData responseMessageData = requestSender.sendRequest(peerTask.getSubProtocol(), requestMessageData, peer); + + result = peerTask.parseResponse(responseMessageData); } - T result = peerTask.parseResponse(responseMessageData); peer.recordUsefulResponse(); - executorResult = new PeerTaskExecutorResult<>(result, PeerTaskExecutorResponseCode.SUCCESS); + executorResult = + new PeerTaskExecutorResult<>( + Optional.ofNullable(result), PeerTaskExecutorResponseCode.SUCCESS); } catch (PeerConnection.PeerNotConnected e) { executorResult = - new PeerTaskExecutorResult<>(null, PeerTaskExecutorResponseCode.PEER_DISCONNECTED); + new PeerTaskExecutorResult<>( + Optional.empty(), PeerTaskExecutorResponseCode.PEER_DISCONNECTED); } catch (InterruptedException | TimeoutException e) { peer.recordRequestTimeout(requestMessageData.getCode()); - executorResult = new PeerTaskExecutorResult<>(null, PeerTaskExecutorResponseCode.TIMEOUT); + executorResult = + new PeerTaskExecutorResult<>(Optional.empty(), PeerTaskExecutorResponseCode.TIMEOUT); } catch (InvalidPeerTaskResponseException e) { peer.recordUselessResponse(e.getMessage()); executorResult = - new PeerTaskExecutorResult<>(null, PeerTaskExecutorResponseCode.INVALID_RESPONSE); + new PeerTaskExecutorResult<>( + Optional.empty(), PeerTaskExecutorResponseCode.INVALID_RESPONSE); } catch (ExecutionException e) { executorResult = - new PeerTaskExecutorResult<>(null, PeerTaskExecutorResponseCode.INTERNAL_SERVER_ERROR); + new PeerTaskExecutorResult<>( + Optional.empty(), PeerTaskExecutorResponseCode.INTERNAL_SERVER_ERROR); } } while (--triesRemaining > 0 - && executorResult.getResponseCode() != PeerTaskExecutorResponseCode.SUCCESS - && executorResult.getResponseCode() != PeerTaskExecutorResponseCode.PEER_DISCONNECTED + && executorResult.responseCode() != PeerTaskExecutorResponseCode.SUCCESS + && executorResult.responseCode() != PeerTaskExecutorResponseCode.PEER_DISCONNECTED && sleepBetweenRetries()); return executorResult; @@ -144,17 +150,4 @@ private boolean sleepBetweenRetries() { return false; } } - - private static boolean isPeerUnused( - final EthPeer ethPeer, final Collection usedEthPeers) { - return !usedEthPeers.contains(ethPeer); - } - - private static boolean isPeerHeightHighEnough(final EthPeer ethPeer, final long requiredHeight) { - return ethPeer.chainState().getEstimatedHeight() >= requiredHeight; - } - - private static boolean isPeerProtocolSuitable(final EthPeer ethPeer, final String protocol) { - return ethPeer.getProtocolName().equals(protocol); - } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResult.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResult.java index f89bc67f61f..86dec85c295 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResult.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResult.java @@ -16,20 +16,5 @@ import java.util.Optional; -public class PeerTaskExecutorResult { - private final Optional result; - private final PeerTaskExecutorResponseCode responseCode; - - public PeerTaskExecutorResult(final T result, final PeerTaskExecutorResponseCode responseCode) { - this.result = Optional.ofNullable(result); - this.responseCode = responseCode; - } - - public Optional getResult() { - return result; - } - - public PeerTaskExecutorResponseCode getResponseCode() { - return responseCode; - } -} +public record PeerTaskExecutorResult( + Optional result, PeerTaskExecutorResponseCode responseCode) {} diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRequestSender.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRequestSender.java index 77ff5e7251d..7a597eca8e8 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRequestSender.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRequestSender.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.ethereum.eth.manager.RequestManager.ResponseStream; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -38,13 +39,13 @@ public PeerTaskRequestSender(final long timeoutMs) { } public MessageData sendRequest( - final String subProtocol, final MessageData requestMessageData, final EthPeer ethPeer) + final SubProtocol subProtocol, final MessageData requestMessageData, final EthPeer ethPeer) throws PeerConnection.PeerNotConnected, ExecutionException, InterruptedException, TimeoutException { ResponseStream responseStream = - ethPeer.send(requestMessageData, subProtocol, ethPeer.getConnection()); + ethPeer.send(requestMessageData, subProtocol.getName(), ethPeer.getConnection()); final CompletableFuture responseMessageDataFuture = new CompletableFuture<>(); responseStream.then( (boolean streamClosed, MessageData message, EthPeer peer) -> { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskBehavior.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRetryBehavior.java similarity index 95% rename from ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskBehavior.java rename to ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRetryBehavior.java index fba9000a741..53e2def6612 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskBehavior.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRetryBehavior.java @@ -14,7 +14,7 @@ */ package org.hyperledger.besu.ethereum.eth.manager.peertask; -public enum PeerTaskBehavior { +public enum PeerTaskRetryBehavior { RETRY_WITH_SAME_PEER, RETRY_WITH_OTHER_PEERS } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java index 21c75b364a0..a79c05e3e6e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java @@ -351,9 +351,7 @@ public void selectTransactions(final PendingTransactions.TransactionSelector sel } }); logDiscardedTransaction(candidatePendingTx, selectionResult); - } - - if (selectionResult.penalize()) { + } else if (selectionResult.penalize()) { ethScheduler.scheduleTxWorkerTask( () -> { synchronized (this) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelectorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelectorTest.java index 8913b80a457..add2b1e612c 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelectorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/DefaultPeerSelectorTest.java @@ -14,19 +14,23 @@ */ package org.hyperledger.besu.ethereum.eth.manager.peertask; +import org.hyperledger.besu.ethereum.eth.EthProtocol; +import org.hyperledger.besu.ethereum.eth.SnapProtocol; +import org.hyperledger.besu.ethereum.eth.manager.ChainState; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.MockPeerConnection; +import org.hyperledger.besu.ethereum.eth.manager.PeerReputation; +import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; +import org.hyperledger.besu.ethereum.p2p.peers.Peer; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; -import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; -import java.time.Clock; -import java.util.Collections; +import java.util.HashSet; import java.util.Set; -import org.apache.tuweni.bytes.Bytes; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; public class DefaultPeerSelectorTest { @@ -34,67 +38,98 @@ public class DefaultPeerSelectorTest { @BeforeEach public void beforeTest() { - peerSelector = new DefaultPeerSelector(); + ProtocolSpec protocolSpec = Mockito.mock(ProtocolSpec.class); + Mockito.when(protocolSpec.isPoS()).thenReturn(false); + peerSelector = new DefaultPeerSelector(() -> protocolSpec); } @Test public void testGetPeer() throws NoAvailablePeerException { - EthPeer protocol1With5ReputationPeer = - createTestPeer(Set.of(Capability.create("capability1", 1)), "protocol1", 5); - peerSelector.addPeer(protocol1With5ReputationPeer); - EthPeer protocol1With4ReputationPeer = - createTestPeer(Set.of(Capability.create("capability1", 1)), "protocol1", 4); - peerSelector.addPeer(protocol1With4ReputationPeer); - EthPeer protocol2With50ReputationPeer = - createTestPeer(Set.of(Capability.create("capability1", 1)), "protocol2", 50); - peerSelector.addPeer(protocol2With50ReputationPeer); - EthPeer protocol2With4ReputationPeer = - createTestPeer(Set.of(Capability.create("capability1", 1)), "protocol2", 4); - peerSelector.addPeer(protocol2With4ReputationPeer); - - EthPeer result = peerSelector.getPeer((p) -> p.getProtocolName().equals("protocol1")); - - Assertions.assertSame(protocol1With5ReputationPeer, result); + EthPeer expectedPeer = createTestPeer(10, EthProtocol.get(), 5); + peerSelector.addPeer(expectedPeer); + EthPeer excludedForLowChainHeightPeer = createTestPeer(5, EthProtocol.get(), 50); + peerSelector.addPeer(excludedForLowChainHeightPeer); + EthPeer excludedForWrongProtocolPeer = createTestPeer(10, SnapProtocol.get(), 50); + peerSelector.addPeer(excludedForWrongProtocolPeer); + EthPeer excludedForLowReputationPeer = createTestPeer(10, EthProtocol.get(), 1); + peerSelector.addPeer(excludedForLowReputationPeer); + EthPeer excludedForBeingAlreadyUsedPeer = createTestPeer(10, EthProtocol.get(), 50); + peerSelector.addPeer(excludedForBeingAlreadyUsedPeer); + + Set usedEthPeers = new HashSet<>(); + usedEthPeers.add(excludedForBeingAlreadyUsedPeer); + + EthPeer result = peerSelector.getPeer(usedEthPeers, 10, EthProtocol.get()); + + Assertions.assertSame(expectedPeer, result); } @Test public void testGetPeerButNoPeerMatchesFilter() { - EthPeer protocol1With5ReputationPeer = - createTestPeer(Set.of(Capability.create("capability1", 1)), "protocol1", 5); - peerSelector.addPeer(protocol1With5ReputationPeer); - EthPeer protocol1With4ReputationPeer = - createTestPeer(Set.of(Capability.create("capability1", 1)), "protocol1", 4); - peerSelector.addPeer(protocol1With4ReputationPeer); - EthPeer protocol2With50ReputationPeer = - createTestPeer(Set.of(Capability.create("capability1", 1)), "protocol2", 50); - peerSelector.addPeer(protocol2With50ReputationPeer); - EthPeer protocol2With4ReputationPeer = - createTestPeer(Set.of(Capability.create("capability1", 1)), "protocol2", 4); - peerSelector.addPeer(protocol2With4ReputationPeer); + EthPeer expectedPeer = createTestPeer(10, EthProtocol.get(), 5); + peerSelector.addPeer(expectedPeer); + EthPeer excludedForLowChainHeightPeer = createTestPeer(5, EthProtocol.get(), 50); + peerSelector.addPeer(excludedForLowChainHeightPeer); + EthPeer excludedForWrongProtocolPeer = createTestPeer(10, SnapProtocol.get(), 50); + peerSelector.addPeer(excludedForWrongProtocolPeer); + EthPeer excludedForLowReputationPeer = createTestPeer(10, EthProtocol.get(), 1); + peerSelector.addPeer(excludedForLowReputationPeer); + EthPeer excludedForBeingAlreadyUsedPeer = createTestPeer(10, EthProtocol.get(), 50); + peerSelector.addPeer(excludedForBeingAlreadyUsedPeer); + + Set usedEthPeers = new HashSet<>(); + usedEthPeers.add(excludedForBeingAlreadyUsedPeer); Assertions.assertThrows( NoAvailablePeerException.class, - () -> peerSelector.getPeer((p) -> p.getProtocolName().equals("fake protocol"))); + () -> peerSelector.getPeer(usedEthPeers, 10, new MockSubProtocol())); } private EthPeer createTestPeer( - final Set connectionCapabilities, - final String protocolName, - final int reputationAdjustment) { - PeerConnection peerConnection = new MockPeerConnection(connectionCapabilities); - EthPeer peer = - new EthPeer( - peerConnection, - protocolName, - null, - Collections.emptyList(), - 1, - Clock.systemUTC(), - Collections.emptyList(), - Bytes.EMPTY); - for (int i = 0; i < reputationAdjustment; i++) { - peer.getReputation().recordUsefulResponse(); + final long chainHeight, final SubProtocol protocol, final int reputation) { + EthPeer ethPeer = Mockito.mock(EthPeer.class); + PeerConnection peerConnection = Mockito.mock(PeerConnection.class); + Peer peer = Mockito.mock(Peer.class); + ChainState chainState = Mockito.mock(ChainState.class); + PeerReputation peerReputation = Mockito.mock(PeerReputation.class); + + Mockito.when(ethPeer.getConnection()).thenReturn(peerConnection); + Mockito.when(peerConnection.getPeer()).thenReturn(peer); + Mockito.when(ethPeer.getProtocolName()).thenReturn(protocol.getName()); + Mockito.when(ethPeer.chainState()).thenReturn(chainState); + Mockito.when(chainState.getEstimatedHeight()).thenReturn(chainHeight); + Mockito.when(ethPeer.getReputation()).thenReturn(peerReputation); + Mockito.when(peerReputation.getScore()).thenReturn(reputation); + + Mockito.when(ethPeer.compareTo(Mockito.any(EthPeer.class))) + .thenAnswer( + (invocationOnMock) -> { + EthPeer otherPeer = invocationOnMock.getArgument(0, EthPeer.class); + return Integer.compare(reputation, otherPeer.getReputation().getScore()); + }); + return ethPeer; + } + + private static class MockSubProtocol implements SubProtocol { + + @Override + public String getName() { + return "Mock"; + } + + @Override + public int messageSpace(final int protocolVersion) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isValidMessageCode(final int protocolVersion, final int code) { + throw new UnsupportedOperationException(); + } + + @Override + public String messageName(final int protocolVersion, final int code) { + throw new UnsupportedOperationException(); } - return peer; } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java index 930f4325b6b..15b1747bc7b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java @@ -15,16 +15,16 @@ package org.hyperledger.besu.ethereum.eth.manager.peertask; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; -import java.util.function.Predicate; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -37,8 +37,8 @@ public class PeerTaskExecutorTest { private @Mock PeerSelector peerSelector; private @Mock PeerTaskRequestSender requestSender; - private @Mock ProtocolSpec protocolSpec; private @Mock PeerTask peerTask; + private @Mock SubProtocol subprotocol; private @Mock MessageData requestMessageData; private @Mock MessageData responseMessageData; private @Mock EthPeer ethPeer; @@ -49,9 +49,7 @@ public class PeerTaskExecutorTest { @BeforeEach public void beforeTest() { mockCloser = MockitoAnnotations.openMocks(this); - peerTaskExecutor = - new PeerTaskExecutor( - peerSelector, requestSender, () -> protocolSpec, new NoOpMetricsSystem()); + peerTaskExecutor = new PeerTaskExecutor(peerSelector, requestSender, new NoOpMetricsSystem()); } @AfterEach @@ -66,12 +64,13 @@ public void testExecuteAgainstPeerWithNoPeerTaskBehaviorsAndSuccessfulFlow() InterruptedException, TimeoutException, InvalidPeerTaskResponseException { - String subprotocol = "subprotocol"; + Object responseObject = new Object(); Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()).thenReturn(Collections.emptyList()); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); + Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenReturn(responseMessageData); Mockito.when(peerTask.parseResponse(responseMessageData)).thenReturn(responseObject); @@ -81,9 +80,9 @@ public void testExecuteAgainstPeerWithNoPeerTaskBehaviorsAndSuccessfulFlow() Mockito.verify(ethPeer).recordUsefulResponse(); Assertions.assertNotNull(result); - Assertions.assertTrue(result.getResult().isPresent()); - Assertions.assertSame(responseObject, result.getResult().get()); - Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.getResponseCode()); + Assertions.assertTrue(result.result().isPresent()); + Assertions.assertSame(responseObject, result.result().get()); + Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.responseCode()); } @Test @@ -93,15 +92,15 @@ public void testExecuteAgainstPeerWithRetryBehaviorsAndSuccessfulFlowAfterFirstF InterruptedException, TimeoutException, InvalidPeerTaskResponseException { - String subprotocol = "subprotocol"; Object responseObject = new Object(); int requestMessageDataCode = 123; Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()) - .thenReturn(List.of(PeerTaskBehavior.RETRY_WITH_SAME_PEER)); + .thenReturn(List.of(PeerTaskRetryBehavior.RETRY_WITH_SAME_PEER)); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); + Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenThrow(new TimeoutException()) .thenReturn(responseMessageData); @@ -114,9 +113,9 @@ public void testExecuteAgainstPeerWithRetryBehaviorsAndSuccessfulFlowAfterFirstF Mockito.verify(ethPeer).recordUsefulResponse(); Assertions.assertNotNull(result); - Assertions.assertTrue(result.getResult().isPresent()); - Assertions.assertSame(responseObject, result.getResult().get()); - Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.getResponseCode()); + Assertions.assertTrue(result.result().isPresent()); + Assertions.assertSame(responseObject, result.result().get()); + Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.responseCode()); } @Test @@ -125,20 +124,19 @@ public void testExecuteAgainstPeerWithNoPeerTaskBehaviorsAndPeerNotConnected() ExecutionException, InterruptedException, TimeoutException { - String subprotocol = "subprotocol"; Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()).thenReturn(Collections.emptyList()); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); + Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenThrow(new PeerConnection.PeerNotConnected("")); PeerTaskExecutorResult result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer); Assertions.assertNotNull(result); - Assertions.assertTrue(result.getResult().isEmpty()); - Assertions.assertEquals( - PeerTaskExecutorResponseCode.PEER_DISCONNECTED, result.getResponseCode()); + Assertions.assertTrue(result.result().isEmpty()); + Assertions.assertEquals(PeerTaskExecutorResponseCode.PEER_DISCONNECTED, result.responseCode()); } @Test @@ -147,12 +145,12 @@ public void testExecuteAgainstPeerWithNoPeerTaskBehaviorsAndTimeoutException() ExecutionException, InterruptedException, TimeoutException { - String subprotocol = "subprotocol"; int requestMessageDataCode = 123; Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()).thenReturn(Collections.emptyList()); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); + Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenThrow(new TimeoutException()); Mockito.when(requestMessageData.getCode()).thenReturn(requestMessageDataCode); @@ -162,8 +160,8 @@ public void testExecuteAgainstPeerWithNoPeerTaskBehaviorsAndTimeoutException() Mockito.verify(ethPeer).recordRequestTimeout(requestMessageDataCode); Assertions.assertNotNull(result); - Assertions.assertTrue(result.getResult().isEmpty()); - Assertions.assertEquals(PeerTaskExecutorResponseCode.TIMEOUT, result.getResponseCode()); + Assertions.assertTrue(result.result().isEmpty()); + Assertions.assertEquals(PeerTaskExecutorResponseCode.TIMEOUT, result.responseCode()); } @Test @@ -173,11 +171,11 @@ public void testExecuteAgainstPeerWithNoPeerTaskBehaviorsAndInvalidResponseMessa InterruptedException, TimeoutException, InvalidPeerTaskResponseException { - String subprotocol = "subprotocol"; Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()).thenReturn(Collections.emptyList()); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); + Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenReturn(responseMessageData); Mockito.when(peerTask.parseResponse(responseMessageData)) @@ -188,9 +186,8 @@ public void testExecuteAgainstPeerWithNoPeerTaskBehaviorsAndInvalidResponseMessa Mockito.verify(ethPeer).recordUselessResponse(null); Assertions.assertNotNull(result); - Assertions.assertTrue(result.getResult().isEmpty()); - Assertions.assertEquals( - PeerTaskExecutorResponseCode.INVALID_RESPONSE, result.getResponseCode()); + Assertions.assertTrue(result.result().isEmpty()); + Assertions.assertEquals(PeerTaskExecutorResponseCode.INVALID_RESPONSE, result.responseCode()); } @Test @@ -202,14 +199,18 @@ public void testExecuteWithNoPeerTaskBehaviorsAndSuccessFlow() TimeoutException, InvalidPeerTaskResponseException, NoAvailablePeerException { - String subprotocol = "subprotocol"; Object responseObject = new Object(); - Mockito.when(peerSelector.getPeer(Mockito.any(Predicate.class))).thenReturn(ethPeer); + Mockito.when( + peerSelector.getPeer( + Mockito.any(Collection.class), Mockito.eq(10L), Mockito.eq(subprotocol))) + .thenReturn(ethPeer); Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()).thenReturn(Collections.emptyList()); + Mockito.when(peerTask.getRequiredBlockNumber()).thenReturn(10L); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); + Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenReturn(responseMessageData); Mockito.when(peerTask.parseResponse(responseMessageData)).thenReturn(responseObject); @@ -219,9 +220,9 @@ public void testExecuteWithNoPeerTaskBehaviorsAndSuccessFlow() Mockito.verify(ethPeer).recordUsefulResponse(); Assertions.assertNotNull(result); - Assertions.assertTrue(result.getResult().isPresent()); - Assertions.assertSame(responseObject, result.getResult().get()); - Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.getResponseCode()); + Assertions.assertTrue(result.result().isPresent()); + Assertions.assertSame(responseObject, result.result().get()); + Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.responseCode()); } @Test @@ -233,18 +234,20 @@ public void testExecuteWithPeerSwitchingAndSuccessFlow() TimeoutException, InvalidPeerTaskResponseException, NoAvailablePeerException { - String subprotocol = "subprotocol"; Object responseObject = new Object(); int requestMessageDataCode = 123; EthPeer peer2 = Mockito.mock(EthPeer.class); - Mockito.when(peerSelector.getPeer(Mockito.any(Predicate.class))) + Mockito.when( + peerSelector.getPeer( + Mockito.any(Collection.class), Mockito.eq(10L), Mockito.eq(subprotocol))) .thenReturn(ethPeer) .thenReturn(peer2); Mockito.when(peerTask.getRequestMessage()).thenReturn(requestMessageData); Mockito.when(peerTask.getPeerTaskBehaviors()) - .thenReturn(List.of(PeerTaskBehavior.RETRY_WITH_OTHER_PEERS)); + .thenReturn(List.of(PeerTaskRetryBehavior.RETRY_WITH_OTHER_PEERS)); + Mockito.when(peerTask.getRequiredBlockNumber()).thenReturn(10L); Mockito.when(peerTask.getSubProtocol()).thenReturn(subprotocol); Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer)) .thenThrow(new TimeoutException()); @@ -259,8 +262,8 @@ public void testExecuteWithPeerSwitchingAndSuccessFlow() Mockito.verify(peer2).recordUsefulResponse(); Assertions.assertNotNull(result); - Assertions.assertTrue(result.getResult().isPresent()); - Assertions.assertSame(responseObject, result.getResult().get()); - Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.getResponseCode()); + Assertions.assertTrue(result.result().isPresent()); + Assertions.assertSame(responseObject, result.result().get()); + Assertions.assertEquals(PeerTaskExecutorResponseCode.SUCCESS, result.responseCode()); } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRequestSenderTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRequestSenderTest.java index 8bc52604db7..4041fb63037 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRequestSenderTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskRequestSenderTest.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.ethereum.eth.manager.RequestManager; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -40,7 +41,7 @@ public void beforeTest() { @Test public void testSendRequest() throws PeerConnection.PeerNotConnected, ExecutionException, InterruptedException { - String subprotocol = "subprotocol"; + SubProtocol subprotocol = Mockito.mock(SubProtocol.class); MessageData requestMessageData = Mockito.mock(MessageData.class); MessageData responseMessageData = Mockito.mock(MessageData.class); EthPeer peer = Mockito.mock(EthPeer.class); @@ -49,7 +50,8 @@ public void testSendRequest() Mockito.mock(RequestManager.ResponseStream.class); Mockito.when(peer.getConnection()).thenReturn(peerConnection); - Mockito.when(peer.send(requestMessageData, subprotocol, peerConnection)) + Mockito.when(subprotocol.getName()).thenReturn("subprotocol"); + Mockito.when(peer.send(requestMessageData, "subprotocol", peerConnection)) .thenReturn(responseStream); CompletableFuture actualResponseMessageDataFuture = diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/DiscoveryPeer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/DiscoveryPeer.java index 9b5981b4978..1888e0df274 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/DiscoveryPeer.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/DiscoveryPeer.java @@ -17,12 +17,12 @@ import org.hyperledger.besu.ethereum.forkid.ForkId; import org.hyperledger.besu.ethereum.p2p.peers.DefaultPeer; import org.hyperledger.besu.ethereum.p2p.peers.Peer; -import org.hyperledger.besu.ethereum.p2p.peers.PeerId; import org.hyperledger.besu.ethereum.rlp.RLPInput; import org.hyperledger.besu.ethereum.rlp.RLPOutput; import org.hyperledger.besu.plugin.data.EnodeURL; import java.util.Optional; +import java.util.concurrent.atomic.AtomicLong; import org.apache.tuweni.bytes.Bytes; import org.ethereum.beacon.discovery.schema.NodeRecord; @@ -37,9 +37,7 @@ public class DiscoveryPeer extends DefaultPeer { private final Endpoint endpoint; // Timestamps. - private long firstDiscovered = 0; - private long lastContacted = 0; - private long lastSeen = 0; + private final AtomicLong firstDiscovered = new AtomicLong(0L); private long lastAttemptedConnection = 0; private NodeRecord nodeRecord; @@ -96,20 +94,11 @@ public void setStatus(final PeerDiscoveryStatus status) { } public long getFirstDiscovered() { - return firstDiscovered; + return firstDiscovered.get(); } - public PeerId setFirstDiscovered(final long firstDiscovered) { - this.firstDiscovered = firstDiscovered; - return this; - } - - public long getLastContacted() { - return lastContacted; - } - - public void setLastContacted(final long lastContacted) { - this.lastContacted = lastContacted; + public void setFirstDiscovered(final long firstDiscovered) { + this.firstDiscovered.compareAndExchange(0L, firstDiscovered); } public long getLastAttemptedConnection() { @@ -120,14 +109,6 @@ public void setLastAttemptedConnection(final long lastAttemptedConnection) { this.lastAttemptedConnection = lastAttemptedConnection; } - public long getLastSeen() { - return lastSeen; - } - - public void setLastSeen(final long lastSeen) { - this.lastSeen = lastSeen; - } - public Endpoint getEndpoint() { return endpoint; } @@ -163,8 +144,6 @@ public String toString() { sb.append("status=").append(status); sb.append(", enode=").append(this.getEnodeURL()); sb.append(", firstDiscovered=").append(firstDiscovered); - sb.append(", lastContacted=").append(lastContacted); - sb.append(", lastSeen=").append(lastSeen); sb.append('}'); return sb.toString(); } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java index 7efc50750fb..703b702e8e7 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java @@ -365,9 +365,7 @@ protected void handleOutgoingPacket(final DiscoveryPeer peer, final Packet packe (res, err) -> { if (err != null) { handleOutgoingPacketError(err, peer, packet); - return; } - peer.setLastContacted(System.currentTimeMillis()); }); } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryStatus.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryStatus.java index 5311a2212a5..28ccac70360 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryStatus.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryStatus.java @@ -35,10 +35,7 @@ public enum PeerDiscoveryStatus { * We have successfully bonded with this {@link DiscoveryPeer}, and we are able to exchange * messages with them. */ - BONDED, - - /** We have requested the ENR record from this {@link DiscoveryPeer} */ - ENR_REQUESTED; + BONDED; @Override public String toString() { diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 11b5216a82b..2261f813546 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -27,6 +27,7 @@ import org.hyperledger.besu.ethereum.p2p.peers.PeerId; import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissions; import org.hyperledger.besu.ethereum.p2p.rlpx.RlpxAgent; +import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection; import org.hyperledger.besu.metrics.BesuMetricCategory; import org.hyperledger.besu.plugin.services.MetricsSystem; import org.hyperledger.besu.plugin.services.metrics.Counter; @@ -43,6 +44,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.function.Predicate; @@ -321,7 +323,6 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { switch (packet.getType()) { case PING: if (peerPermissions.allowInboundBonding(peer)) { - peer.setLastSeen(System.currentTimeMillis()); final PingPacketData ping = packet.getPacketData(PingPacketData.class).get(); if (!PeerDiscoveryStatus.BONDED.equals(peer.getStatus()) && (bondingPeers.getIfPresent(sender.getId()) == null)) { @@ -338,7 +339,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { requestENR(peer); } bondingPeers.invalidate(peerId); - addToPeerTable(peer); + checkBeforeAddingToPeerTable(peer); recursivePeerRefreshState.onBondingComplete(peer); Optional.ofNullable(cachedEnrRequests.getIfPresent(peerId)) .ifPresent(cachedEnrRequest -> processEnrRequest(peer, cachedEnrRequest)); @@ -405,38 +406,45 @@ private List getPeersFromNeighborsPacket(final Packet packet) { .collect(Collectors.toList()); } - private boolean addToPeerTable(final DiscoveryPeer peer) { - final PeerTable.AddResult result = peerTable.tryAdd(peer); - if (result.getOutcome() != PeerTable.AddResult.AddOutcome.INVALID) { - - // Reset the last seen timestamp. - final long now = System.currentTimeMillis(); - if (peer.getFirstDiscovered() == 0) { - peer.setFirstDiscovered(now); - } - peer.setLastSeen(now); + private void checkBeforeAddingToPeerTable(final DiscoveryPeer peer) { + if (peerTable.isIpAddressInvalid(peer.getEndpoint())) { + return; + } - if (peer.getStatus() != PeerDiscoveryStatus.BONDED) { - peer.setStatus(PeerDiscoveryStatus.BONDED); - connectOnRlpxLayer(peer); - } + if (peer.getFirstDiscovered() == 0L) { + connectOnRlpxLayer(peer) + .whenComplete( + (pc, th) -> { + if (th == null || !(th.getCause() instanceof TimeoutException)) { + peer.setStatus(PeerDiscoveryStatus.BONDED); + peer.setFirstDiscovered(System.currentTimeMillis()); + addToPeerTable(peer); + } else { + LOG.debug("Handshake timed out with peer {}", peer.getLoggableId(), th); + peerTable.invalidateIP(peer.getEndpoint()); + } + }); + } else { + peer.setStatus(PeerDiscoveryStatus.BONDED); + addToPeerTable(peer); + } + } - if (result.getOutcome() == PeerTable.AddResult.AddOutcome.ALREADY_EXISTED) { - // Bump peer. - peerTable.tryEvict(peer); - peerTable.tryAdd(peer); - } else if (result.getOutcome() == PeerTable.AddResult.AddOutcome.BUCKET_FULL) { - peerTable.tryEvict(result.getEvictionCandidate()); - peerTable.tryAdd(peer); - } + public void addToPeerTable(final DiscoveryPeer peer) { + final PeerTable.AddResult result = peerTable.tryAdd(peer); - return true; + if (result.getOutcome() == PeerTable.AddResult.AddOutcome.ALREADY_EXISTED) { + // Bump peer. + peerTable.tryEvict(peer); + peerTable.tryAdd(peer); + } else if (result.getOutcome() == PeerTable.AddResult.AddOutcome.BUCKET_FULL) { + peerTable.tryEvict(result.getEvictionCandidate()); + peerTable.tryAdd(peer); } - return false; } - void connectOnRlpxLayer(final DiscoveryPeer peer) { - rlpxAgent.connect(peer); + CompletableFuture connectOnRlpxLayer(final DiscoveryPeer peer) { + return rlpxAgent.connect(peer); } private Optional matchInteraction(final Packet packet) { @@ -512,7 +520,6 @@ void bond(final DiscoveryPeer peer) { return; } - peer.setFirstDiscovered(System.currentTimeMillis()); peer.setStatus(PeerDiscoveryStatus.BONDING); bondingPeers.put(peer.getId(), peer); @@ -719,7 +726,7 @@ public void handleBondingRequest(final DiscoveryPeer peer) { // Load the peer first from the table, then from bonding cache or use the instance that comes in. private DiscoveryPeer resolvePeer(final DiscoveryPeer peer) { - if (peerTable.ipAddressIsInvalid(peer.getEndpoint())) { + if (peerTable.isIpAddressInvalid(peer.getEndpoint())) { return null; } final Optional maybeKnownPeer = diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerTable.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerTable.java index 50250e1c2a0..9f58ab1af97 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerTable.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerTable.java @@ -112,7 +112,7 @@ public Optional get(final PeerId peer) { * @see AddOutcome */ public AddResult tryAdd(final DiscoveryPeer peer) { - if (ipAddressIsInvalid(peer.getEndpoint())) { + if (isIpAddressInvalid(peer.getEndpoint())) { return AddResult.invalid(); } final Bytes id = peer.getId(); @@ -212,7 +212,7 @@ public Stream streamAllPeers() { return Arrays.stream(table).flatMap(e -> e.getPeers().stream()); } - boolean ipAddressIsInvalid(final Endpoint endpoint) { + public boolean isIpAddressInvalid(final Endpoint endpoint) { final String key = getKey(endpoint); if (invalidIPs.contains(key)) { return true; @@ -223,7 +223,7 @@ boolean ipAddressIsInvalid(final Endpoint endpoint) { for (final Bucket bucket : table) { bucket.getPeers().stream() .filter(p -> p.getEndpoint().getHost().equals(endpoint.getHost())) - .forEach(p -> evictAndStore(p, bucket, key)); + .forEach(bucket::evict); } return true; } else { @@ -231,13 +231,13 @@ boolean ipAddressIsInvalid(final Endpoint endpoint) { } } - private void evictAndStore(final DiscoveryPeer peer, final Bucket bucket, final String key) { - bucket.evict(peer); + public void invalidateIP(final Endpoint endpoint) { + final String key = getKey(endpoint); invalidIPs.add(key); } private static String getKey(final Endpoint endpoint) { - return endpoint.getHost() + endpoint.getFunctionalTcpPort(); + return endpoint.getHost() + ":" + endpoint.getFunctionalTcpPort(); } /** diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/RecursivePeerRefreshState.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/RecursivePeerRefreshState.java index 3329dad4ce8..c0442e891f7 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/RecursivePeerRefreshState.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/RecursivePeerRefreshState.java @@ -202,7 +202,7 @@ private boolean satisfiesMapAdditionCriteria(final DiscoveryPeer discoPeer) { return !oneTrueMap.containsKey(discoPeer.getId()) && (initialPeers.contains(discoPeer) || !peerTable.get(discoPeer).isPresent()) && !discoPeer.getId().equals(localPeer.getId()) - && !peerTable.ipAddressIsInvalid(discoPeer.getEndpoint()); + && !peerTable.isIpAddressInvalid(discoPeer.getEndpoint()); } void onNeighboursReceived(final DiscoveryPeer peer, final List peers) { diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java index f386c59a384..045b6a9b6b2 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java @@ -16,7 +16,6 @@ import org.hyperledger.besu.cryptoservices.NodeKey; import org.hyperledger.besu.ethereum.p2p.config.RlpxConfiguration; -import org.hyperledger.besu.ethereum.p2p.discovery.DiscoveryPeer; import org.hyperledger.besu.ethereum.p2p.discovery.internal.PeerTable; import org.hyperledger.besu.ethereum.p2p.peers.LocalNode; import org.hyperledger.besu.ethereum.p2p.peers.Peer; @@ -174,10 +173,6 @@ public void subscribeIncomingConnect(final ConnectCallback callback) { public CompletableFuture connect(final Peer peer) { final CompletableFuture connectionFuture = new CompletableFuture<>(); - if (peer instanceof DiscoveryPeer) { - ((DiscoveryPeer) peer).setLastAttemptedConnection(System.currentTimeMillis()); - } - final EnodeURL enode = peer.getEnodeURL(); new Bootstrap() .group(workers) diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java index 269eb92b084..57eb6f518b9 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java @@ -182,7 +182,7 @@ public void neighborsPacketFromUnbondedPeerIsDropped() { } @Test - public void neighborsPacketLimited() { + public void neighborsPacketLimited() throws InterruptedException { // Start 20 agents with no bootstrap peers. final List otherAgents = helper.startDiscoveryAgents(20, Collections.emptyList()); @@ -192,8 +192,9 @@ public void neighborsPacketLimited() { .map(Optional::get) .collect(Collectors.toList()); - // Start another peer pointing to those 20 agents. + // Start another peer final MockPeerDiscoveryAgent agent = helper.startDiscoveryAgent(otherPeers); + // We used to do a hasSize match but we had issues with duplicate peers getting added to the // list. By moving to a contains we make sure that all the peers are loaded with tolerance for // duplicates. If we fix the duplication problem we should use containsExactlyInAnyOrder to @@ -222,7 +223,7 @@ public void neighborsPacketLimited() { final List incomingPackets = testAgent.getIncomingPackets().stream() .filter(p -> p.packet.getType().equals(PacketType.NEIGHBORS)) - .collect(toList()); + .toList(); assertThat(incomingPackets.size()).isEqualTo(1); final IncomingPacket neighborsPacket = incomingPackets.get(0); assertThat(neighborsPacket.fromAgent).isEqualTo(agent); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryBondingTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryBondingTest.java index 898a415d8a3..64f159c58bc 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryBondingTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryBondingTest.java @@ -47,11 +47,15 @@ public void pongSentUponPing() { final List otherAgentIncomingPongs = otherAgent.getIncomingPackets().stream() .filter(p -> p.packet.getType().equals(PacketType.PONG)) - .collect(Collectors.toList()); + .toList(); assertThat(otherAgentIncomingPongs.size()).isEqualTo(1); assertThat( - otherAgentIncomingPongs.get(0).packet.getPacketData(PongPacketData.class).isPresent()) + otherAgentIncomingPongs + .getFirst() + .packet + .getPacketData(PongPacketData.class) + .isPresent()) .isTrue(); final PongPacketData pong = otherAgentIncomingPongs.get(0).packet.getPacketData(PongPacketData.class).get(); diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java index c29855374ea..dbfb34de28c 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java @@ -16,6 +16,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static java.util.Arrays.asList; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -44,6 +45,7 @@ import java.util.Map; import java.util.Optional; import java.util.OptionalInt; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -301,15 +303,12 @@ public MockPeerDiscoveryAgent build() { final ForkId forkId = new ForkId(Bytes.EMPTY, Bytes.EMPTY); when(mockForkIdManager.getForkIdForChainHead()).thenReturn(forkId); when(mockForkIdManager.peerCheck(forkId)).thenReturn(true); + final RlpxAgent rlpxAgent = mock(RlpxAgent.class); + when(rlpxAgent.connect(any())) + .thenReturn(CompletableFuture.failedFuture(new RuntimeException())); final MockPeerDiscoveryAgent mockPeerDiscoveryAgent = new MockPeerDiscoveryAgent( - nodeKey, - config, - peerPermissions, - agents, - natService, - mockForkIdManager, - mock(RlpxAgent.class)); + nodeKey, config, peerPermissions, agents, natService, mockForkIdManager, rlpxAgent); mockPeerDiscoveryAgent.getAdvertisedPeer().ifPresent(peer -> peer.setNodeRecord(nodeRecord)); return mockPeerDiscoveryAgent; diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java index d75847050d4..7a5fea14d67 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java @@ -40,16 +40,13 @@ public void lastSeenAndFirstDiscoveredTimestampsUpdatedOnMessage() { final Packet pong = helper.createPongPacket(agent, Hash.hash(agentPing.getHash())); helper.sendMessageBetweenAgents(testAgent, agent, pong); - long lastSeen; long firstDiscovered; assertThat(agent.streamDiscoveredPeers()).hasSize(1); DiscoveryPeer p = agent.streamDiscoveredPeers().iterator().next(); - assertThat(p.getLastSeen()).isGreaterThan(0); assertThat(p.getFirstDiscovered()).isGreaterThan(0); - lastSeen = p.getLastSeen(); firstDiscovered = p.getFirstDiscovered(); helper.sendMessageBetweenAgents(testAgent, agent, testAgentPing); @@ -57,52 +54,6 @@ public void lastSeenAndFirstDiscoveredTimestampsUpdatedOnMessage() { assertThat(agent.streamDiscoveredPeers()).hasSize(1); p = agent.streamDiscoveredPeers().iterator().next(); - assertThat(p.getLastSeen()).isGreaterThan(lastSeen); assertThat(p.getFirstDiscovered()).isEqualTo(firstDiscovered); } - - @Test - public void lastContactedTimestampUpdatedOnOutboundMessage() { - final MockPeerDiscoveryAgent agent = helper.startDiscoveryAgent(Collections.emptyList()); - assertThat(agent.streamDiscoveredPeers()).hasSize(0); - - // Start a test peer and send a PING packet to the agent under test. - final MockPeerDiscoveryAgent testAgent = helper.startDiscoveryAgent(); - final Packet ping = helper.createPingPacket(testAgent, agent); - helper.sendMessageBetweenAgents(testAgent, agent, ping); - - assertThat(agent.streamDiscoveredPeers()).hasSize(1); - - final long lastContacted; - final long lastSeen; - final long firstDiscovered; - - DiscoveryPeer peer = agent.streamDiscoveredPeers().iterator().next(); - final long lc = peer.getLastContacted(); - final long ls = peer.getLastSeen(); - final long fd = peer.getFirstDiscovered(); - - assertThat(lc).isGreaterThan(0); - assertThat(ls).isGreaterThan(0); - assertThat(fd).isGreaterThan(0); - - lastContacted = lc; - lastSeen = ls; - firstDiscovered = fd; - - // Send another packet and ensure that timestamps are updated accordingly. - // Sleep beforehand to make sure timestamps will be different. - try { - Thread.sleep(1); - } catch (InterruptedException e) { - // Swallow exception because we only want to pause the test. - } - helper.sendMessageBetweenAgents(testAgent, agent, ping); - - peer = agent.streamDiscoveredPeers().iterator().next(); - - assertThat(peer.getLastContacted()).isGreaterThan(lastContacted); - assertThat(peer.getLastSeen()).isGreaterThan(lastSeen); - assertThat(peer.getFirstDiscovered()).isEqualTo(firstDiscovered); - } } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 1064da78e58..4005673e3d3 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -55,7 +55,9 @@ import java.util.HashMap; import java.util.List; import java.util.Optional; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -468,14 +470,13 @@ public void findNeighborsSentAfterBondingFinished() { .build(); // Mock the creation of the PING packet, so that we can control the hash, which gets validated - // when - // processing the PONG. + // when processing the PONG. final PingPacketData mockPing = PingPacketData.create( Optional.ofNullable(localPeer.getEndpoint()), peers.get(0).getEndpoint(), UInt64.ONE); final Packet mockPacket = Packet.create(PacketType.PING, mockPing, nodeKeys.get(0)); mockPingPacketCreation(mockPacket); - controller.setRetryDelayFunction((prev) -> 999999999L); + controller.setRetryDelayFunction(PeerDiscoveryControllerTest::longDelayFunction); controller.start(); // Verify that the PING was sent. @@ -506,11 +507,68 @@ public void findNeighborsSentAfterBondingFinished() { .isEqualTo(PeerDiscoveryStatus.BONDED); } + @Test + public void addedToInvalidIpsWhenConnectTimedOut() { + // Create a peer + final List nodeKeys = PeerDiscoveryTestHelper.generateNodeKeys(1); + final NodeKey nodeKey = nodeKeys.getFirst(); + final DiscoveryPeer peerThatTimesOut = helper.createDiscoveryPeers(nodeKeys).getFirst(); + + // Initialize the peer controller, using a rlpx agent that times out when asked to connect. + // Set a high controller refresh interval and a high timeout threshold, to avoid retries + // getting in the way of this test. + final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); + RlpxAgent rlpxAgentMock = mock(RlpxAgent.class); + when(rlpxAgentMock.connect(any())) + .thenReturn(CompletableFuture.failedFuture(new Exception(new TimeoutException()))); + controller = + getControllerBuilder() + .outboundMessageHandler(outboundMessageHandler) + .rlpxAgent(rlpxAgentMock) + .build(); + + // Mock the creation of the PING packet, so that we can control the hash, which gets validated + // when processing the PONG. + final PingPacketData mockPing = + PingPacketData.create( + Optional.ofNullable(localPeer.getEndpoint()), + peerThatTimesOut.getEndpoint(), + UInt64.ONE); + final Packet mockPacket = Packet.create(PacketType.PING, mockPing, nodeKey); + mockPingPacketCreation(mockPacket); + controller.setRetryDelayFunction(PeerDiscoveryControllerTest::longDelayFunction); + controller.start(); + + controller.handleBondingRequest(peerThatTimesOut); + + // Verify that the PING was sent. + verify(outboundMessageHandler, times(1)) + .send(eq(peerThatTimesOut), matchPacketOfType(PacketType.PING)); + + // Simulate a PONG message from the peer. + respondWithPong(peerThatTimesOut, nodeKey, mockPacket.getHash()); + + final List peersInTable = controller.streamDiscoveredPeers().toList(); + assertThat(peersInTable).hasSize(0); + assertThat(peersInTable).doesNotContain(peerThatTimesOut); + + // Try bonding again, and check that the peer is not sent the PING packet again + controller.handleBondingRequest(peerThatTimesOut); + + // verify that the ping was not sent, no additional interaction + verify(outboundMessageHandler, times(1)) + .send(eq(peerThatTimesOut), matchPacketOfType(PacketType.PING)); + } + private ControllerBuilder getControllerBuilder() { + final RlpxAgent rlpxAgent = mock(RlpxAgent.class); + when(rlpxAgent.connect(any())) + .thenReturn(CompletableFuture.failedFuture(new RuntimeException())); return ControllerBuilder.create() .nodeKey(localNodeKey) .localPeer(localPeer) - .peerTable(peerTable); + .peerTable(peerTable) + .rlpxAgent(rlpxAgent); } private void respondWithPong( @@ -544,7 +602,7 @@ public void peerSeenTwice() { mockPingPacketCreation(pingPacket); - controller.setRetryDelayFunction((prev) -> 999999999L); + controller.setRetryDelayFunction(PeerDiscoveryControllerTest::longDelayFunction); controller.start(); verify(outboundMessageHandler, times(1)) @@ -994,7 +1052,7 @@ public void shouldAddNewPeerWhenReceivedPongAndPeerTableBucketIsNotFull() { .build(); mockPingPacketCreation(pingPacket); - controller.setRetryDelayFunction((prev) -> 999999999L); + controller.setRetryDelayFunction(PeerDiscoveryControllerTest::longDelayFunction); controller.start(); verify(outboundMessageHandler, times(1)).send(any(), matchPacketOfType(PacketType.PING)); @@ -1689,6 +1747,7 @@ static class ControllerBuilder { private Cache enrs = CacheBuilder.newBuilder().maximumSize(50).expireAfterWrite(10, TimeUnit.SECONDS).build(); private boolean filterOnForkId = false; + private RlpxAgent rlpxAgent; public static ControllerBuilder create() { return new ControllerBuilder(); @@ -1744,6 +1803,11 @@ public ControllerBuilder filterOnForkId(final boolean filterOnForkId) { return this; } + public ControllerBuilder rlpxAgent(final RlpxAgent rlpxAgent) { + this.rlpxAgent = rlpxAgent; + return this; + } + PeerDiscoveryController build() { checkNotNull(nodeKey); if (localPeer == null) { @@ -1752,6 +1816,7 @@ PeerDiscoveryController build() { if (peerTable == null) { peerTable = new PeerTable(localPeer.getId()); } + return spy( PeerDiscoveryController.builder() .nodeKey(nodeKey) @@ -1767,7 +1832,7 @@ PeerDiscoveryController build() { .metricsSystem(new NoOpMetricsSystem()) .cacheForEnrRequests(enrs) .filterOnEnrForkId(filterOnForkId) - .rlpxAgent(mock(RlpxAgent.class)) + .rlpxAgent(rlpxAgent) .build()); } } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java index 6320c909622..b200239fee5 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import org.hyperledger.besu.cryptoservices.NodeKey; import org.hyperledger.besu.ethereum.p2p.discovery.DiscoveryPeer; @@ -34,8 +35,8 @@ import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; -import java.util.stream.Collectors; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.units.bigints.UInt64; @@ -59,6 +60,9 @@ public void tableRefreshSingleNode() { final OutboundMessageHandler outboundMessageHandler = mock(OutboundMessageHandler.class); final MockTimerUtil timer = new MockTimerUtil(); + final RlpxAgent rlpxAgent = mock(RlpxAgent.class); + when(rlpxAgent.connect(any())) + .thenReturn(CompletableFuture.failedFuture(new RuntimeException())); final PeerDiscoveryController controller = spy( PeerDiscoveryController.builder() @@ -70,7 +74,7 @@ public void tableRefreshSingleNode() { .workerExecutor(new BlockingAsyncExecutor()) .tableRefreshIntervalMs(0) .metricsSystem(new NoOpMetricsSystem()) - .rlpxAgent(mock(RlpxAgent.class)) + .rlpxAgent(rlpxAgent) .build()); controller.start(); @@ -117,7 +121,7 @@ public void tableRefreshSingleNode() { final List capturedFindNeighborsPackets = captor.getAllValues().stream() .filter(p -> p.getType().equals(PacketType.FIND_NEIGHBORS)) - .collect(Collectors.toList()); + .toList(); assertThat(capturedFindNeighborsPackets.size()).isEqualTo(5); // Collect targets from find neighbors packets diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerTableTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerTableTest.java index bcef09da817..331711a07f6 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerTableTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerTableTest.java @@ -196,7 +196,7 @@ public void ipAddressIsInvalidReturnsTrue() { final PeerTable.AddResult addResult1 = table.tryAdd(peer1); assertThat(addResult1.getOutcome()).isEqualTo(PeerTable.AddResult.added().getOutcome()); - assertThat(table.ipAddressIsInvalid(peer2.getEndpoint())).isEqualTo(true); + assertThat(table.isIpAddressInvalid(peer2.getEndpoint())).isEqualTo(true); } @Test @@ -210,7 +210,7 @@ public void ipAddressIsInvalidReturnsFalse() { final PeerTable.AddResult addResult1 = table.tryAdd(peer1); assertThat(addResult1.getOutcome()).isEqualTo(PeerTable.AddResult.added().getOutcome()); - assertThat(table.ipAddressIsInvalid(peer2.getEndpoint())).isEqualTo(false); + assertThat(table.isIpAddressInvalid(peer2.getEndpoint())).isEqualTo(false); } @Test diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 7f6a58b66e7..87747b3dcce 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -4816,12 +4816,12 @@ - - - + + + - - + + @@ -4832,52 +4832,52 @@ - - - + + + - - + + - - - + + + - - + + - - - + + + - - + + - - - + + + - - + + - - - + + + - - + + - - - + + + - - + + diff --git a/gradle/versions.gradle b/gradle/versions.gradle index 634c29b1e13..8d84a6be1de 100644 --- a/gradle/versions.gradle +++ b/gradle/versions.gradle @@ -157,7 +157,7 @@ dependencyManagement { dependency 'org.fusesource.jansi:jansi:2.4.1' - dependencySet(group: 'org.hyperledger.besu', version: '0.9.5') { + dependencySet(group: 'org.hyperledger.besu', version: '0.9.6') { entry 'arithmetic' entry 'ipa-multipoint' entry 'bls12-381' diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle index bee57ab5aef..6d3a1f93a9a 100644 --- a/plugin-api/build.gradle +++ b/plugin-api/build.gradle @@ -71,7 +71,7 @@ Calculated : ${currentHash} tasks.register('checkAPIChanges', FileStateChecker) { description = "Checks that the API for the Plugin-API project does not change without deliberate thought" files = sourceSets.main.allJava.files - knownHash = 'V/bdVbzJLjdwch266dHHuxIGwiCRhS4w3jDwHt4TWqg=' + knownHash = '5H+3gUzCwZtLByfnk11kf+kAPwykQ+WR+n3xWgyfsyY=' } check.dependsOn('checkAPIChanges') diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java index 66b1c1f2d65..1c35972a582 100644 --- a/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java +++ b/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java @@ -63,6 +63,7 @@ private enum BaseStatus implements Status { BLOBS_FULL(false, false, false), BLOCK_OCCUPANCY_ABOVE_THRESHOLD(true, false, false), BLOCK_SELECTION_TIMEOUT(true, false, false), + BLOCK_SELECTION_TIMEOUT_INVALID_TX(true, true, true), TX_EVALUATION_TOO_LONG(true, false, true), INVALID_TX_EVALUATION_TOO_LONG(true, true, true), INVALID_TRANSIENT(false, false, true), @@ -121,7 +122,11 @@ public boolean penalize() { public static final TransactionSelectionResult BLOCK_SELECTION_TIMEOUT = new TransactionSelectionResult(BaseStatus.BLOCK_SELECTION_TIMEOUT); - /** Transaction took too much to evaluate, but it was not invalid */ + /** There was no more time to add transaction to the block, and the transaction is invalid */ + public static final TransactionSelectionResult BLOCK_SELECTION_TIMEOUT_INVALID_TX = + new TransactionSelectionResult(BaseStatus.BLOCK_SELECTION_TIMEOUT_INVALID_TX); + + /** Transaction took too much to evaluate, but it was valid */ public static final TransactionSelectionResult TX_EVALUATION_TOO_LONG = new TransactionSelectionResult(BaseStatus.TX_EVALUATION_TOO_LONG);