From 7a4abc6f913bc16ee9e1fe1cdc85b8047f5204fe Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Tue, 14 Feb 2023 13:04:21 +1300 Subject: [PATCH] Handling BlsToExecutionChange operations on reorgs (#6809) --- .../OperationsReOrgManager.java | 9 +++++++++ .../OperationsReOrgManagerTest.java | 18 +++++++++++++++++- .../beaconchain/BeaconChainController.java | 4 ++-- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/OperationsReOrgManager.java b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/OperationsReOrgManager.java index e1a02a0ef49..8b3d08d615d 100644 --- a/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/OperationsReOrgManager.java +++ b/ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/OperationsReOrgManager.java @@ -42,6 +42,7 @@ public class OperationsReOrgManager implements ChainHeadChannel { private final OperationPool attesterSlashingPool; private final AttestationManager attestationManager; private final AggregatingAttestationPool attestationPool; + private final BlsToExecutionOperationPool blsToExecutionOperationPool; private final RecentChainData recentChainData; public OperationsReOrgManager( @@ -50,12 +51,14 @@ public OperationsReOrgManager( final OperationPool exitPool, final AggregatingAttestationPool attestationPool, final AttestationManager attestationManager, + final BlsToExecutionOperationPool blsToExecutionOperationPool, final RecentChainData recentChainData) { this.exitPool = exitPool; this.proposerSlashingPool = proposerSlashingPool; this.attesterSlashingPool = attesterSlashingPool; this.attestationManager = attestationManager; this.attestationPool = attestationPool; + this.blsToExecutionOperationPool = blsToExecutionOperationPool; this.recentChainData = recentChainData; } @@ -101,6 +104,9 @@ private void processNonCanonicalBlockOperations( proposerSlashingPool.addAll(blockBody.getProposerSlashings()); attesterSlashingPool.addAll(blockBody.getAttesterSlashings()); exitPool.addAll(blockBody.getVoluntaryExits()); + blockBody + .getOptionalBlsToExecutionChanges() + .ifPresent(blsToExecutionOperationPool::addAll); processNonCanonicalBlockAttestations(blockBody.getAttestations(), root); }, @@ -160,6 +166,9 @@ private void processCanonicalBlockOperations(final Collection canonical exitPool.removeAll(blockBody.getVoluntaryExits()); attestationPool.onAttestationsIncludedInBlock( block.getSlot(), blockBody.getAttestations()); + blockBody + .getOptionalBlsToExecutionChanges() + .ifPresent(blsToExecutionOperationPool::removeAll); }, () -> LOG.debug( diff --git a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/OperationsReOrgManagerTest.java b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/OperationsReOrgManagerTest.java index 5787b16aaf4..7617f64e58c 100644 --- a/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/OperationsReOrgManagerTest.java +++ b/ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/OperationsReOrgManagerTest.java @@ -48,7 +48,7 @@ @SuppressWarnings("unchecked") public class OperationsReOrgManagerTest { - private final Spec spec = TestSpecFactory.createDefault(); + private final Spec spec = TestSpecFactory.createMinimalCapella(); private final DataStructureUtil dataStructureUtil = new DataStructureUtil(spec); private final OperationPool proposerSlashingOperationPool = @@ -59,6 +59,8 @@ public class OperationsReOrgManagerTest { mock(SimpleOperationPool.class); private final AggregatingAttestationPool attestationPool = mock(AggregatingAttestationPool.class); private final AttestationManager attestationManager = mock(AttestationManager.class); + private final BlsToExecutionOperationPool blsToExecutionOperationPool = + mock(BlsToExecutionOperationPool.class); private final RecentChainData recentChainData = mock(RecentChainData.class); @@ -69,6 +71,7 @@ public class OperationsReOrgManagerTest { exitOperationPool, attestationPool, attestationManager, + blsToExecutionOperationPool, recentChainData); @Test @@ -127,10 +130,14 @@ void shouldRequeueAndRemoveOperations() { verify(proposerSlashingOperationPool).addAll(fork1Block1.getBody().getProposerSlashings()); verify(attesterSlashingOperationPool).addAll(fork1Block1.getBody().getAttesterSlashings()); verify(exitOperationPool).addAll(fork1Block1.getBody().getVoluntaryExits()); + verify(blsToExecutionOperationPool) + .addAll(fork1Block1.getBody().getOptionalBlsToExecutionChanges().orElseThrow()); verify(proposerSlashingOperationPool).addAll(fork1Block2.getBody().getProposerSlashings()); verify(attesterSlashingOperationPool).addAll(fork1Block2.getBody().getAttesterSlashings()); verify(exitOperationPool).addAll(fork1Block2.getBody().getVoluntaryExits()); + verify(blsToExecutionOperationPool) + .addAll(fork1Block2.getBody().getOptionalBlsToExecutionChanges().orElseThrow()); ArgumentCaptor argument = ArgumentCaptor.forClass(ValidateableAttestation.class); @@ -155,6 +162,8 @@ void shouldRequeueAndRemoveOperations() { verify(attestationPool) .onAttestationsIncludedInBlock( fork2Block1.getSlot(), fork2Block1.getBody().getAttestations()); + verify(blsToExecutionOperationPool) + .removeAll(fork2Block1.getBody().getOptionalBlsToExecutionChanges().orElseThrow()); verify(proposerSlashingOperationPool).removeAll(fork2Block2.getBody().getProposerSlashings()); verify(attesterSlashingOperationPool).removeAll(fork2Block2.getBody().getAttesterSlashings()); @@ -162,6 +171,8 @@ void shouldRequeueAndRemoveOperations() { verify(attestationPool) .onAttestationsIncludedInBlock( fork2Block2.getSlot(), fork2Block2.getBody().getAttestations()); + verify(blsToExecutionOperationPool) + .removeAll(fork2Block2.getBody().getOptionalBlsToExecutionChanges().orElseThrow()); } @Test @@ -206,17 +217,22 @@ void shouldOnlyRemoveOperations() { verify(proposerSlashingOperationPool, never()).addAll(any()); verify(attesterSlashingOperationPool, never()).addAll(any()); verify(attestationManager, never()).onAttestation(any()); + verify(blsToExecutionOperationPool, never()).addAll(any()); verify(proposerSlashingOperationPool).removeAll(block2.getBody().getProposerSlashings()); verify(attesterSlashingOperationPool).removeAll(block2.getBody().getAttesterSlashings()); verify(exitOperationPool).removeAll(block2.getBody().getVoluntaryExits()); verify(attestationPool) .onAttestationsIncludedInBlock(block2.getSlot(), block2.getBody().getAttestations()); + verify(blsToExecutionOperationPool) + .removeAll(block2.getBody().getOptionalBlsToExecutionChanges().orElseThrow()); verify(proposerSlashingOperationPool).removeAll(block1.getBody().getProposerSlashings()); verify(attesterSlashingOperationPool).removeAll(block1.getBody().getAttesterSlashings()); verify(exitOperationPool).removeAll(block1.getBody().getVoluntaryExits()); verify(attestationPool) .onAttestationsIncludedInBlock(block1.getSlot(), block1.getBody().getAttestations()); + verify(blsToExecutionOperationPool) + .removeAll(block1.getBody().getOptionalBlsToExecutionChanges().orElseThrow()); } } 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 20deccf9d61..ea0772a1fc0 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 @@ -87,7 +87,6 @@ import tech.pegasys.teku.spec.datastructures.interop.GenesisStateBuilder; import tech.pegasys.teku.spec.datastructures.operations.AttesterSlashing; import tech.pegasys.teku.spec.datastructures.operations.ProposerSlashing; -import tech.pegasys.teku.spec.datastructures.operations.SignedBlsToExecutionChange; import tech.pegasys.teku.spec.datastructures.operations.SignedVoluntaryExit; import tech.pegasys.teku.spec.datastructures.state.AnchorPoint; import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; @@ -219,7 +218,7 @@ public class BeaconChainController extends Service implements BeaconChainControl protected volatile OperationPool attesterSlashingPool; protected volatile OperationPool proposerSlashingPool; protected volatile OperationPool voluntaryExitPool; - protected volatile OperationPool blsToExecutionChangePool; + protected volatile BlsToExecutionOperationPool blsToExecutionChangePool; protected volatile SyncCommitteeContributionPool syncCommitteeContributionPool; protected volatile SyncCommitteeMessagePool syncCommitteeMessagePool; protected volatile WeakSubjectivityValidator weakSubjectivityValidator; @@ -1041,6 +1040,7 @@ protected void initOperationsReOrgManager() { voluntaryExitPool, attestationPool, attestationManager, + blsToExecutionChangePool, recentChainData); eventChannels.subscribe(ChainHeadChannel.class, operationsReOrgManager); }