diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifier.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifier.java index 2c6e364a3bf..975bcc4deee 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifier.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifier.java @@ -13,6 +13,8 @@ package tech.pegasys.teku.statetransition.forkchoice; +import static tech.pegasys.teku.infrastructure.logging.StatusLogger.STATUS_LOG; + import java.util.Collection; import java.util.Optional; import org.apache.logging.log4j.LogManager; @@ -22,6 +24,7 @@ import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.async.eventthread.AsyncRunnerEventThread; import tech.pegasys.teku.infrastructure.async.eventthread.EventThread; +import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; import tech.pegasys.teku.infrastructure.ssz.type.Bytes8; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; @@ -29,7 +32,6 @@ import tech.pegasys.teku.spec.datastructures.operations.versions.bellatrix.BeaconPreparableProposer; import tech.pegasys.teku.spec.executionengine.ExecutionEngineChannel; import tech.pegasys.teku.spec.executionengine.ForkChoiceState; -import tech.pegasys.teku.spec.executionengine.PayloadAttributes; import tech.pegasys.teku.storage.client.RecentChainData; public class ForkChoiceNotifier { @@ -42,8 +44,6 @@ public class ForkChoiceNotifier { private final Spec spec; private ForkChoiceUpdateData forkChoiceUpdateData = new ForkChoiceUpdateData(); - private long payloadAttributesSequenceProducer = 0; - private long payloadAttributesSequenceConsumer = -1; private boolean inSync = false; // Assume we are not in sync at startup. @@ -64,7 +64,8 @@ public static ForkChoiceNotifier create( final AsyncRunnerFactory asyncRunnerFactory, final Spec spec, final ExecutionEngineChannel executionEngineChannel, - final RecentChainData recentChainData) { + final RecentChainData recentChainData, + final Optional proposerDefaultFeeRecipient) { final AsyncRunnerEventThread eventThread = new AsyncRunnerEventThread("forkChoiceNotifier", asyncRunnerFactory); eventThread.start(); @@ -73,7 +74,8 @@ public static ForkChoiceNotifier create( spec, executionEngineChannel, recentChainData, - new PayloadAttributesCalculator(spec, eventThread, recentChainData)); + new PayloadAttributesCalculator( + spec, eventThread, recentChainData, proposerDefaultFeeRecipient)); } public void onUpdatePreparableProposers(final Collection proposers) { @@ -113,6 +115,7 @@ private void internalTerminalBlockReached(Bytes32 executionBlockHash) { /** * @param parentBeaconBlockRoot root of the beacon block the new block will be built on + * @param blockSlot slot of the block being produced, for which the payloadId has been requested * @return must return a Future resolving to: *

Optional.empty() only when is safe to produce a block with an empty execution payload * (after the bellatrix fork and before Terminal Block arrival) @@ -150,7 +153,7 @@ private SafeFuture> internalGetPayloadId( // returns, we save locally the current class reference. ForkChoiceUpdateData localForkChoiceUpdateData = forkChoiceUpdateData; return payloadAttributesCalculator - .calculatePayloadAttributes(blockSlot, inSync, localForkChoiceUpdateData) + .calculatePayloadAttributes(blockSlot, inSync, localForkChoiceUpdateData, true) .thenCompose( newPayloadAttributes -> { @@ -178,6 +181,10 @@ private void internalUpdatePreparableProposers( LOG.debug("internalUpdatePreparableProposers proposers {}", proposers); + if (!payloadAttributesCalculator.isProposerDefaultFeeRecipientDefined()) { + STATUS_LOG.warnMissingProposerDefaultFeeRecipientWithPreparedBeaconProposerBeingCalled(); + } + // Default to the genesis slot if we're pre-genesis. final UInt64 currentSlot = recentChainData.getCurrentSlot().orElse(SpecConfig.GENESIS_SLOT); @@ -218,46 +225,24 @@ private void sendForkChoiceUpdated() { private void updatePayloadAttributes(final UInt64 blockSlot) { LOG.debug("updatePayloadAttributes blockSlot {}", blockSlot); - // we want to preserve ordering in payload calculation, - // so we first generate a sequence for each calculation request - final long sequenceNumber = payloadAttributesSequenceProducer++; - payloadAttributesCalculator - .calculatePayloadAttributes(blockSlot, inSync, forkChoiceUpdateData) - .thenAcceptAsync( - newPayloadAttributes -> - updatePayloadAttributes(blockSlot, newPayloadAttributes, sequenceNumber), + forkChoiceUpdateData + .withPayloadAttributesAsync( + () -> + payloadAttributesCalculator.calculatePayloadAttributes( + blockSlot, inSync, forkChoiceUpdateData, false), eventThread) + .thenAccept( + newForkChoiceUpdateData -> { + if (newForkChoiceUpdateData.isPresent()) { + forkChoiceUpdateData = newForkChoiceUpdateData.get(); + sendForkChoiceUpdated(); + } + }) .finish( error -> LOG.error("Failed to calculate payload attributes for slot {}", blockSlot, error)); } - private boolean updatePayloadAttributes( - final UInt64 blockSlot, - final Optional newPayloadAttributes, - final long sequenceNumber) { - eventThread.checkOnEventThread(); - - LOG.debug( - "updatePayloadAttributes blockSlot {} newPayloadAttributes {}", - blockSlot, - newPayloadAttributes); - - // to preserve ordering we make sure we haven't already calculated a payload that has been - // requested later than the current one - if (sequenceNumber <= payloadAttributesSequenceConsumer) { - LOG.warn("Ignoring calculated payload attributes since it violates ordering"); - return false; - } - payloadAttributesSequenceConsumer = sequenceNumber; - - LOG.debug("updatePayloadAttributes blockSlot {} {}", blockSlot, newPayloadAttributes); - - forkChoiceUpdateData = forkChoiceUpdateData.withPayloadAttributes(newPayloadAttributes); - sendForkChoiceUpdated(); - return true; - } - public PayloadAttributesCalculator getPayloadAttributesCalculator() { return payloadAttributesCalculator; } diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceUpdateData.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceUpdateData.java index 6d49bfe7a65..ae4c978288a 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceUpdateData.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceUpdateData.java @@ -14,7 +14,9 @@ package tech.pegasys.teku.statetransition.forkchoice; import com.google.common.base.MoreObjects; +import com.google.common.base.Supplier; import java.util.Optional; +import java.util.concurrent.Executor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.tuweni.bytes.Bytes32; @@ -35,6 +37,9 @@ public class ForkChoiceUpdateData { private final SafeFuture> payloadId = new SafeFuture<>(); private boolean sent = false; + private long payloadAttributesSequenceProducer = 0; + private long payloadAttributesSequenceConsumer = -1; + public ForkChoiceUpdateData() { this.forkChoiceState = new ForkChoiceState( @@ -87,6 +92,29 @@ public ForkChoiceUpdateData withTerminalBlockHash(final Bytes32 terminalBlockHas forkChoiceState, payloadAttributes, Optional.of(terminalBlockHash)); } + public SafeFuture> withPayloadAttributesAsync( + final Supplier>> payloadAttributesCalculator, + final Executor executor) { + // we want to preserve ordering in payload calculation, + // so we first generate a sequence for each calculation request + final long sequenceNumber = payloadAttributesSequenceProducer++; + + return payloadAttributesCalculator + .get() + .thenApplyAsync( + newPayloadAttributes -> { + // to preserve ordering we make sure we haven't already calculated a payload that has + // been requested later than the current one + if (sequenceNumber <= payloadAttributesSequenceConsumer) { + LOG.warn("Ignoring calculated payload attributes since it violates ordering"); + return Optional.empty(); + } + payloadAttributesSequenceConsumer = sequenceNumber; + return Optional.of(this.withPayloadAttributes(newPayloadAttributes)); + }, + executor); + } + public boolean isPayloadIdSuitable(final Bytes32 parentExecutionHash, final UInt64 timestamp) { if (payloadAttributes.isEmpty()) { LOG.debug("isPayloadIdSuitable - payloadAttributes.isEmpty returning false"); diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/PayloadAttributesCalculator.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/PayloadAttributesCalculator.java index 31bffc93613..5b6579f6b42 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/PayloadAttributesCalculator.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/forkchoice/PayloadAttributesCalculator.java @@ -13,6 +13,8 @@ package tech.pegasys.teku.statetransition.forkchoice; +import static tech.pegasys.teku.infrastructure.logging.ValidatorLogger.VALIDATOR_LOGGER; + import com.google.common.collect.ImmutableMap; import java.util.Collection; import java.util.List; @@ -23,6 +25,7 @@ import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.infrastructure.async.SafeFuture; import tech.pegasys.teku.infrastructure.async.eventthread.EventThread; +import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot; @@ -39,12 +42,17 @@ public class PayloadAttributesCalculator { private final EventThread eventThread; private final RecentChainData recentChainData; private final Map proposerInfoByValidatorIndex = new ConcurrentHashMap<>(); + private final Optional proposerDefaultFeeRecipient; public PayloadAttributesCalculator( - final Spec spec, final EventThread eventThread, final RecentChainData recentChainData) { + final Spec spec, + final EventThread eventThread, + final RecentChainData recentChainData, + final Optional proposerDefaultFeeRecipient) { this.spec = spec; this.eventThread = eventThread; this.recentChainData = recentChainData; + this.proposerDefaultFeeRecipient = proposerDefaultFeeRecipient; } public void updateProposers( @@ -64,7 +72,8 @@ public void updateProposers( public SafeFuture> calculatePayloadAttributes( final UInt64 blockSlot, final boolean inSync, - final ForkChoiceUpdateData forkChoiceUpdateData) { + final ForkChoiceUpdateData forkChoiceUpdateData, + final boolean mandatoryPayloadAttributes) { eventThread.checkOnEventThread(); if (!inSync) { // We don't produce blocks while syncing so don't bother preparing the payload @@ -82,11 +91,17 @@ public SafeFuture> calculatePayloadAttributes( final UInt64 epoch = spec.computeEpochAtSlot(blockSlot); return getStateInEpoch(epoch) .thenApplyAsync( - maybeState -> calculatePayloadAttributes(blockSlot, epoch, maybeState), eventThread); + maybeState -> + calculatePayloadAttributes( + blockSlot, epoch, maybeState, mandatoryPayloadAttributes), + eventThread); } private Optional calculatePayloadAttributes( - final UInt64 blockSlot, final UInt64 epoch, final Optional maybeState) { + final UInt64 blockSlot, + final UInt64 epoch, + final Optional maybeState, + final boolean mandatoryPayloadAttributes) { eventThread.checkOnEventThread(); if (maybeState.isEmpty()) { return Optional.empty(); @@ -94,13 +109,27 @@ private Optional calculatePayloadAttributes( final BeaconState state = maybeState.get(); final UInt64 proposerIndex = UInt64.valueOf(spec.getBeaconProposerIndex(state, blockSlot)); final ProposerInfo proposerInfo = proposerInfoByValidatorIndex.get(proposerIndex); - if (proposerInfo == null) { + if (proposerInfo == null && !mandatoryPayloadAttributes) { // Proposer is not one of our validators. No need to propose a block. return Optional.empty(); } final UInt64 timestamp = spec.computeTimeAtSlot(state, blockSlot); final Bytes32 random = spec.getRandaoMix(state, epoch); - return Optional.of(new PayloadAttributes(timestamp, random, proposerInfo.feeRecipient)); + return Optional.of( + new PayloadAttributes(timestamp, random, getFeeRecipient(proposerInfo, blockSlot))); + } + + // this function MUST return a fee recipient. + private Bytes20 getFeeRecipient(final ProposerInfo proposerInfo, final UInt64 blockSlot) { + if (proposerInfo != null) { + return proposerInfo.feeRecipient; + } + if (proposerDefaultFeeRecipient.isPresent()) { + VALIDATOR_LOGGER.executionPayloadPreparedUsingBeaconDefaultFeeRecipient(blockSlot); + return proposerDefaultFeeRecipient.get(); + } + throw new IllegalStateException( + "Unable to determine proposer fee recipient address for slot " + blockSlot); } private SafeFuture> getStateInEpoch(final UInt64 requiredEpoch) { @@ -130,4 +159,8 @@ public List> getData() { .build()) .collect(Collectors.toList()); } + + public boolean isProposerDefaultFeeRecipientDefined() { + return proposerDefaultFeeRecipient.isPresent(); + } } diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifierTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifierTest.java index f5de064c0d0..3707e4f1e20 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifierTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceNotifierTest.java @@ -69,6 +69,8 @@ class ForkChoiceNotifierTest { private RecentChainData recentChainData; private ForkChoiceStrategy forkChoiceStrategy; private PayloadAttributesCalculator payloadAttributesCalculator; + private Optional defaultFeeRecipient = + Optional.of(Bytes20.fromHexString("0x2Df386eFF130f991321bfC4F8372Ba838b9AB14B")); private final ExecutionEngineChannel executionEngineChannel = mock(ExecutionEngineChannel.class); @@ -86,11 +88,20 @@ public static void resetSession() { @BeforeEach void setUp() { + setUp(false); + } + + void setUp(final boolean doNotInitializeWithDefaultFeeRecipient) { // initialize post-merge by default storageSystem = InMemoryStorageSystemBuilder.buildDefault(spec); recentChainData = storageSystem.recentChainData(); payloadAttributesCalculator = - spy(new PayloadAttributesCalculator(spec, eventThread, recentChainData)); + spy( + new PayloadAttributesCalculator( + spec, + eventThread, + recentChainData, + doNotInitializeWithDefaultFeeRecipient ? Optional.empty() : defaultFeeRecipient)); notifier = new ForkChoiceNotifier( eventThread, @@ -115,7 +126,9 @@ void reInitializePreMerge() { storageSystem = InMemoryStorageSystemBuilder.buildDefault(spec); recentChainData = storageSystem.recentChainData(); payloadAttributesCalculator = - spy(new PayloadAttributesCalculator(spec, eventThread, recentChainData)); + spy( + new PayloadAttributesCalculator( + spec, eventThread, recentChainData, defaultFeeRecipient)); notifier = new ForkChoiceNotifier( eventThread, @@ -210,7 +223,7 @@ void onForkChoiceUpdated_shouldNotSendNotificationOfOutOfOrderPayloadAttributes( return deferredResponseA; }) .when(payloadAttributesCalculator) - .calculatePayloadAttributes(any(), anyBoolean(), any()); + .calculatePayloadAttributes(any(), anyBoolean(), any(), anyBoolean()); notifier.onForkChoiceUpdated(forkChoiceState); // calculate attributes for slot 2 // it is called once with no attributes. the one with attributes is pending @@ -219,7 +232,7 @@ void onForkChoiceUpdated_shouldNotSendNotificationOfOutOfOrderPayloadAttributes( // forward to real method call doAnswer(InvocationOnMock::callRealMethod) .when(payloadAttributesCalculator) - .calculatePayloadAttributes(any(), anyBoolean(), any()); + .calculatePayloadAttributes(any(), anyBoolean(), any(), anyBoolean()); storageSystem .chainUpdater() @@ -458,8 +471,8 @@ void getPayloadId_shouldObtainAPayloadIdWhenProposingTheMergeBlock() { notifier.onTerminalBlockReached(terminalBlockHash); - validateGetPayloadIOnTheFlyRetrieval( - blockSlot, blockRoot, forkChoiceState, payloadId, payloadAttributes); + validateGetPayloadIdOnTheFlyRetrieval( + blockSlot, blockRoot, forkChoiceState, payloadId, payloadAttributes, false); } @Test @@ -486,8 +499,8 @@ void getPayloadId_shouldObtainAPayloadIdOnPostMergeBlockNonFinalized() { final Bytes8 payloadId = dataStructureUtil.randomBytes8(); - validateGetPayloadIOnTheFlyRetrieval( - blockSlot, blockRoot, nonFinalizedForkChoiceState, payloadId, payloadAttributes); + validateGetPayloadIdOnTheFlyRetrieval( + blockSlot, blockRoot, nonFinalizedForkChoiceState, payloadId, payloadAttributes, false); } @Test @@ -508,8 +521,55 @@ void getPayloadId_shouldObtainAPayloadIdOnPostMergeBlockFinalized() { final Bytes32 blockRoot = recentChainData.getBestBlockRoot().orElseThrow(); final PayloadAttributes payloadAttributes = withProposerForSlot(headState, blockSlot); - validateGetPayloadIOnTheFlyRetrieval( - blockSlot, blockRoot, finalizedForkChoiceState, payloadId, payloadAttributes); + validateGetPayloadIdOnTheFlyRetrieval( + blockSlot, blockRoot, finalizedForkChoiceState, payloadId, payloadAttributes, false); + } + + @Test + void getPayloadId_shouldObtainAPayloadIdOnPostMergeBlockFinalizedEvenIfProposerNotPrepared() { + final Bytes8 payloadId = dataStructureUtil.randomBytes8(); + + // current slot: 1 + + // send post-merge onForkChoiceUpdated (with finalized block state) + ForkChoiceState finalizedForkChoiceState = getCurrentForkChoiceState(); + assertThat(finalizedForkChoiceState.getFinalizedExecutionBlockHash()) + .isNotEqualTo(Bytes32.ZERO); + notifier.onForkChoiceUpdated(finalizedForkChoiceState); + verify(executionEngineChannel).forkChoiceUpdated(finalizedForkChoiceState, Optional.empty()); + + final BeaconState headState = recentChainData.getBestState().orElseThrow(); + final UInt64 blockSlot = headState.getSlot().plus(2); // proposing slot 3 + final Bytes32 blockRoot = recentChainData.getBestBlockRoot().orElseThrow(); + final PayloadAttributes payloadAttributes = + withProposerForSlotButDoNotPrepare(headState, blockSlot, defaultFeeRecipient); + + validateGetPayloadIdOnTheFlyRetrieval( + blockSlot, blockRoot, finalizedForkChoiceState, payloadId, payloadAttributes, false); + } + + @Test + void getPayloadId_shouldFailIfDefaultFeeRecipientIsNotConfigured() { + setUp(true); + final Bytes8 payloadId = dataStructureUtil.randomBytes8(); + + // current slot: 1 + + // send post-merge onForkChoiceUpdated (with finalized block state) + ForkChoiceState finalizedForkChoiceState = getCurrentForkChoiceState(); + assertThat(finalizedForkChoiceState.getFinalizedExecutionBlockHash()) + .isNotEqualTo(Bytes32.ZERO); + notifier.onForkChoiceUpdated(finalizedForkChoiceState); + verify(executionEngineChannel).forkChoiceUpdated(finalizedForkChoiceState, Optional.empty()); + + final BeaconState headState = recentChainData.getBestState().orElseThrow(); + final UInt64 blockSlot = headState.getSlot().plus(2); // proposing slot 3 + final Bytes32 blockRoot = recentChainData.getBestBlockRoot().orElseThrow(); + final PayloadAttributes payloadAttributes = + withProposerForSlotButDoNotPrepare(headState, blockSlot, Optional.empty()); + + validateGetPayloadIdOnTheFlyRetrieval( + blockSlot, blockRoot, finalizedForkChoiceState, payloadId, payloadAttributes, true); } @Test @@ -551,12 +611,13 @@ void getPayloadId_preMergeShouldReturnEmptyBeforeTheFirstForkChoiceState() { .isCompletedWithEmptyOptional(); } - private void validateGetPayloadIOnTheFlyRetrieval( + private void validateGetPayloadIdOnTheFlyRetrieval( final UInt64 blockSlot, final Bytes32 blockRoot, final ForkChoiceState forkChoiceState, final Bytes8 payloadId, - final PayloadAttributes payloadAttributes) { + final PayloadAttributes payloadAttributes, + final boolean mustFail) { final SafeFuture responseFuture = new SafeFuture<>(); storageSystem.chainUpdater().setCurrentSlot(blockSlot); @@ -571,7 +632,11 @@ private void validateGetPayloadIOnTheFlyRetrieval( responseFuture.complete( new ForkChoiceUpdatedResult(ForkChoiceUpdatedStatus.SUCCESS, Optional.of(payloadId))); - assertThatSafeFuture(futurePayloadId).isCompletedWithOptionalContaining(payloadId); + if (mustFail) { + assertThatSafeFuture(futurePayloadId).isCompletedExceptionally(); + } else { + assertThatSafeFuture(futurePayloadId).isCompletedWithOptionalContaining(payloadId); + } } private PayloadAttributes withProposerForSlot(final UInt64 blockSlot) { @@ -584,14 +649,32 @@ private PayloadAttributes withProposerForSlot(final UInt64 blockSlot) { return withProposerForSlot(state, blockSlot); } + private PayloadAttributes withProposerForSlotButDoNotPrepare( + final BeaconState headState, + final UInt64 blockSlot, + final Optional overrideFeeRecipient) { + return withProposerForSlot(headState, blockSlot, false, overrideFeeRecipient); + } + private PayloadAttributes withProposerForSlot( final BeaconState headState, final UInt64 blockSlot) { + return withProposerForSlot(headState, blockSlot, true, Optional.empty()); + } + + private PayloadAttributes withProposerForSlot( + final BeaconState headState, + final UInt64 blockSlot, + final boolean doPrepare, + final Optional overrideFeeRecipient) { final int block2Proposer = spec.getBeaconProposerIndex(headState, blockSlot); - final PayloadAttributes payloadAttributes = getExpectedPayloadAttributes(headState, blockSlot); - notifier.onUpdatePreparableProposers( - List.of( - new BeaconPreparableProposer( - UInt64.valueOf(block2Proposer), payloadAttributes.getFeeRecipient()))); + final PayloadAttributes payloadAttributes = + getExpectedPayloadAttributes(headState, blockSlot, overrideFeeRecipient); + if (doPrepare) { + notifier.onUpdatePreparableProposers( + List.of( + new BeaconPreparableProposer( + UInt64.valueOf(block2Proposer), payloadAttributes.getFeeRecipient()))); + } return payloadAttributes; } @@ -600,9 +683,9 @@ private List withProposerForTwoSlots( final int block2Proposer1 = spec.getBeaconProposerIndex(headState, blockSlot1); final int block2Proposer2 = spec.getBeaconProposerIndex(headState, blockSlot2); final PayloadAttributes payloadAttributes1 = - getExpectedPayloadAttributes(headState, blockSlot1); + getExpectedPayloadAttributes(headState, blockSlot1, Optional.empty()); final PayloadAttributes payloadAttributes2 = - getExpectedPayloadAttributes(headState, blockSlot2); + getExpectedPayloadAttributes(headState, blockSlot2, Optional.empty()); if (block2Proposer1 == block2Proposer2) { throw new UnsupportedOperationException( @@ -618,8 +701,10 @@ private List withProposerForTwoSlots( } private PayloadAttributes getExpectedPayloadAttributes( - final BeaconState headState, final UInt64 blockSlot) { - final Bytes20 feeRecipient = dataStructureUtil.randomBytes20(); + final BeaconState headState, + final UInt64 blockSlot, + final Optional overrideFeeRecipient) { + final Bytes20 feeRecipient = overrideFeeRecipient.orElse(dataStructureUtil.randomBytes20()); final UInt64 timestamp = spec.computeTimeAtSlot(headState, blockSlot); final Bytes32 random = spec.getRandaoMix(headState, UInt64.ZERO); return new PayloadAttributes(timestamp, random, feeRecipient); diff --git a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java index ae1550cd0f1..64163332921 100644 --- a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java +++ b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/StatusLogger.java @@ -71,6 +71,23 @@ public void warnBellatrixParameterChanged(final String parameterName, final Stri Color.YELLOW)); } + public void warnMissingProposerDefaultFeeRecipientWithRestAPIEnabled() { + log.warn( + print( + "No default proposer fee recipient has been specified and rest API is enabled. " + + "If a Validator Client is going to use this node it is strongly recommended to " + + "configure it to avoid possible block production failures", + Color.RED)); + } + + public void warnMissingProposerDefaultFeeRecipientWithPreparedBeaconProposerBeingCalled() { + log.warn( + print( + "Validator Client detected and no default proposer fee recipient configured! " + + "it is strongly recommended to configure it to avoid possible block production failures", + Color.RED)); + } + public void fatalError(final String description, final Throwable cause) { log.fatal("Exiting due to fatal error in {}", description, cause); } diff --git a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/ValidatorLogger.java b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/ValidatorLogger.java index b8a7486a73d..cfd59afbede 100644 --- a/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/ValidatorLogger.java +++ b/infrastructure/logging/src/main/java/tech/pegasys/teku/infrastructure/logging/ValidatorLogger.java @@ -158,4 +158,14 @@ public void beaconProposerPreparationFailed(final Throwable error) { final String errorString = String.format("%sFailed to send proposers to Beacon Node", PREFIX); log.error(ColorConsolePrinter.print(errorString, Color.RED), error); } + + public void executionPayloadPreparedUsingBeaconDefaultFeeRecipient(final UInt64 slot) { + log.warn( + ColorConsolePrinter.print( + "Beacon Node has been requested to produce a block for slot " + + slot + + " but proposer hasn't been prepared by any Validator Client. " + + "Using Beacon Node's default fee recipient.", + Color.YELLOW)); + } } 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 e69acf674b6..865358e635e 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 @@ -40,6 +40,7 @@ import tech.pegasys.teku.infrastructure.events.EventChannels; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; import tech.pegasys.teku.infrastructure.io.PortAvailability; +import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; import tech.pegasys.teku.infrastructure.time.TimeProvider; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.infrastructure.version.VersionProvider; @@ -873,7 +874,26 @@ protected void initOperationsReOrgManager() { protected void initForkChoiceNotifier() { LOG.debug("BeaconChainController.initForkChoiceNotifier()"); forkChoiceNotifier = - ForkChoiceNotifier.create(asyncRunnerFactory, spec, executionEngine, recentChainData); + ForkChoiceNotifier.create( + asyncRunnerFactory, + spec, + executionEngine, + recentChainData, + getProposerDefaultFeeRecipient()); + } + + private Optional getProposerDefaultFeeRecipient() { + if (!spec.isMilestoneSupported(SpecMilestone.BELLATRIX)) { + return Optional.of(Bytes20.ZERO); + } + + Optional defaultFeeRecipient = + beaconConfig.validatorConfig().getProposerDefaultFeeRecipient(); + if (defaultFeeRecipient.isEmpty() && beaconConfig.beaconRestApiConfig().isRestApiEnabled()) { + STATUS_LOG.warnMissingProposerDefaultFeeRecipientWithRestAPIEnabled(); + } + + return defaultFeeRecipient; } protected void setupInitialState(final RecentChainData client) { 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 b4ac0ea8d64..8743cbc706c 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 @@ -16,8 +16,8 @@ import java.nio.file.Path; import java.util.Optional; import org.apache.tuweni.bytes.Bytes32; -import picocli.CommandLine; import picocli.CommandLine.Help.Visibility; +import picocli.CommandLine.Mixin; import picocli.CommandLine.Option; import tech.pegasys.teku.cli.converter.GraffitiConverter; import tech.pegasys.teku.config.TekuConfiguration; @@ -27,9 +27,12 @@ public class ValidatorOptions { - @CommandLine.Mixin(name = "Validator Keys") + @Mixin(name = "Validator Keys") private ValidatorKeysOptions validatorKeysOptions; + @Mixin(name = "Validator Proposer") + private ValidatorProposerOptions validatorProposerOptions; + @Option( names = {"--validators-graffiti"}, converter = GraffitiConverter.class, @@ -106,15 +109,6 @@ public class ValidatorOptions { arity = "0..1") private boolean generateEarlyAttestations = ValidatorConfig.DEFAULT_GENERATE_EARLY_ATTESTATIONS; - @Option( - names = {"--Xvalidators-suggested-fee-recipient-address"}, - paramLabel = "

", - description = - "Suggested fee recipient sent to the execution engine, which could use it as fee recipient when producing a new execution block.", - arity = "0..1", - hidden = true) - private String suggestedFeeRecipient = null; - public void configure(TekuConfiguration.Builder builder) { if (validatorPerformanceTrackingEnabled != null) { if (validatorPerformanceTrackingEnabled) { @@ -135,8 +129,8 @@ public void configure(TekuConfiguration.Builder builder) { new FileBackedGraffitiProvider( Optional.ofNullable(graffiti), Optional.ofNullable(graffitiFile))) .useDependentRoots(useDependentRoots) - .generateEarlyAttestations(generateEarlyAttestations) - .suggestedFeeRecipient(suggestedFeeRecipient)); + .generateEarlyAttestations(generateEarlyAttestations)); + validatorProposerOptions.configure(builder); validatorKeysOptions.configure(builder); } } diff --git a/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java new file mode 100644 index 00000000000..614d79d170d --- /dev/null +++ b/teku/src/main/java/tech/pegasys/teku/cli/options/ValidatorProposerOptions.java @@ -0,0 +1,32 @@ +/* + * Copyright 2022 ConsenSys AG. + * + * 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.cli.options; + +import picocli.CommandLine.Option; +import tech.pegasys.teku.config.TekuConfiguration; + +public class ValidatorProposerOptions { + @Option( + names = {"--Xvalidators-proposer-default-fee-recipient"}, + paramLabel = "
", + description = + "Default fee recipient sent to the execution engine, which could use it as fee recipient when producing a new execution block.", + arity = "0..1", + hidden = true) + private String proposerDefaultFeeRecipient = null; + + public void configure(TekuConfiguration.Builder builder) { + builder.validator(config -> config.proposerDefaultFeeRecipient(proposerDefaultFeeRecipient)); + } +} 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 ffb35e2e0a9..330524d8275 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 @@ -102,16 +102,17 @@ void beaconNodeApiEndpoint_shouldBeEmptyByDefault() { @Test public void shouldReportEmptyIfFeeRecipientNotSpecified() { final TekuConfiguration config = getTekuConfigurationFromArguments(); - assertThat(config.validatorClient().getValidatorConfig().getSuggestedFeeRecipient()).isEmpty(); + assertThat(config.validatorClient().getValidatorConfig().getProposerDefaultFeeRecipient()) + .isEmpty(); } @Test public void shouldReportAddressIfFeeRecipientSpecified() { final String[] args = { - "--Xvalidators-suggested-fee-recipient-address", "0xfe3b557e8fb62b89f4916b721be55ceb828dbd73" + "--Xvalidators-proposer-default-fee-recipient", "0xfe3b557e8fb62b89f4916b721be55ceb828dbd73" }; final TekuConfiguration config = getTekuConfigurationFromArguments(args); - assertThat(config.validatorClient().getValidatorConfig().getSuggestedFeeRecipient()) + assertThat(config.validatorClient().getValidatorConfig().getProposerDefaultFeeRecipient()) .isEqualTo( Optional.of(Eth1Address.fromHexString("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"))); } 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 41715638b5c..c6f2b5c9de6 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 @@ -57,7 +57,7 @@ public class ValidatorConfig { private final int validatorExternalSignerConcurrentRequestLimit; private final boolean useDependentRoots; private final boolean generateEarlyAttestations; - private final Optional suggestedFeeRecipient; + private final Optional proposerDefaultFeeRecipient; private ValidatorConfig( final List validatorKeys, @@ -76,7 +76,7 @@ private ValidatorConfig( final int validatorExternalSignerConcurrentRequestLimit, final boolean useDependentRoots, final boolean generateEarlyAttestations, - final Optional suggestedFeeRecipient) { + final Optional proposerDefaultFeeRecipient) { this.validatorKeys = validatorKeys; this.validatorExternalSignerPublicKeySources = validatorExternalSignerPublicKeySources; this.validatorExternalSignerUrl = validatorExternalSignerUrl; @@ -96,7 +96,7 @@ private ValidatorConfig( validatorExternalSignerConcurrentRequestLimit; this.useDependentRoots = useDependentRoots; this.generateEarlyAttestations = generateEarlyAttestations; - this.suggestedFeeRecipient = suggestedFeeRecipient; + this.proposerDefaultFeeRecipient = proposerDefaultFeeRecipient; } public static Builder builder() { @@ -160,16 +160,16 @@ public boolean useDependentRoots() { return useDependentRoots; } - public Optional getSuggestedFeeRecipient() { - validateFeeRecipient(); - return suggestedFeeRecipient; + public Optional getProposerDefaultFeeRecipient() { + validateProposerDefaultFeeRecipient(); + return proposerDefaultFeeRecipient; } - private void validateFeeRecipient() { - if (suggestedFeeRecipient.isEmpty() + private void validateProposerDefaultFeeRecipient() { + if (proposerDefaultFeeRecipient.isEmpty() && !(validatorKeys.isEmpty() && validatorExternalSignerPublicKeySources.isEmpty())) { throw new InvalidConfigurationException( - "Invalid configuration. --Xvalidators-suggested-fee-recipient-address must be specified when Bellatrix milestone is active"); + "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active"); } } @@ -194,7 +194,7 @@ public static final class Builder { DEFAULT_VALIDATOR_EXTERNAL_SIGNER_SLASHING_PROTECTION_ENABLED; private boolean useDependentRoots = DEFAULT_USE_DEPENDENT_ROOTS; private boolean generateEarlyAttestations = DEFAULT_GENERATE_EARLY_ATTESTATIONS; - private Optional suggestedFeeRecipient = Optional.empty(); + private Optional proposerDefaultFeeRecipient = Optional.empty(); private Builder() {} @@ -289,16 +289,17 @@ public Builder generateEarlyAttestations(final boolean generateEarlyAttestations return this; } - public Builder suggestedFeeRecipient(final Eth1Address suggestedFeeRecipient) { - this.suggestedFeeRecipient = Optional.ofNullable(suggestedFeeRecipient); + public Builder proposerDefaultFeeRecipient(final Eth1Address proposerDefaultFeeRecipient) { + this.proposerDefaultFeeRecipient = Optional.ofNullable(proposerDefaultFeeRecipient); return this; } - public Builder suggestedFeeRecipient(final String suggestedFeeRecipient) { - if (suggestedFeeRecipient == null) { - this.suggestedFeeRecipient = Optional.empty(); + public Builder proposerDefaultFeeRecipient(final String proposerDefaultFeeRecipient) { + if (proposerDefaultFeeRecipient == null) { + this.proposerDefaultFeeRecipient = Optional.empty(); } else { - this.suggestedFeeRecipient = Optional.of(Eth1Address.fromHexString(suggestedFeeRecipient)); + this.proposerDefaultFeeRecipient = + Optional.of(Eth1Address.fromHexString(proposerDefaultFeeRecipient)); } return this; } @@ -325,7 +326,7 @@ public ValidatorConfig build() { validatorExternalSignerConcurrentRequestLimit, useDependentRoots, generateEarlyAttestations, - suggestedFeeRecipient); + proposerDefaultFeeRecipient); } private void validateExternalSignerUrlAndPublicKeys() { diff --git a/validator/api/src/test/java/tech/pegasys/teku/validator/api/ValidatorConfigTest.java b/validator/api/src/test/java/tech/pegasys/teku/validator/api/ValidatorConfigTest.java index 69f3e20a9d2..364fd8166a8 100644 --- a/validator/api/src/test/java/tech/pegasys/teku/validator/api/ValidatorConfigTest.java +++ b/validator/api/src/test/java/tech/pegasys/teku/validator/api/ValidatorConfigTest.java @@ -130,9 +130,9 @@ public void bellatrix_shouldThrowExceptionIfExternalSignerPublicKeySourcesIsSpec .build(); Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) - .isThrownBy(config::getSuggestedFeeRecipient) + .isThrownBy(config::getProposerDefaultFeeRecipient) .withMessageContaining( - "Invalid configuration. --Xvalidators-suggested-fee-recipient-address must be specified when Bellatrix milestone is active"); + "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active"); } @Test @@ -140,9 +140,9 @@ public void bellatrix_shouldThrowExceptionIfValidatorKeysAreSpecified() { final ValidatorConfig config = configBuilder.validatorKeys(List.of("some string")).build(); Assertions.assertThatExceptionOfType(InvalidConfigurationException.class) - .isThrownBy(config::getSuggestedFeeRecipient) + .isThrownBy(config::getProposerDefaultFeeRecipient) .withMessageContaining( - "Invalid configuration. --Xvalidators-suggested-fee-recipient-address must be specified when Bellatrix milestone is active"); + "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active"); } @Test @@ -153,10 +153,10 @@ public void bellatrix_noExceptionThrownIfIfExternalSignerPublicKeySourcesIsSpeci .validatorExternalSignerPublicKeySources( List.of(BLSTestUtil.randomKeyPair(0).getPublicKey().toString())) .validatorExternalSignerUrl(URI.create("http://localhost:9000").toURL()) - .suggestedFeeRecipient("0x0000000000000000000000000000000000000000") + .proposerDefaultFeeRecipient("0x0000000000000000000000000000000000000000") .build(); - Assertions.assertThatCode(config::getSuggestedFeeRecipient).doesNotThrowAnyException(); + Assertions.assertThatCode(config::getProposerDefaultFeeRecipient).doesNotThrowAnyException(); } @Test @@ -164,10 +164,10 @@ public void bellatrix_noExceptionThrownIfIfValidatorKeysAreSpecified() { final ValidatorConfig config = configBuilder .validatorKeys(List.of("some string")) - .suggestedFeeRecipient( + .proposerDefaultFeeRecipient( Eth1Address.fromHexString("0x0000000000000000000000000000000000000000")) .build(); - Assertions.assertThatCode(config::getSuggestedFeeRecipient).doesNotThrowAnyException(); + Assertions.assertThatCode(config::getProposerDefaultFeeRecipient).doesNotThrowAnyException(); } } diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java index 54f2958ca6d..da4b623842a 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/BeaconProposerPreparer.java @@ -19,9 +19,9 @@ import java.util.stream.Collectors; import org.apache.tuweni.bytes.Bytes32; import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException; +import tech.pegasys.teku.infrastructure.ssz.type.Bytes20; import tech.pegasys.teku.infrastructure.unsigned.UInt64; import tech.pegasys.teku.spec.Spec; -import tech.pegasys.teku.spec.datastructures.eth1.Eth1Address; import tech.pegasys.teku.spec.datastructures.operations.versions.bellatrix.BeaconPreparableProposer; import tech.pegasys.teku.validator.api.ValidatorApiChannel; import tech.pegasys.teku.validator.api.ValidatorTimingChannel; @@ -32,13 +32,13 @@ public class BeaconProposerPreparer implements ValidatorTimingChannel { private final ValidatorIndexProvider validatorIndexProvider; private final OwnedValidators validators; private final Spec spec; - private final Optional feeRecipient; + private final Optional feeRecipient; private boolean firstCallDone = false; public BeaconProposerPreparer( ValidatorApiChannel validatorApiChannel, ValidatorIndexProvider validatorIndexProvider, - Optional feeRecipient, + Optional feeRecipient, OwnedValidators validators, Spec spec) { this.validatorApiChannel = validatorApiChannel; @@ -67,11 +67,11 @@ public void onSlot(UInt64 slot) { } } - private Eth1Address getFeeRecipient() { + private Bytes20 getFeeRecipient() { return feeRecipient.orElseThrow( () -> new InvalidConfigurationException( - "Invalid configuration. --Xvalidators-suggested-fee-recipient-address must be specified when Bellatrix milestone is active")); + "Invalid configuration. --Xvalidators-proposer-default-fee-recipient must be specified when Bellatrix milestone is active")); } @Override diff --git a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java index 7e41dbe5c90..fd4b0dcb7ce 100644 --- a/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java +++ b/validator/client/src/main/java/tech/pegasys/teku/validator/client/ValidatorClientService.java @@ -238,7 +238,7 @@ private void initializeValidators( new BeaconProposerPreparer( validatorApiChannel, validatorIndexProvider, - config.getValidatorConfig().getSuggestedFeeRecipient(), + config.getValidatorConfig().getProposerDefaultFeeRecipient(), validators, spec)); }