From 37f5307b4bfd52ae46ddd581b80305f4ab008684 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Sat, 22 Jun 2024 10:31:37 +0100 Subject: [PATCH] improvement --- .../coordinator/GraffitiBuilder.java | 18 ++++----- .../coordinator/AbstractBlockFactoryTest.java | 2 +- .../BlockOperationSelectorFactoryTest.java | 2 +- .../coordinator/GraffitiBuilderTest.java | 39 ++++++++----------- .../infrastructure/logging/EventLogger.java | 6 +-- .../beaconchain/BeaconChainController.java | 4 +- 6 files changed, 29 insertions(+), 42 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 77e07220618..f2bddbc70bf 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 @@ -46,14 +46,10 @@ public class GraffitiBuilder implements ExecutionClientVersionChannel { private final ClientGraffitiAppendFormat clientGraffitiAppendFormat; private final ClientVersion consensusClientVersion; - private final Optional defaultUserGraffiti; - public GraffitiBuilder( - final ClientGraffitiAppendFormat clientGraffitiAppendFormat, - final Optional defaultUserGraffiti) { + public GraffitiBuilder(final ClientGraffitiAppendFormat clientGraffitiAppendFormat) { this.clientGraffitiAppendFormat = clientGraffitiAppendFormat; this.consensusClientVersion = createTekuClientVersion(); - this.defaultUserGraffiti = defaultUserGraffiti; } private ClientVersion createTekuClientVersion() { @@ -73,18 +69,18 @@ public ClientVersion getConsensusClientVersion() { @Override public void onExecutionClientVersion(final ClientVersion executionClientVersion) { this.executionClientVersion = Optional.of(executionClientVersion); - logDefaultGraffiti(); + logGraffitiWatermark(); } @Override public void onExecutionClientVersionNotAvailable() { - logDefaultGraffiti(); + logGraffitiWatermark(); } - private void logDefaultGraffiti() { - final Optional defaultGraffiti = Optional.of(buildGraffiti(defaultUserGraffiti)); - EVENT_LOG.logDefaultGraffiti( - extractGraffiti(defaultGraffiti, calculateGraffitiLength(defaultGraffiti))); + private void logGraffitiWatermark() { + final Optional graffitiWatermark = Optional.of(buildGraffiti(Optional.empty())); + EVENT_LOG.logGraffitiWatermark( + extractGraffiti(graffitiWatermark, calculateGraffitiLength(graffitiWatermark))); } public Bytes32 buildGraffiti(final Optional userGraffiti) { 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 ae25358e2ea..e2d7f42cde1 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 @@ -124,7 +124,7 @@ public abstract class AbstractBlockFactoryTest { protected ExecutionPayloadResult cachedExecutionPayloadResult = null; protected GraffitiBuilder graffitiBuilder = - new GraffitiBuilder(ClientGraffitiAppendFormat.DISABLED, Optional.empty()); + new GraffitiBuilder(ClientGraffitiAppendFormat.DISABLED); @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 2ef3063a0fd..f0c4d75deed 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 @@ -186,7 +186,7 @@ class BlockOperationSelectorFactoryTest { new CapturingBeaconBlockBodyBuilder(false, false); private final GraffitiBuilder graffitiBuilder = - new GraffitiBuilder(ClientGraffitiAppendFormat.DISABLED, Optional.empty()); + new GraffitiBuilder(ClientGraffitiAppendFormat.DISABLED); private final BlockOperationSelectorFactory factoryElectra = 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 e3013d36f4b..849eec474ed 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 @@ -37,12 +37,10 @@ public class GraffitiBuilderTest { private ClientGraffitiAppendFormat clientGraffitiAppendFormat = AUTO; - private Optional userGraffiti = Optional.empty(); - private GraffitiBuilder graffitiBuilder = - new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); + private GraffitiBuilder graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat); private static final ClientVersion TEKU_CLIENT_VERSION = - new GraffitiBuilder(DISABLED, Optional.empty()).getConsensusClientVersion(); + new GraffitiBuilder(DISABLED).getConsensusClientVersion(); private static final ClientVersion BESU_CLIENT_VERSION = new ClientVersion("BU", "Besu", "23.4.1", Bytes4.fromHexString("abcdef12")); @@ -58,41 +56,37 @@ public class GraffitiBuilderTest { @BeforeEach public void setup() { this.clientGraffitiAppendFormat = AUTO; - this.userGraffiti = Optional.empty(); - this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); + this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat); } @Test - public void onExecutionClientVersion_shouldLogDefaultGraffiti() { + public void onExecutionClientVersion_shouldLogGraffitiWatermark() { + this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat); 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" + "Using graffiti watermark: \"TK" + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + "BUabcdef12\". " - + "To change check validator graffiti options."); + + "This will be appended to any user-defined graffiti or used if none is defined. Refer to validator graffiti options to customize."); } } @Test - public void onExecutionClientVersion_shouldLogDefaultMergedGraffiti() { - this.graffitiBuilder = - new GraffitiBuilder( - clientGraffitiAppendFormat, Optional.of(Bytes32Parser.toBytes32(ASCII_GRAFFITI_20))); + public void onExecutionClientVersionNotAvailable_shouldLogGraffitiWatermark() { try (final LogCaptor logCaptor = LogCaptor.forClass(EventLogger.class)) { - graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); + graffitiBuilder.onExecutionClientVersionNotAvailable(); 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."); + "Using graffiti watermark: \"TK" + + TEKU_CLIENT_VERSION.commit().toUnprefixedHexString() + + "\". This will be appended to any user-defined graffiti or used if none is defined. Refer to validator graffiti options to customize."); } } @Test public void buildGraffiti_shouldNotFail() { this.graffitiBuilder = - new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti) { + new GraffitiBuilder(clientGraffitiAppendFormat) { @Override protected int calculateGraffitiLength(final Optional graffiti) { throw new RuntimeException(""); @@ -105,10 +99,9 @@ protected int calculateGraffitiLength(final Optional graffiti) { @Test public void buildGraffiti_shouldPreferCallInput() { - final Bytes32 defaultGraffiti = Bytes32Parser.toBytes32(asciiGraffiti32); final Bytes32 userGraffiti = Bytes32Parser.toBytes32(ASCII_GRAFFITI_20); final Bytes32 expectedGraffiti = Bytes32Parser.toBytes32(ASCII_GRAFFITI_20 + " TK"); - this.graffitiBuilder = new GraffitiBuilder(CLIENT_CODES, Optional.of(defaultGraffiti)); + this.graffitiBuilder = new GraffitiBuilder(CLIENT_CODES); assertThat(graffitiBuilder.buildGraffiti(Optional.of(userGraffiti))) .isEqualTo(expectedGraffiti); } @@ -119,7 +112,7 @@ public void buildGraffiti_shouldProvideCorrectOutput( final ClientGraffitiAppendFormat clientGraffitiAppendFormat, final Optional maybeUserGraffiti, final String expectedGraffiti) { - this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); + this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat); graffitiBuilder.onExecutionClientVersion(BESU_CLIENT_VERSION); final Bytes32 expectedGraffitiBytes = Bytes32Parser.toBytes32(expectedGraffiti); assertThat( @@ -140,7 +133,7 @@ public void buildGraffiti_shouldProvideCorrectOutput_whenElInfoNa( final ClientGraffitiAppendFormat clientGraffitiAppendFormat, final Optional maybeUserGraffiti, final String expectedGraffiti) { - this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat, userGraffiti); + this.graffitiBuilder = new GraffitiBuilder(clientGraffitiAppendFormat); final Bytes32 expectedGraffitiBytes = Bytes32Parser.toBytes32(expectedGraffiti); assertThat( new String( 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 486daacd973..4893ec3e908 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,11 +174,11 @@ public void logExecutionClientVersion(final String name, final String version) { log.info("Execution Client version: {} {}", name, version); } - public void logDefaultGraffiti(final String graffiti) { + public void logGraffitiWatermark(final String graffitiWatermark) { log.info( - "Using default graffiti: \"{}\". This will be appended to any user-defined graffiti or used if none is defined. " + "Using graffiti watermark: \"{}\". This will be appended to any user-defined graffiti or used if none is defined. " + "Refer to validator graffiti options to customize.", - graffiti); + graffitiWatermark); } public void builderIsNotAvailable(final String errorMessage) { 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 a4ec12fe4d1..1782ea9a90d 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 @@ -900,9 +900,7 @@ public void initRewardCalculator() { public void initValidatorApiHandler() { LOG.debug("BeaconChainController.initValidatorApiHandler()"); final GraffitiBuilder graffitiBuilder = - new GraffitiBuilder( - beaconConfig.validatorConfig().getClientGraffitiAppendFormat(), - beaconConfig.validatorConfig().getGraffitiProvider().get()); + new GraffitiBuilder(beaconConfig.validatorConfig().getClientGraffitiAppendFormat()); eventChannels.subscribe(ExecutionClientVersionChannel.class, graffitiBuilder); final ExecutionClientVersionProvider executionClientVersionProvider = new ExecutionClientVersionProvider(