Skip to content

Commit

Permalink
Feature/engine api override (#4190)
Browse files Browse the repository at this point in the history
* initial impl of engine-api override:
* remove reliance on isMergeEnabled in pre-merge block header validation rules
* create a merge-specific block header validtion stack rather than leveraging mainnet
* re-add --engine-api-enabled cli param
* make engine api usable without a merge config.
leaves the door open to supporting the engine api with other MiningCoordinators, but disables all but engine_exchangeTransitionConfiguration in the absence of mergeCoordinator

Signed-off-by: garyschulte <garyschulte@gmail.com>
  • Loading branch information
garyschulte committed Jul 31, 2022
1 parent 992534e commit 826648b
Show file tree
Hide file tree
Showing 25 changed files with 122 additions and 73 deletions.
2 changes: 1 addition & 1 deletion besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public RunnerBuilder jsonRpcConfiguration(final JsonRpcConfiguration jsonRpcConf

public RunnerBuilder engineJsonRpcConfiguration(
final JsonRpcConfiguration engineJsonRpcConfiguration) {
this.engineJsonRpcConfiguration = Optional.of(engineJsonRpcConfiguration);
this.engineJsonRpcConfiguration = Optional.ofNullable(engineJsonRpcConfiguration);
return this;
}

Expand Down
27 changes: 14 additions & 13 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,9 @@ static class GraphQlOptionGroup {
static class EngineRPCOptionGroup {
@Option(
names = {"--engine-rpc-enabled"},
description = "deprectaed parameter, do not use.",
hidden = true)
private final Boolean deprecatedIsEngineRpcEnabled = false;
description =
"enable the engine api, even in the absence of merge-specific configurations.")
private final Boolean overrideEngineRpcEnabled = false;

@Option(
names = {"--engine-rpc-port", "--engine-rpc-http-port"},
Expand Down Expand Up @@ -1933,9 +1933,11 @@ private void configure() throws Exception {
jsonRpcConfiguration =
jsonRpcConfiguration(
jsonRPCHttpOptionGroup.rpcHttpPort, jsonRPCHttpOptionGroup.rpcHttpApis, hostsAllowlist);
engineJsonRpcConfiguration =
createEngineJsonRpcConfiguration(
engineRPCOptionGroup.engineRpcPort, engineRPCOptionGroup.engineHostsAllowlist);
if (isEngineApiEnabled()) {
engineJsonRpcConfiguration =
createEngineJsonRpcConfiguration(
engineRPCOptionGroup.engineRpcPort, engineRPCOptionGroup.engineHostsAllowlist);
}
p2pTLSConfiguration = p2pTLSConfigOptions.p2pTLSConfiguration(commandLine);
graphQLConfiguration = graphQLConfiguration();
webSocketConfiguration =
Expand Down Expand Up @@ -2129,12 +2131,7 @@ private JsonRpcConfiguration createEngineJsonRpcConfiguration(
final Integer listenPort, final List<String> allowCallsFrom) {
JsonRpcConfiguration engineConfig =
jsonRpcConfiguration(listenPort, Arrays.asList("ENGINE", "ETH"), allowCallsFrom);
if (engineRPCOptionGroup.deprecatedIsEngineRpcEnabled) {
logger.warn(
"--engine-api-enabled parameter has been deprecated and will be removed in a future release. "
+ "Merge support is implicitly enabled by the presence of terminalTotalDifficulty in the genesis config.");
}
engineConfig.setEnabled(isMergeEnabled());
engineConfig.setEnabled(isEngineApiEnabled());
if (!engineRPCOptionGroup.isEngineAuthDisabled) {
engineConfig.setAuthenticationEnabled(true);
engineConfig.setAuthenticationAlgorithm(JwtAlgorithm.HS256);
Expand Down Expand Up @@ -3098,7 +3095,7 @@ private List<Integer> getEffectivePorts() {
effectivePorts,
jsonRPCWebsocketOptionGroup.rpcWsPort,
jsonRPCWebsocketOptionGroup.isRpcWsEnabled);
addPortIfEnabled(effectivePorts, engineRPCOptionGroup.engineRpcPort, isMergeEnabled());
addPortIfEnabled(effectivePorts, engineRPCOptionGroup.engineRpcPort, isEngineApiEnabled());
addPortIfEnabled(
effectivePorts, metricsOptionGroup.metricsPort, metricsOptionGroup.isMetricsEnabled);
addPortIfEnabled(
Expand Down Expand Up @@ -3222,6 +3219,10 @@ private boolean isMergeEnabled() {
return MergeConfigOptions.isMergeEnabled();
}

private boolean isEngineApiEnabled() {
return engineRPCOptionGroup.overrideEngineRpcEnabled || isMergeEnabled();
}

public static List<String> getJDKEnabledCypherSuites() {
try {
SSLContext context = SSLContext.getInstance("TLS");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2760,6 +2760,7 @@ public void rpcHttpTlsWarnIfCipherSuitesSpecifiedWithoutTls() {

parseCommand(
"--rpc-http-enabled",
"--engine-rpc-enabled",
"--rpc-http-host",
host,
"--rpc-http-port",
Expand Down
1 change: 1 addition & 0 deletions besu/src/test/resources/everything_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ random-peer-priority-enabled=false
host-whitelist=["all"]
host-allowlist=["all"]
engine-host-allowlist=["all"]
engine-rpc-enabled=true
engine-jwt-disabled=true
engine-jwt-secret="/tmp/jwt.hex"
required-blocks=["8675309=123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,27 @@
*/
package org.hyperledger.besu.consensus.merge;

import static org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderValidator.MIN_GAS_LIMIT;
import static org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderValidator.TIMESTAMP_TOLERANCE_S;

import org.hyperledger.besu.consensus.merge.headervalidationrules.ConstantOmmersHashRule;
import org.hyperledger.besu.consensus.merge.headervalidationrules.IncrementalTimestampRule;
import org.hyperledger.besu.consensus.merge.headervalidationrules.MergeUnfinalizedValidationRule;
import org.hyperledger.besu.consensus.merge.headervalidationrules.NoDifficultyRule;
import org.hyperledger.besu.consensus.merge.headervalidationrules.NoNonceRule;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator;
import org.hyperledger.besu.ethereum.mainnet.EpochCalculator;
import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderValidator;
import org.hyperledger.besu.ethereum.mainnet.PoWHasher;
import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket;
import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.AncestryValidationRule;
import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.BaseFeeMarketBlockHeaderGasPriceValidationRule;
import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.ExtraDataMaxLengthValidationRule;
import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.GasLimitRangeAndDeltaValidationRule;
import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.GasUsageValidationRule;
import org.hyperledger.besu.ethereum.mainnet.headervalidationrules.TimestampBoundedByFutureParameter;

import java.util.Optional;

Expand All @@ -40,7 +50,7 @@ public class MergeValidationRulesetFactory {
* <p>Specifically the set of rules provided by this function are to be used for a Mainnet Merge
* chain.
*
* @param feeMarket the applicable {@link FeeMarket}, either PGA or BaseFee.
* @param feeMarket the applicable {@link FeeMarket}
* @return the header validator.
*/
public static BlockHeaderValidator.Builder mergeBlockHeaderValidator(final FeeMarket feeMarket) {
Expand All @@ -49,14 +59,17 @@ public static BlockHeaderValidator.Builder mergeBlockHeaderValidator(final FeeMa
return MainnetBlockHeaderValidator.createPgaBlockHeaderValidator(
preMergeCalculator, PoWHasher.ETHASH_LIGHT);
} else {
return MainnetBlockHeaderValidator.createBaseFeeMarketValidator(
Optional.of(feeMarket)
.filter(FeeMarket::implementsBaseFee)
.map(BaseFeeMarket.class::cast)
.orElseThrow(
() ->
new RuntimeException(
"Invalid configuration: missing BaseFeeMarket for merge net")))
var baseFeeMarket = (BaseFeeMarket) feeMarket;

return new BlockHeaderValidator.Builder()
.addRule(new AncestryValidationRule())
.addRule(new GasUsageValidationRule())
.addRule(
new GasLimitRangeAndDeltaValidationRule(
MIN_GAS_LIMIT, Long.MAX_VALUE, Optional.of(baseFeeMarket)))
.addRule(new TimestampBoundedByFutureParameter(TIMESTAMP_TOLERANCE_S))
.addRule(new ExtraDataMaxLengthValidationRule(BlockHeader.MAX_EXTRA_DATA_BYTES))
.addRule((new BaseFeeMarketBlockHeaderGasPriceValidationRule(baseFeeMarket)))
.addRule(new MergeUnfinalizedValidationRule())
.addRule(new ConstantOmmersHashRule())
.addRule(new NoNonceRule())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ PayloadIdentifier preparePayload(
final Bytes32 random,
final Address feeRecipient);

@Override
default boolean isCompatibleWithEngineApi() {
return true;
}

Result rememberBlock(final Block block);

Result validateBlock(final Block block);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;

import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.function.Supplier;

import io.vertx.core.Vertx;
import org.slf4j.Logger;
Expand All @@ -40,13 +42,15 @@ public enum EngineStatus {
public static final long ENGINE_API_LOGGING_THRESHOLD = 60000L;
private final Vertx syncVertx;
private static final Logger LOG = LoggerFactory.getLogger(ExecutionEngineJsonRpcMethod.class);
protected final MergeContext mergeContext;
protected final Optional<MergeContext> mergeContextOptional;
protected final Supplier<MergeContext> mergeContext;
protected final ProtocolContext protocolContext;

protected ExecutionEngineJsonRpcMethod(final Vertx vertx, final ProtocolContext protocolContext) {
this.syncVertx = vertx;
this.protocolContext = protocolContext;
this.mergeContext = protocolContext.getConsensusContext(MergeContext.class);
this.mergeContextOptional = protocolContext.safeConsensusContext(MergeContext.class);
this.mergeContext = mergeContextOptional::orElseThrow;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,23 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EngineExchangeTransitionConfigurationResult;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.Difficulty;

import java.util.Optional;

import io.vertx.core.Vertx;
import io.vertx.core.json.Json;
import org.apache.tuweni.units.bigints.UInt256;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class EngineExchangeTransitionConfiguration extends ExecutionEngineJsonRpcMethod {
private static final Logger LOG =
LoggerFactory.getLogger(EngineExchangeTransitionConfiguration.class);

// use (2^256 - 2^10) if engine is enabled in the absence of a TTD configuration
static final Difficulty FALLBACK_TTD_DEFAULT =
Difficulty.MAX_VALUE.subtract(UInt256.valueOf(1024L));
static final long QOS_TIMEOUT_MILLIS = 120000L;

private final QosTimer qosTimer;
Expand Down Expand Up @@ -76,11 +81,14 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
"received transitionConfiguration: {}",
() -> Json.encodePrettily(remoteTransitionConfiguration));

final Optional<BlockHeader> maybeTerminalPoWBlockHeader = mergeContext.getTerminalPoWBlock();
final Optional<BlockHeader> maybeTerminalPoWBlockHeader =
mergeContextOptional.get().getTerminalPoWBlock();

final EngineExchangeTransitionConfigurationResult localTransitionConfiguration =
new EngineExchangeTransitionConfigurationResult(
mergeContext.getTerminalTotalDifficulty(),
mergeContextOptional
.map(c -> c.getTerminalTotalDifficulty())
.orElse(FALLBACK_TTD_DEFAULT),
maybeTerminalPoWBlockHeader.map(BlockHeader::getHash).orElse(Hash.ZERO),
maybeTerminalPoWBlockHeader.map(BlockHeader::getNumber).orElse(0L));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,12 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
Optional.ofNullable(forkChoice.getFinalizedBlockHash())
.filter(finalized -> !finalized.isZero());

mergeContext.fireNewUnverifiedForkchoiceMessageEvent(
forkChoice.getHeadBlockHash(), maybeFinalizedHash, forkChoice.getSafeBlockHash());
mergeContext
.get()
.fireNewUnverifiedForkchoiceMessageEvent(
forkChoice.getHeadBlockHash(), maybeFinalizedHash, forkChoice.getSafeBlockHash());

if (mergeContext.isSyncing()) {
if (mergeContext.get().isSyncing()) {
return syncingResponse(requestId, forkChoice);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public String getName() {
public JsonRpcResponse syncResponse(final JsonRpcRequestContext request) {
final PayloadIdentifier payloadId = request.getRequiredParameter(0, PayloadIdentifier.class);

final Optional<Block> block = mergeContext.retrieveBlockById(payloadId);
final Optional<Block> block = mergeContext.get().retrieveBlockById(payloadId);
if (block.isPresent()) {
var proposal = block.get();
var proposalHeader = proposal.getHeader();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,10 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
final var block =
new Block(newBlockHeader, new BlockBody(transactions, Collections.emptyList()));

if (mergeContext.isSyncing() || parentHeader.isEmpty()) {
if (mergeContext.get().isSyncing() || parentHeader.isEmpty()) {
LOG.debug(
"isSyncing: {} parentHeaderMissing: {}, adding {} to backwardsync",
mergeContext.isSyncing(),
mergeContext.get().isSyncing(),
parentHeader.isEmpty(),
block.getHash());
mergeCoordinator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.hyperledger.besu.ethereum.blockcreation.MiningCoordinator;

import java.util.Map;
import java.util.Optional;

import io.vertx.core.Vertx;
import io.vertx.core.VertxOptions;
Expand All @@ -34,12 +35,16 @@ public class ExecutionEngineJsonRpcMethods extends ApiGroupJsonRpcMethods {

private final BlockResultFactory blockResultFactory = new BlockResultFactory();

private final MergeMiningCoordinator mergeCoordinator;
private final Optional<MergeMiningCoordinator> mergeCoordinator;
private final ProtocolContext protocolContext;

ExecutionEngineJsonRpcMethods(
final MiningCoordinator miningCoordinator, final ProtocolContext protocolContext) {
this.mergeCoordinator = (MergeMiningCoordinator) miningCoordinator;
this.mergeCoordinator =
Optional.ofNullable(miningCoordinator)
.filter(mc -> mc.isCompatibleWithEngineApi())
.map(MergeMiningCoordinator.class::cast);

this.protocolContext = protocolContext;
}

Expand All @@ -51,10 +56,14 @@ protected String getApiGroup() {
@Override
protected Map<String, JsonRpcMethod> create() {
Vertx syncVertx = Vertx.vertx(new VertxOptions().setWorkerPoolSize(1));
return mapOf(
new EngineGetPayload(syncVertx, protocolContext, blockResultFactory),
new EngineNewPayload(syncVertx, protocolContext, mergeCoordinator),
new EngineForkchoiceUpdated(syncVertx, protocolContext, mergeCoordinator),
new EngineExchangeTransitionConfiguration(syncVertx, protocolContext));
if (mergeCoordinator.isPresent()) {
return mapOf(
new EngineGetPayload(syncVertx, protocolContext, blockResultFactory),
new EngineNewPayload(syncVertx, protocolContext, mergeCoordinator.get()),
new EngineForkchoiceUpdated(syncVertx, protocolContext, mergeCoordinator.get()),
new EngineExchangeTransitionConfiguration(syncVertx, protocolContext));
} else {
return mapOf(new EngineExchangeTransitionConfiguration(syncVertx, protocolContext));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package org.hyperledger.besu.ethereum.api.jsonrpc.methods;

import org.hyperledger.besu.config.GenesisConfigOptions;
import org.hyperledger.besu.config.MergeConfigOptions;
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.filter.FilterManager;
Expand Down Expand Up @@ -95,6 +94,7 @@ public Map<String, JsonRpcMethod> methods(
blockchainQueries, protocolSchedule, metricsSystem, transactionPool, dataDir),
new EeaJsonRpcMethods(
blockchainQueries, protocolSchedule, transactionPool, privacyParameters),
new ExecutionEngineJsonRpcMethods(miningCoordinator, protocolContext),
new GoQuorumJsonRpcPrivacyMethods(
blockchainQueries, protocolSchedule, transactionPool, privacyParameters),
new EthJsonRpcMethods(
Expand Down Expand Up @@ -127,11 +127,6 @@ public Map<String, JsonRpcMethod> methods(
new TxPoolJsonRpcMethods(transactionPool),
new PluginsJsonRpcMethods(namedPlugins));

if (MergeConfigOptions.isMergeEnabled()) {
enabled.putAll(
new ExecutionEngineJsonRpcMethods(miningCoordinator, protocolContext).create(rpcApis));
}

for (final JsonRpcMethods apiGroup : availableApiGroups) {
enabled.putAll(apiGroup.create(rpcApis));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineExchangeTransitionConfiguration.FALLBACK_TTD_DEFAULT;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -70,6 +71,7 @@ public class EngineExchangeTransitionConfigurationTest {
@Before
public void setUp() {
when(protocolContext.getConsensusContext(Mockito.any())).thenReturn(mergeContext);
when(protocolContext.safeConsensusContext(Mockito.any())).thenReturn(Optional.of(mergeContext));

this.method = new EngineExchangeTransitionConfiguration(vertx, protocolContext);
}
Expand Down Expand Up @@ -114,6 +116,21 @@ public void shouldReturnZerosOnTerminalPoWBlockHeaderEmpty() {
assertThat(result.getTerminalBlockNumber()).isEqualTo(0L);
}

@Test
public void shouldReturnDefaultOnNoTerminalTotalDifficultyConfigured() {
when(mergeContext.getTerminalPoWBlock()).thenReturn(Optional.empty());

var response =
resp(
new EngineExchangeTransitionConfigurationParameter(
"0", Hash.ZERO.toHexString(), new UnsignedLongParameter(0L)));

var result = fromSuccessResp(response);
assertThat(result.getTerminalTotalDifficulty()).isEqualTo(FALLBACK_TTD_DEFAULT);
assertThat(result.getTerminalBlockHash()).isEqualTo(Hash.ZERO);
assertThat(result.getTerminalBlockNumber()).isEqualTo(0L);
}

@Test
public void shouldReturnConfigurationOnConfigurationMisMatch() {
final BlockHeader fakeBlockHeader = createBlockHeader(Hash.fromHexStringLenient("0x01"), 42);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public class EngineForkchoiceUpdatedTest {

@Before
public void before() {
when(protocolContext.getConsensusContext(Mockito.any())).thenReturn(mergeContext);
when(protocolContext.safeConsensusContext(Mockito.any())).thenReturn(Optional.of(mergeContext));
when(protocolContext.getBlockchain()).thenReturn(blockchain);
this.method = new EngineForkchoiceUpdated(vertx, protocolContext, mergeCoordinator);
}
Expand Down
Loading

0 comments on commit 826648b

Please sign in to comment.