Handling BlsToExecutionChange operations on reorgs (#6809)

Consensys · Feb 14, 2023 · 7a4abc6 · 7a4abc6
1 parent 385eaa8
commit 7a4abc6
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 3 deletions.
diff --git a/...atetransition/src/main/java/tech/pegasys/teku/statetransition/OperationsReOrgManager.java b/...atetransition/src/main/java/tech/pegasys/teku/statetransition/OperationsReOrgManager.java
@@ -42,6 +42,7 @@ public class OperationsReOrgManager implements ChainHeadChannel {
   private final OperationPool<AttesterSlashing> 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<SignedVoluntaryExit> 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<Bytes32> canonical
                             exitPool.removeAll(blockBody.getVoluntaryExits());
                             attestationPool.onAttestationsIncludedInBlock(
                                 block.getSlot(), blockBody.getAttestations());
+                            blockBody
+                                .getOptionalBlsToExecutionChanges()
+                                .ifPresent(blsToExecutionOperationPool::removeAll);
                           },
                           () ->
                               LOG.debug(

diff --git a/...ransition/src/test/java/tech/pegasys/teku/statetransition/OperationsReOrgManagerTest.java b/...ransition/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<ProposerSlashing> 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<ValidateableAttestation> argument =
         ArgumentCaptor.forClass(ValidateableAttestation.class);
@@ -155,13 +162,17 @@ 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());
     verify(exitOperationPool).removeAll(fork2Block2.getBody().getVoluntaryExits());
     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/...aconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java b/...aconchain/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<AttesterSlashing> attesterSlashingPool;
   protected volatile OperationPool<ProposerSlashing> proposerSlashingPool;
   protected volatile OperationPool<SignedVoluntaryExit> voluntaryExitPool;
-  protected volatile OperationPool<SignedBlsToExecutionChange> 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);
   }