From 28bc1c891b5aeff4c3cfde9f26ee2e95b23c812c Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Mar 2024 00:35:15 +0400 Subject: [PATCH 1/8] Add clients information to user's graffiti when producing a block --- .../BlockProposalAcceptanceTest.java | 101 +++++ .../test/acceptance/dsl/TekuBeaconNode.java | 14 + .../acceptance/dsl/TekuNodeConfigBuilder.java | 6 + beacon/validator/build.gradle | 1 + .../BlockOperationSelectorFactory.java | 8 +- .../DefaultGraffitiProviderImpl.java | 117 ----- .../coordinator/GraffitiBuilder.java | 175 ++++++++ .../coordinator/AbstractBlockFactoryTest.java | 4 + .../coordinator/BlockFactoryDenebTest.java | 5 +- .../coordinator/BlockFactoryPhase0Test.java | 5 +- .../BlockOperationSelectorFactoryTest.java | 12 +- .../DefaultGraffitiProviderImplTest.java | 110 ----- .../coordinator/GraffitiBuilderTest.java | 424 ++++++++++++++++++ .../ExecutionClientVersionChannel.java | 13 +- .../ExecutionClientVersionProvider.java | 81 ++++ .../ExecutionClientVersionProviderTest.java | 93 ++++ .../execution/ClientVersion.java | 2 + .../infrastructure/logging/EventLogger.java | 7 + infrastructure/version/build.gradle | 1 + .../version/VersionProvider.java | 13 + .../beaconchain/BeaconChainController.java | 18 +- .../teku/cli/options/ValidatorOptions.java | 20 + .../cli/options/ValidatorOptionsTest.java | 20 + .../api/ClientGraffitiAppendFormat.java | 31 ++ .../teku/validator/api/ValidatorConfig.java | 24 +- 25 files changed, 1049 insertions(+), 256 deletions(-) create mode 100644 acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java delete mode 100644 beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProviderImpl.java create mode 100644 beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java delete mode 100644 beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProviderImplTest.java create mode 100644 beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java rename beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProvider.java => ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionChannel.java (58%) create mode 100644 ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProvider.java create mode 100644 ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java create mode 100644 validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java diff --git a/acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java b/acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java new file mode 100644 index 00000000000..5d579c84008 --- /dev/null +++ b/acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/BlockProposalAcceptanceTest.java @@ -0,0 +1,101 @@ +/* + * Copyright Consensys Software Inc., 2022 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.test.acceptance; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.io.Resources; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Locale; +import org.apache.tuweni.bytes.Bytes32; +import org.junit.jupiter.api.Test; +import tech.pegasys.teku.api.schema.bellatrix.SignedBeaconBlockBellatrix; +import tech.pegasys.teku.infrastructure.unsigned.UInt64; +import tech.pegasys.teku.test.acceptance.dsl.AcceptanceTestBase; +import tech.pegasys.teku.test.acceptance.dsl.GenesisGenerator.InitialStateData; +import tech.pegasys.teku.test.acceptance.dsl.TekuBeaconNode; +import tech.pegasys.teku.test.acceptance.dsl.TekuNodeConfigBuilder; +import tech.pegasys.teku.test.acceptance.dsl.TekuValidatorNode; +import tech.pegasys.teku.test.acceptance.dsl.tools.deposits.ValidatorKeystores; + +public class BlockProposalAcceptanceTest extends AcceptanceTestBase { + private static final URL JWT_FILE = Resources.getResource("auth/ee-jwt-secret.hex"); + + @Test + void shouldHaveCorrectFeeRecipientAndGraffiti() throws Exception { + final String networkName = "swift"; + + final ValidatorKeystores validatorKeystores = + createTekuDepositSender(networkName).generateValidatorKeys(8); + + final InitialStateData genesis = + createGenesisGenerator() + .network(networkName) + .withAltairEpoch(UInt64.ZERO) + .withBellatrixEpoch(UInt64.ZERO) + .validatorKeys(validatorKeystores, validatorKeystores) + .generate(); + + final String defaultFeeRecipient = "0xFE3B557E8Fb62b89F4916B721be55cEb828dBd73"; + final String userGraffiti = "My block \uD83D\uDE80"; // 13 bytes + final TekuBeaconNode beaconNode = + createTekuBeaconNode( + TekuNodeConfigBuilder.createBeaconNode() + .withStubExecutionEngine() + .withJwtSecretFile(JWT_FILE) + .withNetwork(networkName) + .withInitialState(genesis) + .withAltairEpoch(UInt64.ZERO) + .withBellatrixEpoch(UInt64.ZERO) + .withValidatorProposerDefaultFeeRecipient(defaultFeeRecipient) + .build()); + final TekuValidatorNode validatorClient = + createValidatorNode( + TekuNodeConfigBuilder.createValidatorClient() + .withReadOnlyKeystorePath(validatorKeystores) + .withValidatorProposerDefaultFeeRecipient(defaultFeeRecipient) + .withInteropModeDisabled() + .withBeaconNodes(beaconNode) + .withGraffiti(userGraffiti) + .withNetwork("auto") + .build()); + + beaconNode.start(); + validatorClient.start(); + + beaconNode.waitForBlockSatisfying( + block -> { + assertThat(block).isInstanceOf(SignedBeaconBlockBellatrix.class); + final SignedBeaconBlockBellatrix bellatrixBlock = (SignedBeaconBlockBellatrix) block; + assertThat( + bellatrixBlock.getMessage().getBody().executionPayload.feeRecipient.toHexString()) + .isEqualTo(defaultFeeRecipient.toLowerCase(Locale.ROOT)); + final Bytes32 graffiti = bellatrixBlock.getMessage().getBody().graffiti; + final String graffitiMessage = + new String( + Arrays.copyOfRange( + graffiti.toArray(), 0, 32 - graffiti.numberOfTrailingZeroBytes()), + StandardCharsets.UTF_8); + // 13 bytes + 1 byte + assertThat(graffitiMessage).startsWith(userGraffiti + " "); + // 18 bytes left, so 12 bytes client footprint: TKxxxxELxxxx. 20 bytes with full commits + // doesn't fit + assertThat(graffitiMessage).contains("TK"); + // stub execution endpoint. + assertThat(graffitiMessage).endsWith("SB0000"); + }); + } +} diff --git a/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java b/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java index 095fec6abad..85a28bd1328 100644 --- a/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java +++ b/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java @@ -46,6 +46,7 @@ import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; +import org.assertj.core.api.ThrowingConsumer; import org.testcontainers.containers.Network; import org.testcontainers.containers.wait.strategy.HttpWaitStrategy; import tech.pegasys.teku.api.response.v1.EventType; @@ -484,6 +485,19 @@ public void waitForNonDefaultExecutionPayload() { MINUTES); } + public void waitForBlockSatisfying(final ThrowingConsumer consumer) { + LOG.debug("Wait for a block satisfying certain assertions"); + + waitFor( + () -> { + final Optional block = fetchHeadBlock(); + assertThat(block).isPresent(); + assertThat(block.get()).satisfies(consumer); + }, + 5, + MINUTES); + } + public void waitForGenesisWithNonDefaultExecutionPayload() { LOG.debug("Wait for genesis block containing a non default execution payload"); diff --git a/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java b/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java index ba9d142399a..8f9c84f3bcd 100644 --- a/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java +++ b/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuNodeConfigBuilder.java @@ -520,6 +520,12 @@ public TekuNodeConfigBuilder withGenesisTime(int time) { return this; } + public TekuNodeConfigBuilder withGraffiti(final String graffiti) { + LOG.debug("validators-graffiti: {}", graffiti); + configMap.put("validators-graffiti", graffiti); + return this; + } + private TekuNodeConfigBuilder withPrivateKey(PrivKey privKey) throws IOException { mustBe(NodeType.BEACON_NODE); this.maybePrivKey = Optional.ofNullable(privKey); diff --git a/beacon/validator/build.gradle b/beacon/validator/build.gradle index f76d2f2de89..d069f585e01 100644 --- a/beacon/validator/build.gradle +++ b/beacon/validator/build.gradle @@ -18,6 +18,7 @@ dependencies { implementation project(':ethereum:pow:api') implementation project(':ethereum:pow:merkletree') implementation project(':ethereum:spec') + implementation project(':ethereum:executionclient') implementation project(':networking:eth2') implementation project(':infrastructure:logging') implementation project(':infrastructure:ssz') diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java index f68b1714944..1f7d73209eb 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactory.java @@ -72,7 +72,7 @@ public class BlockOperationSelectorFactory { private final SyncCommitteeContributionPool contributionPool; private final DepositProvider depositProvider; private final Eth1DataCache eth1DataCache; - private final DefaultGraffitiProvider defaultGraffitiProvider; + private final GraffitiBuilder graffitiBuilder; private final ForkChoiceNotifier forkChoiceNotifier; private final ExecutionLayerBlockProductionManager executionLayerBlockProductionManager; @@ -86,7 +86,7 @@ public BlockOperationSelectorFactory( final SyncCommitteeContributionPool contributionPool, final DepositProvider depositProvider, final Eth1DataCache eth1DataCache, - final DefaultGraffitiProvider defaultGraffitiProvider, + final GraffitiBuilder graffitiBuilder, final ForkChoiceNotifier forkChoiceNotifier, final ExecutionLayerBlockProductionManager executionLayerBlockProductionManager) { this.spec = spec; @@ -98,7 +98,7 @@ public BlockOperationSelectorFactory( this.contributionPool = contributionPool; this.depositProvider = depositProvider; this.eth1DataCache = eth1DataCache; - this.defaultGraffitiProvider = defaultGraffitiProvider; + this.graffitiBuilder = graffitiBuilder; this.forkChoiceNotifier = forkChoiceNotifier; this.executionLayerBlockProductionManager = executionLayerBlockProductionManager; } @@ -147,7 +147,7 @@ public Function> createSelector( bodyBuilder .randaoReveal(randaoReveal) .eth1Data(eth1Data) - .graffiti(optionalGraffiti.orElse(defaultGraffitiProvider.getDefaultGraffiti())) + .graffiti(graffitiBuilder.buildGraffiti(optionalGraffiti)) .attestations(attestations) .proposerSlashings(proposerSlashings) .attesterSlashings(attesterSlashings) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProviderImpl.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProviderImpl.java deleted file mode 100644 index c329bb25ef9..00000000000 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProviderImpl.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Copyright Consensys Software Inc., 2024 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.validator.coordinator; - -import static tech.pegasys.teku.infrastructure.logging.EventLogger.EVENT_LOG; - -import java.nio.charset.StandardCharsets; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.apache.tuweni.bytes.Bytes; -import org.apache.tuweni.bytes.Bytes32; -import tech.pegasys.teku.ethereum.events.ExecutionClientEventsChannel; -import tech.pegasys.teku.infrastructure.bytes.Bytes4; -import tech.pegasys.teku.infrastructure.version.VersionProvider; -import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; -import tech.pegasys.teku.spec.executionlayer.ExecutionLayerChannel; - -/** - * Based on Specify Client Versions on - * Engine API. This provider will return EL & Teku client version graffiti when the EL supports - * version specification and {@link ExecutionLayerChannel#engineGetClientVersion(ClientVersion)} has - * been called. Otherwise, it will return an only Teku version graffiti fallback. - */ -public class DefaultGraffitiProviderImpl - implements DefaultGraffitiProvider, ExecutionClientEventsChannel { - - private static final Logger LOG = LogManager.getLogger(); - - private final AtomicBoolean lastExecutionClientAvailability = new AtomicBoolean(true); - - private final ExecutionLayerChannel executionLayerChannel; - private final ClientVersion tekuClientVersion; - private final AtomicReference defaultGraffiti; - - public DefaultGraffitiProviderImpl(final ExecutionLayerChannel executionLayerChannel) { - this.executionLayerChannel = executionLayerChannel; - this.tekuClientVersion = createTekuClientVersion(); - this.defaultGraffiti = new AtomicReference<>(getGraffitiFallback()); - // update default graffiti on initialization - updateDefaultGraffiti(); - } - - @Override - public Bytes32 getDefaultGraffiti() { - return defaultGraffiti.get(); - } - - @Override - public void onAvailabilityUpdated(final boolean isAvailable) { - // only update graffiti after EL has been unavailable - if (isAvailable && lastExecutionClientAvailability.compareAndSet(false, true)) { - updateDefaultGraffiti(); - } else { - lastExecutionClientAvailability.set(isAvailable); - } - } - - private ClientVersion createTekuClientVersion() { - return new ClientVersion( - ClientVersion.TEKU_CLIENT_CODE, - VersionProvider.CLIENT_IDENTITY, - VersionProvider.IMPLEMENTATION_VERSION, - VersionProvider.COMMIT_HASH - .map(commitHash -> Bytes4.fromHexString(commitHash.substring(0, 8))) - .orElse(Bytes4.ZERO)); - } - - private Bytes32 getGraffitiFallback() { - final String graffiti = - VersionProvider.CLIENT_IDENTITY + "/" + VersionProvider.IMPLEMENTATION_VERSION; - return convertToBytes32(graffiti); - } - - private void updateDefaultGraffiti() { - executionLayerChannel - .engineGetClientVersion(tekuClientVersion) - .thenAccept( - clientVersions -> { - final ClientVersion executionClientVersion = clientVersions.get(0); - EVENT_LOG.logExecutionClientVersion( - executionClientVersion.name(), executionClientVersion.version()); - defaultGraffiti.set(getExecutionAndTekuGraffiti(executionClientVersion)); - }) - .finish(ex -> LOG.debug("Exception while calling engine_getClientVersion", ex)); - } - - private Bytes32 getExecutionAndTekuGraffiti(final ClientVersion executionClientVersion) { - final String graffiti = - executionClientVersion.code() - + executionClientVersion.commit().toUnprefixedHexString() - + tekuClientVersion.code() - + tekuClientVersion.commit().toUnprefixedHexString(); - return convertToBytes32(graffiti); - } - - private Bytes32 convertToBytes32(final String graffiti) { - final Bytes graffitiBytes = Bytes.wrap(graffiti.getBytes(StandardCharsets.UTF_8)); - if (graffitiBytes.size() <= Bytes32.SIZE) { - return Bytes32.rightPad(graffitiBytes); - } else { - return Bytes32.wrap(graffitiBytes.slice(0, Bytes32.SIZE)); - } - } -} diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java new file mode 100644 index 00000000000..541148d0c3b --- /dev/null +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java @@ -0,0 +1,175 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.validator.coordinator; + +import static tech.pegasys.teku.infrastructure.logging.EventLogger.EVENT_LOG; + +import com.google.common.annotations.VisibleForTesting; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Locale; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.tuweni.bytes.Bytes; +import org.apache.tuweni.bytes.Bytes32; +import tech.pegasys.teku.ethereum.executionclient.ExecutionClientVersionChannel; +import tech.pegasys.teku.infrastructure.version.VersionProvider; +import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; +import tech.pegasys.teku.validator.api.Bytes32Parser; +import tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat; + +/** + * Builds graffiti from user's graffiti input and CL/EL client info according to the + * clientGraffitiAppendFormat configuration. + */ +public class GraffitiBuilder implements ExecutionClientVersionChannel { + private static final Logger LOG = LogManager.getLogger(); + + private static final String SPACE = " "; + + private final ClientGraffitiAppendFormat clientGraffitiAppendFormat; + private final ClientVersion consensusClientVersion; + private final AtomicReference executionClientVersion; + private final Optional defaultUserGraffiti; + + public GraffitiBuilder( + final ClientGraffitiAppendFormat clientGraffitiAppendFormat, + final Optional defaultUserGraffiti) { + this.clientGraffitiAppendFormat = clientGraffitiAppendFormat; + this.consensusClientVersion = VersionProvider.createTekuClientVersion(); + this.executionClientVersion = new AtomicReference<>(ClientVersion.UNKNOWN); + this.defaultUserGraffiti = defaultUserGraffiti; + } + + @Override + public void onExecutionClientVersion(final ClientVersion executionClientVersion) { + this.executionClientVersion.set(executionClientVersion); + final Optional graffiti = Optional.of(buildGraffiti(defaultUserGraffiti)); + EVENT_LOG.logDefaultGraffiti(extractGraffiti(graffiti, calculateGraffitiLength(graffiti))); + } + + public Bytes32 buildGraffiti(final Optional userGraffiti) { + try { + final int userGraffitiLength = calculateGraffitiLength(userGraffiti); + + return switch (clientGraffitiAppendFormat) { + case AUTO_END -> { + final int clientInfoLength = Bytes32.SIZE - 1 - userGraffitiLength; + yield joinNonEmpty( + SPACE, + extractGraffiti(userGraffiti, userGraffitiLength), + formatClientInfo(clientInfoLength)); + } + case AUTO_START -> { + final int clientInfoLength = Bytes32.SIZE - 1 - userGraffitiLength; + yield joinNonEmpty( + SPACE, + formatClientInfo(clientInfoLength), + extractGraffiti(userGraffiti, userGraffitiLength)); + } + case NAME_END -> { + final int clientInfoLength = Integer.min(Bytes32.SIZE - 1 - userGraffitiLength, 4); + yield joinNonEmpty( + SPACE, + extractGraffiti(userGraffiti, userGraffitiLength), + formatClientInfo(clientInfoLength)); + } + case NAME_START -> { + final int clientInfoLength = Integer.min(Bytes32.SIZE - 1 - userGraffitiLength, 4); + yield joinNonEmpty( + SPACE, + formatClientInfo(clientInfoLength), + extractGraffiti(userGraffiti, userGraffitiLength)); + } + case NONE -> userGraffiti.orElse(Bytes32.ZERO); + }; + } catch (final Exception ex) { + LOG.error("Unexpected error when preparing block graffiti", ex); + return userGraffiti.orElse(Bytes32.ZERO); + } + } + + @VisibleForTesting + protected String extractGraffiti(final Optional graffiti, final int length) { + return graffiti + .map(Bytes::toArray) + .map(bytes -> Arrays.copyOf(bytes, length)) + .map(bytes -> new String(bytes, StandardCharsets.UTF_8)) + .orElse(""); + } + + @VisibleForTesting + protected int calculateGraffitiLength(final Optional graffiti) { + return Bytes32.SIZE - graffiti.map(Bytes::numberOfTrailingZeroBytes).orElse(Bytes32.SIZE); + } + + @VisibleForTesting + protected Bytes32 joinNonEmpty(final String delimiter, final String... parts) { + final String graffiti = + Arrays.stream(parts).filter(part -> !part.isEmpty()).collect(Collectors.joining(delimiter)); + return Bytes32Parser.toBytes32(graffiti); + } + + @VisibleForTesting + protected String formatClientInfo(final int length) { + final String safeConsensusCode = extractClientCodeSafely(consensusClientVersion); + final String safeExecutionCode = extractClientCodeSafely(executionClientVersion.get()); + // LH1be52536BU0f91a674 + if (length >= 20) { + return String.format( + "%s%s%s%s", + safeConsensusCode, + consensusClientVersion.commit().toUnprefixedHexString(), + safeExecutionCode, + executionClientVersion.get().commit().toUnprefixedHexString()); + } + // LH1be5BU0f91 + if (length >= 12) { + return String.format( + "%s%s%s%s", + safeConsensusCode, + consensusClientVersion.commit().toUnprefixedHexString().substring(0, 4), + safeExecutionCode, + executionClientVersion.get().commit().toUnprefixedHexString().substring(0, 4)); + } + // LH1bBU0f + if (length >= 8) { + return String.format( + "%s%s%s%s", + safeConsensusCode, + consensusClientVersion.commit().toUnprefixedHexString().substring(0, 2), + safeExecutionCode, + executionClientVersion.get().commit().toUnprefixedHexString().substring(0, 2)); + } + // LHBU + if (length >= 4) { + return String.format("%s%s", safeConsensusCode, safeExecutionCode); + } + + return ""; + } + + protected String extractClientCodeSafely(final ClientVersion clientVersion) { + final boolean isValid = + clientVersion.code() != null + && clientVersion.code().length() >= 2 + && clientVersion.code().substring(0, 2).matches("[a-zA-Z]{2}"); + return isValid + ? clientVersion.code().substring(0, 2).toUpperCase(Locale.ROOT) + : ClientVersion.UNKNOWN.code(); + } +} diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java index d4f250eca28..3fb4773e510 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java @@ -85,6 +85,7 @@ import tech.pegasys.teku.storage.client.RecentChainData; import tech.pegasys.teku.storage.storageSystem.InMemoryStorageSystemBuilder; import tech.pegasys.teku.storage.storageSystem.StorageSystem; +import tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat; @SuppressWarnings("unchecked") public abstract class AbstractBlockFactoryTest { @@ -117,6 +118,9 @@ public abstract class AbstractBlockFactoryTest { protected ExecutionPayloadResult cachedExecutionPayloadResult = null; + protected GraffitiBuilder graffitiBuilder = + new GraffitiBuilder(ClientGraffitiAppendFormat.NONE, Optional.empty()); + @BeforeAll public static void initSession() { AbstractBlockProcessor.depositSignatureVerifier = BLSSignatureVerifier.NO_OP; diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java index 6f9bc6d2451..13d834c83d9 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryDenebTest.java @@ -19,7 +19,6 @@ import java.util.List; import java.util.stream.IntStream; -import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.Test; import tech.pegasys.teku.infrastructure.ssz.SszCollection; import tech.pegasys.teku.infrastructure.ssz.SszList; @@ -178,8 +177,6 @@ void shouldCreateValidBlobSidecarsForBlindedBlock() { @Override public BlockFactory createBlockFactory(final Spec spec) { - final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec); - final Bytes32 graffiti = dataStructureUtil.randomBytes32(); return new BlockFactoryDeneb( spec, new BlockOperationSelectorFactory( @@ -192,7 +189,7 @@ public BlockFactory createBlockFactory(final Spec spec) { syncCommitteeContributionPool, depositProvider, eth1DataCache, - () -> graffiti, + graffitiBuilder, forkChoiceNotifier, executionLayer)); } diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryPhase0Test.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryPhase0Test.java index 56797866c3c..8a4296ece14 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryPhase0Test.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockFactoryPhase0Test.java @@ -18,7 +18,6 @@ import static org.mockito.Mockito.verify; import static tech.pegasys.teku.spec.datastructures.blocks.blockbody.versions.altair.SyncAggregateAssert.assertThatSyncAggregate; -import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.Test; import tech.pegasys.teku.infrastructure.ssz.SszList; import tech.pegasys.teku.infrastructure.unsigned.UInt64; @@ -187,8 +186,6 @@ void shouldCreateEmptyBlobSidecarsForBlindedBlock() { @Override public BlockFactory createBlockFactory(final Spec spec) { - final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec); - final Bytes32 graffiti = dataStructureUtil.randomBytes32(); return new BlockFactoryPhase0( spec, new BlockOperationSelectorFactory( @@ -201,7 +198,7 @@ public BlockFactory createBlockFactory(final Spec spec) { syncCommitteeContributionPool, depositProvider, eth1DataCache, - () -> graffiti, + graffitiBuilder, forkChoiceNotifier, executionLayer)); } diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java index ca8839aed5c..5828e105f3c 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java @@ -89,6 +89,7 @@ import tech.pegasys.teku.statetransition.synccommittee.SignedContributionAndProofValidator; import tech.pegasys.teku.statetransition.synccommittee.SyncCommitteeContributionPool; import tech.pegasys.teku.statetransition.validation.OperationValidator; +import tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat; class BlockOperationSelectorFactoryTest { private final Spec spec = TestSpecFactory.createMinimalDeneb(); @@ -175,6 +176,9 @@ class BlockOperationSelectorFactoryTest { private final CapturingBeaconBlockBodyBuilder bodyBuilder = new CapturingBeaconBlockBodyBuilder(false); + private final GraffitiBuilder graffitiBuilder = + new GraffitiBuilder(ClientGraffitiAppendFormat.NONE, Optional.empty()); + private final BlockOperationSelectorFactory factory = new BlockOperationSelectorFactory( spec, @@ -186,7 +190,7 @@ class BlockOperationSelectorFactoryTest { contributionPool, depositProvider, eth1DataCache, - () -> defaultGraffiti, + graffitiBuilder, forkChoiceNotifier, executionLayer); private final BlockOperationSelectorFactory factoryBellatrix = @@ -200,7 +204,7 @@ class BlockOperationSelectorFactoryTest { contributionPool, depositProvider, eth1DataCache, - () -> defaultGraffiti, + graffitiBuilder, forkChoiceNotifier, executionLayer); private ExecutionPayloadContext executionPayloadContext; @@ -290,7 +294,7 @@ void shouldIncludeValidOperations() { parentRoot, blockSlotState, randaoReveal, - Optional.empty(), + Optional.of(defaultGraffiti), Optional.empty(), Optional.empty(), BlockProductionPerformance.NOOP) @@ -378,7 +382,7 @@ void shouldNotIncludeInvalidOperations() { parentRoot, blockSlotState, randaoReveal, - Optional.empty(), + Optional.of(defaultGraffiti), Optional.empty(), Optional.empty(), BlockProductionPerformance.NOOP) diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProviderImplTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProviderImplTest.java deleted file mode 100644 index 3deb776f118..00000000000 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProviderImplTest.java +++ /dev/null @@ -1,110 +0,0 @@ -/* - * Copyright Consensys Software Inc., 2024 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.validator.coordinator; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.nio.charset.StandardCharsets; -import java.util.List; -import org.apache.tuweni.bytes.Bytes32; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import tech.pegasys.teku.infrastructure.async.SafeFuture; -import tech.pegasys.teku.infrastructure.bytes.Bytes4; -import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; -import tech.pegasys.teku.spec.executionlayer.ExecutionLayerChannel; - -class DefaultGraffitiProviderImplTest { - - private final ExecutionLayerChannel executionLayerChannel = mock(ExecutionLayerChannel.class); - - @BeforeEach - public void setUp() { - final ClientVersion executionClientVersion = - new ClientVersion("BU", "besu", "1.0.0", Bytes4.fromHexString("8dba2981")); - when(executionLayerChannel.engineGetClientVersion(any())) - .thenReturn(SafeFuture.completedFuture(List.of(executionClientVersion))); - } - - @Test - public void getsExecutionAndTekuDefaultGraffitiIfEngineClientVersionSucceedsOnInitialization() { - final DefaultGraffitiProviderImpl defaultGraffitiProvider = - new DefaultGraffitiProviderImpl(executionLayerChannel); - - assertExecutionAndTekuGraffiti(defaultGraffitiProvider.getDefaultGraffiti()); - } - - @Test - public void getsDefaultGraffitiFallbackIfEngineGetClientVersionFailsOnInitialization() { - when(executionLayerChannel.engineGetClientVersion(any())) - .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); - - final DefaultGraffitiProviderImpl defaultGraffitiProvider = - new DefaultGraffitiProviderImpl(executionLayerChannel); - - assertGraffitiFallback(defaultGraffitiProvider.getDefaultGraffiti()); - } - - @Test - public void doesNotTryToUpdateDefaultGraffitiIfElHasNotBeenUnavailable() { - final DefaultGraffitiProviderImpl defaultGraffitiProvider = - new DefaultGraffitiProviderImpl(executionLayerChannel); - - assertExecutionAndTekuGraffiti(defaultGraffitiProvider.getDefaultGraffiti()); - - defaultGraffitiProvider.onAvailabilityUpdated(true); - - // no change in graffiti - assertExecutionAndTekuGraffiti(defaultGraffitiProvider.getDefaultGraffiti()); - // EL called only one time - verify(executionLayerChannel, times(1)).engineGetClientVersion(any()); - } - - @Test - public void updatesDefaultGraffitiIfElIsAvailableAfterBeingUnavailable() { - final DefaultGraffitiProviderImpl defaultGraffitiProvider = - new DefaultGraffitiProviderImpl(executionLayerChannel); - - assertExecutionAndTekuGraffiti(defaultGraffitiProvider.getDefaultGraffiti()); - - final ClientVersion updatedExecutionClientVersion = - new ClientVersion("BU", "besu", "1.0.1", Bytes4.fromHexString("efd1bc70")); - when(executionLayerChannel.engineGetClientVersion(any())) - .thenReturn(SafeFuture.completedFuture(List.of(updatedExecutionClientVersion))); - - defaultGraffitiProvider.onAvailabilityUpdated(false); - defaultGraffitiProvider.onAvailabilityUpdated(true); - - // graffiti has changed - final Bytes32 changedGraffiti = defaultGraffitiProvider.getDefaultGraffiti(); - assertThat(new String(changedGraffiti.toArrayUnsafe(), StandardCharsets.UTF_8)) - .matches("BUefd1bc70TK[0-9a-fA-F]{8}.+"); - // EL called two times - verify(executionLayerChannel, times(2)).engineGetClientVersion(any()); - } - - private void assertExecutionAndTekuGraffiti(final Bytes32 graffiti) { - assertThat(new String(graffiti.toArrayUnsafe(), StandardCharsets.UTF_8)) - .matches("BU8dba2981TK[0-9a-fA-F]{8}.+"); - } - - private void assertGraffitiFallback(final Bytes32 graffiti) { - assertThat(new String(graffiti.toArrayUnsafe(), StandardCharsets.UTF_8)).startsWith("teku/v"); - } -} diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java new file mode 100644 index 00000000000..dfad73aa301 --- /dev/null +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java @@ -0,0 +1,424 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.validator.coordinator; + +import static org.assertj.core.api.Assertions.assertThat; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.AUTO_END; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.AUTO_START; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NAME_END; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NAME_START; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NONE; + +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Optional; +import java.util.stream.Stream; +import org.apache.tuweni.bytes.Bytes32; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import tech.pegasys.infrastructure.logging.LogCaptor; +import tech.pegasys.teku.infrastructure.bytes.Bytes4; +import tech.pegasys.teku.infrastructure.logging.EventLogger; +import tech.pegasys.teku.infrastructure.version.VersionProvider; +import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; +import tech.pegasys.teku.validator.api.Bytes32Parser; +import tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat; + +public class GraffitiBuilderTest { + private ClientGraffitiAppendFormat clientGraffitiAppendFormat = AUTO_END; + private Optional userGraffiti = Optional.empty(); + private GraffitiBuilder graffitiBuilder = + new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); + + private static final ClientVersion TEKU_CLIENT_VERSION = + VersionProvider.createTekuClientVersion(); + private static final ClientVersion BESU_CLIENT_VERSION = + new ClientVersion("BU", "Besu", "23.4.1", Bytes4.fromHexString("abcdef12")); + + private final String asciiGraffiti0 = ""; + private static final String ASCII_GRAFFITI_20 = "I've proposed ablock"; + private final String asciiGraffiti32 = "I've proposed a good Teku block!"; + + private static final String UTF_8_GRAFFITI_4 = "\uD83D\uDE80"; + private final String utf8Graffiti20 = "\uD83D\uDE80 !My block! \uD83D\uDE80"; + + @BeforeEach + public void setup() { + this.clientGraffitiAppendFormat = AUTO_END; + this.userGraffiti = Optional.empty(); + this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); + } + + @Test + public void onExecutionClientVersion_shouldLogDefaultGraffiti() { + try (final LogCaptor logCaptor = LogCaptor.forClass(EventLogger.class)) { + graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); + logCaptor.assertInfoLog( + "Default graffiti to use when building block without external VC: \"TK" + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + "BUabcdef12\". " + + "To change check validator graffiti options."); + } + } + + @Test + public void onExecutionClientVersion_shouldLogDefaultMergedGraffiti() { + this.graffitiBuilder = + new GraffitiBuilder( + clientGraffitiAppendFormat, Optional.of(Bytes32Parser.toBytes32(ASCII_GRAFFITI_20))); + try (final LogCaptor logCaptor = LogCaptor.forClass(EventLogger.class)) { + graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); + logCaptor.assertInfoLog( + "Default graffiti to use when building block without external VC: \"I've proposed ablock TK" + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) + + "BUab\". " + + "To change check validator graffiti options."); + } + } + + @Test + public void buildGraffiti_shouldNotFail() { + this.graffitiBuilder = + new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti) { + @Override + protected int calculateGraffitiLength(Optional graffiti) { + throw new RuntimeException(""); + } + }; + assertThat(graffitiBuilder.buildGraffiti(Optional.empty())).isEqualTo(Bytes32.ZERO); + final Bytes32 graffiti = Bytes32Parser.toBytes32(ASCII_GRAFFITI_20); + assertThat(graffitiBuilder.buildGraffiti(Optional.of(graffiti))).isEqualTo(graffiti); + } + + @ParameterizedTest() + @MethodSource("getBuildGraffitiFixtures") + public void buildGraffiti_shouldProvideCorrectOutput( + final ClientGraffitiAppendFormat clientGraffitiAppendFormat, + final Optional maybeUserGraffiti, + final String expectedGraffiti) { + this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); + graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); + final Bytes32 expectedGraffitiBytes = Bytes32Parser.toBytes32(expectedGraffiti); + assertThat( + new String( + Arrays.copyOfRange( + expectedGraffitiBytes.toArray(), + 0, + 32 - expectedGraffitiBytes.numberOfTrailingZeroBytes()), + StandardCharsets.UTF_8)) + .isEqualTo(expectedGraffiti); + assertThat(graffitiBuilder.buildGraffiti(maybeUserGraffiti.map(Bytes32Parser::toBytes32))) + .isEqualTo(expectedGraffitiBytes); + } + + @Test + public void extractGraffiti_shouldReturnCorrectGraffiti() { + assertThat(graffitiBuilder.extractGraffiti(Optional.empty(), 0)).isEqualTo(""); + assertThat( + graffitiBuilder.extractGraffiti( + Optional.of(Bytes32Parser.toBytes32(asciiGraffiti0)), 0)) + .isEqualTo(""); + assertThat( + graffitiBuilder.extractGraffiti( + Optional.of(Bytes32Parser.toBytes32(ASCII_GRAFFITI_20)), 20)) + .isEqualTo(ASCII_GRAFFITI_20); + assertThat( + graffitiBuilder.extractGraffiti( + Optional.of(Bytes32Parser.toBytes32(asciiGraffiti32)), 32)) + .isEqualTo(asciiGraffiti32); + assertThat( + graffitiBuilder.extractGraffiti( + Optional.of(Bytes32Parser.toBytes32(asciiGraffiti32)), 16)) + .isEqualTo(asciiGraffiti32.substring(0, 16)); + assertThat( + graffitiBuilder.extractGraffiti( + Optional.of(Bytes32Parser.toBytes32(UTF_8_GRAFFITI_4)), 4)) + .isEqualTo(UTF_8_GRAFFITI_4); + assertThat( + graffitiBuilder.extractGraffiti( + Optional.of(Bytes32Parser.toBytes32(utf8Graffiti20)), 20)) + .isEqualTo(utf8Graffiti20); + assertThat( + graffitiBuilder.extractGraffiti( + Optional.of(Bytes32Parser.toBytes32(utf8Graffiti20)), 24)) + .isEqualTo(utf8Graffiti20 + "\u0000\u0000\u0000\u0000"); + } + + @Test + public void calculateGraffitiLength_shouldReturnCorrectLength() { + assertThat(graffitiBuilder.calculateGraffitiLength(Optional.empty())).isEqualTo(0); + assertThat( + graffitiBuilder.calculateGraffitiLength( + Optional.of(Bytes32Parser.toBytes32(asciiGraffiti0)))) + .isEqualTo(0); + assertThat( + graffitiBuilder.calculateGraffitiLength( + Optional.of(Bytes32Parser.toBytes32(ASCII_GRAFFITI_20)))) + .isEqualTo(20); + assertThat( + graffitiBuilder.calculateGraffitiLength( + Optional.of(Bytes32Parser.toBytes32(asciiGraffiti32)))) + .isEqualTo(32); + assertThat( + graffitiBuilder.calculateGraffitiLength( + Optional.of(Bytes32Parser.toBytes32(UTF_8_GRAFFITI_4)))) + .isEqualTo(4); + assertThat( + graffitiBuilder.calculateGraffitiLength( + Optional.of(Bytes32Parser.toBytes32(utf8Graffiti20)))) + .isEqualTo(20); + } + + @Test + public void join_shouldJoinStringsSkippingEmpty() { + assertThat(graffitiBuilder.joinNonEmpty("", "aa", "bb", "cc")) + .isEqualTo(Bytes32Parser.toBytes32("aabbcc")); + assertThat(graffitiBuilder.joinNonEmpty(" ", "aa", "", "cc")) + .isEqualTo(Bytes32Parser.toBytes32("aa cc")); + assertThat(graffitiBuilder.joinNonEmpty(" ", "", "bb", "cc")) + .isEqualTo(Bytes32Parser.toBytes32("bb cc")); + assertThat(graffitiBuilder.joinNonEmpty(" ", "", "", "")).isEqualTo(Bytes32.ZERO); + } + + @Test + public void formatClientInfo_shouldFitInDesiredLength() { + graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); + + // 20: LH1be52536BU0f91a674 + assertThat(graffitiBuilder.formatClientInfo(30)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(20)); + assertThat(graffitiBuilder.formatClientInfo(20)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(20)); + + // 12: LH1be5BU0f91 + assertThat(graffitiBuilder.formatClientInfo(19)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4) + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4)) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(12)); + assertThat(graffitiBuilder.formatClientInfo(12)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4) + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4)) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(12)); + + // 8: LH1bBU0f + assertThat(graffitiBuilder.formatClientInfo(11)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(8)); + assertThat(graffitiBuilder.formatClientInfo(8)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(8)); + + // 4: LHBU + assertThat(graffitiBuilder.formatClientInfo(7)) + .isEqualTo(TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(4)); + assertThat(graffitiBuilder.formatClientInfo(4)) + .isEqualTo(TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(4)); + + // Empty + assertThat(graffitiBuilder.formatClientInfo(3)) + .isEqualTo("") + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(0)); + assertThat(graffitiBuilder.formatClientInfo(0)) + .isEqualTo("") + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(0)); + assertThat(graffitiBuilder.formatClientInfo(-1)) + .isEqualTo("") + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(0)); + } + + @ParameterizedTest() + @MethodSource("getClientCodes") + public void formatClientInfo_shouldHandleBadCode(final String code, final String expectedCode) { + graffitiBuilder.onExecutionClientVersion( + new ClientVersion( + code, + BESU_CLIENT_VERSION.name(), + BESU_CLIENT_VERSION.version(), + BESU_CLIENT_VERSION.commit())); + + // 20: LH1be52536BU0f91a674 + assertThat(graffitiBuilder.formatClientInfo(20)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + expectedCode + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(20)); + + // 12: LH1be5BU0f91 + assertThat(graffitiBuilder.formatClientInfo(12)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4) + + expectedCode + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4)) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(12)); + + // 8: LH1bBU0f + assertThat(graffitiBuilder.formatClientInfo(8)) + .isEqualTo( + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) + + expectedCode + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(8)); + + // 4: LHBU + assertThat(graffitiBuilder.formatClientInfo(4)) + .isEqualTo(TEKU_CLIENT_VERSION.code() + expectedCode) + .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(4)); + } + + private static Stream getClientCodes() { + return Stream.of( + Arguments.of("bu", "BU"), + Arguments.of("bU", "BU"), + Arguments.of("bur", "BU"), + Arguments.of("b", "NA"), + Arguments.of("12", "NA"), + Arguments.of("", "NA"), + Arguments.of(null, "NA")); + } + + private static Stream getBuildGraffitiFixtures() { + return Stream.of( + Arguments.of( + AUTO_END, + Optional.empty(), + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()), + Arguments.of( + AUTO_END, + Optional.of("small"), + "small " + + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()), + Arguments.of( + AUTO_END, + Optional.of(UTF_8_GRAFFITI_4), + UTF_8_GRAFFITI_4 + + " " + + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()), + Arguments.of( + AUTO_END, + Optional.of(ASCII_GRAFFITI_20), + ASCII_GRAFFITI_20 + + " " + + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)), + Arguments.of( + AUTO_START, + Optional.empty(), + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()), + Arguments.of( + AUTO_START, + Optional.of("small"), + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString() + + " small"), + Arguments.of( + AUTO_START, + Optional.of(UTF_8_GRAFFITI_4), + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString() + + " " + + UTF_8_GRAFFITI_4), + Arguments.of( + AUTO_START, + Optional.of(ASCII_GRAFFITI_20), + TEKU_CLIENT_VERSION.code() + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) + + BESU_CLIENT_VERSION.code() + + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) + + " " + + ASCII_GRAFFITI_20), + Arguments.of( + NAME_END, Optional.empty(), TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), + Arguments.of( + NAME_END, + Optional.of("small"), + "small " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), + Arguments.of( + NAME_END, + Optional.of(UTF_8_GRAFFITI_4), + UTF_8_GRAFFITI_4 + " " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), + Arguments.of( + NAME_END, + Optional.of(ASCII_GRAFFITI_20), + ASCII_GRAFFITI_20 + " " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), + Arguments.of( + NAME_START, Optional.empty(), TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), + Arguments.of( + NAME_START, + Optional.of("small"), + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code() + " small"), + Arguments.of( + NAME_START, + Optional.of(UTF_8_GRAFFITI_4), + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code() + " " + UTF_8_GRAFFITI_4), + Arguments.of( + NAME_START, + Optional.of(ASCII_GRAFFITI_20), + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code() + " " + ASCII_GRAFFITI_20), + Arguments.of(NONE, Optional.empty(), ""), + Arguments.of(NONE, Optional.of("small"), "small"), + Arguments.of(NONE, Optional.of(UTF_8_GRAFFITI_4), UTF_8_GRAFFITI_4), + Arguments.of(NONE, Optional.of(ASCII_GRAFFITI_20), ASCII_GRAFFITI_20)); + } +} diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProvider.java b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionChannel.java similarity index 58% rename from beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProvider.java rename to ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionChannel.java index 6ac5af1e4c4..872f50a40bd 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DefaultGraffitiProvider.java +++ b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionChannel.java @@ -11,11 +11,14 @@ * specific language governing permissions and limitations under the License. */ -package tech.pegasys.teku.validator.coordinator; +package tech.pegasys.teku.ethereum.executionclient; -import org.apache.tuweni.bytes.Bytes32; +import tech.pegasys.teku.ethereum.events.ExecutionClientEventsChannel; +import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; -@FunctionalInterface -public interface DefaultGraffitiProvider { - Bytes32 getDefaultGraffiti(); +public interface ExecutionClientVersionChannel extends ExecutionClientEventsChannel { + void onExecutionClientVersion(ClientVersion executionClientVersion); + + @Override + default void onAvailabilityUpdated(boolean isAvailable) {} } diff --git a/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProvider.java b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProvider.java new file mode 100644 index 00000000000..7815a7aa2cf --- /dev/null +++ b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProvider.java @@ -0,0 +1,81 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.ethereum.executionclient; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import tech.pegasys.teku.ethereum.events.ExecutionClientEventsChannel; +import tech.pegasys.teku.infrastructure.version.VersionProvider; +import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; +import tech.pegasys.teku.spec.executionlayer.ExecutionLayerChannel; + +/** + * Based on Specify Client Versions on + * Engine API. This provider will publish EL client version on {@link + * ExecutionClientVersionChannel} when the EL supports version specification and {@link + * ExecutionLayerChannel#engineGetClientVersion(ClientVersion)} has been called. + */ +public class ExecutionClientVersionProvider implements ExecutionClientEventsChannel { + + private static final Logger LOG = LogManager.getLogger(); + + private final AtomicBoolean lastExecutionClientAvailability = new AtomicBoolean(true); + + private final ExecutionLayerChannel executionLayerChannel; + private final ExecutionClientVersionChannel executionClientVersionChannel; + private final ClientVersion consensusClientVersion; + private final AtomicReference executionClientVersion = new AtomicReference<>(null); + + public ExecutionClientVersionProvider( + final ExecutionLayerChannel executionLayerChannel, + final ExecutionClientVersionChannel executionClientVersionChannel) { + this.executionLayerChannel = executionLayerChannel; + this.executionClientVersionChannel = executionClientVersionChannel; + this.consensusClientVersion = VersionProvider.createTekuClientVersion(); + // update client info on initialization + updateClientInfo(); + } + + @Override + public void onAvailabilityUpdated(final boolean isAvailable) { + // only update info after EL has been unavailable + if (isAvailable && lastExecutionClientAvailability.compareAndSet(false, true)) { + updateClientInfo(); + } else { + lastExecutionClientAvailability.set(isAvailable); + } + } + + private void updateClientInfo() { + executionLayerChannel + .engineGetClientVersion(consensusClientVersion) + .thenAccept( + clientVersions -> { + final ClientVersion executionClientVersion = clientVersions.get(0); + updateVersionIfNeeded(executionClientVersion); + }) + .finish(ex -> LOG.debug("Exception while calling engine_getClientVersion", ex)); + } + + private synchronized void updateVersionIfNeeded(final ClientVersion executionClientVersion) { + if (executionClientVersion.equals(this.executionClientVersion.get())) { + return; + } + + this.executionClientVersion.set(executionClientVersion); + executionClientVersionChannel.onExecutionClientVersion(executionClientVersion); + } +} diff --git a/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java b/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java new file mode 100644 index 00000000000..168769150e0 --- /dev/null +++ b/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java @@ -0,0 +1,93 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.ethereum.executionclient; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import tech.pegasys.teku.infrastructure.async.SafeFuture; +import tech.pegasys.teku.infrastructure.bytes.Bytes4; +import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; +import tech.pegasys.teku.spec.executionlayer.ExecutionLayerChannel; + +public class ExecutionClientVersionProviderTest { + + private final ExecutionLayerChannel executionLayerChannel = mock(ExecutionLayerChannel.class); + private final ExecutionClientVersionChannel publishChannel = + mock(ExecutionClientVersionChannel.class); + private final ClientVersion executionClientVersion = + new ClientVersion("BU", "besu", "1.0.0", Bytes4.fromHexString("8dba2981")); + + @BeforeEach + public void setUp() { + when(executionLayerChannel.engineGetClientVersion(any())) + .thenReturn(SafeFuture.completedFuture(List.of(executionClientVersion))); + } + + @Test + public void doesNotPublishExecutionClientVersionIfFailed() { + when(executionLayerChannel.engineGetClientVersion(any())) + .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); + + new ExecutionClientVersionProvider(executionLayerChannel, publishChannel); + verify(publishChannel, never()).onExecutionClientVersion(any()); + } + + @Test + public void doesNotTryToUpdateExecutionClientVersionIfElHasNotBeenUnavailable() { + final ExecutionClientVersionProvider executionClientVersionProvider = + new ExecutionClientVersionProvider(executionLayerChannel, publishChannel); + + executionClientVersionProvider.onAvailabilityUpdated(true); + // EL called only one time + verify(executionLayerChannel, times(1)).engineGetClientVersion(any()); + } + + @Test + public void updatesExecutionClientVersionElIsAvailableAfterBeingUnavailable() { + final ExecutionClientVersionProvider executionClientVersionProvider = + new ExecutionClientVersionProvider(executionLayerChannel, publishChannel); + + verify(publishChannel).onExecutionClientVersion(executionClientVersion); + + final ClientVersion updatedExecutionClientVersion = + new ClientVersion("BU", "besu", "1.0.1", Bytes4.fromHexString("efd1bc70")); + when(executionLayerChannel.engineGetClientVersion(any())) + .thenReturn(SafeFuture.completedFuture(List.of(updatedExecutionClientVersion))); + + executionClientVersionProvider.onAvailabilityUpdated(false); + executionClientVersionProvider.onAvailabilityUpdated(true); + // EL called two times + verify(executionLayerChannel, times(2)).engineGetClientVersion(any()); + + verify(publishChannel).onExecutionClientVersion(updatedExecutionClientVersion); + + executionClientVersionProvider.onAvailabilityUpdated(false); + executionClientVersionProvider.onAvailabilityUpdated(true); + + // EL called three times + verify(executionLayerChannel, times(3)).engineGetClientVersion(any()); + + // 1st time - executionClientVersion, 2nd time - updatedExecutionClientVersion, 3rd time - + // ignoring the same + verify(publishChannel, times(2)).onExecutionClientVersion(any()); + } +} diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/ClientVersion.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/ClientVersion.java index 9771c05a897..c2e77e36e3e 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/ClientVersion.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/execution/ClientVersion.java @@ -18,4 +18,6 @@ public record ClientVersion(String code, String name, String version, Bytes4 commit) { public static final String TEKU_CLIENT_CODE = "TK"; + + public static final ClientVersion UNKNOWN = new ClientVersion("NA", "", "", Bytes4.ZERO); } diff --git a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/EventLogger.java b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/EventLogger.java index 9ea8e1c4e1a..6ae514219bb 100644 --- a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/EventLogger.java +++ b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/EventLogger.java @@ -174,6 +174,13 @@ public void logExecutionClientVersion(final String name, final String version) { log.info("Execution Client version: {} {}", name, version); } + public void logDefaultGraffiti(final String graffiti) { + log.info( + "Default graffiti to use when building block without external VC: \"{}\". " + + "To change check validator graffiti options.", + graffiti); + } + public void builderIsNotAvailable(final String errorMessage) { final String builderIsNotAvailableEventLog = String.format( diff --git a/infrastructure/version/build.gradle b/infrastructure/version/build.gradle index 9b5de221ac3..96ebdad29d0 100644 --- a/infrastructure/version/build.gradle +++ b/infrastructure/version/build.gradle @@ -16,5 +16,6 @@ tasks.withType(JavaCompile).configureEach { dependencies { + implementation project(':ethereum:spec') implementation 'org.apache.commons:commons-lang3' } diff --git a/infrastructure/version/src/main/java/tech/pegasys/teku/infrastructure/version/VersionProvider.java b/infrastructure/version/src/main/java/tech/pegasys/teku/infrastructure/version/VersionProvider.java index a5b65cf862d..7f1874754ad 100644 --- a/infrastructure/version/src/main/java/tech/pegasys/teku/infrastructure/version/VersionProvider.java +++ b/infrastructure/version/src/main/java/tech/pegasys/teku/infrastructure/version/VersionProvider.java @@ -21,6 +21,8 @@ import java.util.Optional; import java.util.Properties; import org.apache.commons.lang3.StringUtils; +import tech.pegasys.teku.infrastructure.bytes.Bytes4; +import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; public class VersionProvider { public static final String ENV_XDG_DATA_HOME = "XDG_DATA_HOME"; @@ -32,6 +34,17 @@ public class VersionProvider { CLIENT_IDENTITY + "/" + IMPLEMENTATION_VERSION + "/" + detectOS() + "/" + detectJvm(); public static final Optional COMMIT_HASH = getCommitHash(); + public static ClientVersion createTekuClientVersion() { + // TODO: supplier.memoize + return new ClientVersion( + ClientVersion.TEKU_CLIENT_CODE, + VersionProvider.CLIENT_IDENTITY, + VersionProvider.IMPLEMENTATION_VERSION, + VersionProvider.COMMIT_HASH + .map(commitHash -> Bytes4.fromHexString(commitHash.substring(0, 8))) + .orElse(Bytes4.ZERO)); + } + public static String defaultStoragePath() { final String detectedOS = normalizeOS(normalize("os.name")); return defaultStoragePathForNormalizedOS(detectedOS, System.getenv()); diff --git a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java index 032edc0634b..58da5de4544 100644 --- a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java +++ b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java @@ -53,6 +53,8 @@ import tech.pegasys.teku.ethereum.events.ExecutionClientEventsChannel; import tech.pegasys.teku.ethereum.events.SlotEventsChannel; import tech.pegasys.teku.ethereum.execution.types.Eth1Address; +import tech.pegasys.teku.ethereum.executionclient.ExecutionClientVersionChannel; +import tech.pegasys.teku.ethereum.executionclient.ExecutionClientVersionProvider; import tech.pegasys.teku.ethereum.performance.trackers.BlockProductionPerformanceFactory; import tech.pegasys.teku.ethereum.pow.api.Eth1EventsChannel; import tech.pegasys.teku.infrastructure.async.AsyncRunner; @@ -186,12 +188,12 @@ import tech.pegasys.teku.validator.coordinator.ActiveValidatorTracker; import tech.pegasys.teku.validator.coordinator.BlockFactory; import tech.pegasys.teku.validator.coordinator.BlockOperationSelectorFactory; -import tech.pegasys.teku.validator.coordinator.DefaultGraffitiProviderImpl; import tech.pegasys.teku.validator.coordinator.DepositProvider; import tech.pegasys.teku.validator.coordinator.DutyMetrics; import tech.pegasys.teku.validator.coordinator.Eth1DataCache; import tech.pegasys.teku.validator.coordinator.Eth1DataProvider; import tech.pegasys.teku.validator.coordinator.Eth1VotingPeriod; +import tech.pegasys.teku.validator.coordinator.GraffitiBuilder; import tech.pegasys.teku.validator.coordinator.MilestoneBasedBlockFactory; import tech.pegasys.teku.validator.coordinator.ValidatorApiHandler; import tech.pegasys.teku.validator.coordinator.performance.DefaultPerformanceTracker; @@ -877,8 +879,14 @@ public void initRewardCalculator() { public void initValidatorApiHandler() { LOG.debug("BeaconChainController.initValidatorApiHandler()"); - final DefaultGraffitiProviderImpl defaultGraffitiProvider = - new DefaultGraffitiProviderImpl(executionLayer); + final ExecutionClientVersionProvider executionClientVersionProvider = + new ExecutionClientVersionProvider( + executionLayer, eventChannels.getPublisher(ExecutionClientVersionChannel.class)); + final GraffitiBuilder graffitiBuilder = + new GraffitiBuilder( + beaconConfig.validatorConfig().getClientGraffitiAppendFormat(), + beaconConfig.validatorConfig().getGraffitiProvider().get()); + eventChannels.subscribe(ExecutionClientVersionChannel.class, graffitiBuilder); final BlockOperationSelectorFactory operationSelector = new BlockOperationSelectorFactory( spec, @@ -890,7 +898,7 @@ public void initValidatorApiHandler() { syncCommitteeContributionPool, depositProvider, eth1DataCache, - defaultGraffitiProvider, + graffitiBuilder, forkChoiceNotifier, executionLayerBlockProductionManager); final BlockFactory blockFactory = new MilestoneBasedBlockFactory(spec, operationSelector); @@ -940,7 +948,7 @@ public void initValidatorApiHandler() { blockProductionPerformanceFactory); eventChannels .subscribe(SlotEventsChannel.class, activeValidatorTracker) - .subscribe(ExecutionClientEventsChannel.class, defaultGraffitiProvider) + .subscribe(ExecutionClientEventsChannel.class, executionClientVersionProvider) .subscribeMultithreaded( ValidatorApiChannel.class, validatorApiHandler, diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java index ff1c8c8d4b5..ef6277f2d32 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java @@ -27,6 +27,7 @@ import picocli.CommandLine.Option; import tech.pegasys.teku.cli.converter.GraffitiConverter; import tech.pegasys.teku.config.TekuConfiguration; +import tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat; import tech.pegasys.teku.validator.api.FileBackedGraffitiProvider; import tech.pegasys.teku.validator.api.ValidatorConfig; import tech.pegasys.teku.validator.api.ValidatorPerformanceTrackingMode; @@ -55,6 +56,24 @@ public class ValidatorOptions { arity = "1") private Path graffitiFile; + @Option( + names = {"--validators-graffiti-client-append-format"}, + paramLabel = "", + description = + """ + Appends CL and EL clients information separated with a space to user's graffiti on the Beacon Node. + AUTO modes tries to provide complete version information if there is enough space for it. + To disable use NONE. Available options: + AUTO_END: (default) Clients info is appended after user's graffiti if any. + AUTO_START: Clients info is added before user's graffiti. + NAME_END: Only clients names are appended after user's graffiti. Good when you don't want to disclose clients versions. + NAME_START: Clients names are added before user's graffiti. + NONE: Clients information is not added to the graffiti. + """, + arity = "1") + private ClientGraffitiAppendFormat clientGraffitiAppendFormat = + ValidatorConfig.DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT; + @Option( names = {"--validators-performance-tracking-mode"}, paramLabel = "", @@ -176,6 +195,7 @@ public void configure(TekuConfiguration.Builder builder) { .graffitiProvider( new FileBackedGraffitiProvider( Optional.ofNullable(graffiti), Optional.ofNullable(graffitiFile))) + .clientGraffitiAppendFormat(clientGraffitiAppendFormat) .generateEarlyAttestations(generateEarlyAttestations) .executorMaxQueueSize(executorMaxQueueSize) .doppelgangerDetectionEnabled(doppelgangerDetectionEnabled) diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java index cd10c37c9a9..ba54400ee92 100644 --- a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java +++ b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java @@ -15,6 +15,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static tech.pegasys.teku.validator.api.ValidatorConfig.DEFAULT_BUILDER_REGISTRATION_GAS_LIMIT; +import static tech.pegasys.teku.validator.api.ValidatorConfig.DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT; import java.net.MalformedURLException; import java.net.URL; @@ -27,6 +28,7 @@ import tech.pegasys.teku.config.TekuConfiguration; import tech.pegasys.teku.ethereum.execution.types.Eth1Address; import tech.pegasys.teku.infrastructure.unsigned.UInt64; +import tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat; import tech.pegasys.teku.validator.api.ValidatorConfig; public class ValidatorOptionsTest extends AbstractBeaconNodeCommandTest { @@ -235,6 +237,24 @@ public void shouldSetShutdownWhenValidatorSlashedEnabled() { assertThat(config.isShutdownWhenValidatorSlashedEnabled()).isTrue(); } + @Test + public void shouldSetDefaultGraffitiClientAppend() { + final ValidatorConfig config = + getTekuConfigurationFromArguments().validatorClient().getValidatorConfig(); + assertThat(config.getClientGraffitiAppendFormat()) + .isEqualTo(DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT); + } + + @Test + public void shouldOverrideGraffitiClientAppend() { + final ValidatorConfig config = + getTekuConfigurationFromArguments("--validators-graffiti-client-append-format=NAME_END") + .validatorClient() + .getValidatorConfig(); + assertThat(config.getClientGraffitiAppendFormat()) + .isEqualTo(ClientGraffitiAppendFormat.NAME_END); + } + @Test public void shouldNotUseDvtSelectionsEndpointByDefault() { final String[] args = {}; diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java new file mode 100644 index 00000000000..5625756c543 --- /dev/null +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java @@ -0,0 +1,31 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.validator.api; + +public enum ClientGraffitiAppendFormat { + // Appends comprehensive client info if there is a space for it. Reduces verbosity when needed. + // Client info is separated with a space after user's graffiti if any. + AUTO_END, + // Prepends comprehensive client info with a space before user's graffiti if there is a room for + // it. + AUTO_START, + // Only encoded client name is appended with space prefix. Good when you don't want to disclose + // client + // version. + NAME_END, + // Client name is prepended with a space before user's graffiti. + NAME_START, + // Client information is not appended to the graffiti. + NONE; +} diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java index eb8cb7727f0..807c5858029 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java @@ -60,6 +60,8 @@ public class ValidatorConfig { public static final boolean DEFAULT_VALIDATOR_EXTERNAL_SIGNER_SLASHING_PROTECTION_ENABLED = true; public static final boolean DEFAULT_GENERATE_EARLY_ATTESTATIONS = true; public static final Optional DEFAULT_GRAFFITI = Optional.empty(); + public static final ClientGraffitiAppendFormat DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT = + ClientGraffitiAppendFormat.AUTO_END; public static final boolean DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_ENABLED = false; public static final boolean DEFAULT_BUILDER_REGISTRATION_DEFAULT_ENABLED = false; public static final boolean DEFAULT_VALIDATOR_BLINDED_BLOCKS_ENABLED = false; @@ -78,6 +80,7 @@ public class ValidatorConfig { private final Path validatorExternalSignerTruststore; private final Path validatorExternalSignerTruststorePasswordFile; private final GraffitiProvider graffitiProvider; + private final ClientGraffitiAppendFormat clientGraffitiAppendFormat; private final ValidatorPerformanceTrackingMode validatorPerformanceTrackingMode; private final boolean validatorKeystoreLockingEnabled; private final Optional> beaconNodeApiEndpoints; @@ -120,6 +123,7 @@ private ValidatorConfig( final Path validatorExternalSignerTruststorePasswordFile, final Optional> beaconNodeApiEndpoints, final GraffitiProvider graffitiProvider, + final ClientGraffitiAppendFormat clientGraffitiAppendFormat, final ValidatorPerformanceTrackingMode validatorPerformanceTrackingMode, final boolean validatorKeystoreLockingEnabled, final boolean validatorExternalSignerSlashingProtectionEnabled, @@ -158,6 +162,7 @@ private ValidatorConfig( this.validatorExternalSignerTruststorePasswordFile = validatorExternalSignerTruststorePasswordFile; this.graffitiProvider = graffitiProvider; + this.clientGraffitiAppendFormat = clientGraffitiAppendFormat; this.validatorKeystoreLockingEnabled = validatorKeystoreLockingEnabled; this.beaconNodeApiEndpoints = beaconNodeApiEndpoints; this.validatorPerformanceTrackingMode = validatorPerformanceTrackingMode; @@ -249,6 +254,10 @@ public GraffitiProvider getGraffitiProvider() { return graffitiProvider; } + public ClientGraffitiAppendFormat getClientGraffitiAppendFormat() { + return clientGraffitiAppendFormat; + } + public List getValidatorKeys() { return validatorKeys; } @@ -370,6 +379,8 @@ public static final class Builder { private Path validatorExternalSignerTruststorePasswordFile; private GraffitiProvider graffitiProvider = new FileBackedGraffitiProvider(DEFAULT_GRAFFITI, Optional.empty()); + private ClientGraffitiAppendFormat clientGraffitiAppendFormat = + DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT; private ValidatorPerformanceTrackingMode validatorPerformanceTrackingMode = ValidatorPerformanceTrackingMode.DEFAULT_MODE; private boolean validatorKeystoreLockingEnabled = DEFAULT_VALIDATOR_KEYSTORE_LOCKING_ENABLED; @@ -490,18 +501,24 @@ public Builder beaconNodeApiEndpoints(final List beaconNodeApiEndpoints) { return this; } - public Builder graffitiProvider(GraffitiProvider graffitiProvider) { + public Builder graffitiProvider(final GraffitiProvider graffitiProvider) { this.graffitiProvider = graffitiProvider; return this; } + public Builder clientGraffitiAppendFormat( + final ClientGraffitiAppendFormat clientGraffitiAppendFormat) { + this.clientGraffitiAppendFormat = clientGraffitiAppendFormat; + return this; + } + public Builder validatorPerformanceTrackingMode( - ValidatorPerformanceTrackingMode validatorPerformanceTrackingMode) { + final ValidatorPerformanceTrackingMode validatorPerformanceTrackingMode) { this.validatorPerformanceTrackingMode = validatorPerformanceTrackingMode; return this; } - public Builder validatorKeystoreLockingEnabled(boolean validatorKeystoreLockingEnabled) { + public Builder validatorKeystoreLockingEnabled(final boolean validatorKeystoreLockingEnabled) { this.validatorKeystoreLockingEnabled = validatorKeystoreLockingEnabled; return this; } @@ -670,6 +687,7 @@ public ValidatorConfig build() { validatorExternalSignerTruststorePasswordFile, beaconNodeApiEndpoints, graffitiProvider, + clientGraffitiAppendFormat, validatorPerformanceTrackingMode, validatorKeystoreLockingEnabled, validatorExternalSignerSlashingProtectionEnabled, From 58ec4e75a256da90fd952e44fee020e179ca0d27 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Mar 2024 16:27:33 +0400 Subject: [PATCH 2/8] Fix createTekuVersion import dependency issues --- .../validator/coordinator/GraffitiBuilder.java | 17 ++++++++++++++++- .../ExecutionClientVersionProvider.java | 6 +++--- .../ExecutionClientVersionProviderTest.java | 9 ++++++--- infrastructure/version/build.gradle | 1 - .../infrastructure/version/VersionProvider.java | 13 ------------- .../beaconchain/BeaconChainController.java | 8 +++++--- 6 files changed, 30 insertions(+), 24 deletions(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java index 541148d0c3b..db272258b28 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java @@ -27,6 +27,7 @@ import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.ethereum.executionclient.ExecutionClientVersionChannel; +import tech.pegasys.teku.infrastructure.bytes.Bytes4; import tech.pegasys.teku.infrastructure.version.VersionProvider; import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; import tech.pegasys.teku.validator.api.Bytes32Parser; @@ -50,11 +51,25 @@ public GraffitiBuilder( final ClientGraffitiAppendFormat clientGraffitiAppendFormat, final Optional defaultUserGraffiti) { this.clientGraffitiAppendFormat = clientGraffitiAppendFormat; - this.consensusClientVersion = VersionProvider.createTekuClientVersion(); + this.consensusClientVersion = createTekuClientVersion(); this.executionClientVersion = new AtomicReference<>(ClientVersion.UNKNOWN); this.defaultUserGraffiti = defaultUserGraffiti; } + private ClientVersion createTekuClientVersion() { + return new ClientVersion( + ClientVersion.TEKU_CLIENT_CODE, + VersionProvider.CLIENT_IDENTITY, + VersionProvider.IMPLEMENTATION_VERSION, + VersionProvider.COMMIT_HASH + .map(commitHash -> Bytes4.fromHexString(commitHash.substring(0, 8))) + .orElse(Bytes4.ZERO)); + } + + public ClientVersion getConsensusClientVersion() { + return consensusClientVersion; + } + @Override public void onExecutionClientVersion(final ClientVersion executionClientVersion) { this.executionClientVersion.set(executionClientVersion); diff --git a/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProvider.java b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProvider.java index 7815a7aa2cf..0ccb2419e34 100644 --- a/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProvider.java +++ b/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProvider.java @@ -18,7 +18,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import tech.pegasys.teku.ethereum.events.ExecutionClientEventsChannel; -import tech.pegasys.teku.infrastructure.version.VersionProvider; import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; import tech.pegasys.teku.spec.executionlayer.ExecutionLayerChannel; @@ -41,10 +40,11 @@ public class ExecutionClientVersionProvider implements ExecutionClientEventsChan public ExecutionClientVersionProvider( final ExecutionLayerChannel executionLayerChannel, - final ExecutionClientVersionChannel executionClientVersionChannel) { + final ExecutionClientVersionChannel executionClientVersionChannel, + final ClientVersion consensusClientVersion) { this.executionLayerChannel = executionLayerChannel; this.executionClientVersionChannel = executionClientVersionChannel; - this.consensusClientVersion = VersionProvider.createTekuClientVersion(); + this.consensusClientVersion = consensusClientVersion; // update client info on initialization updateClientInfo(); } diff --git a/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java b/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java index 168769150e0..27f55026630 100644 --- a/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java +++ b/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java @@ -47,14 +47,16 @@ public void doesNotPublishExecutionClientVersionIfFailed() { when(executionLayerChannel.engineGetClientVersion(any())) .thenReturn(SafeFuture.failedFuture(new IllegalStateException("oopsy"))); - new ExecutionClientVersionProvider(executionLayerChannel, publishChannel); + new ExecutionClientVersionProvider( + executionLayerChannel, publishChannel, ClientVersion.UNKNOWN); verify(publishChannel, never()).onExecutionClientVersion(any()); } @Test public void doesNotTryToUpdateExecutionClientVersionIfElHasNotBeenUnavailable() { final ExecutionClientVersionProvider executionClientVersionProvider = - new ExecutionClientVersionProvider(executionLayerChannel, publishChannel); + new ExecutionClientVersionProvider( + executionLayerChannel, publishChannel, ClientVersion.UNKNOWN); executionClientVersionProvider.onAvailabilityUpdated(true); // EL called only one time @@ -64,7 +66,8 @@ public void doesNotTryToUpdateExecutionClientVersionIfElHasNotBeenUnavailable() @Test public void updatesExecutionClientVersionElIsAvailableAfterBeingUnavailable() { final ExecutionClientVersionProvider executionClientVersionProvider = - new ExecutionClientVersionProvider(executionLayerChannel, publishChannel); + new ExecutionClientVersionProvider( + executionLayerChannel, publishChannel, ClientVersion.UNKNOWN); verify(publishChannel).onExecutionClientVersion(executionClientVersion); diff --git a/infrastructure/version/build.gradle b/infrastructure/version/build.gradle index 96ebdad29d0..9b5de221ac3 100644 --- a/infrastructure/version/build.gradle +++ b/infrastructure/version/build.gradle @@ -16,6 +16,5 @@ tasks.withType(JavaCompile).configureEach { dependencies { - implementation project(':ethereum:spec') implementation 'org.apache.commons:commons-lang3' } diff --git a/infrastructure/version/src/main/java/tech/pegasys/teku/infrastructure/version/VersionProvider.java b/infrastructure/version/src/main/java/tech/pegasys/teku/infrastructure/version/VersionProvider.java index 7f1874754ad..a5b65cf862d 100644 --- a/infrastructure/version/src/main/java/tech/pegasys/teku/infrastructure/version/VersionProvider.java +++ b/infrastructure/version/src/main/java/tech/pegasys/teku/infrastructure/version/VersionProvider.java @@ -21,8 +21,6 @@ import java.util.Optional; import java.util.Properties; import org.apache.commons.lang3.StringUtils; -import tech.pegasys.teku.infrastructure.bytes.Bytes4; -import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; public class VersionProvider { public static final String ENV_XDG_DATA_HOME = "XDG_DATA_HOME"; @@ -34,17 +32,6 @@ public class VersionProvider { CLIENT_IDENTITY + "/" + IMPLEMENTATION_VERSION + "/" + detectOS() + "/" + detectJvm(); public static final Optional COMMIT_HASH = getCommitHash(); - public static ClientVersion createTekuClientVersion() { - // TODO: supplier.memoize - return new ClientVersion( - ClientVersion.TEKU_CLIENT_CODE, - VersionProvider.CLIENT_IDENTITY, - VersionProvider.IMPLEMENTATION_VERSION, - VersionProvider.COMMIT_HASH - .map(commitHash -> Bytes4.fromHexString(commitHash.substring(0, 8))) - .orElse(Bytes4.ZERO)); - } - public static String defaultStoragePath() { final String detectedOS = normalizeOS(normalize("os.name")); return defaultStoragePathForNormalizedOS(detectedOS, System.getenv()); diff --git a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java index 58da5de4544..56dd56300e3 100644 --- a/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java +++ b/services/beaconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java @@ -879,13 +879,15 @@ public void initRewardCalculator() { public void initValidatorApiHandler() { LOG.debug("BeaconChainController.initValidatorApiHandler()"); - final ExecutionClientVersionProvider executionClientVersionProvider = - new ExecutionClientVersionProvider( - executionLayer, eventChannels.getPublisher(ExecutionClientVersionChannel.class)); final GraffitiBuilder graffitiBuilder = new GraffitiBuilder( beaconConfig.validatorConfig().getClientGraffitiAppendFormat(), beaconConfig.validatorConfig().getGraffitiProvider().get()); + final ExecutionClientVersionProvider executionClientVersionProvider = + new ExecutionClientVersionProvider( + executionLayer, + eventChannels.getPublisher(ExecutionClientVersionChannel.class), + graffitiBuilder.getConsensusClientVersion()); eventChannels.subscribe(ExecutionClientVersionChannel.class, graffitiBuilder); final BlockOperationSelectorFactory operationSelector = new BlockOperationSelectorFactory( From 799d59c196c8200c9832de936dcc292510f7471f Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Mar 2024 16:31:07 +0400 Subject: [PATCH 3/8] Decrease waiting time for correct block in test --- .../pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java b/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java index 85a28bd1328..baac1dd7d38 100644 --- a/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java +++ b/acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java @@ -485,16 +485,16 @@ public void waitForNonDefaultExecutionPayload() { MINUTES); } - public void waitForBlockSatisfying(final ThrowingConsumer consumer) { + public void waitForBlockSatisfying(final ThrowingConsumer assertions) { LOG.debug("Wait for a block satisfying certain assertions"); waitFor( () -> { final Optional block = fetchHeadBlock(); assertThat(block).isPresent(); - assertThat(block.get()).satisfies(consumer); + assertThat(block.get()).satisfies(assertions); }, - 5, + 1, MINUTES); } From c5241da3c8c63bfd281133675cb1c27972214d1a Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Mar 2024 16:49:34 +0400 Subject: [PATCH 4/8] Remove extra ClientGraffitiAppendFormat options --- .../coordinator/GraffitiBuilder.java | 18 +---- .../coordinator/GraffitiBuilderTest.java | 76 ++++--------------- .../teku/cli/options/ValidatorOptions.java | 10 +-- .../cli/options/ValidatorOptionsTest.java | 3 +- .../api/ClientGraffitiAppendFormat.java | 10 +-- .../teku/validator/api/ValidatorConfig.java | 2 +- 6 files changed, 22 insertions(+), 97 deletions(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java index db272258b28..a4eb5cec3ed 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java @@ -82,34 +82,20 @@ public Bytes32 buildGraffiti(final Optional userGraffiti) { final int userGraffitiLength = calculateGraffitiLength(userGraffiti); return switch (clientGraffitiAppendFormat) { - case AUTO_END -> { + case AUTO -> { final int clientInfoLength = Bytes32.SIZE - 1 - userGraffitiLength; yield joinNonEmpty( SPACE, extractGraffiti(userGraffiti, userGraffitiLength), formatClientInfo(clientInfoLength)); } - case AUTO_START -> { - final int clientInfoLength = Bytes32.SIZE - 1 - userGraffitiLength; - yield joinNonEmpty( - SPACE, - formatClientInfo(clientInfoLength), - extractGraffiti(userGraffiti, userGraffitiLength)); - } - case NAME_END -> { + case NAME -> { final int clientInfoLength = Integer.min(Bytes32.SIZE - 1 - userGraffitiLength, 4); yield joinNonEmpty( SPACE, extractGraffiti(userGraffiti, userGraffitiLength), formatClientInfo(clientInfoLength)); } - case NAME_START -> { - final int clientInfoLength = Integer.min(Bytes32.SIZE - 1 - userGraffitiLength, 4); - yield joinNonEmpty( - SPACE, - formatClientInfo(clientInfoLength), - extractGraffiti(userGraffiti, userGraffitiLength)); - } case NONE -> userGraffiti.orElse(Bytes32.ZERO); }; } catch (final Exception ex) { diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java index dfad73aa301..f57ca5b7530 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java @@ -14,10 +14,8 @@ package tech.pegasys.teku.validator.coordinator; import static org.assertj.core.api.Assertions.assertThat; -import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.AUTO_END; -import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.AUTO_START; -import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NAME_END; -import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NAME_START; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.AUTO; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NAME; import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NONE; import java.nio.charset.StandardCharsets; @@ -33,19 +31,18 @@ import tech.pegasys.infrastructure.logging.LogCaptor; import tech.pegasys.teku.infrastructure.bytes.Bytes4; import tech.pegasys.teku.infrastructure.logging.EventLogger; -import tech.pegasys.teku.infrastructure.version.VersionProvider; import tech.pegasys.teku.spec.datastructures.execution.ClientVersion; import tech.pegasys.teku.validator.api.Bytes32Parser; import tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat; public class GraffitiBuilderTest { - private ClientGraffitiAppendFormat clientGraffitiAppendFormat = AUTO_END; + private ClientGraffitiAppendFormat clientGraffitiAppendFormat = AUTO; private Optional userGraffiti = Optional.empty(); private GraffitiBuilder graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); private static final ClientVersion TEKU_CLIENT_VERSION = - VersionProvider.createTekuClientVersion(); + new GraffitiBuilder(NONE, Optional.empty()).getConsensusClientVersion(); private static final ClientVersion BESU_CLIENT_VERSION = new ClientVersion("BU", "Besu", "23.4.1", Bytes4.fromHexString("abcdef12")); @@ -58,7 +55,7 @@ public class GraffitiBuilderTest { @BeforeEach public void setup() { - this.clientGraffitiAppendFormat = AUTO_END; + this.clientGraffitiAppendFormat = AUTO; this.userGraffiti = Optional.empty(); this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); } @@ -323,14 +320,14 @@ private static Stream getClientCodes() { private static Stream getBuildGraffitiFixtures() { return Stream.of( Arguments.of( - AUTO_END, + AUTO, Optional.empty(), TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()), Arguments.of( - AUTO_END, + AUTO, Optional.of("small"), "small " + TEKU_CLIENT_VERSION.code() @@ -338,7 +335,7 @@ private static Stream getBuildGraffitiFixtures() { + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()), Arguments.of( - AUTO_END, + AUTO, Optional.of(UTF_8_GRAFFITI_4), UTF_8_GRAFFITI_4 + " " @@ -347,7 +344,7 @@ private static Stream getBuildGraffitiFixtures() { + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()), Arguments.of( - AUTO_END, + AUTO, Optional.of(ASCII_GRAFFITI_20), ASCII_GRAFFITI_20 + " " @@ -356,66 +353,19 @@ private static Stream getBuildGraffitiFixtures() { + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)), Arguments.of( - AUTO_START, - Optional.empty(), - TEKU_CLIENT_VERSION.code() - + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() - + BESU_CLIENT_VERSION.code() - + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()), - Arguments.of( - AUTO_START, - Optional.of("small"), - TEKU_CLIENT_VERSION.code() - + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() - + BESU_CLIENT_VERSION.code() - + BESU_CLIENT_VERSION.commit().toUnprefixedHexString() - + " small"), - Arguments.of( - AUTO_START, - Optional.of(UTF_8_GRAFFITI_4), - TEKU_CLIENT_VERSION.code() - + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() - + BESU_CLIENT_VERSION.code() - + BESU_CLIENT_VERSION.commit().toUnprefixedHexString() - + " " - + UTF_8_GRAFFITI_4), - Arguments.of( - AUTO_START, - Optional.of(ASCII_GRAFFITI_20), - TEKU_CLIENT_VERSION.code() - + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) - + BESU_CLIENT_VERSION.code() - + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) - + " " - + ASCII_GRAFFITI_20), - Arguments.of( - NAME_END, Optional.empty(), TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), + NAME, Optional.empty(), TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - NAME_END, + NAME, Optional.of("small"), "small " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - NAME_END, + NAME, Optional.of(UTF_8_GRAFFITI_4), UTF_8_GRAFFITI_4 + " " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - NAME_END, + NAME, Optional.of(ASCII_GRAFFITI_20), ASCII_GRAFFITI_20 + " " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), - Arguments.of( - NAME_START, Optional.empty(), TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), - Arguments.of( - NAME_START, - Optional.of("small"), - TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code() + " small"), - Arguments.of( - NAME_START, - Optional.of(UTF_8_GRAFFITI_4), - TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code() + " " + UTF_8_GRAFFITI_4), - Arguments.of( - NAME_START, - Optional.of(ASCII_GRAFFITI_20), - TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code() + " " + ASCII_GRAFFITI_20), Arguments.of(NONE, Optional.empty(), ""), Arguments.of(NONE, Optional.of("small"), "small"), Arguments.of(NONE, Optional.of(UTF_8_GRAFFITI_4), UTF_8_GRAFFITI_4), diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java index ef6277f2d32..c1ed78fa8f7 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java @@ -59,16 +59,12 @@ public class ValidatorOptions { @Option( names = {"--validators-graffiti-client-append-format"}, paramLabel = "", + showDefaultValue = Visibility.ALWAYS, description = """ Appends CL and EL clients information separated with a space to user's graffiti on the Beacon Node. - AUTO modes tries to provide complete version information if there is enough space for it. - To disable use NONE. Available options: - AUTO_END: (default) Clients info is appended after user's graffiti if any. - AUTO_START: Clients info is added before user's graffiti. - NAME_END: Only clients names are appended after user's graffiti. Good when you don't want to disclose clients versions. - NAME_START: Clients names are added before user's graffiti. - NONE: Clients information is not added to the graffiti. + AUTO mode tries to provide complete version information if there is enough space for it. + NAME adds only CL and EL name, to disable use NONE. (Valid values: ${COMPLETION-CANDIDATES}) """, arity = "1") private ClientGraffitiAppendFormat clientGraffitiAppendFormat = diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java index ba54400ee92..512c3e15f26 100644 --- a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java +++ b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java @@ -251,8 +251,7 @@ public void shouldOverrideGraffitiClientAppend() { getTekuConfigurationFromArguments("--validators-graffiti-client-append-format=NAME_END") .validatorClient() .getValidatorConfig(); - assertThat(config.getClientGraffitiAppendFormat()) - .isEqualTo(ClientGraffitiAppendFormat.NAME_END); + assertThat(config.getClientGraffitiAppendFormat()).isEqualTo(ClientGraffitiAppendFormat.NAME); } @Test diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java index 5625756c543..8231d7939be 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java @@ -16,16 +16,10 @@ public enum ClientGraffitiAppendFormat { // Appends comprehensive client info if there is a space for it. Reduces verbosity when needed. // Client info is separated with a space after user's graffiti if any. - AUTO_END, + AUTO, // Prepends comprehensive client info with a space before user's graffiti if there is a room for // it. - AUTO_START, - // Only encoded client name is appended with space prefix. Good when you don't want to disclose - // client - // version. - NAME_END, - // Client name is prepended with a space before user's graffiti. - NAME_START, + NAME, // Client information is not appended to the graffiti. NONE; } diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java index 807c5858029..7653c59fa24 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ValidatorConfig.java @@ -61,7 +61,7 @@ public class ValidatorConfig { public static final boolean DEFAULT_GENERATE_EARLY_ATTESTATIONS = true; public static final Optional DEFAULT_GRAFFITI = Optional.empty(); public static final ClientGraffitiAppendFormat DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT = - ClientGraffitiAppendFormat.AUTO_END; + ClientGraffitiAppendFormat.AUTO; public static final boolean DEFAULT_VALIDATOR_PROPOSER_CONFIG_REFRESH_ENABLED = false; public static final boolean DEFAULT_BUILDER_REGISTRATION_DEFAULT_ENABLED = false; public static final boolean DEFAULT_VALIDATOR_BLINDED_BLOCKS_ENABLED = false; From 71b776367c9143c42d55ef6ff221af926c97ec0e Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Mar 2024 18:43:31 +0400 Subject: [PATCH 5/8] Fix the test --- .../tech/pegasys/teku/cli/options/ValidatorOptionsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java index 512c3e15f26..75a4b882533 100644 --- a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java +++ b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java @@ -248,7 +248,7 @@ public void shouldSetDefaultGraffitiClientAppend() { @Test public void shouldOverrideGraffitiClientAppend() { final ValidatorConfig config = - getTekuConfigurationFromArguments("--validators-graffiti-client-append-format=NAME_END") + getTekuConfigurationFromArguments("--validators-graffiti-client-append-format=NAME") .validatorClient() .getValidatorConfig(); assertThat(config.getClientGraffitiAppendFormat()).isEqualTo(ClientGraffitiAppendFormat.NAME); From a622b5fc4179b96d6a4bd2459401ef8435289401 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Mar 2024 19:15:35 +0400 Subject: [PATCH 6/8] Split big tests, to have less assertions in every test case --- .../coordinator/GraffitiBuilderTest.java | 93 +++++++++++++++++-- 1 file changed, 86 insertions(+), 7 deletions(-) diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java index f57ca5b7530..54e5c88ad23 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java @@ -101,7 +101,7 @@ protected int calculateGraffitiLength(Optional graffiti) { assertThat(graffitiBuilder.buildGraffiti(Optional.of(graffiti))).isEqualTo(graffiti); } - @ParameterizedTest() + @ParameterizedTest(name = "format={0}, userGraffiti={1}") @MethodSource("getBuildGraffitiFixtures") public void buildGraffiti_shouldProvideCorrectOutput( final ClientGraffitiAppendFormat clientGraffitiAppendFormat, @@ -123,12 +123,16 @@ public void buildGraffiti_shouldProvideCorrectOutput( } @Test - public void extractGraffiti_shouldReturnCorrectGraffiti() { + public void extractGraffiti_shouldReturnEmptyString() { assertThat(graffitiBuilder.extractGraffiti(Optional.empty(), 0)).isEqualTo(""); assertThat( graffitiBuilder.extractGraffiti( Optional.of(Bytes32Parser.toBytes32(asciiGraffiti0)), 0)) .isEqualTo(""); + } + + @Test + public void extractGraffiti_shouldReturnAsciiString() { assertThat( graffitiBuilder.extractGraffiti( Optional.of(Bytes32Parser.toBytes32(ASCII_GRAFFITI_20)), 20)) @@ -141,6 +145,10 @@ public void extractGraffiti_shouldReturnCorrectGraffiti() { graffitiBuilder.extractGraffiti( Optional.of(Bytes32Parser.toBytes32(asciiGraffiti32)), 16)) .isEqualTo(asciiGraffiti32.substring(0, 16)); + } + + @Test + public void extractGraffiti_shouldReturnUtf8String() { assertThat( graffitiBuilder.extractGraffiti( Optional.of(Bytes32Parser.toBytes32(UTF_8_GRAFFITI_4)), 4)) @@ -156,12 +164,16 @@ public void extractGraffiti_shouldReturnCorrectGraffiti() { } @Test - public void calculateGraffitiLength_shouldReturnCorrectLength() { + public void calculateGraffitiLength_shouldHandleEmptyString() { assertThat(graffitiBuilder.calculateGraffitiLength(Optional.empty())).isEqualTo(0); assertThat( graffitiBuilder.calculateGraffitiLength( Optional.of(Bytes32Parser.toBytes32(asciiGraffiti0)))) .isEqualTo(0); + } + + @Test + public void calculateGraffitiLength_shouldHandleAsciiString() { assertThat( graffitiBuilder.calculateGraffitiLength( Optional.of(Bytes32Parser.toBytes32(ASCII_GRAFFITI_20)))) @@ -170,6 +182,10 @@ public void calculateGraffitiLength_shouldReturnCorrectLength() { graffitiBuilder.calculateGraffitiLength( Optional.of(Bytes32Parser.toBytes32(asciiGraffiti32)))) .isEqualTo(32); + } + + @Test + public void calculateGraffitiLength_shouldHandleUtf8String() { assertThat( graffitiBuilder.calculateGraffitiLength( Optional.of(Bytes32Parser.toBytes32(UTF_8_GRAFFITI_4)))) @@ -181,9 +197,15 @@ public void calculateGraffitiLength_shouldReturnCorrectLength() { } @Test - public void join_shouldJoinStringsSkippingEmpty() { + public void joinNonEmpty_shouldJoin() { assertThat(graffitiBuilder.joinNonEmpty("", "aa", "bb", "cc")) .isEqualTo(Bytes32Parser.toBytes32("aabbcc")); + assertThat(graffitiBuilder.joinNonEmpty(" ", "aa", "bb", "cc")) + .isEqualTo(Bytes32Parser.toBytes32("aa bb cc")); + } + + @Test + public void joinNonEmpty_shouldJoinSkippingEmpty() { assertThat(graffitiBuilder.joinNonEmpty(" ", "aa", "", "cc")) .isEqualTo(Bytes32Parser.toBytes32("aa cc")); assertThat(graffitiBuilder.joinNonEmpty(" ", "", "bb", "cc")) @@ -192,7 +214,7 @@ public void join_shouldJoinStringsSkippingEmpty() { } @Test - public void formatClientInfo_shouldFitInDesiredLength() { + public void formatClientInfo_shouldRenderClientNamesAndFullCommit() { graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // 20: LH1be52536BU0f91a674 @@ -210,6 +232,11 @@ public void formatClientInfo_shouldFitInDesiredLength() { + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(20)); + } + + @Test + public void formatClientInfo_shouldRenderClientNamesAndHalfCommit() { + graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // 12: LH1be5BU0f91 assertThat(graffitiBuilder.formatClientInfo(19)) @@ -226,6 +253,11 @@ public void formatClientInfo_shouldFitInDesiredLength() { + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4)) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(12)); + } + + @Test + public void formatClientInfo_shouldRenderClientNamesAnd1stCommitByte() { + graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // 8: LH1bBU0f assertThat(graffitiBuilder.formatClientInfo(11)) @@ -242,6 +274,11 @@ public void formatClientInfo_shouldFitInDesiredLength() { + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(8)); + } + + @Test + public void formatClientInfo_shouldRenderClientNames() { + graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // 4: LHBU assertThat(graffitiBuilder.formatClientInfo(7)) @@ -250,6 +287,11 @@ public void formatClientInfo_shouldFitInDesiredLength() { assertThat(graffitiBuilder.formatClientInfo(4)) .isEqualTo(TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(4)); + } + + @Test + public void formatClientInfo_shouldSkipClientsInfoWhenNotEnoughSpace() { + graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // Empty assertThat(graffitiBuilder.formatClientInfo(3)) @@ -263,9 +305,10 @@ public void formatClientInfo_shouldFitInDesiredLength() { .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(0)); } - @ParameterizedTest() + @ParameterizedTest(name = "code={0}") @MethodSource("getClientCodes") - public void formatClientInfo_shouldHandleBadCode(final String code, final String expectedCode) { + public void formatClientInfo_shouldHandleBadCodeOnClientNamesAndFullCommit( + final String code, final String expectedCode) { graffitiBuilder.onExecutionClientVersion( new ClientVersion( code, @@ -281,6 +324,18 @@ public void formatClientInfo_shouldHandleBadCode(final String code, final String + expectedCode + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(20)); + } + + @ParameterizedTest(name = "code={0}") + @MethodSource("getClientCodes") + public void formatClientInfo_shouldHandleBadCodeOnClientNamesAndHalfCommit( + final String code, final String expectedCode) { + graffitiBuilder.onExecutionClientVersion( + new ClientVersion( + code, + BESU_CLIENT_VERSION.name(), + BESU_CLIENT_VERSION.version(), + BESU_CLIENT_VERSION.commit())); // 12: LH1be5BU0f91 assertThat(graffitiBuilder.formatClientInfo(12)) @@ -290,6 +345,18 @@ public void formatClientInfo_shouldHandleBadCode(final String code, final String + expectedCode + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4)) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(12)); + } + + @ParameterizedTest(name = "code={0}") + @MethodSource("getClientCodes") + public void formatClientInfo_shouldHandleBadCodeOnClientNamesAnd1stCommitByte( + final String code, final String expectedCode) { + graffitiBuilder.onExecutionClientVersion( + new ClientVersion( + code, + BESU_CLIENT_VERSION.name(), + BESU_CLIENT_VERSION.version(), + BESU_CLIENT_VERSION.commit())); // 8: LH1bBU0f assertThat(graffitiBuilder.formatClientInfo(8)) @@ -299,6 +366,18 @@ public void formatClientInfo_shouldHandleBadCode(final String code, final String + expectedCode + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(8)); + } + + @ParameterizedTest(name = "code={0}") + @MethodSource("getClientCodes") + public void formatClientInfo_shouldHandleBadCodeOnClientNames( + final String code, final String expectedCode) { + graffitiBuilder.onExecutionClientVersion( + new ClientVersion( + code, + BESU_CLIENT_VERSION.name(), + BESU_CLIENT_VERSION.version(), + BESU_CLIENT_VERSION.commit())); // 4: LHBU assertThat(graffitiBuilder.formatClientInfo(4)) From 8774ac3042f42f682338b94fe2789cb3c2d42297 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 20 Mar 2024 21:01:59 +0400 Subject: [PATCH 7/8] Polishing per feedback --- .../coordinator/GraffitiBuilder.java | 19 +++---- .../coordinator/AbstractBlockFactoryTest.java | 2 +- .../BlockOperationSelectorFactoryTest.java | 2 +- .../coordinator/GraffitiBuilderTest.java | 54 ++++++++++--------- .../ExecutionClientVersionProviderTest.java | 2 +- .../teku/cli/options/ValidatorOptions.java | 7 +-- .../cli/options/ValidatorOptionsTest.java | 9 ++-- .../api/ClientGraffitiAppendFormat.java | 14 ++--- 8 files changed, 54 insertions(+), 55 deletions(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java index a4eb5cec3ed..3fb65db980e 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java @@ -34,8 +34,8 @@ import tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat; /** - * Builds graffiti from user's graffiti input and CL/EL client info according to the - * clientGraffitiAppendFormat configuration. + * Generates graffiti by combining user-supplied graffiti with information from CL/EL clients, + * according to the clientGraffitiAppendFormat configuration. */ public class GraffitiBuilder implements ExecutionClientVersionChannel { private static final Logger LOG = LogManager.getLogger(); @@ -73,8 +73,9 @@ public ClientVersion getConsensusClientVersion() { @Override public void onExecutionClientVersion(final ClientVersion executionClientVersion) { this.executionClientVersion.set(executionClientVersion); - final Optional graffiti = Optional.of(buildGraffiti(defaultUserGraffiti)); - EVENT_LOG.logDefaultGraffiti(extractGraffiti(graffiti, calculateGraffitiLength(graffiti))); + final Optional defaultGraffiti = Optional.of(buildGraffiti(defaultUserGraffiti)); + EVENT_LOG.logDefaultGraffiti( + extractGraffiti(defaultGraffiti, calculateGraffitiLength(defaultGraffiti))); } public Bytes32 buildGraffiti(final Optional userGraffiti) { @@ -87,16 +88,16 @@ public Bytes32 buildGraffiti(final Optional userGraffiti) { yield joinNonEmpty( SPACE, extractGraffiti(userGraffiti, userGraffitiLength), - formatClientInfo(clientInfoLength)); + formatClientsInfo(clientInfoLength)); } - case NAME -> { + case CLIENT_NAMES -> { final int clientInfoLength = Integer.min(Bytes32.SIZE - 1 - userGraffitiLength, 4); yield joinNonEmpty( SPACE, extractGraffiti(userGraffiti, userGraffitiLength), - formatClientInfo(clientInfoLength)); + formatClientsInfo(clientInfoLength)); } - case NONE -> userGraffiti.orElse(Bytes32.ZERO); + case DISABLED -> userGraffiti.orElse(Bytes32.ZERO); }; } catch (final Exception ex) { LOG.error("Unexpected error when preparing block graffiti", ex); @@ -126,7 +127,7 @@ protected Bytes32 joinNonEmpty(final String delimiter, final String... parts) { } @VisibleForTesting - protected String formatClientInfo(final int length) { + protected String formatClientsInfo(final int length) { final String safeConsensusCode = extractClientCodeSafely(consensusClientVersion); final String safeExecutionCode = extractClientCodeSafely(executionClientVersion.get()); // LH1be52536BU0f91a674 diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java index 3fb4773e510..045ed39e5e2 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/AbstractBlockFactoryTest.java @@ -119,7 +119,7 @@ public abstract class AbstractBlockFactoryTest { protected ExecutionPayloadResult cachedExecutionPayloadResult = null; protected GraffitiBuilder graffitiBuilder = - new GraffitiBuilder(ClientGraffitiAppendFormat.NONE, Optional.empty()); + new GraffitiBuilder(ClientGraffitiAppendFormat.DISABLED, Optional.empty()); @BeforeAll public static void initSession() { diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java index 5828e105f3c..7094d834dc0 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/BlockOperationSelectorFactoryTest.java @@ -177,7 +177,7 @@ class BlockOperationSelectorFactoryTest { new CapturingBeaconBlockBodyBuilder(false); private final GraffitiBuilder graffitiBuilder = - new GraffitiBuilder(ClientGraffitiAppendFormat.NONE, Optional.empty()); + new GraffitiBuilder(ClientGraffitiAppendFormat.DISABLED, Optional.empty()); private final BlockOperationSelectorFactory factory = new BlockOperationSelectorFactory( diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java index 54e5c88ad23..6bf6b79b894 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java @@ -15,8 +15,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.AUTO; -import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NAME; -import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.NONE; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.CLIENT_NAMES; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.DISABLED; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -42,7 +42,7 @@ public class GraffitiBuilderTest { new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); private static final ClientVersion TEKU_CLIENT_VERSION = - new GraffitiBuilder(NONE, Optional.empty()).getConsensusClientVersion(); + new GraffitiBuilder(DISABLED, Optional.empty()).getConsensusClientVersion(); private static final ClientVersion BESU_CLIENT_VERSION = new ClientVersion("BU", "Besu", "23.4.1", Bytes4.fromHexString("abcdef12")); @@ -218,14 +218,14 @@ public void formatClientInfo_shouldRenderClientNamesAndFullCommit() { graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // 20: LH1be52536BU0f91a674 - assertThat(graffitiBuilder.formatClientInfo(30)) + assertThat(graffitiBuilder.formatClientsInfo(30)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString()) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(20)); - assertThat(graffitiBuilder.formatClientInfo(20)) + assertThat(graffitiBuilder.formatClientsInfo(20)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() @@ -239,14 +239,14 @@ public void formatClientInfo_shouldRenderClientNamesAndHalfCommit() { graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // 12: LH1be5BU0f91 - assertThat(graffitiBuilder.formatClientInfo(19)) + assertThat(graffitiBuilder.formatClientsInfo(19)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4) + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4)) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(12)); - assertThat(graffitiBuilder.formatClientInfo(12)) + assertThat(graffitiBuilder.formatClientsInfo(12)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4) @@ -260,14 +260,14 @@ public void formatClientInfo_shouldRenderClientNamesAnd1stCommitByte() { graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // 8: LH1bBU0f - assertThat(graffitiBuilder.formatClientInfo(11)) + assertThat(graffitiBuilder.formatClientsInfo(11)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(8)); - assertThat(graffitiBuilder.formatClientInfo(8)) + assertThat(graffitiBuilder.formatClientsInfo(8)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) @@ -281,10 +281,10 @@ public void formatClientInfo_shouldRenderClientNames() { graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // 4: LHBU - assertThat(graffitiBuilder.formatClientInfo(7)) + assertThat(graffitiBuilder.formatClientsInfo(7)) .isEqualTo(TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(4)); - assertThat(graffitiBuilder.formatClientInfo(4)) + assertThat(graffitiBuilder.formatClientsInfo(4)) .isEqualTo(TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(4)); } @@ -294,13 +294,13 @@ public void formatClientInfo_shouldSkipClientsInfoWhenNotEnoughSpace() { graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); // Empty - assertThat(graffitiBuilder.formatClientInfo(3)) + assertThat(graffitiBuilder.formatClientsInfo(3)) .isEqualTo("") .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(0)); - assertThat(graffitiBuilder.formatClientInfo(0)) + assertThat(graffitiBuilder.formatClientsInfo(0)) .isEqualTo("") .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(0)); - assertThat(graffitiBuilder.formatClientInfo(-1)) + assertThat(graffitiBuilder.formatClientsInfo(-1)) .isEqualTo("") .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(0)); } @@ -317,7 +317,7 @@ public void formatClientInfo_shouldHandleBadCodeOnClientNamesAndFullCommit( BESU_CLIENT_VERSION.commit())); // 20: LH1be52536BU0f91a674 - assertThat(graffitiBuilder.formatClientInfo(20)) + assertThat(graffitiBuilder.formatClientsInfo(20)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() @@ -338,7 +338,7 @@ public void formatClientInfo_shouldHandleBadCodeOnClientNamesAndHalfCommit( BESU_CLIENT_VERSION.commit())); // 12: LH1be5BU0f91 - assertThat(graffitiBuilder.formatClientInfo(12)) + assertThat(graffitiBuilder.formatClientsInfo(12)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 4) @@ -359,7 +359,7 @@ public void formatClientInfo_shouldHandleBadCodeOnClientNamesAnd1stCommitByte( BESU_CLIENT_VERSION.commit())); // 8: LH1bBU0f - assertThat(graffitiBuilder.formatClientInfo(8)) + assertThat(graffitiBuilder.formatClientsInfo(8)) .isEqualTo( TEKU_CLIENT_VERSION.code() + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2) @@ -380,7 +380,7 @@ public void formatClientInfo_shouldHandleBadCodeOnClientNames( BESU_CLIENT_VERSION.commit())); // 4: LHBU - assertThat(graffitiBuilder.formatClientInfo(4)) + assertThat(graffitiBuilder.formatClientsInfo(4)) .isEqualTo(TEKU_CLIENT_VERSION.code() + expectedCode) .satisfies(s -> assertThat(s.getBytes(StandardCharsets.UTF_8).length).isEqualTo(4)); } @@ -432,22 +432,24 @@ private static Stream getBuildGraffitiFixtures() { + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)), Arguments.of( - NAME, Optional.empty(), TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), + CLIENT_NAMES, + Optional.empty(), + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - NAME, + CLIENT_NAMES, Optional.of("small"), "small " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - NAME, + CLIENT_NAMES, Optional.of(UTF_8_GRAFFITI_4), UTF_8_GRAFFITI_4 + " " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - NAME, + CLIENT_NAMES, Optional.of(ASCII_GRAFFITI_20), ASCII_GRAFFITI_20 + " " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), - Arguments.of(NONE, Optional.empty(), ""), - Arguments.of(NONE, Optional.of("small"), "small"), - Arguments.of(NONE, Optional.of(UTF_8_GRAFFITI_4), UTF_8_GRAFFITI_4), - Arguments.of(NONE, Optional.of(ASCII_GRAFFITI_20), ASCII_GRAFFITI_20)); + Arguments.of(DISABLED, Optional.empty(), ""), + Arguments.of(DISABLED, Optional.of("small"), "small"), + Arguments.of(DISABLED, Optional.of(UTF_8_GRAFFITI_4), UTF_8_GRAFFITI_4), + Arguments.of(DISABLED, Optional.of(ASCII_GRAFFITI_20), ASCII_GRAFFITI_20)); } } diff --git a/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java b/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java index 27f55026630..0e09e90f4c5 100644 --- a/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java +++ b/ethereum/executionclient/src/test/java/tech/pegasys/teku/ethereum/executionclient/ExecutionClientVersionProviderTest.java @@ -64,7 +64,7 @@ public void doesNotTryToUpdateExecutionClientVersionIfElHasNotBeenUnavailable() } @Test - public void updatesExecutionClientVersionElIsAvailableAfterBeingUnavailable() { + public void updatesExecutionClientVersionWhenElIsAvailableAfterBeingUnavailable() { final ExecutionClientVersionProvider executionClientVersionProvider = new ExecutionClientVersionProvider( executionLayerChannel, publishChannel, ClientVersion.UNKNOWN); diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java index c826897b1a5..f0788cbe715 100644 --- a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorOptions.java @@ -61,11 +61,8 @@ public class ValidatorOptions { paramLabel = "", showDefaultValue = Visibility.ALWAYS, description = - """ - Appends CL and EL clients information separated with a space to user's graffiti on the Beacon Node. - AUTO mode tries to provide complete version information if there is enough space for it. - NAME adds only CL and EL name, to disable use NONE. (Valid values: ${COMPLETION-CANDIDATES}) - """, + "Appends CL and EL clients information with a space to user's graffiti " + + "when producing a block on the Beacon Node. (Valid values: ${COMPLETION-CANDIDATES})", arity = "1") private ClientGraffitiAppendFormat clientGraffitiAppendFormat = ValidatorConfig.DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT; diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java index 75a4b882533..7ee2282fec6 100644 --- a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java +++ b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java @@ -15,7 +15,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static tech.pegasys.teku.validator.api.ValidatorConfig.DEFAULT_BUILDER_REGISTRATION_GAS_LIMIT; -import static tech.pegasys.teku.validator.api.ValidatorConfig.DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT; import java.net.MalformedURLException; import java.net.URL; @@ -241,17 +240,17 @@ public void shouldSetShutdownWhenValidatorSlashedEnabled() { public void shouldSetDefaultGraffitiClientAppend() { final ValidatorConfig config = getTekuConfigurationFromArguments().validatorClient().getValidatorConfig(); - assertThat(config.getClientGraffitiAppendFormat()) - .isEqualTo(DEFAULT_CLIENT_GRAFFITI_APPEND_FORMAT); + assertThat(config.getClientGraffitiAppendFormat()).isEqualTo(ClientGraffitiAppendFormat.AUTO); } @Test public void shouldOverrideGraffitiClientAppend() { final ValidatorConfig config = - getTekuConfigurationFromArguments("--validators-graffiti-client-append-format=NAME") + getTekuConfigurationFromArguments("--validators-graffiti-client-append-format=CLIENT_NAMES") .validatorClient() .getValidatorConfig(); - assertThat(config.getClientGraffitiAppendFormat()).isEqualTo(ClientGraffitiAppendFormat.NAME); + assertThat(config.getClientGraffitiAppendFormat()) + .isEqualTo(ClientGraffitiAppendFormat.CLIENT_NAMES); } @Test diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java index 8231d7939be..f718384fded 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java @@ -14,12 +14,12 @@ package tech.pegasys.teku.validator.api; public enum ClientGraffitiAppendFormat { - // Appends comprehensive client info if there is a space for it. Reduces verbosity when needed. - // Client info is separated with a space after user's graffiti if any. + // Appends comprehensive clients information if there is a space for it. + // Reduces verbosity with less space or completely skips adding clients information. + // Clients info is separated with a space after user's graffiti if any. AUTO, - // Prepends comprehensive client info with a space before user's graffiti if there is a room for - // it. - NAME, - // Client information is not appended to the graffiti. - NONE; + // Appends client name codes if there is a space for it. + CLIENT_NAMES, + // Clients information is not appended to the graffiti. + DISABLED; } From b11aad93bdc582779fbd0291b45ff6a4605bd873 Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Tue, 26 Mar 2024 10:22:45 +0400 Subject: [PATCH 8/8] Rename CLIENT_NAMES -> CLIENT_CODES --- .../teku/validator/coordinator/GraffitiBuilder.java | 2 +- .../validator/coordinator/GraffitiBuilderTest.java | 10 +++++----- .../pegasys/teku/cli/options/ValidatorOptionsTest.java | 4 ++-- .../teku/validator/api/ClientGraffitiAppendFormat.java | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java index 3fb65db980e..eb346831356 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilder.java @@ -90,7 +90,7 @@ yield joinNonEmpty( extractGraffiti(userGraffiti, userGraffitiLength), formatClientsInfo(clientInfoLength)); } - case CLIENT_NAMES -> { + case CLIENT_CODES -> { final int clientInfoLength = Integer.min(Bytes32.SIZE - 1 - userGraffitiLength, 4); yield joinNonEmpty( SPACE, diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java index 6bf6b79b894..ea990b5249e 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/GraffitiBuilderTest.java @@ -15,7 +15,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.AUTO; -import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.CLIENT_NAMES; +import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.CLIENT_CODES; import static tech.pegasys.teku.validator.api.ClientGraffitiAppendFormat.DISABLED; import java.nio.charset.StandardCharsets; @@ -432,19 +432,19 @@ private static Stream getBuildGraffitiFixtures() { + BESU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.commit().toUnprefixedHexString().substring(0, 2)), Arguments.of( - CLIENT_NAMES, + CLIENT_CODES, Optional.empty(), TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - CLIENT_NAMES, + CLIENT_CODES, Optional.of("small"), "small " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - CLIENT_NAMES, + CLIENT_CODES, Optional.of(UTF_8_GRAFFITI_4), UTF_8_GRAFFITI_4 + " " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of( - CLIENT_NAMES, + CLIENT_CODES, Optional.of(ASCII_GRAFFITI_20), ASCII_GRAFFITI_20 + " " + TEKU_CLIENT_VERSION.code() + BESU_CLIENT_VERSION.code()), Arguments.of(DISABLED, Optional.empty(), ""), diff --git a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java index 7ee2282fec6..c3ca6e4a966 100644 --- a/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java +++ b/teku/src/test/java/tech/pegasys/teku/cli/options/ValidatorOptionsTest.java @@ -246,11 +246,11 @@ public void shouldSetDefaultGraffitiClientAppend() { @Test public void shouldOverrideGraffitiClientAppend() { final ValidatorConfig config = - getTekuConfigurationFromArguments("--validators-graffiti-client-append-format=CLIENT_NAMES") + getTekuConfigurationFromArguments("--validators-graffiti-client-append-format=CLIENT_CODES") .validatorClient() .getValidatorConfig(); assertThat(config.getClientGraffitiAppendFormat()) - .isEqualTo(ClientGraffitiAppendFormat.CLIENT_NAMES); + .isEqualTo(ClientGraffitiAppendFormat.CLIENT_CODES); } @Test diff --git a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java index f718384fded..5caeef6c1c4 100644 --- a/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java +++ b/validator/api/src/main/java/tech/pegasys/teku/validator/api/ClientGraffitiAppendFormat.java @@ -19,7 +19,7 @@ public enum ClientGraffitiAppendFormat { // Clients info is separated with a space after user's graffiti if any. AUTO, // Appends client name codes if there is a space for it. - CLIENT_NAMES, + CLIENT_CODES, // Clients information is not appended to the graffiti. DISABLED; }