From d0a6a70cc8c1cad977b08f6ad1e5a72796b08f13 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Mon, 30 Oct 2023 18:12:04 -0600 Subject: [PATCH 1/4] Correct reference test blobgas calculation (#6107) * Correct reference test blobgas calculation Fix tpyo that resulted in an NPE in t8n blob gas calculations. Signed-off-by: Danno Ferrin * changelog Signed-off-by: Danno Ferrin * spotless Signed-off-by: Danno Ferrin --------- Signed-off-by: Danno Ferrin --- CHANGELOG.md | 1 + .../besu/ethereum/referencetests/ReferenceTestEnv.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f77745e7d43..a1007ea0073 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ - Upgrade netty to address CVE-2023-44487, CVE-2023-34462 [#6100](https://github.com/hyperledger/besu/pull/6100) - Upgrade grpc to address CVE-2023-32731, CVE-2023-33953, CVE-2023-44487, CVE-2023-4785 [#6100](https://github.com/hyperledger/besu/pull/6100) +- Fix blob gas calculation in reference tests [#6107](https://github.com/hyperledger/besu/pull/6107) --- diff --git a/ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestEnv.java b/ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestEnv.java index 230efc4f92c..ae12be1b50e 100644 --- a/ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestEnv.java +++ b/ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestEnv.java @@ -233,7 +233,7 @@ public BlockHeader updateFromParentValues(final ProtocolSpec protocolSpec) { protocolSpec .getGasCalculator() .computeExcessBlobGas( - Long.decode(parentExcessBlobGas), Long.decode(parentGasUsed)))); + Long.decode(parentExcessBlobGas), Long.decode(parentBlobGasUsed)))); } return builder.buildBlockHeader(); From 311570f4900442fa44e2ba36617a7b9846a2ece8 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 31 Oct 2023 14:27:18 +0100 Subject: [PATCH 2/4] Restore javadoc and sources jar as trusted artifacts (#6109) Makes Idea happy again as documented here https://docs.gradle.org/6.8.3/userguide/dependency_verification.html#sec:skipping-javadocs Signed-off-by: Fabio Di Fabio --- gradle/verification-metadata.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index b3af3b6affd..efd59b95677 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -3,6 +3,10 @@ true false + + + + From de8ca108a36680f8db9e9bf59f1b5b1a6d141a0f Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 31 Oct 2023 15:00:11 +0100 Subject: [PATCH 3/4] Add API to set and get the minGasPrice at runtime (#6097) Signed-off-by: Fabio Di Fabio --- CHANGELOG.md | 1 + .../besu/ethereum/api/jsonrpc/RpcMethod.java | 2 + .../methods/miner/MinerGetMinGasPrice.java | 43 ++++++++++ .../methods/miner/MinerSetMinGasPrice.java | 59 +++++++++++++ .../jsonrpc/methods/MinerJsonRpcMethods.java | 6 +- .../miner/MinerGetMinGasPriceTest.java | 67 +++++++++++++++ .../miner/MinerSetMinGasPriceTest.java | 75 ++++++++++++++++ .../selectors/PriceTransactionSelector.java | 29 +++---- .../AbstractBlockTransactionSelectorTest.java | 86 +++++++++++++++++++ .../feemarket/TransactionPriceCalculator.java | 4 +- 10 files changed, 353 insertions(+), 19 deletions(-) create mode 100644 ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinGasPrice.java create mode 100644 ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinGasPrice.java create mode 100644 ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinGasPriceTest.java create mode 100644 ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinGasPriceTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index a1007ea0073..d1430c0daf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Ethereum Classic Spiral network upgrade [#6078](https://github.com/hyperledger/besu/pull/6078) - Add a method to read from a `Memory` instance without altering its inner state [#6073](https://github.com/hyperledger/besu/pull/6073) - Accept `input` and `data` field for the payload of transaction-related RPC methods [#6094](https://github.com/hyperledger/besu/pull/6094) +- Add APIs to set and get the min gas price a transaction must pay for being selected during block creation [#6097](https://github.com/hyperledger/besu/pull/6097) ### Bug fixes diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethod.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethod.java index 78e3f078f7d..746224f3a40 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethod.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethod.java @@ -155,6 +155,8 @@ public enum RpcMethod { MINER_STOP("miner_stop"), MINER_GET_MIN_PRIORITY_FEE("miner_getMinPriorityFee"), MINER_SET_MIN_PRIORITY_FEE("miner_setMinPriorityFee"), + MINER_GET_MIN_GAS_PRICE("miner_getMinGasPrice"), + MINER_SET_MIN_GAS_PRICE("miner_setMinGasPrice"), NET_ENODE("net_enode"), NET_LISTENING("net_listening"), NET_PEER_COUNT("net_peerCount"), diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinGasPrice.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinGasPrice.java new file mode 100644 index 00000000000..bbd53d2a18b --- /dev/null +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinGasPrice.java @@ -0,0 +1,43 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * 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.api.jsonrpc.internal.methods.miner; + +import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity; +import org.hyperledger.besu.ethereum.core.MiningParameters; + +public class MinerGetMinGasPrice implements JsonRpcMethod { + private final MiningParameters miningParameters; + + public MinerGetMinGasPrice(final MiningParameters miningParameters) { + this.miningParameters = miningParameters; + } + + @Override + public String getName() { + return RpcMethod.MINER_GET_MIN_GAS_PRICE.getMethodName(); + } + + @Override + public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { + return new JsonRpcSuccessResponse( + requestContext.getRequest().getId(), + Quantity.create(miningParameters.getMinTransactionGasPrice())); + } +} diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinGasPrice.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinGasPrice.java new file mode 100644 index 00000000000..3bb6678d9f0 --- /dev/null +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinGasPrice.java @@ -0,0 +1,59 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * 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.api.jsonrpc.internal.methods.miner; + +import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; +import org.hyperledger.besu.ethereum.core.MiningParameters; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class MinerSetMinGasPrice implements JsonRpcMethod { + private static final Logger LOG = LoggerFactory.getLogger(MinerSetMinGasPrice.class); + + private final MiningParameters miningParameters; + + public MinerSetMinGasPrice(final MiningParameters miningParameters) { + this.miningParameters = miningParameters; + } + + @Override + public String getName() { + return RpcMethod.MINER_SET_MIN_GAS_PRICE.getMethodName(); + } + + @Override + public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { + try { + final Wei minGasPrice = + Wei.fromHexString(requestContext.getRequiredParameter(0, String.class)); + miningParameters.setMinTransactionGasPrice(minGasPrice); + LOG.debug("min gas price changed to {}", minGasPrice.toHumanReadableString()); + return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); + } catch (final IllegalArgumentException invalidJsonRpcParameters) { + return new JsonRpcErrorResponse( + requestContext.getRequest().getId(), + new JsonRpcError(RpcErrorType.INVALID_PARAMS, invalidJsonRpcParameters.getMessage())); + } + } +} diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/MinerJsonRpcMethods.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/MinerJsonRpcMethods.java index 1742fe1e552..8afaf6a5873 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/MinerJsonRpcMethods.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/MinerJsonRpcMethods.java @@ -17,9 +17,11 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerChangeTargetGasLimit; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerGetMinGasPrice; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerGetMinPriorityFee; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerSetCoinbase; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerSetEtherbase; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerSetMinGasPrice; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerSetMinPriorityFee; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerStart; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.miner.MinerStop; @@ -54,6 +56,8 @@ protected Map create() { new MinerSetEtherbase(minerSetCoinbase), new MinerChangeTargetGasLimit(miningCoordinator), new MinerGetMinPriorityFee(miningParameters), - new MinerSetMinPriorityFee(miningParameters)); + new MinerSetMinPriorityFee(miningParameters), + new MinerGetMinGasPrice(miningParameters), + new MinerSetMinGasPrice(miningParameters)); } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinGasPriceTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinGasPriceTest.java new file mode 100644 index 00000000000..ebb8035b5f7 --- /dev/null +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinGasPriceTest.java @@ -0,0 +1,67 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * 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.api.jsonrpc.internal.methods.miner; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity; +import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; +import org.hyperledger.besu.ethereum.core.MiningParameters; + +import org.junit.jupiter.api.Test; + +public class MinerGetMinGasPriceTest { + + @Test + public void shouldReturnDefaultMinGasPrice() { + final MiningParameters miningParameters = ImmutableMiningParameters.newDefault(); + final MinerGetMinGasPrice method = new MinerGetMinGasPrice(miningParameters); + final JsonRpcRequestContext request = + new JsonRpcRequestContext(new JsonRpcRequest("2.0", method.getName(), new Object[] {})); + + final JsonRpcResponse expected = + new JsonRpcSuccessResponse( + request.getRequest().getId(), + Quantity.create(MiningParameters.MutableInitValues.DEFAULT_MIN_TRANSACTION_GAS_PRICE)); + + final JsonRpcResponse actual = method.response(request); + assertThat(actual).usingRecursiveComparison().isEqualTo(expected); + } + + @Test + public void shouldReturnSetAtRuntimeMinGasPrice() { + final MiningParameters miningParameters = ImmutableMiningParameters.newDefault(); + final MinerGetMinGasPrice method = new MinerGetMinGasPrice(miningParameters); + + final Wei minGasPriceAtRuntime = Wei.of(2000); + + miningParameters.setMinTransactionGasPrice(minGasPriceAtRuntime); + + final JsonRpcRequestContext request = + new JsonRpcRequestContext(new JsonRpcRequest("2.0", method.getName(), new Object[] {})); + + final JsonRpcResponse expected = + new JsonRpcSuccessResponse( + request.getRequest().getId(), Quantity.create(minGasPriceAtRuntime)); + + final JsonRpcResponse actual = method.response(request); + assertThat(actual).usingRecursiveComparison().isEqualTo(expected); + } +} diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinGasPriceTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinGasPriceTest.java new file mode 100644 index 00000000000..824b8f8fb81 --- /dev/null +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinGasPriceTest.java @@ -0,0 +1,75 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * 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.api.jsonrpc.internal.methods.miner; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.hyperledger.besu.datatypes.Wei; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; +import org.hyperledger.besu.ethereum.core.MiningParameters; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class MinerSetMinGasPriceTest { + MiningParameters miningParameters = MiningParameters.newDefault(); + private MinerSetMinGasPrice method; + + @BeforeEach + public void setUp() { + method = new MinerSetMinGasPrice(miningParameters); + } + + @Test + public void shouldReturnFalseWhenParameterIsInvalid() { + final String newMinGasPrice = "-1"; + final var request = request(newMinGasPrice); + + method.response(request); + final JsonRpcResponse expected = + new JsonRpcErrorResponse( + request.getRequest().getId(), + new JsonRpcError( + RpcErrorType.INVALID_PARAMS, + "Illegal character '-' found at index 0 in hex binary representation")); + + final JsonRpcResponse actual = method.response(request); + assertThat(actual).usingRecursiveComparison().isEqualTo(expected); + } + + @Test + public void shouldChangeMinPriorityFee() { + final String newMinGasPrice = "0x10"; + final var request = request(newMinGasPrice); + method.response(request); + final JsonRpcResponse expected = new JsonRpcSuccessResponse(request.getRequest().getId(), true); + + final JsonRpcResponse actual = method.response(request); + assertThat(actual).usingRecursiveComparison().isEqualTo(expected); + assertThat(miningParameters.getMinTransactionGasPrice()) + .isEqualTo(Wei.fromHexString(newMinGasPrice)); + } + + private JsonRpcRequestContext request(final String newMinGasPrice) { + return new JsonRpcRequestContext( + new JsonRpcRequest("2.0", method.getName(), new Object[] {newMinGasPrice})); + } +} diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java index dee4316245c..a38fd4a0a4d 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java @@ -71,30 +71,27 @@ public TransactionSelectionResult evaluateTransactionPostProcessing( */ private boolean transactionCurrentPriceBelowMin(final PendingTransaction pendingTransaction) { final Transaction transaction = pendingTransaction.getTransaction(); - // Here we only care about EIP1159 since for Frontier and local transactions the checks - // that we do when accepting them in the pool are enough - if (transaction.getType().supports1559FeeMarket() && !pendingTransaction.hasPriority()) { - - // For EIP1559 transactions, the price is dynamic and depends on network conditions, so we can - // only calculate at this time the current minimum price the transaction is willing to pay - // and if it is above the minimum accepted by the node. - // If below we do not delete the transaction, since when we added the transaction to the pool, - // we assured sure that the maxFeePerGas is >= of the minimum price accepted by the node - // and so the price of the transaction could satisfy this rule in the future - final Wei currentMinTransactionGasPriceInBlock = + // Priority txs are exempt from this check + if (!pendingTransaction.hasPriority()) { + // since the minGasPrice can change at runtime, we need to recheck it everytime + final Wei transactionGasPriceInBlock = context .feeMarket() .getTransactionPriceCalculator() .price(transaction, context.processableBlockHeader().getBaseFee()); + if (context .miningParameters() .getMinTransactionGasPrice() - .compareTo(currentMinTransactionGasPriceInBlock) + .compareTo(transactionGasPriceInBlock) > 0) { - LOG.trace( - "Current gas fee of {} is lower than configured minimum {}, skipping", - pendingTransaction, - context.miningParameters().getMinTransactionGasPrice()); + LOG.atTrace() + .setMessage( + "Current gas price of {} is {} and lower than the configured minimum {}, skipping") + .addArgument(pendingTransaction::toTraceLog) + .addArgument(transactionGasPriceInBlock) + .addArgument(context.miningParameters()::getMinTransactionGasPrice) + .log(); return true; } } 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 1ddcd264ea0..63f9476dcb3 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 @@ -737,6 +737,92 @@ public void transactionWithIncorrectNonceRemainsInPoolAndNotSelected() { TransactionInvalidReason.NONCE_TOO_HIGH.name()))); } + @Test + public void increaseOfMinGasPriceAtRuntimeExcludeTxFromBeingSelected() { + final Transaction transaction = createTransaction(0, Wei.of(7L), 100_000); + transactionPool.addRemoteTransactions(List.of(transaction)); + + ensureTransactionIsValid(transaction, 0, 5); + + final ProcessableBlockHeader blockHeader = createBlock(500_000); + + final Address miningBeneficiary = AddressHelpers.ofValue(1); + + final BlockTransactionSelector selector = + createBlockSelector( + transactionProcessor, + blockHeader, + Wei.ZERO, + miningBeneficiary, + Wei.ZERO, + MIN_OCCUPANCY_80_PERCENT); + + // raise the minGasPrice at runtime from 1 wei to 10 wei + miningParameters.setMinTransactionGasPrice(Wei.of(10)); + + final TransactionSelectionResults results = selector.buildTransactionListForBlock(); + + // now the tx gasPrice is below the new minGasPrice, it is not selected but stays in the pool + assertThat(results.getSelectedTransactions()).isEmpty(); + assertThat(results.getNotSelectedTransactions()) + .containsOnly(entry(transaction, TransactionSelectionResult.CURRENT_TX_PRICE_BELOW_MIN)); + assertThat(transactionPool.getPendingTransactions()) + .map(PendingTransaction::getTransaction) + .containsOnly(transaction); + } + + @Test + public void decreaseOfMinGasPriceAtRuntimeIncludeTxThatWasPreviouslyNotSelected() { + final Transaction transaction = createTransaction(0, Wei.of(7L), 100_000); + transactionPool.addRemoteTransactions(List.of(transaction)); + + ensureTransactionIsValid(transaction, 0, 5); + + final ProcessableBlockHeader blockHeader = createBlock(500_000); + + final Address miningBeneficiary = AddressHelpers.ofValue(1); + + final BlockTransactionSelector selector1 = + createBlockSelector( + transactionProcessor, + blockHeader, + Wei.ZERO, + miningBeneficiary, + Wei.ZERO, + MIN_OCCUPANCY_80_PERCENT); + + // raise the minGasPrice at runtime from 1 wei to 10 wei + miningParameters.setMinTransactionGasPrice(Wei.of(10)); + + final TransactionSelectionResults results1 = selector1.buildTransactionListForBlock(); + + // now the tx gasPrice is below the new minGasPrice, it is not selected but stays in the pool + assertThat(results1.getSelectedTransactions()).isEmpty(); + assertThat(results1.getNotSelectedTransactions()) + .containsOnly(entry(transaction, TransactionSelectionResult.CURRENT_TX_PRICE_BELOW_MIN)); + assertThat(transactionPool.getPendingTransactions()) + .map(PendingTransaction::getTransaction) + .containsOnly(transaction); + + // decrease the minGasPrice at runtime from 10 wei to 5 wei + miningParameters.setMinTransactionGasPrice(Wei.of(5)); + + final BlockTransactionSelector selector2 = + createBlockSelector( + transactionProcessor, + blockHeader, + Wei.ZERO, + miningBeneficiary, + Wei.ZERO, + MIN_OCCUPANCY_80_PERCENT); + + final TransactionSelectionResults results2 = selector2.buildTransactionListForBlock(); + + // now the tx gasPrice is above the new minGasPrice and it is selected + assertThat(results2.getSelectedTransactions()).contains(transaction); + assertThat(results2.getNotSelectedTransactions()).isEmpty(); + } + protected BlockTransactionSelector createBlockSelector( final MainnetTransactionProcessor transactionProcessor, final ProcessableBlockHeader blockHeader, diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/feemarket/TransactionPriceCalculator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/feemarket/TransactionPriceCalculator.java index f2b8637d591..35e2acc1464 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/feemarket/TransactionPriceCalculator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/feemarket/TransactionPriceCalculator.java @@ -37,9 +37,9 @@ static TransactionPriceCalculator eip1559() { } final Wei maxPriorityFeePerGas = transaction.getMaxPriorityFeePerGas().orElseThrow(); final Wei maxFeePerGas = transaction.getMaxFeePerGas().orElseThrow(); - Wei price = maxPriorityFeePerGas.add(baseFee); + final Wei price = maxPriorityFeePerGas.add(baseFee); if (price.compareTo(maxFeePerGas) > 0) { - price = maxFeePerGas; + return maxFeePerGas; } return price; }; From 84dee295d9c31b371bf6d68da169129ed5bb5c01 Mon Sep 17 00:00:00 2001 From: Matt Whitehead Date: Tue, 31 Oct 2023 15:12:19 +0000 Subject: [PATCH 4/4] Don't put NONCE_TOO_LOW transactions into the invalid nonce cache (#6067) * Don't put NONCE_TOO_LOW transactions into the invalid nonce cache Signed-off-by: Matthew Whitehead * Update unit tests Signed-off-by: Matthew Whitehead * Use list of errors to ignore Signed-off-by: Matthew Whitehead --------- Signed-off-by: Matthew Whitehead Signed-off-by: Matt Whitehead Signed-off-by: Fabio Di Fabio Co-authored-by: Fabio Di Fabio --- .../besu/ethereum/eth/transactions/TransactionPool.java | 7 ++++++- .../sorter/AbstractLegacyTransactionPoolTest.java | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java index c41027deb1b..9e0ca14a284 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java @@ -55,6 +55,7 @@ import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Comparator; import java.util.IntSummaryStatistics; @@ -93,6 +94,8 @@ public class TransactionPool implements BlockAddedObserver { private static final Logger LOG = LoggerFactory.getLogger(TransactionPool.class); private static final Logger LOG_FOR_REPLAY = LoggerFactory.getLogger("LOG_FOR_REPLAY"); + private static final List INVALID_TX_CACHE_IGNORED_ERRORS = + new ArrayList<>(Arrays.asList(TransactionInvalidReason.NONCE_TOO_LOW)); private final Supplier pendingTransactionsSupplier; private final PluginTransactionValidator pluginTransactionValidator; private volatile PendingTransactions pendingTransactions; @@ -277,7 +280,9 @@ private ValidationResult addTransaction( .log(); metrics.incrementRejected( isLocal, hasPriority, validationResult.result.getInvalidReason(), "txpool"); - if (!isLocal) { + if (!isLocal + && !INVALID_TX_CACHE_IGNORED_ERRORS.contains( + validationResult.result.getInvalidReason())) { pendingTransactions.signalInvalidAndRemoveDependentTransactions(transaction); } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractLegacyTransactionPoolTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractLegacyTransactionPoolTest.java index 1b35bd26516..46d735f9954 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractLegacyTransactionPoolTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractLegacyTransactionPoolTest.java @@ -16,7 +16,7 @@ import static java.util.Arrays.asList; import static java.util.Collections.singletonList; -import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.NONCE_TOO_LOW; +import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.TRANSACTION_ALREADY_KNOWN; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; @@ -38,7 +38,7 @@ public abstract class AbstractLegacyTransactionPoolTest extends AbstractTransact public void shouldNotAddRemoteTransactionsWhenThereIsALowestInvalidNonceForTheSender() { givenTransactionIsValid(transaction1); when(transactionValidatorFactory.get().validate(eq(transaction0), any(Optional.class), any())) - .thenReturn(ValidationResult.invalid(NONCE_TOO_LOW)); + .thenReturn(ValidationResult.invalid(TRANSACTION_ALREADY_KNOWN)); transactionPool.addRemoteTransactions(asList(transaction0, transaction1));