Skip to content

Commit

Permalink
[4844] Add excessDataGas and dataGasUsed validation to EngineNewPaylo…
Browse files Browse the repository at this point in the history
…adV3 (hyperledger#5683)

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
  • Loading branch information
Gabriel-Trintinalia authored and jflo committed Jul 19, 2023
1 parent 537a0b1 commit c611766
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
import org.hyperledger.besu.datatypes.DataGas;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.VersionedHash;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.BlockProcessingResult;
import org.hyperledger.besu.ethereum.ProtocolContext;
Expand All @@ -53,6 +54,9 @@
import org.hyperledger.besu.ethereum.mainnet.BodyValidation;
import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
import org.hyperledger.besu.ethereum.mainnet.feemarket.ExcessDataGasCalculator;
import org.hyperledger.besu.ethereum.rlp.RLPException;
import org.hyperledger.besu.ethereum.trie.MerkleTrieException;
import org.hyperledger.besu.plugin.services.exception.StorageException;
Expand All @@ -66,6 +70,7 @@
import io.vertx.core.Vertx;
import io.vertx.core.json.Json;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -103,14 +108,17 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)

Object reqId = requestContext.getRequest().getId();

final Optional<BlockHeader> maybeParentHeader =
protocolContext.getBlockchain().getBlockHeader(blockParam.getParentHash());

LOG.atTrace()
.setMessage("blockparam: {}")
.addArgument(() -> Json.encodePrettily(blockParam))
.log();

var forkValidationResult = validateForkSupported(reqId, blockParam);
if (forkValidationResult.isPresent()) {
return forkValidationResult.get();
ValidationResult<JsonRpcError> forkValidationResult = validateForkSupported(reqId, blockParam);
if (!forkValidationResult.isValid()) {
return new JsonRpcErrorResponse(reqId, forkValidationResult.getInvalidReason());
}

final Optional<List<Withdrawal>> maybeWithdrawals =
Expand Down Expand Up @@ -199,11 +207,20 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
return respondWithInvalid(reqId, blockParam, null, getInvalidBlockHashStatus(), errorMessage);
}

// Validate transactions
var transactionValidationResult =
validateTransactions(blockParam, reqId, transactions, maybeVersionedHashParam);
if (transactionValidationResult.isPresent()) {
return transactionValidationResult.get();
ValidationResult<JsonRpcError> blobValidationResult =
validateBlobs(
transactions,
newBlockHeader,
maybeParentHeader,
maybeVersionedHashParam,
protocolSchedule.getByBlockHeader(newBlockHeader));
if (!blobValidationResult.isValid()) {
return respondWithInvalid(
reqId,
blockParam,
null,
getInvalidBlockHashStatus(),
blobValidationResult.getErrorMessage());
}

// do we already have this payload
Expand All @@ -222,8 +239,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
"Block already present in bad block manager.");
}

