Move block header errors from trace/debug to info (#1568) (#1706)

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 <danno.ferrin@gmail.com>
pull/1719/head
Danno Ferrin 4 years ago committed by GitHub
parent bd5ef64f81
commit 422ebf2a49
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 6
      CHANGELOG.md
  2. 15
      consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueDifficultyValidationRule.java
  3. 14
      consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CliqueExtraDataValidationRule.java
  4. 13
      consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/CoinbaseHeaderValidationRule.java
  5. 12
      consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/SignerRateLimitValidationRule.java
  6. 2
      consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/headervalidationrules/VoteValidationRule.java
  7. 4
      consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCoinbaseValidationRule.java
  8. 10
      consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftCommitSealsValidationRule.java
  9. 14
      consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java
  10. 2
      consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftVanityDataValidationRule.java
  11. 25
      consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java
  12. 2
      consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/VoteValidationRule.java
  13. 2
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetBlockBodyValidator.java
  14. 7
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/AncestryValidationRule.java
  15. 2
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/CalculatedDifficultyValidationRule.java
  16. 4
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ConstantFieldValidationRule.java
  17. 4
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/EIP1559BlockHeaderGasPriceValidationRule.java
  18. 2
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ExtraDataMaxLengthValidationRule.java
  19. 9
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasLimitRangeAndDeltaValidationRule.java
  20. 2
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/GasUsageValidationRule.java
  21. 10
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/ProofOfWorkValidationRule.java
  22. 2
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java
  23. 2
      ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/headervalidationrules/TimestampMoreRecentThanParent.java

@ -4,11 +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)
* Enabled eth65 by default [#1682](https://github.com/hyperledger/besu/pull/1682)
* Fixed gasPrice parameter not always respected when passed to `eth_estimateGas` endpoint [\#1636](https://github.com/hyperledger/besu/pull/1636)
* Enabled eth65 by default [\#1682](https://github.com/hyperledger/besu/pull/1682)
### 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

@ -23,8 +23,13 @@ import org.hyperledger.besu.ethereum.mainnet.AttachedBlockHeaderValidationRule;
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 class CliqueDifficultyValidationRule implements AttachedBlockHeaderValida
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

@ -66,10 +66,12 @@ public class CliqueExtraDataValidationRule implements AttachedBlockHeaderValidat
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 @@ public class CliqueExtraDataValidationRule implements AttachedBlockHeaderValidat
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;
}
}

@ -19,8 +19,13 @@ import org.hyperledger.besu.consensus.common.EpochManager;
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 class CoinbaseHeaderValidationRule implements DetachedBlockHeaderValidati
// 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;
}

@ -20,14 +20,24 @@ import org.hyperledger.besu.ethereum.core.Address;
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

@ -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;

@ -48,8 +48,8 @@ public class IbftCoinbaseValidationRule implements AttachedBlockHeaderValidation
final Collection<Address> 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;

@ -58,7 +58,7 @@ public class IbftCommitSealsValidationRule implements AttachedBlockHeaderValidat
final List<Address> 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 @@ public class IbftCommitSealsValidationRule implements AttachedBlockHeaderValidat
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;

@ -54,8 +54,8 @@ public class IbftValidatorsValidationRule implements AttachedBlockHeaderValidati
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 class IbftValidatorsValidationRule implements AttachedBlockHeaderValidati
final Collection<Address> 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;
}

@ -32,7 +32,7 @@ public class IbftVanityDataValidationRule implements AttachedBlockHeaderValidati
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;

@ -65,7 +65,7 @@ public class IbftExtraDataValidationRule implements AttachedBlockHeaderValidatio
final Collection<Address> 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 class IbftExtraDataValidationRule implements AttachedBlockHeaderValidatio
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 @@ public class IbftExtraDataValidationRule implements AttachedBlockHeaderValidatio
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;
}

@ -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;

@ -121,7 +121,7 @@ public class MainnetBlockBodyValidator implements BlockBodyValidator {
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;
}

@ -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());

@ -41,7 +41,7 @@ public class CalculatedDifficultyValidationRule implements AttachedBlockHeaderVa
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);

@ -40,8 +40,8 @@ public class ConstantFieldValidationRule<T> implements DetachedBlockHeaderValida
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);

@ -50,7 +50,7 @@ public class EIP1559BlockHeaderGasPriceValidationRule implements DetachedBlockHe
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 class EIP1559BlockHeaderGasPriceValidationRule implements DetachedBlockHe
}
return baseFee == currentBaseFee && eip1559.isValidBaseFee(parentBaseFee, currentBaseFee);
} catch (final EIP1559MissingBaseFeeFromBlockHeader e) {
LOG.trace(e.getMessage());
LOG.info("Invalid block header: " + e.getMessage());
return false;
}
}

@ -41,7 +41,7 @@ public class ExtraDataMaxLengthValidationRule implements DetachedBlockHeaderVali
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);

@ -40,8 +40,11 @@ public class GasLimitRangeAndDeltaValidationRule extends AbstractGasLimitSpecifi
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 class GasLimitRangeAndDeltaValidationRule extends AbstractGasLimitSpecifi
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;
}

@ -48,7 +48,7 @@ public class GasUsageValidationRule implements DetachedBlockHeaderValidationRule
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());

@ -55,10 +55,10 @@ public final class ProofOfWorkValidationRule implements DetachedBlockHeaderValid
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 final class ProofOfWorkValidationRule implements DetachedBlockHeaderValid
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 final class ProofOfWorkValidationRule implements DetachedBlockHeaderValid
: 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 final class ProofOfWorkValidationRule implements DetachedBlockHeaderValid
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(),

@ -49,7 +49,7 @@ public class TimestampBoundedByFutureParameter implements DetachedBlockHeaderVal
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);

@ -46,7 +46,7 @@ public class TimestampMoreRecentThanParent implements DetachedBlockHeaderValidat
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,

Loading…
Cancel
Save