From 6b174e84f1510e6dd0914a74897be908d76ef81d Mon Sep 17 00:00:00 2001 From: Matt Whitehead Date: Tue, 13 Feb 2024 20:42:42 +0000 Subject: [PATCH 1/7] Log changes in BFT validators (#6437) * Log changes in BFT validators Signed-off-by: Matthew Whitehead * Move the log entry to the block expiry handler, not the round change validator, and only log on round 0 Signed-off-by: Matthew Whitehead * Remove unused imports Signed-off-by: Matthew Whitehead * Javadoc Signed-off-by: Matthew Whitehead * Remaining javadoc fix Signed-off-by: Matthew Whitehead * Better javadoc fix Signed-off-by: Matthew Whitehead * Spotless Signed-off-by: Matthew Whitehead * Spotless Signed-off-by: Matthew Whitehead * Merge fix Signed-off-by: Matthew Whitehead --------- Signed-off-by: Matthew Whitehead Signed-off-by: Matt Whitehead --- .../statemachine/IbftBlockHeightManager.java | 28 ++++++++++++++++ .../ibft/statemachine/IbftRound.java | 4 ++- .../validation/MessageValidatorFactory.java | 31 +++++++++++++++--- .../statemachine/QbftBlockHeightManager.java | 28 ++++++++++++++++ .../validation/MessageValidatorFactory.java | 32 ++++++++++++++++--- 5 files changed, 114 insertions(+), 9 deletions(-) diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftBlockHeightManager.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftBlockHeightManager.java index 371cdb61f77..64dd67bc793 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftBlockHeightManager.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftBlockHeightManager.java @@ -32,6 +32,7 @@ import org.hyperledger.besu.consensus.ibft.payload.MessageFactory; import org.hyperledger.besu.consensus.ibft.validation.FutureRoundProposalMessageValidator; import org.hyperledger.besu.consensus.ibft.validation.MessageValidatorFactory; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException; @@ -122,6 +123,9 @@ public IbftBlockHeightManager( @Override public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifier) { + + logValidatorChanges(currentRound); + if (roundIdentifier.equals(currentRound.getRoundIdentifier())) { final long headerTimeStampSeconds = Math.round(clock.millis() / 1000D); currentRound.createAndSendProposalMessage(headerTimeStampSeconds); @@ -133,6 +137,30 @@ public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifie } } + /** + * If the list of validators for the next block to be proposed/imported has changed from the + * previous block, log the change. Only log for round 0 (i.e. once per block). + * + * @param ibftRound The current round + */ + private void logValidatorChanges(final IbftRound ibftRound) { + if (ibftRound.getRoundIdentifier().getRoundNumber() == 0) { + final Collection
previousValidators = + MessageValidatorFactory.getValidatorsForBlock(ibftRound.protocolContext, parentHeader); + final Collection
validatorsForHeight = + MessageValidatorFactory.getValidatorsAfterBlock(ibftRound.protocolContext, parentHeader); + if (!(validatorsForHeight.containsAll(previousValidators)) + || !(previousValidators.containsAll(validatorsForHeight))) { + LOG.info( + "Validator list change. Previous chain height {}: {}. Current chain height {}: {}.", + parentHeader.getNumber(), + previousValidators, + parentHeader.getNumber() + 1, + validatorsForHeight); + } + } + } + @Override public void roundExpired(final RoundExpiry expire) { if (!expire.getView().equals(currentRound.getRoundIdentifier())) { diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java index de6af7b4f79..ec4405e0485 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java @@ -57,7 +57,9 @@ public class IbftRound { private final Subscribers observers; private final RoundState roundState; private final BlockCreator blockCreator; - private final ProtocolContext protocolContext; + /** The protocol context. */ + protected final ProtocolContext protocolContext; + private final ProtocolSchedule protocolSchedule; private final NodeKey nodeKey; private final MessageFactory messageFactory; // used only to create stored local msgs diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/validation/MessageValidatorFactory.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/validation/MessageValidatorFactory.java index 5c61bd68d32..308058bec11 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/validation/MessageValidatorFactory.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/validation/MessageValidatorFactory.java @@ -54,18 +54,41 @@ public MessageValidatorFactory( this.bftExtraDataCodec = bftExtraDataCodec; } - private Collection
getValidatorsAfterBlock(final BlockHeader parentHeader) { + /** + * Get the list of validators that are applicable after the given block + * + * @param protocolContext the protocol context + * @param parentHeader the parent header + * @return the list of validators + */ + public static Collection
getValidatorsAfterBlock( + final ProtocolContext protocolContext, final BlockHeader parentHeader) { return protocolContext .getConsensusContext(BftContext.class) .getValidatorProvider() .getValidatorsAfterBlock(parentHeader); } + /** + * Get the list of validators that are applicable for the given block + * + * @param protocolContext the protocol context + * @param parentHeader the parent header + * @return the list of validators + */ + public static Collection
getValidatorsForBlock( + final ProtocolContext protocolContext, final BlockHeader parentHeader) { + return protocolContext + .getConsensusContext(BftContext.class) + .getValidatorProvider() + .getValidatorsForBlock(parentHeader); + } + private SignedDataValidator createSignedDataValidator( final ConsensusRoundIdentifier roundIdentifier, final BlockHeader parentHeader) { return new SignedDataValidator( - getValidatorsAfterBlock(parentHeader), + getValidatorsAfterBlock(protocolContext, parentHeader), proposerSelector.selectProposerForRound(roundIdentifier), roundIdentifier); } @@ -79,7 +102,7 @@ private SignedDataValidator createSignedDataValidator( */ public MessageValidator createMessageValidator( final ConsensusRoundIdentifier roundIdentifier, final BlockHeader parentHeader) { - final Collection
validators = getValidatorsAfterBlock(parentHeader); + final Collection
validators = getValidatorsAfterBlock(protocolContext, parentHeader); final BftBlockInterface bftBlockInterface = protocolContext.getConsensusContext(BftContext.class).getBlockInterface(); @@ -106,7 +129,7 @@ public MessageValidator createMessageValidator( */ public RoundChangeMessageValidator createRoundChangeMessageValidator( final long chainHeight, final BlockHeader parentHeader) { - final Collection
validators = getValidatorsAfterBlock(parentHeader); + final Collection
validators = getValidatorsAfterBlock(protocolContext, parentHeader); final BftBlockInterface bftBlockInterface = protocolContext.getConsensusContext(BftContext.class).getBlockInterface(); diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java index 45e97ed6795..0940d35df6b 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java @@ -27,6 +27,7 @@ import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; import org.hyperledger.besu.consensus.qbft.validation.FutureRoundProposalMessageValidator; import org.hyperledger.besu.consensus.qbft.validation.MessageValidatorFactory; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException; @@ -126,6 +127,9 @@ public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifie startNewRound(0); final QbftRound qbftRound = currentRound.get(); + + logValidatorChanges(qbftRound); + // mining will be checked against round 0 as the current round is initialised to 0 above final boolean isProposer = finalState.isLocalNodeProposerForRound(qbftRound.getRoundIdentifier()); @@ -143,6 +147,30 @@ public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifie } } + /** + * If the list of validators for the next block to be proposed/imported has changed from the + * previous block, log the change. Only log for round 0 (i.e. once per block). + * + * @param qbftRound The current round + */ + private void logValidatorChanges(final QbftRound qbftRound) { + if (qbftRound.getRoundIdentifier().getRoundNumber() == 0) { + final Collection
previousValidators = + MessageValidatorFactory.getValidatorsForBlock(qbftRound.protocolContext, parentHeader); + final Collection
validatorsForHeight = + MessageValidatorFactory.getValidatorsAfterBlock(qbftRound.protocolContext, parentHeader); + if (!(validatorsForHeight.containsAll(previousValidators)) + || !(previousValidators.containsAll(validatorsForHeight))) { + LOG.info( + "Validator list change. Previous chain height {}: {}. Current chain height {}: {}.", + parentHeader.getNumber(), + previousValidators, + parentHeader.getNumber() + 1, + validatorsForHeight); + } + } + } + @Override public void roundExpired(final RoundExpiry expire) { if (currentRound.isEmpty()) { diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/MessageValidatorFactory.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/MessageValidatorFactory.java index 24ab9c8666e..b5164746bf6 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/MessageValidatorFactory.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validation/MessageValidatorFactory.java @@ -55,13 +55,36 @@ public MessageValidatorFactory( this.bftExtraDataCodec = bftExtraDataCodec; } - private Collection
getValidatorsAfterBlock(final BlockHeader parentHeader) { + /** + * Get the list of validators that are applicable after the given block + * + * @param protocolContext the protocol context + * @param parentHeader the parent header + * @return the list of validators + */ + public static Collection
getValidatorsAfterBlock( + final ProtocolContext protocolContext, final BlockHeader parentHeader) { return protocolContext .getConsensusContext(BftContext.class) .getValidatorProvider() .getValidatorsAfterBlock(parentHeader); } + /** + * Get the list of validators that are applicable for the given block + * + * @param protocolContext the protocol context + * @param parentHeader the parent header + * @return the list of validators + */ + public static Collection
getValidatorsForBlock( + final ProtocolContext protocolContext, final BlockHeader parentHeader) { + return protocolContext + .getConsensusContext(BftContext.class) + .getValidatorProvider() + .getValidatorsForBlock(parentHeader); + } + /** * Create round change message validator. * @@ -72,7 +95,8 @@ private Collection
getValidatorsAfterBlock(final BlockHeader parentHead public RoundChangeMessageValidator createRoundChangeMessageValidator( final long chainHeight, final BlockHeader parentHeader) { - final Collection
validatorsForHeight = getValidatorsAfterBlock(parentHeader); + final Collection
validatorsForHeight = + getValidatorsAfterBlock(protocolContext, parentHeader); final RoundChangePayloadValidator roundChangePayloadValidator = new RoundChangePayloadValidator(validatorsForHeight, chainHeight); @@ -95,8 +119,8 @@ public RoundChangeMessageValidator createRoundChangeMessageValidator( */ public MessageValidator createMessageValidator( final ConsensusRoundIdentifier roundIdentifier, final BlockHeader parentHeader) { - - final Collection
validatorsForHeight = getValidatorsAfterBlock(parentHeader); + final Collection
validatorsForHeight = + getValidatorsAfterBlock(protocolContext, parentHeader); final ProposalValidator proposalValidator = new ProposalValidator( From 2160c1155459e5a5702747a423e4743896cdf8cc Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 14 Feb 2024 09:03:41 +1000 Subject: [PATCH 2/7] add profile info to help footer (#6493) * add profile info to footer Signed-off-by: Sally MacFarlane --------- Signed-off-by: Sally MacFarlane --- .../main/java/org/hyperledger/besu/cli/BesuCommand.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 0d252d203f5..201568ac8fd 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -255,8 +255,12 @@ synopsisHeading = "%n", descriptionHeading = "%n@|bold,fg(cyan) Description:|@%n%n", optionListHeading = "%n@|bold,fg(cyan) Options:|@%n", - footerHeading = "%n", - footer = "Besu is licensed under the Apache License 2.0") + footerHeading = "%nBesu is licensed under the Apache License 2.0%n", + footer = { + "%n%n@|fg(cyan) To get started quickly, just choose a network to sync and a profile to run with suggested defaults:|@", + "%n@|fg(cyan) for Mainnet|@ --network=mainnet --profile=[minimalist_staker|staker]", + "%nMore info and other profiles at https://besu.hyperledger.org%n" + }) public class BesuCommand implements DefaultCommandValues, Runnable { @SuppressWarnings("PrivateStaticFinalLoggers") From 61c7e18ab446a7383bafc2e10d96a88595cf1986 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 14 Feb 2024 15:42:12 +1000 Subject: [PATCH 3/7] Add tests for user profiles (#6568) * test each profile * removed duplicated line * use non-deprecated name for sync mode Signed-off-by: Sally MacFarlane --------- Signed-off-by: Sally MacFarlane --- .../cli/CascadingDefaultProviderTest.java | 2 - .../hyperledger/besu/cli/ProfilesTest.java | 37 +++++++++++++++++++ .../resources/profiles/minimalist-staker.toml | 2 +- 3 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 besu/src/test/java/org/hyperledger/besu/cli/ProfilesTest.java diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CascadingDefaultProviderTest.java b/besu/src/test/java/org/hyperledger/besu/cli/CascadingDefaultProviderTest.java index e9707f1dfde..fcba28bd61b 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CascadingDefaultProviderTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CascadingDefaultProviderTest.java @@ -182,8 +182,6 @@ public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() { assertThat(syncConfig.getSyncMode()).isEqualTo(SyncMode.FAST); assertThat(syncConfig.getFastSyncMinimumPeerCount()).isEqualTo(5); - assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); - assertThat(commandOutput.toString(UTF_8)).isEmpty(); assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/ProfilesTest.java b/besu/src/test/java/org/hyperledger/besu/cli/ProfilesTest.java new file mode 100644 index 00000000000..3c1a5f75cf5 --- /dev/null +++ b/besu/src/test/java/org/hyperledger/besu/cli/ProfilesTest.java @@ -0,0 +1,37 @@ +/* + * 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.cli; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; + +import org.hyperledger.besu.cli.config.ProfileName; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +public class ProfilesTest extends CommandTestAbstract { + + /** Test if besu will validate the combination of options within the given profile. */ + @ParameterizedTest + @EnumSource(ProfileName.class) + public void testProfileWithNoOverrides_doesNotError(final ProfileName profileName) { + + parseCommand("--profile", profileName.name()); + + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } +} diff --git a/config/src/main/resources/profiles/minimalist-staker.toml b/config/src/main/resources/profiles/minimalist-staker.toml index f465b00bf43..fcdbf64363f 100644 --- a/config/src/main/resources/profiles/minimalist-staker.toml +++ b/config/src/main/resources/profiles/minimalist-staker.toml @@ -1,4 +1,4 @@ -sync-mode="X_CHECKPOINT" +sync-mode="CHECKPOINT" data-storage-format="BONSAI" bonsai-historical-block-limit=128 max-peers=25 \ No newline at end of file From 8b64023a121ea996ef60e4b7e2299c5807683f90 Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Wed, 14 Feb 2024 14:35:23 -0500 Subject: [PATCH 4/7] Blob gas fee checking (#6546) * invalidate txs below blob base fee * updates tests to provide blob prices * correction to reftests --------- Signed-off-by: Justin Florentine Co-authored-by: Fabio Di Fabio --- .../besu/services/BesuEventsImplTest.java | 5 +- .../api/jsonrpc/EngineJsonRpcService.java | 1 + .../mainnet/MainnetTransactionProcessor.java | 5 +- .../mainnet/MainnetTransactionValidator.java | 19 +++- .../PermissionTransactionValidator.java | 3 +- .../mainnet/TransactionValidationParams.java | 2 +- .../mainnet/TransactionValidator.java | 1 + .../transaction/TransactionInvalidReason.java | 1 + .../MainnetTransactionProcessorTest.java | 6 +- .../MainnetTransactionValidatorTest.java | 91 ++++++++++++++++--- .../eth/transactions/TransactionPool.java | 4 +- .../AbstractTransactionPoolTest.java | 12 ++- .../besu/ethereum/core/TransactionTest.java | 2 +- 13 files changed, 124 insertions(+), 28 deletions(-) diff --git a/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java b/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java index 93cf830faf1..87b6255b593 100644 --- a/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java +++ b/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java @@ -137,7 +137,10 @@ public void setUp() { .thenReturn(mockTransactionValidatorFactory); lenient().when(mockProtocolSpec.getFeeMarket()).thenReturn(FeeMarket.london(0L)); lenient() - .when(mockTransactionValidatorFactory.get().validate(any(), any(Optional.class), any())) + .when( + mockTransactionValidatorFactory + .get() + .validate(any(), any(Optional.class), any(Optional.class), any())) .thenReturn(ValidationResult.valid()); lenient() .when(mockTransactionValidatorFactory.get().validateForSender(any(), any(), any())) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java index fad3bffce6c..2e0fd4064d3 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/EngineJsonRpcService.java @@ -428,6 +428,7 @@ private Router buildRouter() { .handler( BodyHandler.create() .setUploadsDirectory(dataDir.resolve("uploads").toString()) + .setBodyLimit(128 * 1024 * 1024) .setDeleteUploadedFilesOnEnd(true)); router.route("/").method(HttpMethod.GET).handler(this::handleEmptyRequest); router 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 4ec3eea8605..1756501814c 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 @@ -273,7 +273,10 @@ public TransactionProcessingResult processTransaction( LOG.trace("Starting execution of {}", transaction); ValidationResult validationResult = transactionValidator.validate( - transaction, blockHeader.getBaseFee(), transactionValidationParams); + transaction, + blockHeader.getBaseFee(), + Optional.ofNullable(blobGasPrice), + transactionValidationParams); // Make sure the transaction is intrinsically valid before trying to // compare against a sender account (because the transaction may not // be signed correctly to extract the sender). diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java index 5fd841c1c39..1e8c620ad16 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java @@ -83,6 +83,7 @@ public MainnetTransactionValidator( public ValidationResult validate( final Transaction transaction, final Optional baseFee, + final Optional blobFee, final TransactionValidationParams transactionValidationParams) { final ValidationResult signatureResult = validateTransactionSignature(transaction); @@ -128,17 +129,18 @@ public ValidationResult validate( transaction.getPayload().size(), maxInitcodeSize)); } - return validateCostAndFee(transaction, baseFee, transactionValidationParams); + return validateCostAndFee(transaction, baseFee, blobFee, transactionValidationParams); } private ValidationResult validateCostAndFee( final Transaction transaction, final Optional maybeBaseFee, + final Optional maybeBlobFee, final TransactionValidationParams transactionValidationParams) { if (maybeBaseFee.isPresent()) { final Wei price = feeMarket.getTransactionPriceCalculator().price(transaction, maybeBaseFee); - if (!transactionValidationParams.isAllowMaxFeeGasBelowBaseFee() + if (!transactionValidationParams.allowUnderpriced() && price.compareTo(maybeBaseFee.orElseThrow()) < 0) { return ValidationResult.invalid( TransactionInvalidReason.GAS_PRICE_BELOW_CURRENT_BASE_FEE, @@ -168,6 +170,19 @@ private ValidationResult validateCostAndFee( "total blob gas %d exceeds max blob gas per block %d", txTotalBlobGas, gasLimitCalculator.currentBlobGasLimit())); } + if (maybeBlobFee.isEmpty()) { + throw new IllegalArgumentException( + "blob fee must be provided from blocks containing blobs"); + } else if (!transactionValidationParams.allowUnderpriced() + && maybeBlobFee.get().compareTo(transaction.getMaxFeePerBlobGas().get()) > 0) { + return ValidationResult.invalid( + TransactionInvalidReason.BLOB_GAS_PRICE_BELOW_CURRENT_BLOB_BASE_FEE, + String.format( + "max fee per blob gas less than block blob gas fee: address %s blobGasFeeCap: %s, blobBaseFee: %s", + transaction.getSender().toHexString(), + transaction.getMaxFeePerBlobGas().get().toHumanReadableString(), + maybeBlobFee.get().toHumanReadableString())); + } } final long intrinsicGasCost = diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PermissionTransactionValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PermissionTransactionValidator.java index 5325567d0f9..43b3d8d2be1 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PermissionTransactionValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PermissionTransactionValidator.java @@ -43,8 +43,9 @@ public PermissionTransactionValidator( public ValidationResult validate( final Transaction transaction, final Optional baseFee, + final Optional blobBaseFee, final TransactionValidationParams transactionValidationParams) { - return delegate.validate(transaction, baseFee, transactionValidationParams); + return delegate.validate(transaction, baseFee, blobBaseFee, transactionValidationParams); } @Override diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidationParams.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidationParams.java index c60f376907d..be2185b09a7 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidationParams.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidationParams.java @@ -46,7 +46,7 @@ default boolean isAllowExceedingBalance() { } @Value.Default - default boolean isAllowMaxFeeGasBelowBaseFee() { + default boolean allowUnderpriced() { return false; } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidator.java index a70de0bcd7f..dc96ac46fcd 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidator.java @@ -35,6 +35,7 @@ public interface TransactionValidator { ValidationResult validate( Transaction transaction, Optional baseFee, + Optional blobBaseFee, TransactionValidationParams transactionValidationParams); /** 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 55d7e431c8b..acdb73c6978 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 @@ -43,6 +43,7 @@ public enum TransactionInvalidReason { TOTAL_BLOB_GAS_TOO_HIGH, GAS_PRICE_TOO_LOW, GAS_PRICE_BELOW_CURRENT_BASE_FEE, + BLOB_GAS_PRICE_BELOW_CURRENT_BLOB_BASE_FEE, MAX_FEE_PER_GAS_BELOW_CURRENT_BASE_FEE, TX_FEECAP_EXCEEDED, INTERNAL_ERROR, diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessorTest.java index 3e40784ad42..ae95014150a 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessorTest.java @@ -103,7 +103,7 @@ void shouldWarmCoinbaseIfRequested() { when(transaction.getPayload()).thenReturn(Bytes.EMPTY); when(transaction.getSender()).thenReturn(senderAddress); when(transaction.getValue()).thenReturn(Wei.ZERO); - when(transactionValidatorFactory.get().validate(any(), any(), any())) + when(transactionValidatorFactory.get().validate(any(), any(), any(), any())) .thenReturn(ValidationResult.valid()); when(transactionValidatorFactory.get().validateForSender(any(), any(), any())) .thenReturn(ValidationResult.valid()); @@ -168,7 +168,7 @@ void shouldTraceEndTxOnFailingTransaction(final Exception exception) { when(transaction.getPayload()).thenReturn(Bytes.EMPTY); when(transaction.getSender()).thenReturn(senderAddress); when(transaction.getValue()).thenReturn(Wei.ZERO); - when(transactionValidatorFactory.get().validate(any(), any(), any())) + when(transactionValidatorFactory.get().validate(any(), any(), any(), any())) .thenReturn(ValidationResult.valid()); when(transactionValidatorFactory.get().validateForSender(any(), any(), any())) .thenReturn(ValidationResult.valid()); @@ -255,7 +255,7 @@ void shouldCallTransactionValidatorWithExpectedTransactionValidationParams() { private ArgumentCaptor transactionValidationParamCaptor() { final ArgumentCaptor txValidationParamCaptor = ArgumentCaptor.forClass(TransactionValidationParams.class); - when(transactionValidatorFactory.get().validate(any(), any(), any())) + when(transactionValidatorFactory.get().validate(any(), any(), any(), any())) .thenReturn(ValidationResult.valid()); // returning invalid transaction to halt method execution when(transactionValidatorFactory diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java index 5ab351c2efa..622d868a0ba 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidatorTest.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.mainnet.TransactionValidationParams.processingBlockParams; import static org.hyperledger.besu.ethereum.mainnet.TransactionValidationParams.transactionPoolParams; +import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.BLOB_GAS_PRICE_BELOW_CURRENT_BLOB_BASE_FEE; import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.GAS_PRICE_BELOW_CURRENT_BASE_FEE; import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.INVALID_TRANSACTION_FORMAT; import static org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason.MAX_PRIORITY_FEE_PER_GAS_EXCEEDS_MAX_FEE_PER_GAS; @@ -128,7 +129,9 @@ public void shouldRejectTransactionIfIntrinsicGasExceedsGasLimit() { .createTransaction(senderKeys); when(gasCalculator.transactionIntrinsicGasCost(any(), anyBoolean())).thenReturn(50L); - assertThat(validator.validate(transaction, Optional.empty(), transactionValidationParams)) + assertThat( + validator.validate( + transaction, Optional.empty(), Optional.empty(), transactionValidationParams)) .isEqualTo( ValidationResult.invalid(TransactionInvalidReason.INTRINSIC_GAS_EXCEEDS_GAS_LIMIT)); } @@ -138,7 +141,9 @@ public void shouldRejectTransactionWhenTransactionHasChainIdAndValidatorDoesNot( final TransactionValidator validator = createTransactionValidator( gasCalculator, GasLimitCalculator.constant(), false, Optional.empty()); - assertThat(validator.validate(basicTransaction, Optional.empty(), transactionValidationParams)) + assertThat( + validator.validate( + basicTransaction, Optional.empty(), Optional.empty(), transactionValidationParams)) .isEqualTo( ValidationResult.invalid( TransactionInvalidReason.REPLAY_PROTECTED_SIGNATURES_NOT_SUPPORTED)); @@ -152,7 +157,9 @@ public void shouldRejectTransactionWhenTransactionHasIncorrectChainId() { GasLimitCalculator.constant(), false, Optional.of(BigInteger.valueOf(2))); - assertThat(validator.validate(basicTransaction, Optional.empty(), transactionValidationParams)) + assertThat( + validator.validate( + basicTransaction, Optional.empty(), Optional.empty(), transactionValidationParams)) .isEqualTo(ValidationResult.invalid(TransactionInvalidReason.WRONG_CHAIN_ID)); } @@ -300,13 +307,60 @@ public void shouldRejectTransactionWithMaxPriorityFeeGreaterThanMaxFee() { .signAndBuild(new SECP256K1().generateKeyPair()); final ValidationResult validationResult = - validator.validate(transaction, Optional.of(Wei.ONE), transactionValidationParams); + validator.validate( + transaction, Optional.of(Wei.ONE), Optional.empty(), transactionValidationParams); assertThat(validationResult) .isEqualTo(ValidationResult.invalid(MAX_PRIORITY_FEE_PER_GAS_EXCEEDS_MAX_FEE_PER_GAS)); assertThat(validationResult.getErrorMessage()) .isEqualTo("max priority fee per gas cannot be greater than max fee per gas"); } + @Test + public void shouldRejectTransactionWithMaxBlobPriorityFeeSmallerThanBlobBaseFee() { + final TransactionValidator validator = + createTransactionValidator( + gasCalculator, + GasLimitCalculator.constant(), + FeeMarket.cancun(0L, Optional.empty()), + false, + Optional.of(BigInteger.ONE), + Set.of( + new TransactionType[] { + TransactionType.FRONTIER, + TransactionType.ACCESS_LIST, + TransactionType.EIP1559, + TransactionType.BLOB + }), + Integer.MAX_VALUE); + + BlobTestFixture blobTestFixture = new BlobTestFixture(); + BlobsWithCommitments bwc = blobTestFixture.createBlobsWithCommitments(1); + + final Transaction transaction = + new TransactionTestFixture() + .to(Optional.of(Address.fromHexString("0xDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF"))) + .type(TransactionType.BLOB) + .chainId(Optional.of(BigInteger.ONE)) + .maxFeePerGas(Optional.of(Wei.of(15))) + .maxFeePerBlobGas(Optional.of(Wei.of(7))) + .maxPriorityFeePerGas(Optional.of(Wei.of(1))) + .blobsWithCommitments(Optional.of(bwc)) + .versionedHashes(Optional.of(bwc.getVersionedHashes())) + .createTransaction(senderKeys); + + final ValidationResult validationResult = + validator.validate( + transaction, + Optional.of(Wei.ONE), + Optional.of(Wei.of(10)), + transactionValidationParams); + assertThat(validationResult) + .isEqualTo(ValidationResult.invalid(BLOB_GAS_PRICE_BELOW_CURRENT_BLOB_BASE_FEE)); + assertThat(validationResult.getErrorMessage()) + .matches( + "max fee per blob gas less than block blob gas fee: address 0x[0-9a-f]+ blobGasFeeCap: 7 wei, blobBaseFee: 10 wei"); + } + @Test public void shouldAcceptOnlyTransactionsInAcceptedTransactionTypes() { final TransactionValidator frontierValidator = @@ -339,14 +393,15 @@ public void shouldAcceptOnlyTransactionsInAcceptedTransactionTypes() { .createTransaction(senderKeys); assertThat( - frontierValidator.validate(transaction, Optional.empty(), transactionValidationParams)) + frontierValidator.validate( + transaction, Optional.empty(), Optional.empty(), transactionValidationParams)) .isEqualTo(ValidationResult.invalid(INVALID_TRANSACTION_FORMAT)); when(gasCalculator.transactionIntrinsicGasCost(any(), anyBoolean())).thenReturn(0L); assertThat( eip1559Validator.validate( - transaction, Optional.of(Wei.ONE), transactionValidationParams)) + transaction, Optional.of(Wei.ONE), Optional.empty(), transactionValidationParams)) .isEqualTo(ValidationResult.valid()); } @@ -369,7 +424,8 @@ public void shouldRejectTransactionIfEIP1559TransactionGasPriceLessBaseFee() { .chainId(Optional.of(BigInteger.ONE)) .createTransaction(senderKeys); final Optional basefee = Optional.of(Wei.of(150000L)); - assertThat(validator.validate(transaction, basefee, transactionValidationParams)) + assertThat( + validator.validate(transaction, basefee, Optional.empty(), transactionValidationParams)) .isEqualTo(ValidationResult.invalid(GAS_PRICE_BELOW_CURRENT_BASE_FEE)); } @@ -393,7 +449,9 @@ public void shouldAcceptZeroGasPriceTransactionIfBaseFeeIsZero() { .chainId(Optional.of(BigInteger.ONE)) .createTransaction(senderKeys); - assertThat(validator.validate(transaction, zeroBaseFee, transactionValidationParams)) + assertThat( + validator.validate( + transaction, zeroBaseFee, Optional.empty(), transactionValidationParams)) .isEqualTo(ValidationResult.valid()); } @@ -418,7 +476,8 @@ public void shouldAcceptValidEIP1559() { final Optional basefee = Optional.of(Wei.of(150000L)); when(gasCalculator.transactionIntrinsicGasCost(any(), anyBoolean())).thenReturn(50L); - assertThat(validator.validate(transaction, basefee, transactionValidationParams)) + assertThat( + validator.validate(transaction, basefee, Optional.empty(), transactionValidationParams)) .isEqualTo(ValidationResult.valid()); } @@ -444,7 +503,10 @@ public void shouldValidate1559TransactionWithPriceLowerThanBaseFeeForTransaction assertThat( validator.validate( - transaction, Optional.of(Wei.ONE), TransactionValidationParams.transactionPool())) + transaction, + Optional.of(Wei.ONE), + Optional.empty(), + TransactionValidationParams.transactionPool())) .isEqualTo(ValidationResult.valid()); } @@ -466,7 +528,8 @@ public void shouldRejectTooLargeInitcode() { .chainId(Optional.of(BigInteger.ONE)) .createTransaction(senderKeys); var validationResult = - validator.validate(bigPayload, Optional.empty(), transactionValidationParams); + validator.validate( + bigPayload, Optional.empty(), Optional.empty(), transactionValidationParams); assertThat(validationResult.isValid()).isFalse(); assertThat(validationResult.getInvalidReason()) @@ -511,7 +574,8 @@ public void shouldRejectContractCreateWithBlob() { .versionedHashes(Optional.of(List.of(VersionedHash.DEFAULT_VERSIONED_HASH))) .createTransaction(senderKeys); var validationResult = - validator.validate(blobTx, Optional.empty(), transactionValidationParams); + validator.validate( + blobTx, Optional.empty(), Optional.of(Wei.of(15)), transactionValidationParams); if (!validationResult.isValid()) { System.out.println( validationResult.getInvalidReason() + " " + validationResult.getErrorMessage()); @@ -549,7 +613,8 @@ public void shouldAcceptTransactionWithAtLeastOneBlob() { .versionedHashes(Optional.of(bwc.getVersionedHashes())) .createTransaction(senderKeys); var validationResult = - validator.validate(blobTx, Optional.empty(), transactionValidationParams); + validator.validate( + blobTx, Optional.empty(), Optional.of(Wei.of(15)), transactionValidationParams); if (!validationResult.isValid()) { System.out.println( validationResult.getInvalidReason() + " " + validationResult.getErrorMessage()); 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 a3d164105d0..dab6d13a240 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 @@ -399,7 +399,6 @@ private ValidationResultAndAccount validateTransaction( final FeeMarket feeMarket = protocolSchedule.getByBlockHeader(chainHeadBlockHeader).getFeeMarket(); - final TransactionInvalidReason priceInvalidReason = validatePrice(transaction, isLocal, hasPriority, feeMarket); if (priceInvalidReason != null) { @@ -411,6 +410,9 @@ private ValidationResultAndAccount validateTransaction( .validate( transaction, chainHeadBlockHeader.getBaseFee(), + Optional.of( + Wei.ZERO), // TransactionValidationParams.transactionPool() allows underpriced + // txs TransactionValidationParams.transactionPool()); if (!basicValidationResult.isValid()) { return new ValidationResultAndAccount(basicValidationResult); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java index c34b4f0906d..040a653fa99 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java @@ -551,11 +551,11 @@ public void shouldNotAddRemoteTransactionsThatAreInvalidAccordingToStateDependen assertTransactionNotPending(transaction1); verify(transactionBroadcaster).onTransactionsAdded(singletonList(transaction0)); verify(transactionValidatorFactory.get()) - .validate(eq(transaction0), any(Optional.class), any()); + .validate(eq(transaction0), any(Optional.class), any(Optional.class), any()); verify(transactionValidatorFactory.get()) .validateForSender(eq(transaction0), eq(null), any(TransactionValidationParams.class)); verify(transactionValidatorFactory.get()) - .validate(eq(transaction1), any(Optional.class), any()); + .validate(eq(transaction1), any(Optional.class), any(Optional.class), any()); verify(transactionValidatorFactory.get()).validateForSender(eq(transaction1), any(), any()); verifyNoMoreInteractions(transactionValidatorFactory.get()); } @@ -726,7 +726,9 @@ public void shouldCallValidatorWithExpectedValidationParameters() { final ArgumentCaptor txValidationParamCaptor = ArgumentCaptor.forClass(TransactionValidationParams.class); - when(transactionValidatorFactory.get().validate(eq(transaction0), any(Optional.class), any())) + when(transactionValidatorFactory + .get() + .validate(eq(transaction0), any(Optional.class), any(Optional.class), any())) .thenReturn(valid()); when(transactionValidatorFactory .get() @@ -1345,7 +1347,9 @@ protected void addAndAssertTransactionViaApiInvalid( @SuppressWarnings("unchecked") protected void givenTransactionIsValid(final Transaction transaction) { - when(transactionValidatorFactory.get().validate(eq(transaction), any(Optional.class), any())) + when(transactionValidatorFactory + .get() + .validate(eq(transaction), any(Optional.class), any(Optional.class), any())) .thenReturn(valid()); when(transactionValidatorFactory .get() diff --git a/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/core/TransactionTest.java b/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/core/TransactionTest.java index 45ba21c7e0c..bf17f2f96ec 100644 --- a/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/core/TransactionTest.java +++ b/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/core/TransactionTest.java @@ -178,7 +178,7 @@ public void milestone( final Transaction transaction = Transaction.readFrom(RLP.input(rlp)); final ValidationResult validation = transactionValidator(milestone) - .validate(transaction, baseFee, TransactionValidationParams.processingBlock()); + .validate(transaction, baseFee, Optional.empty(), TransactionValidationParams.processingBlock()); if (!validation.isValid()) { throw new RuntimeException( String.format( From de9b301372ccf14be1eca6d13ec7b8a1ed62c698 Mon Sep 17 00:00:00 2001 From: garyschulte Date: Wed, 14 Feb 2024 16:56:01 -0800 Subject: [PATCH 5/7] bump rev (#6575) Signed-off-by: garyschulte --- CHANGELOG.md | 16 ++++++++++++++-- gradle.properties | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e01d115b51a..2135395bcb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 24.1.2-SNAPSHOT +## 24.2.0-SNAPSHOT ### Breaking Changes - Following the OpenMetrics convention, the updated Prometheus client adds the `_total` suffix to every metrics of type counter, with the effect that some existing metrics have been renamed to have this suffix. If you are using the official Besu Grafana dashboard [(available here)](https://grafana.com/grafana/dashboards/16455-besu-full/), just update it to the latest revision, that accepts the old and the new name of the affected metrics. If you have a custom dashboard or use the metrics in other ways, then you need to manually update it to support the new naming. @@ -9,7 +9,6 @@ - `--Xfilter-on-enr-fork-id` has been removed. To disable the feature use `--filter-on-enr-fork-id=false`. - `--engine-jwt-enabled` has been removed. Use `--engine-jwt-disabled` instead. [#6491](https://github.com/hyperledger/besu/pull/6491) - ### Deprecations - X_SNAP and X_CHECKPOINT are marked for deprecation and will be removed in 24.4.0 in favor of SNAP and CHECKPOINT [#6405](https://github.com/hyperledger/besu/pull/6405) - `--Xsnapsync-synchronizer-flat-db-healing-enabled` is deprecated (always enabled). [#6499](https://github.com/hyperledger/besu/pull/6499) @@ -38,6 +37,19 @@ ### Download Links +## 24.1.2 + +### Bug fixes +- Fix ETC Spiral upgrade breach of consensus [#6524](https://github.com/hyperledger/besu/pull/6524) + +### Additions and Improvements +- Adds timestamp to enable Cancun upgrade on mainnet [#6545](https://github.com/hyperledger/besu/pull/6545) +- Github Actions based build.[#6427](https://github.com/hyperledger/besu/pull/6427) + +### Download Links +https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/24.1.2/besu-24.1.2.zip / sha256 9033f300edd81c770d3aff27a29f59dd4b6142a113936886a8f170718e412971 +https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/24.1.2/besu-24.1.2.tar.gz / sha256 082db8cf4fb67527aa0dd757e5d254b3b497f5027c23287f9c0a74a6a743bf08 + ## 24.1.1 ### Breaking Changes diff --git a/gradle.properties b/gradle.properties index 8fe780b083a..cc51c2b730f 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=24.1.2-SNAPSHOT +version=24.2.0-SNAPSHOT org.gradle.welcome=never # Set exports/opens flags required by Google Java Format and ErrorProne plugins. (JEP-396) From c913b6dff99078a923bb927cfb791c92ace903ad Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Thu, 15 Feb 2024 14:13:27 +1000 Subject: [PATCH 6/7] rocksdb usage - Replace Column Size with more accurate Total Size (#6540) Add rocksdb x-stats command to print various stats for each column family --------- Signed-off-by: Simon Dudley --- CHANGELOG.md | 2 + .../subcommands/storage/RocksDbHelper.java | 218 ++++++++++++++++-- .../storage/RocksDbSubCommand.java | 61 ++++- .../storage/StorageSubCommand.java | 6 +- .../storage/TrieLogSubCommand.java | 11 +- 5 files changed, 266 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2135395bcb0..1ede741f64c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ - Support for `shanghaiTime` fork and Shanghai EVM smart contracts in QBFT/IBFT chains [#6353](https://github.com/hyperledger/besu/pull/6353) - Change ExecutionHaltReason for contract creation collision case to return ILLEGAL_STATE_CHANGE [#6518](https://github.com/hyperledger/besu/pull/6518) - Experimental feature `--Xbonsai-code-using-code-hash-enabled` for storing Bonsai code storage by code hash [#6505](https://github.com/hyperledger/besu/pull/6505) +- More accurate column size `storage rocksdb usage` subcommand [#6540](https://github.com/hyperledger/besu/pull/6540) +- Adds `storage rocksdb x-stats` subcommand [#6540](https://github.com/hyperledger/besu/pull/6540) ### Bug fixes - Fix the way an advertised host configured with `--p2p-host` is treated when communicating with the originator of a PING packet [#6225](https://github.com/hyperledger/besu/pull/6225) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbHelper.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbHelper.java index 7f2fc609a1b..2b122979830 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbHelper.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbHelper.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.List; import java.util.function.BiConsumer; +import java.util.stream.Stream; import org.bouncycastle.util.Arrays; import org.rocksdb.ColumnFamilyDescriptor; @@ -66,36 +67,210 @@ static void forEachColumnFamily( } } - static void printUsageForColumnFamily( + static void printStatsForColumnFamily( final RocksDB rocksdb, final ColumnFamilyHandle cfHandle, final PrintWriter out) - throws RocksDBException, NumberFormatException { + throws RocksDBException { final String size = rocksdb.getProperty(cfHandle, "rocksdb.estimate-live-data-size"); final String numberOfKeys = rocksdb.getProperty(cfHandle, "rocksdb.estimate-num-keys"); - boolean emptyColumnFamily = false; - if (!size.isBlank() && !numberOfKeys.isBlank()) { + final long sizeLong = Long.parseLong(size); + final long numberOfKeysLong = Long.parseLong(numberOfKeys); + if (!size.isBlank() + && !numberOfKeys.isBlank() + && isPopulatedColumnFamily(sizeLong, numberOfKeysLong)) { + out.println("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-="); + out.println("Column Family: " + getNameById(cfHandle.getName())); + + final String prefix = "rocksdb."; + final String cfstats = "cfstats"; + final String cfstats_no_file_histogram = "cfstats-no-file-histogram"; + final String cf_file_histogram = "cf-file-histogram"; + final String cf_write_stall_stats = "cf-write-stall-stats"; + final String dbstats = "dbstats"; + final String db_write_stall_stats = "db-write-stall-stats"; + final String levelstats = "levelstats"; + final String block_cache_entry_stats = "block-cache-entry-stats"; + final String fast_block_cache_entry_stats = "fast-block-cache-entry-stats"; + final String num_immutable_mem_table = "num-immutable-mem-table"; + final String num_immutable_mem_table_flushed = "num-immutable-mem-table-flushed"; + final String mem_table_flush_pending = "mem-table-flush-pending"; + final String compaction_pending = "compaction-pending"; + final String background_errors = "background-errors"; + final String cur_size_active_mem_table = "cur-size-active-mem-table"; + final String cur_size_all_mem_tables = "cur-size-all-mem-tables"; + final String size_all_mem_tables = "size-all-mem-tables"; + final String num_entries_active_mem_table = "num-entries-active-mem-table"; + final String num_entries_imm_mem_tables = "num-entries-imm-mem-tables"; + final String num_deletes_active_mem_table = "num-deletes-active-mem-table"; + final String num_deletes_imm_mem_tables = "num-deletes-imm-mem-tables"; + final String estimate_num_keys = "estimate-num-keys"; + final String estimate_table_readers_mem = "estimate-table-readers-mem"; + final String is_file_deletions_enabled = "is-file-deletions-enabled"; + final String num_snapshots = "num-snapshots"; + final String oldest_snapshot_time = "oldest-snapshot-time"; + final String oldest_snapshot_sequence = "oldest-snapshot-sequence"; + final String num_live_versions = "num-live-versions"; + final String current_version_number = "current-super-version-number"; + final String estimate_live_data_size = "estimate-live-data-size"; + final String min_log_number_to_keep_str = "min-log-number-to-keep"; + final String min_obsolete_sst_number_to_keep_str = "min-obsolete-sst-number-to-keep"; + final String base_level_str = "base-level"; + final String total_sst_files_size = "total-sst-files-size"; + final String live_sst_files_size = "live-sst-files-size"; + final String obsolete_sst_files_size = "obsolete-sst-files-size"; + final String live_sst_files_size_at_temperature = "live-sst-files-size-at-temperature"; + final String estimate_pending_comp_bytes = "estimate-pending-compaction-bytes"; + final String aggregated_table_properties = "aggregated-table-properties"; + final String num_running_compactions = "num-running-compactions"; + final String num_running_flushes = "num-running-flushes"; + final String actual_delayed_write_rate = "actual-delayed-write-rate"; + final String is_write_stopped = "is-write-stopped"; + final String estimate_oldest_key_time = "estimate-oldest-key-time"; + final String block_cache_capacity = "block-cache-capacity"; + final String block_cache_usage = "block-cache-usage"; + final String block_cache_pinned_usage = "block-cache-pinned-usage"; + final String options_statistics = "options-statistics"; + final String num_blob_files = "num-blob-files"; + final String blob_stats = "blob-stats"; + final String total_blob_file_size = "total-blob-file-size"; + final String live_blob_file_size = "live-blob-file-size"; + final String live_blob_file_garbage_size = "live-blob-file-garbage-size"; + final String blob_cache_capacity = "blob-cache-capacity"; + final String blob_cache_usage = "blob-cache-usage"; + final String blob_cache_pinned_usage = "blob-cache-pinned-usage"; + Stream.of( + cfstats, + cfstats_no_file_histogram, + cf_file_histogram, + cf_write_stall_stats, + dbstats, + db_write_stall_stats, + levelstats, + block_cache_entry_stats, + fast_block_cache_entry_stats, + num_immutable_mem_table, + num_immutable_mem_table_flushed, + mem_table_flush_pending, + compaction_pending, + background_errors, + cur_size_active_mem_table, + cur_size_all_mem_tables, + size_all_mem_tables, + num_entries_active_mem_table, + num_entries_imm_mem_tables, + num_deletes_active_mem_table, + num_deletes_imm_mem_tables, + estimate_num_keys, + estimate_table_readers_mem, + is_file_deletions_enabled, + num_snapshots, + oldest_snapshot_time, + oldest_snapshot_sequence, + num_live_versions, + current_version_number, + estimate_live_data_size, + min_log_number_to_keep_str, + min_obsolete_sst_number_to_keep_str, + base_level_str, + total_sst_files_size, + live_sst_files_size, + obsolete_sst_files_size, + live_sst_files_size_at_temperature, + estimate_pending_comp_bytes, + aggregated_table_properties, + num_running_compactions, + num_running_flushes, + actual_delayed_write_rate, + is_write_stopped, + estimate_oldest_key_time, + block_cache_capacity, + block_cache_usage, + block_cache_pinned_usage, + options_statistics, + num_blob_files, + blob_stats, + total_blob_file_size, + live_blob_file_size, + live_blob_file_garbage_size, + blob_cache_capacity, + blob_cache_usage, + blob_cache_pinned_usage) + .forEach( + prop -> { + try { + final String value = rocksdb.getProperty(cfHandle, prefix + prop); + if (!value.isBlank()) { + out.println(prop + ": " + value); + } + } catch (RocksDBException e) { + LOG.debug("couldn't get property {}", prop); + } + }); + out.println("=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-="); + } + } + + static ColumnFamilyUsage getAndPrintUsageForColumnFamily( + final RocksDB rocksdb, final ColumnFamilyHandle cfHandle, final PrintWriter out) + throws RocksDBException, NumberFormatException { + final String numberOfKeys = rocksdb.getProperty(cfHandle, "rocksdb.estimate-num-keys"); + if (!numberOfKeys.isBlank()) { try { - final long sizeLong = Long.parseLong(size); final long numberOfKeysLong = Long.parseLong(numberOfKeys); final String totalSstFilesSize = rocksdb.getProperty(cfHandle, "rocksdb.total-sst-files-size"); final long totalSstFilesSizeLong = !totalSstFilesSize.isBlank() ? Long.parseLong(totalSstFilesSize) : 0; - if (sizeLong == 0 && numberOfKeysLong == 0) { - emptyColumnFamily = true; - } - if (!emptyColumnFamily) { + final String totalBlobFilesSize = + rocksdb.getProperty(cfHandle, "rocksdb.total-blob-file-size"); + final long totalBlobFilesSizeLong = + !totalBlobFilesSize.isBlank() ? Long.parseLong(totalBlobFilesSize) : 0; + + final long totalFilesSize = totalSstFilesSizeLong + totalBlobFilesSizeLong; + if (isPopulatedColumnFamily(0, numberOfKeysLong)) { printLine( out, getNameById(cfHandle.getName()), rocksdb.getProperty(cfHandle, "rocksdb.estimate-num-keys"), - formatOutputSize(sizeLong), - formatOutputSize(totalSstFilesSizeLong)); + formatOutputSize(totalFilesSize), + formatOutputSize(totalSstFilesSizeLong), + formatOutputSize(totalBlobFilesSizeLong)); } + return new ColumnFamilyUsage( + getNameById(cfHandle.getName()), + numberOfKeysLong, + totalFilesSize, + totalSstFilesSizeLong, + totalBlobFilesSizeLong); } catch (NumberFormatException e) { LOG.error("Failed to parse string into long: " + e.getMessage()); } } + // return empty usage on error + return new ColumnFamilyUsage(getNameById(cfHandle.getName()), 0, 0, 0, 0); + } + + static void printTotals(final PrintWriter out, final List columnFamilyUsages) { + final long totalKeys = columnFamilyUsages.stream().mapToLong(ColumnFamilyUsage::keys).sum(); + final long totalSize = + columnFamilyUsages.stream().mapToLong(ColumnFamilyUsage::totalSize).sum(); + final long totalSsts = + columnFamilyUsages.stream().mapToLong(ColumnFamilyUsage::sstFilesSize).sum(); + final long totalBlobs = + columnFamilyUsages.stream().mapToLong(ColumnFamilyUsage::blobFilesSize).sum(); + printSeparator(out); + printLine( + out, + "ESTIMATED TOTAL", + String.valueOf(totalKeys), + formatOutputSize(totalSize), + formatOutputSize(totalSsts), + formatOutputSize(totalBlobs)); + printSeparator(out); + } + + private static boolean isPopulatedColumnFamily(final long size, final long numberOfKeys) { + return size != 0 || numberOfKeys != 0; } static String formatOutputSize(final long size) { @@ -123,19 +298,28 @@ private static String getNameById(final byte[] id) { } static void printTableHeader(final PrintWriter out) { + printSeparator(out); out.format( - "| Column Family | Keys | Column Size | SST Files Size |\n"); + "| Column Family | Keys | Total Size | SST Files Size | Blob Files Size | \n"); + printSeparator(out); + } + + private static void printSeparator(final PrintWriter out) { out.format( - "|--------------------------------|-----------------|--------------|-----------------|\n"); + "|--------------------------------|-----------------|-------------|-----------------|------------------|\n"); } static void printLine( final PrintWriter out, final String cfName, final String keys, - final String columnSize, - final String sstFilesSize) { - final String format = "| %-30s | %-15s | %-12s | %-15s |\n"; - out.format(format, cfName, keys, columnSize, sstFilesSize); + final String totalFilesSize, + final String sstFilesSize, + final String blobFilesSize) { + final String format = "| %-30s | %-15s | %-11s | %-15s | %-16s |\n"; + out.format(format, cfName, keys, totalFilesSize, sstFilesSize, blobFilesSize); } + + record ColumnFamilyUsage( + String name, long keys, long totalSize, long sstFilesSize, long blobFilesSize) {} } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbSubCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbSubCommand.java index 0beddfdcbca..e7bfe05cf1d 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbSubCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbSubCommand.java @@ -19,6 +19,8 @@ import org.hyperledger.besu.cli.util.VersionProvider; import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.List; import org.rocksdb.RocksDBException; import picocli.CommandLine; @@ -31,12 +33,12 @@ description = "Print RocksDB information", mixinStandardHelpOptions = true, versionProvider = VersionProvider.class, - subcommands = {RocksDbSubCommand.RocksDbUsage.class}) + subcommands = {RocksDbSubCommand.RocksDbUsage.class, RocksDbSubCommand.RocksDbStats.class}) public class RocksDbSubCommand implements Runnable { @SuppressWarnings("unused") @ParentCommand - private StorageSubCommand parentCommand; + private StorageSubCommand storageSubCommand; @SuppressWarnings("unused") @CommandLine.Spec @@ -60,7 +62,7 @@ static class RocksDbUsage implements Runnable { @SuppressWarnings("unused") @ParentCommand - private RocksDbSubCommand parentCommand; + private RocksDbSubCommand rocksDbSubCommand; @Override public void run() { @@ -68,9 +70,9 @@ public void run() { final PrintWriter out = spec.commandLine().getOut(); final String dbPath = - parentCommand - .parentCommand - .parentCommand + rocksDbSubCommand + .storageSubCommand + .besuCommand .dataDir() .toString() .concat("/") @@ -78,11 +80,56 @@ public void run() { RocksDbHelper.printTableHeader(out); + final List columnFamilyUsages = new ArrayList<>(); RocksDbHelper.forEachColumnFamily( dbPath, (rocksdb, cfHandle) -> { try { - RocksDbHelper.printUsageForColumnFamily(rocksdb, cfHandle, out); + columnFamilyUsages.add( + RocksDbHelper.getAndPrintUsageForColumnFamily(rocksdb, cfHandle, out)); + } catch (RocksDBException e) { + throw new RuntimeException(e); + } + }); + RocksDbHelper.printTotals(out, columnFamilyUsages); + } + } + + @Command( + name = "x-stats", + description = "Print rocksdb stats", + mixinStandardHelpOptions = true, + versionProvider = VersionProvider.class) + static class RocksDbStats implements Runnable { + + @SuppressWarnings("unused") + @CommandLine.Spec + private CommandLine.Model.CommandSpec spec; + + @SuppressWarnings("unused") + @ParentCommand + private RocksDbSubCommand rocksDbSubCommand; + + @Override + public void run() { + + final PrintWriter out = spec.commandLine().getOut(); + + final String dbPath = + rocksDbSubCommand + .storageSubCommand + .besuCommand + .dataDir() + .toString() + .concat("/") + .concat(DATABASE_PATH); + + out.println("Column Family Stats..."); + RocksDbHelper.forEachColumnFamily( + dbPath, + (rocksdb, cfHandle) -> { + try { + RocksDbHelper.printStatsForColumnFamily(rocksdb, cfHandle, out); } catch (RocksDBException e) { throw new RuntimeException(e); } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/StorageSubCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/StorageSubCommand.java index 0dbfa3d22c5..57eb50846d8 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/StorageSubCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/StorageSubCommand.java @@ -57,7 +57,7 @@ public class StorageSubCommand implements Runnable { @SuppressWarnings("unused") @ParentCommand - BesuCommand parentCommand; + BesuCommand besuCommand; @SuppressWarnings("unused") @Spec @@ -104,8 +104,8 @@ public void run() { private StorageProvider getStorageProvider() { // init collection of ignorable segments - parentCommand.parentCommand.setIgnorableStorageSegments(); - return parentCommand.parentCommand.getStorageProvider(); + parentCommand.besuCommand.setIgnorableStorageSegments(); + return parentCommand.besuCommand.getStorageProvider(); } private void revert(final StorageProvider storageProvider) { diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java index 9317dde1304..5ff5396508c 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java @@ -78,7 +78,7 @@ public void run() { } private static BesuController createBesuController() { - return parentCommand.parentCommand.buildController(); + return parentCommand.besuCommand.buildController(); } @Command( @@ -132,7 +132,7 @@ public void run() { final TrieLogContext context = getTrieLogContext(); final Path dataDirectoryPath = Paths.get( - TrieLogSubCommand.parentCommand.parentCommand.dataDir().toAbsolutePath().toString()); + TrieLogSubCommand.parentCommand.besuCommand.dataDir().toAbsolutePath().toString()); LOG.info("Estimating trie logs size before pruning..."); long sizeBefore = estimatedSizeOfTrieLogs(); @@ -167,7 +167,7 @@ public void run() { private long estimatedSizeOfTrieLogs() { final String dbPath = TrieLogSubCommand.parentCommand - .parentCommand + .besuCommand .dataDir() .toString() .concat("/") @@ -180,6 +180,7 @@ private long estimatedSizeOfTrieLogs() { (rocksdb, cfHandle) -> { try { if (Arrays.equals(cfHandle.getName(), TRIE_LOG_STORAGE.getId())) { + // TODO SLD use sst + blob? estimatedSaving.set( Long.parseLong( rocksdb.getProperty(cfHandle, "rocksdb.estimate-live-data-size"))); @@ -233,7 +234,7 @@ public void run() { trieLogFilePath = Paths.get( TrieLogSubCommand.parentCommand - .parentCommand + .besuCommand .dataDir() .resolve("trie-logs.bin") .toAbsolutePath() @@ -283,7 +284,7 @@ public void run() { trieLogFilePath = Paths.get( TrieLogSubCommand.parentCommand - .parentCommand + .besuCommand .dataDir() .resolve("trie-logs.bin") .toAbsolutePath() From 8be243f7923773257f6e9a10dc6e58184f6dc4c5 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Thu, 15 Feb 2024 17:20:03 +1000 Subject: [PATCH 7/7] Use better savings estimate for x-trie-log prune (#6577) Signed-off-by: Simon Dudley --- .../cli/subcommands/storage/RocksDbSubCommand.java | 10 ++++------ .../cli/subcommands/storage/TrieLogSubCommand.java | 11 +++++++---- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbSubCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbSubCommand.java index e7bfe05cf1d..9ca55cbc38b 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbSubCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/RocksDbSubCommand.java @@ -74,9 +74,8 @@ public void run() { .storageSubCommand .besuCommand .dataDir() - .toString() - .concat("/") - .concat(DATABASE_PATH); + .resolve(DATABASE_PATH) + .toString(); RocksDbHelper.printTableHeader(out); @@ -120,9 +119,8 @@ public void run() { .storageSubCommand .besuCommand .dataDir() - .toString() - .concat("/") - .concat(DATABASE_PATH); + .resolve(DATABASE_PATH) + .toString(); out.println("Column Family Stats..."); RocksDbHelper.forEachColumnFamily( diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java index 5ff5396508c..47bad2292e4 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java @@ -180,10 +180,13 @@ private long estimatedSizeOfTrieLogs() { (rocksdb, cfHandle) -> { try { if (Arrays.equals(cfHandle.getName(), TRIE_LOG_STORAGE.getId())) { - // TODO SLD use sst + blob? - estimatedSaving.set( - Long.parseLong( - rocksdb.getProperty(cfHandle, "rocksdb.estimate-live-data-size"))); + + final long sstSize = + Long.parseLong(rocksdb.getProperty(cfHandle, "rocksdb.total-sst-files-size")); + final long blobSize = + Long.parseLong(rocksdb.getProperty(cfHandle, "rocksdb.total-blob-file-size")); + + estimatedSaving.set(sstSize + blobSize); } } catch (RocksDBException | NumberFormatException e) { throw new RuntimeException(e);