final Optional<BlockHeader> maybeParentHeader =
protocolContext.getBlockchain().getBlockHeader(blockParam.getParentHash());
if (maybeParentHeader.isPresent()
&& (blockParam.getTimestamp() <= maybeParentHeader.get().getTimestamp())) {
return respondWithInvalid(
Expand All @@ -245,7 +260,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
.addArgument(block::toLogString)
.log();
mergeCoordinator.appendNewPayloadToSync(block);

return respondWith(reqId, blockParam, null, SYNCING);
}

Expand Down Expand Up @@ -361,17 +375,79 @@ protected EngineStatus getInvalidBlockHashStatus() {
return INVALID;
}

protected Optional<JsonRpcResponse> validateForkSupported(
protected ValidationResult<JsonRpcError> validateForkSupported(
final Object id, final EnginePayloadParameter payloadParameter) {
return Optional.empty();
return ValidationResult.valid();
}

protected Optional<JsonRpcResponse> validateTransactions(
final EnginePayloadParameter blockParam,
final Object reqId,
protected ValidationResult<JsonRpcError> validateBlobs(
final List<Transaction> transactions,
final Optional<List<String>> maybeVersionedHashParam) {
return Optional.empty();
final BlockHeader header,
final Optional<BlockHeader> maybeParentHeader,
final Optional<List<String>> maybeVersionedHashParam,
final ProtocolSpec protocolSpec) {

var blobTransactions =
transactions.stream().filter(transaction -> transaction.getType().supportsBlob()).toList();

List<Bytes32> versionedHashesParam =
maybeVersionedHashParam
.map(strings -> strings.stream().map(Bytes32::fromHexStringStrict).toList())
.orElseGet(ArrayList::new);

final List<Bytes32> transactionVersionedHashes = new ArrayList<>();
for (Transaction transaction : blobTransactions) {
var versionedHashes = transaction.getVersionedHashes();
// blob transactions must have at least one blob
if (versionedHashes.isEmpty()) {
return ValidationResult.invalid(
JsonRpcError.INVALID_PARAMS, "There must be at least one blob");
}
transactionVersionedHashes.addAll(
versionedHashes.get().stream().map(VersionedHash::toBytes).toList());
}

// Validate versionedHashesParam
if (!versionedHashesParam.equals(transactionVersionedHashes)) {
return ValidationResult.invalid(
JsonRpcError.INVALID_PARAMS,
"Versioned hashes from blob transactions do not match expected values");
}

// Validate excessDataGas
if (maybeParentHeader.isPresent()) {
if (!validateExcessDataGas(header, maybeParentHeader.get(), protocolSpec)) {
return ValidationResult.invalid(
JsonRpcError.INVALID_PARAMS,
"Payload excessDataGas does not match calculated excessDataGas");
}
}

// Validate dataGasUsed
if (header.getDataGasUsed().isPresent()) {
if (!validateDataGasUsed(header, versionedHashesParam, protocolSpec)) {
return ValidationResult.invalid(
JsonRpcError.INVALID_PARAMS,
"Payload DataGasUsed does not match calculated DataGasUsed");
}
}
return ValidationResult.valid();
}

private boolean validateExcessDataGas(
final BlockHeader header, final BlockHeader parentHeader, final ProtocolSpec protocolSpec) {
DataGas calculatedDataGas =
ExcessDataGasCalculator.calculateExcessDataGasForParent(protocolSpec, parentHeader);
return header.getExcessDataGas().orElse(DataGas.ZERO).equals(calculatedDataGas);
}

private boolean validateDataGasUsed(
final BlockHeader header,
final List<Bytes32> maybeVersionedHashParam,
final ProtocolSpec protocolSpec) {
var calculatedDataGas =
protocolSpec.getGasCalculator().dataGasUsed(maybeVersionedHashParam.size());
return header.getDataGasUsed().orElse(0L).equals(calculatedDataGas);
}

private void logImportedBlockInfo(final Block block, final double timeInS) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@
import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.mainnet.ValidationResult;

import java.util.List;
import java.util.Optional;

import io.vertx.core.Vertx;

Expand Down Expand Up @@ -50,4 +58,14 @@ protected boolean requireTerminalPoWBlockValidation() {
protected EngineStatus getInvalidBlockHashStatus() {
return INVALID_BLOCK_HASH;
}

@Override
protected ValidationResult<JsonRpcError> validateBlobs(
final List<Transaction> transactions,
final BlockHeader header,
final Optional<BlockHeader> maybeParentHeader,
final Optional<List<String>> maybeVersionedHashParam,
final ProtocolSpec protocolSpec) {
return ValidationResult.valid();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,16 @@
import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
import org.hyperledger.besu.ethereum.mainnet.ValidationResult;

import java.util.List;
import java.util.Optional;

import io.vertx.core.Vertx;

Expand All @@ -38,4 +46,14 @@ public EngineNewPayloadV2(
public String getName() {
return RpcMethod.ENGINE_NEW_PAYLOAD_V2.getMethodName();
}

@Override
protected ValidationResult<JsonRpcError> validateBlobs(
final List<Transaction> transactions,
final BlockHeader header,
final Optional<BlockHeader> maybeParentHeader,
final Optional<List<String>> maybeVersionedHashParam,
final ProtocolSpec protocolSpec) {
return ValidationResult.valid();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,16 @@
*/
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.INVALID_PARAMS;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.UNSUPPORTED_FORK;

import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
import org.hyperledger.besu.datatypes.VersionedHash;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import org.hyperledger.besu.ethereum.mainnet.ValidationResult;

import io.vertx.core.Vertx;
import org.apache.tuweni.bytes.Bytes32;

public class EngineNewPayloadV3 extends AbstractEngineNewPayload {

Expand All @@ -58,73 +47,21 @@ public String getName() {
}

@Override
protected Optional<JsonRpcResponse> validateForkSupported(
protected ValidationResult<JsonRpcError> validateForkSupported(
final Object reqId, final EnginePayloadParameter payloadParameter) {
var cancun = timestampSchedule.hardforkFor(s -> s.fork().name().equalsIgnoreCase("Cancun"));

if (cancun.isPresent() && payloadParameter.getTimestamp() >= cancun.get().milestone()) {
if (payloadParameter.getDataGasUsed() == null
|| payloadParameter.getExcessDataGas() == null) {
return Optional.of(new JsonRpcErrorResponse(reqId, INVALID_PARAMS));
return ValidationResult.invalid(JsonRpcError.INVALID_PARAMS);
}
} else {
if (payloadParameter.getDataGasUsed() != null
|| payloadParameter.getExcessDataGas() != null) {
return Optional.of(new JsonRpcErrorResponse(reqId, UNSUPPORTED_FORK));
}
}
return Optional.empty();
}

@Override
protected Optional<JsonRpcResponse> validateTransactions(
final EnginePayloadParameter blockParam,
final Object reqId,
final List<Transaction> transactions,
final Optional<List<String>> maybeVersionedHashParam) {

var blobTransactions =
transactions.stream().filter(transaction -> transaction.getType().supportsBlob()).toList();

return validateBlobTransactions(blockParam, reqId, blobTransactions, maybeVersionedHashParam);
}

private Optional<JsonRpcResponse> validateBlobTransactions(
final EnginePayloadParameter blockParam,
final Object reqId,
final List<Transaction> blobTransactions,
final Optional<List<String>> maybeVersionedHashParam) {

List<Bytes32> versionedHashesParam =
maybeVersionedHashParam
.map(strings -> strings.stream().map(Bytes32::fromHexStringStrict).toList())
.orElseGet(ArrayList::new);

final List<Bytes32> transactionVersionedHashes = new ArrayList<>();

for (Transaction transaction : blobTransactions) {
if (transaction.getType().supportsBlob()) {
var versionedHashes = transaction.getVersionedHashes();
if (versionedHashes.isEmpty()) {
return Optional.of(
respondWithInvalid(
reqId, blockParam, null, INVALID, "There must be at least one blob"));
}
transactionVersionedHashes.addAll(
versionedHashes.get().stream().map(VersionedHash::toBytes).toList());
return ValidationResult.invalid(JsonRpcError.UNSUPPORTED_FORK);
}
}

// check list contents
if (!versionedHashesParam.equals(transactionVersionedHashes)) {
return Optional.of(
respondWithInvalid(
reqId,
blockParam,
null,
INVALID,
"Versioned hashes from blob transactions do not match expected values"));
}
return Optional.empty();
return ValidationResult.valid();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -558,20 +558,6 @@ public void shouldReturnValidIfDepositsIsNotNull_WhenDepositsAllowed() {
assertValidResponse(mockHeader, resp);
}

@Test
public void shouldReturnValidIfProtocolScheduleIsEmpty() {
when(protocolSchedule.getByBlockHeader(any())).thenReturn(null);
BlockHeader mockHeader =
setupValidPayload(
new BlockProcessingResult(Optional.of(new BlockProcessingOutputs(null, List.of()))),
Optional.empty(),
Optional.empty());

var resp = resp(mockPayload(mockHeader, Collections.emptyList()));

assertValidResponse(mockHeader, resp);
}

protected JsonRpcResponse resp(final EnginePayloadParameter payload) {
return method.response(
new JsonRpcRequestContext(
Expand Down Expand Up @@ -629,7 +615,7 @@ private EnginePayloadParameter mockPayload(
}

@NotNull
private BlockHeader setupValidPayload(
BlockHeader setupValidPayload(
final BlockProcessingResult value,
final Optional<List<Withdrawal>> maybeWithdrawals,
final Optional<List<Deposit>> maybeDeposits) {
Expand Down Expand Up @@ -689,7 +675,7 @@ protected BlockHeader createBlockHeader(
return mockHeader;
}

private void assertValidResponse(final BlockHeader mockHeader, final JsonRpcResponse resp) {
void assertValidResponse(final BlockHeader mockHeader, final JsonRpcResponse resp) {
EnginePayloadStatusResult res = fromSuccessResp(resp);
assertThat(res.getLatestValidHash().get()).isEqualTo(mockHeader.getHash());
assertThat(res.getStatusAsString()).isEqualTo(VALID.name());
Expand Down
Loading

0 comments on commit c611766

Please sign in to comment.