Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/engine api override #4190

Merged
merged 7 commits into from
Jul 31, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,6 +21,7 @@
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;

Expand All @@ -40,13 +41,13 @@ 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> 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.mergeContext = protocolContext.safeConsensusContext(MergeContext.class);
}

@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,12 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
"received transitionConfiguration: {}",
() -> Json.encodePrettily(remoteTransitionConfiguration));

final Optional<BlockHeader> maybeTerminalPoWBlockHeader = mergeContext.getTerminalPoWBlock();
final Optional<BlockHeader> maybeTerminalPoWBlockHeader =
mergeContext.flatMap(c -> c.getTerminalPoWBlock());

final EngineExchangeTransitionConfigurationResult localTransitionConfiguration =
new EngineExchangeTransitionConfigurationResult(
mergeContext.getTerminalTotalDifficulty(),
mergeContext.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.ifPresent(
c ->
c.fireNewUnverifiedForkchoiceMessageEvent(
forkChoice.getHeadBlockHash(), maybeFinalizedHash, forkChoice.getSafeBlockHash()));

if (mergeContext.isSyncing()) {
if (mergeContext.map(c -> c.isSyncing()).orElse(Boolean.TRUE)) {
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.flatMap(c -> c.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.map(c -> c.isSyncing()).orElse(Boolean.TRUE) || parentHeader.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kinda hard to read. It looks like it evaluates to a no-op, does anyone else find this harder to read?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see how it is harder to read, it is a side effect of making mergeContext an Optional. looking for an approach that is easier on the 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supplier/function reference ftw 👍

LOG.debug(
"isSyncing: {} parentHeaderMissing: {}, adding {} to backwardsync",
mergeContext.isSyncing(),
mergeContext.map(c -> c.isSyncing()).orElse(null),
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
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public class EngineGetPayloadTest {
@Before
public void before() {
when(mergeContext.retrieveBlockById(mockPid)).thenReturn(Optional.of(mockBlock));
when(protocolContext.getConsensusContext(Mockito.any())).thenReturn(mergeContext);
when(protocolContext.safeConsensusContext(Mockito.any())).thenReturn(Optional.of(mergeContext));
this.method = new EngineGetPayload(vertx, protocolContext, factory);
}

Expand Down
Loading