From f64736d137176f075574ba9250d1be207d91a1a9 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Mon, 14 Dec 2020 10:39:01 -0700 Subject: [PATCH 1/2] Move block header errors from trace/debug to info (#1568) Move the block header validation errors logging levels from trace and debug to info. Also, include a standard prefix "Invalid block header: " in each of the log lines. Signed-off-by: Danno Ferrin --- .../CliqueDifficultyValidationRule.java | 15 ++++++++++- .../CliqueExtraDataValidationRule.java | 14 ++++++----- .../CoinbaseHeaderValidationRule.java | 13 ++++++++-- .../SignerRateLimitValidationRule.java | 12 ++++++++- .../VoteValidationRule.java | 2 +- .../IbftCoinbaseValidationRule.java | 4 +-- .../IbftCommitSealsValidationRule.java | 10 ++++---- .../IbftValidatorsValidationRule.java | 14 ++++++----- .../IbftVanityDataValidationRule.java | 2 +- .../IbftExtraDataValidationRule.java | 25 +++++++++++-------- .../VoteValidationRule.java | 2 +- .../mainnet/MainnetBlockBodyValidator.java | 2 +- .../AncestryValidationRule.java | 7 +++--- .../CalculatedDifficultyValidationRule.java | 2 +- .../ConstantFieldValidationRule.java | 4 +-- ...1559BlockHeaderGasPriceValidationRule.java | 4 +-- .../ExtraDataMaxLengthValidationRule.java | 2 +- .../GasLimitRangeAndDeltaValidationRule.java | 9 ++++--- .../GasUsageValidationRule.java | 2 +- .../ProofOfWorkValidationRule.java | 10 ++++---- .../TimestampBoundedByFutureParameter.java | 2 +- .../TimestampMoreRecentThanParent.java | 2 +- 22 files changed, 100 insertions(+), 59 deletions(-) diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueDifficultyValidationRule.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueDifficultyValidationRule.java index e02cd47eede..3473c3e48a3 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueDifficultyValidationRule.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueDifficultyValidationRule.java @@ -23,8 +23,13 @@ import java.math.BigInteger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class CliqueDifficultyValidationRule implements AttachedBlockHeaderValidationRule { + private static final Logger LOG = LogManager.getLogger(); + @Override public boolean validate( final BlockHeader header, final BlockHeader parent, final ProtocolContext protocolContext) { @@ -36,7 +41,15 @@ public boolean validate( final BigInteger actualDifficulty = header.getDifficulty().toBigInteger(); - return expectedDifficulty.equals(actualDifficulty); + if (!expectedDifficulty.equals(actualDifficulty)) { + LOG.info( + "Invalid block header: difficulty {} does not equal expected difficulty {}", + actualDifficulty, + expectedDifficulty); + return false; + } + + return true; } @Override diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java index 790479eef62..1dde4df723e 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java @@ -66,10 +66,12 @@ public boolean validate( return extraDataIsValid(storedValidators, header); } catch (final RLPException ex) { - LOG.trace("ExtraData field was unable to be deserialised into an Clique Struct.", ex); + LOG.info( + "Invalid block header: ExtraData field was unable to be deserialised into an Clique Struct.", + ex); return false; } catch (final IllegalArgumentException ex) { - LOG.trace("Failed to verify extra data", ex); + LOG.info("Invalid block header: Failed to verify extra data", ex); return false; } } @@ -81,21 +83,21 @@ private boolean extraDataIsValid( final Address proposer = cliqueExtraData.getProposerAddress(); if (!expectedValidators.contains(proposer)) { - LOG.trace("Proposer sealing block is not a member of the signers."); + LOG.info("Invalid block header: Proposer sealing block is not a member of the signers."); return false; } if (epochManager.isEpochBlock(header.getNumber())) { if (!Iterables.elementsEqual(cliqueExtraData.getValidators(), expectedValidators)) { - LOG.trace( - "Incorrect signers. Expected {} but got {}.", + LOG.info( + "Invalid block header: Incorrect signers. Expected {} but got {}.", expectedValidators, cliqueExtraData.getValidators()); return false; } } else { if (!cliqueExtraData.getValidators().isEmpty()) { - LOG.trace("Signer list on non-epoch blocks must be empty."); + LOG.info("Invalid block header: Signer list on non-epoch blocks must be empty."); return false; } } diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CoinbaseHeaderValidationRule.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CoinbaseHeaderValidationRule.java index 287a8f23a0e..e85d45e78e1 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CoinbaseHeaderValidationRule.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CoinbaseHeaderValidationRule.java @@ -19,8 +19,13 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.mainnet.DetachedBlockHeaderValidationRule; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class CoinbaseHeaderValidationRule implements DetachedBlockHeaderValidationRule { + private static final Logger LOG = LogManager.getLogger(); + private final EpochManager epochManager; public CoinbaseHeaderValidationRule(final EpochManager epochManager) { @@ -31,8 +36,12 @@ public CoinbaseHeaderValidationRule(final EpochManager epochManager) { // The coinbase field is used for voting nodes in/out of the validator group. However, no votes // are allowed to be cast on epoch blocks public boolean validate(final BlockHeader header, final BlockHeader parent) { - if (epochManager.isEpochBlock(header.getNumber())) { - return header.getCoinbase().equals(CliqueBlockInterface.NO_VOTE_SUBJECT); + if (epochManager.isEpochBlock(header.getNumber()) + && !header.getCoinbase().equals(CliqueBlockInterface.NO_VOTE_SUBJECT)) { + LOG.info( + "Invalid block header: No clique in/out voting may occur on epoch blocks ({})", + header.getNumber()); + return false; } return true; } diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/SignerRateLimitValidationRule.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/SignerRateLimitValidationRule.java index 9017d37c942..48ebc42cd37 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/SignerRateLimitValidationRule.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/SignerRateLimitValidationRule.java @@ -20,14 +20,24 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.mainnet.AttachedBlockHeaderValidationRule; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class SignerRateLimitValidationRule implements AttachedBlockHeaderValidationRule { + private static final Logger LOG = LogManager.getLogger(); + @Override public boolean validate( final BlockHeader header, final BlockHeader parent, final ProtocolContext protocolContext) { final Address blockSigner = CliqueHelpers.getProposerOfBlock(header); - return CliqueHelpers.addressIsAllowedToProduceNextBlock(blockSigner, protocolContext, parent); + if (!CliqueHelpers.addressIsAllowedToProduceNextBlock(blockSigner, protocolContext, parent)) { + LOG.info("Invalid block header: {} is not allowed to produce next block", blockSigner); + return false; + } + + return true; } @Override diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/VoteValidationRule.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/VoteValidationRule.java index 109b8edff7f..2f8362edaa0 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/VoteValidationRule.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/VoteValidationRule.java @@ -36,7 +36,7 @@ public class VoteValidationRule implements DetachedBlockHeaderValidationRule { public boolean validate(final BlockHeader header, final BlockHeader parent) { final long nonce = header.getNonce(); if (!CliqueBlockInterface.isValidVoteValue(nonce)) { - LOG.trace("Nonce value ({}) is neither auth or drop.", nonce); + LOG.info("Invalid block header: Nonce value ({}) is neither auth or drop.", nonce); return false; } return true; diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java index b8fc446973c..3c29d2f06c8 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java @@ -48,8 +48,8 @@ public boolean validate( final Collection
storedValidators = validatorProvider.getValidators(); if (!storedValidators.contains(proposer)) { - LOGGER.trace( - "Block proposer is not a member of the validators. proposer={}, validators={}", + LOGGER.info( + "Invalid block header: Block proposer is not a member of the validators. proposer={}, validators={}", proposer, storedValidators); return false; diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java index 58716700176..81c009d1d96 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java @@ -58,7 +58,7 @@ public boolean validate( final List
committersWithoutDuplicates = new ArrayList<>(new HashSet<>(committers)); if (committers.size() != committersWithoutDuplicates.size()) { - LOGGER.trace("Duplicated seals found in header."); + LOGGER.info("Invalid block header: Duplicated seals found in header."); return false; } @@ -70,16 +70,16 @@ private boolean validateCommitters( final int minimumSealsRequired = calculateRequiredValidatorQuorum(storedValidators.size()); if (committers.size() < minimumSealsRequired) { - LOGGER.trace( - "Insufficient committers to seal block. (Required {}, received {})", + LOGGER.info( + "Invalid block header: Insufficient committers to seal block. (Required {}, received {})", minimumSealsRequired, committers.size()); return false; } if (!storedValidators.containsAll(committers)) { - LOGGER.trace( - "Not all committers are in the locally maintained validator list. validators={} committers={}", + LOGGER.info( + "Invalid block header: Not all committers are in the locally maintained validator list. validators={} committers={}", storedValidators, committers); return false; diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java index 852d004eb71..abaff656be5 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java @@ -54,8 +54,8 @@ public boolean validate( new TreeSet<>(ibftExtraData.getValidators()); if (!Iterables.elementsEqual(ibftExtraData.getValidators(), sortedReportedValidators)) { - LOGGER.trace( - "Validators are not sorted in ascending order. Expected {} but got {}.", + LOGGER.info( + "Invalid block header: Validators are not sorted in ascending order. Expected {} but got {}.", sortedReportedValidators, ibftExtraData.getValidators()); return false; @@ -63,18 +63,20 @@ public boolean validate( final Collection
storedValidators = validatorProvider.getValidators(); if (!Iterables.elementsEqual(ibftExtraData.getValidators(), storedValidators)) { - LOGGER.trace( - "Incorrect validators. Expected {} but got {}.", + LOGGER.info( + "Invalid block header: Incorrect validators. Expected {} but got {}.", storedValidators, ibftExtraData.getValidators()); return false; } } catch (final RLPException ex) { - LOGGER.trace("ExtraData field was unable to be deserialised into an IBFT Struct.", ex); + LOGGER.info( + "Invalid block header: ExtraData field was unable to be deserialised into an IBFT Struct.", + ex); return false; } catch (final IllegalArgumentException ex) { - LOGGER.trace("Failed to verify extra data", ex); + LOGGER.info("Invalid block header: Failed to verify extra data", ex); return false; } diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftVanityDataValidationRule.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftVanityDataValidationRule.java index d4bb690cff3..38cb20dd9e1 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftVanityDataValidationRule.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftVanityDataValidationRule.java @@ -32,7 +32,7 @@ public boolean validate( final IbftExtraData extraData = IbftExtraData.decode(header); if (extraData.getVanityData().size() != IbftExtraData.EXTRA_VANITY_LENGTH) { - LOG.trace("Ibft Extra Data does not contain 32 bytes of vanity data."); + LOG.info("Invalid block header: Ibft Extra Data does not contain 32 bytes of vanity data."); return false; } return true; diff --git a/consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java b/consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java index f8050d4955c..4f02c2969c3 100644 --- a/consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java +++ b/consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java @@ -65,7 +65,7 @@ public boolean validate( final Collection
storedValidators = validatorProvider.getValidators(); if (!storedValidators.contains(proposer)) { - LOG.trace("Proposer sealing block is not a member of the validators."); + LOG.info("Invalid block header: Proposer sealing block is not a member of the validators."); return false; } @@ -81,29 +81,31 @@ public boolean validate( new TreeSet<>(ibftExtraData.getValidators()); if (!Iterables.elementsEqual(ibftExtraData.getValidators(), sortedReportedValidators)) { - LOG.trace( - "Validators are not sorted in ascending order. Expected {} but got {}.", + LOG.info( + "Invalid block header: Validators are not sorted in ascending order. Expected {} but got {}.", sortedReportedValidators, ibftExtraData.getValidators()); return false; } if (!Iterables.elementsEqual(ibftExtraData.getValidators(), storedValidators)) { - LOG.trace( - "Incorrect validators. Expected {} but got {}.", + LOG.info( + "Invalid block header: Incorrect validators. Expected {} but got {}.", storedValidators, ibftExtraData.getValidators()); return false; } } catch (final RLPException ex) { - LOG.trace("ExtraData field was unable to be deserialised into an IBFT Struct.", ex); + LOG.info( + "Invalid block header: ExtraData field was unable to be deserialised into an IBFT Struct.", + ex); return false; } catch (final IllegalArgumentException ex) { - LOG.trace("Failed to verify extra data", ex); + LOG.info("Invalid block header: Failed to verify extra data", ex); return false; } catch (final RuntimeException ex) { - LOG.trace("Failed to find validators at parent"); + LOG.info("Invalid block header: Failed to find validators at parent"); return false; } @@ -116,15 +118,16 @@ private boolean validateCommitters( final int minimumSealsRequired = IbftHelpers.calculateRequiredValidatorQuorum(storedValidators.size()); if (committers.size() < minimumSealsRequired) { - LOG.trace( - "Insufficient committers to seal block. (Required {}, received {})", + LOG.info( + "Invalid block header: Insufficient committers to seal block. (Required {}, received {})", minimumSealsRequired, committers.size()); return false; } if (!storedValidators.containsAll(committers)) { - LOG.trace("Not all committers are in the locally maintained validator list."); + LOG.info( + "Invalid block header: Not all committers are in the locally maintained validator list."); return false; } diff --git a/consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/VoteValidationRule.java b/consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/VoteValidationRule.java index e49ac848bca..0929968df86 100644 --- a/consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/VoteValidationRule.java +++ b/consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/VoteValidationRule.java @@ -36,7 +36,7 @@ public class VoteValidationRule implements DetachedBlockHeaderValidationRule { public boolean validate(final BlockHeader header, final BlockHeader parent) { final long nonce = header.getNonce(); if (!IbftLegacyBlockInterface.isValidVoteValue(nonce)) { - LOG.trace("Nonce value ({}) is neither auth or drop.", nonce); + LOG.info("Invalid block header: Nonce value ({}) is neither auth or drop.", nonce); return false; } return true; diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockBodyValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockBodyValidator.java index 2255f92233c..7af751d1f79 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockBodyValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockBodyValidator.java @@ -121,7 +121,7 @@ public boolean validateBodyLight( private static boolean validateTransactionsRoot(final Bytes32 expected, final Bytes32 actual) { if (!expected.equals(actual)) { - LOG.warn( + LOG.info( "Invalid block: transaction root mismatch (expected={}, actual={})", expected, actual); return false; } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/AncestryValidationRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/AncestryValidationRule.java index 9dac6af53c2..6dc9d922bd5 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/AncestryValidationRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/AncestryValidationRule.java @@ -30,16 +30,15 @@ public class AncestryValidationRule implements DetachedBlockHeaderValidationRule @Override public boolean validate(final BlockHeader header, final BlockHeader parent) { if (!header.getParentHash().equals(parent.getHash())) { - LOG.trace( - "Invalid parent block header. Parent hash {} does not match " - + "supplied parent header {}.", + LOG.info( + "Invalid block header: Parent hash {} does not match " + "supplied parent header {}.", header.getParentHash(), parent.getHash()); return false; } if (header.getNumber() != (parent.getNumber() + 1)) { - LOG.trace( + LOG.info( "Invalid block header: number {} is not one more than parent number {}", header.getNumber(), parent.getNumber()); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/CalculatedDifficultyValidationRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/CalculatedDifficultyValidationRule.java index 3a2865b5712..cd429805f3b 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/CalculatedDifficultyValidationRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/CalculatedDifficultyValidationRule.java @@ -41,7 +41,7 @@ public boolean validate( difficultyCalculator.nextDifficulty(header.getTimestamp(), parent, context); if (actualDifficulty.compareTo(expectedDifficulty) != 0) { - LOG.trace( + LOG.info( "Invalid block header: difficulty {} does not equal expected difficulty {}", actualDifficulty, expectedDifficulty); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ConstantFieldValidationRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ConstantFieldValidationRule.java index 78a068b7bf8..4f1bfc38c81 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ConstantFieldValidationRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ConstantFieldValidationRule.java @@ -40,8 +40,8 @@ public ConstantFieldValidationRule( public boolean validate(final BlockHeader header, final BlockHeader parent) { final T actualValue = accessor.apply(header); if (!actualValue.equals(expectedValue)) { - LOG.trace( - "{} failed validation. Actual != Expected ({} != {}).", + LOG.info( + "Invalid block header: Field {} failed validation. Actual != Expected ({} != {}).", fieldName, actualValue, expectedValue); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/EIP1559BlockHeaderGasPriceValidationRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/EIP1559BlockHeaderGasPriceValidationRule.java index 30329fc76d3..e599d6e8f94 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/EIP1559BlockHeaderGasPriceValidationRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/EIP1559BlockHeaderGasPriceValidationRule.java @@ -50,7 +50,7 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { eip1559.computeBaseFee( header.getNumber(), parentBaseFee, parent.getGasUsed(), targetGasUsed); if (baseFee != header.getBaseFee().orElseThrow()) { - LOG.trace( + LOG.info( "Invalid block header: basefee {} does not equal expected basefee {}", header.getBaseFee().orElseThrow(), baseFee); @@ -58,7 +58,7 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { } return baseFee == currentBaseFee && eip1559.isValidBaseFee(parentBaseFee, currentBaseFee); } catch (final EIP1559MissingBaseFeeFromBlockHeader e) { - LOG.trace(e.getMessage()); + LOG.info("Invalid block header: " + e.getMessage()); return false; } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ExtraDataMaxLengthValidationRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ExtraDataMaxLengthValidationRule.java index c8267cb92f2..5f7e6428f27 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ExtraDataMaxLengthValidationRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ExtraDataMaxLengthValidationRule.java @@ -41,7 +41,7 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { private boolean validateExtraData(final Bytes extraData) { if (extraData.size() > maxExtraDataBytes) { - LOG.trace( + LOG.info( "Invalid block header: extra data field length {} is greater {}", extraData.size(), maxExtraDataBytes); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasLimitRangeAndDeltaValidationRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasLimitRangeAndDeltaValidationRule.java index c884f26bfe8..24be8f89590 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasLimitRangeAndDeltaValidationRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasLimitRangeAndDeltaValidationRule.java @@ -40,8 +40,11 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { final long gasLimit = header.getGasLimit(); if ((gasLimit < minGasLimit) || (gasLimit > maxGasLimit)) { - LOG.trace( - "Header gasLimit = {}, outside range {} --> {}", gasLimit, minGasLimit, maxGasLimit); + LOG.info( + "Invalid block header: gasLimit = {} is outside range {} --> {}", + gasLimit, + minGasLimit, + maxGasLimit); return false; } @@ -49,7 +52,7 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { final long difference = Math.abs(parentGasLimit - gasLimit); final long bounds = deltaBound(parentGasLimit); if (Long.compareUnsigned(difference, bounds) >= 0) { - LOG.trace( + LOG.info( "Invalid block header: gas limit delta {} is out of bounds of {}", difference, bounds); return false; } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasUsageValidationRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasUsageValidationRule.java index 8115bf5a701..189046f4843 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasUsageValidationRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasUsageValidationRule.java @@ -48,7 +48,7 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { slackCoefficient = eip1559.get().getFeeMarket().getSlackCoefficient(); } if (header.getGasUsed() > (header.getGasLimit() * slackCoefficient)) { - LOG.trace( + LOG.info( "Invalid block header: gas used {} exceeds gas limit {}", header.getGasUsed(), header.getGasLimit()); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ProofOfWorkValidationRule.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ProofOfWorkValidationRule.java index 407b7ccd0f3..f9f2be91ebc 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ProofOfWorkValidationRule.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ProofOfWorkValidationRule.java @@ -55,10 +55,10 @@ public ProofOfWorkValidationRule( public boolean validate(final BlockHeader header, final BlockHeader parent) { if (includeBaseFee) { if (!ExperimentalEIPs.eip1559Enabled) { - LOG.warn("Invalid block header: EIP-1559 feature flag must be enabled --Xeip1559-enabled"); + LOG.info("Invalid block header: EIP-1559 feature flag must be enabled --Xeip1559-enabled"); return false; } else if (header.getBaseFee().isEmpty()) { - LOG.warn("Invalid block header: missing mandatory base fee."); + LOG.info("Invalid block header: missing mandatory base fee."); return false; } } @@ -69,7 +69,7 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { hashBuffer, header.getNonce(), header.getNumber(), epochCalculator, headerHash.toArray()); if (header.getDifficulty().isZero()) { - LOG.trace("Rejecting header because difficulty is 0"); + LOG.info("Invalid block header: difficulty is 0"); return false; } final BigInteger difficulty = header.getDifficulty().toBytes().toUnsignedBigInteger(); @@ -79,7 +79,7 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { : UInt256.valueOf(ETHASH_TARGET_UPPER_BOUND.divide(difficulty)); final UInt256 result = UInt256.fromBytes(Bytes32.wrap(hashBuffer, 32)); if (result.compareTo(target) > 0) { - LOG.warn( + LOG.info( "Invalid block header: the EthHash result {} was greater than the target {}.\n" + "Failing header:\n{}", result, @@ -91,7 +91,7 @@ public boolean validate(final BlockHeader header, final BlockHeader parent) { final Hash mixedHash = Hash.wrap(Bytes32.leftPad(Bytes.wrap(hashBuffer).slice(0, Bytes32.SIZE))); if (!header.getMixHash().equals(mixedHash)) { - LOG.warn( + LOG.info( "Invalid block header: header mixed hash {} does not equal calculated mixed hash {}.\n" + "Failing header:\n{}", header.getMixHash(), diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java index 7be019522b1..68b5ae94cc6 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java @@ -49,7 +49,7 @@ private boolean validateHeaderNotAheadOfCurrentSystemTime(final long timestamp) TimeUnit.SECONDS.convert(System.currentTimeMillis(), TimeUnit.MILLISECONDS) + acceptableClockDriftSeconds; if (Long.compareUnsigned(timestamp, timestampMargin) > 0) { - LOG.trace( + LOG.info( "Invalid block header: timestamp {} is greater than the timestamp margin {}", timestamp, timestampMargin); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampMoreRecentThanParent.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampMoreRecentThanParent.java index 330045f3264..08ce0173b3a 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampMoreRecentThanParent.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampMoreRecentThanParent.java @@ -46,7 +46,7 @@ private boolean validateHeaderSufficientlyAheadOfParent( final long timestamp, final long parentTimestamp) { final long secondsSinceParent = timestamp - parentTimestamp; if (secondsSinceParent < minimumSecondsSinceParent) { - LOG.trace( + LOG.info( "Invalid block header: timestamp {} is only {} seconds newer than parent timestamp {}. Minimum {} seconds", timestamp, secondsSinceParent, From 90176ed279ddf0caf3e326eeb8ebfab4b0cabc60 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Mon, 14 Dec 2020 10:45:37 -0700 Subject: [PATCH 2/2] changelog Signed-off-by: Danno Ferrin --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5b00398789..606655b9037 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,11 @@ ### Additions and Improvements * Added `memory` as an option to `--key-value-storage`. This ephemeral storage is intended for sync testing and debugging. [\#1617](https://github.com/hyperledger/besu/pull/1617) -* Fixed gasPrice parameter not always respected when passed to `eth_estimateGas` endpoint [#1636](https://github.com/hyperledger/besu/pull/1636) +* Fixed gasPrice parameter not always respected when passed to `eth_estimateGas` endpoint [\#1636](https://github.com/hyperledger/besu/pull/1636) ### Bug Fixes +* Block Validation Errors should be at least INFO level not DEBUG or TRACE. Bug [\#1568](https://github.com/hyperledger/besu/pull/1568) PR [\#1706](https://github.com/hyperledger/besu/pull/1706) #### Previously identified known issues