DepositWithIndex: replace inheritance with composition (#8249)

* Make an explicit composition in DepositWithIndex instead of inheritance
* Make explicit methods to get either SszList<Deposit> SSZ structure or plain List<DepositWithIndex> from DepositProvider
* Remove <? extends Deposit> across the code

beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/DepositProvider.java

@@ -103,13 +103,13 @@ public synchronized void onDepositsFromBlock(final DepositsFromBlockEvent event)
     event.getDeposits().stream()
         .map(depositUtil::convertDepositEventToOperationDeposit)
         .forEach(
-            deposit -> {
+            depositWithIndex -> {
               if (!recentChainData.isPreGenesis()) {
-                LOG.debug("About to process deposit: {}", deposit.getIndex());
+                LOG.debug("About to process deposit: {}", depositWithIndex.index());
               }
 
-              depositNavigableMap.put(deposit.getIndex(), deposit);
-              depositMerkleTree.pushLeaf(deposit.getData().hashTreeRoot());
+              depositNavigableMap.put(depositWithIndex.index(), depositWithIndex);
+              depositMerkleTree.pushLeaf(depositWithIndex.deposit().getData().hashTreeRoot());
             });
     depositCounter.inc(event.getDeposits().size());
     eth1DataCache.onBlockWithDeposit(
@@ -211,9 +211,17 @@ public synchronized SszList<Deposit> getDeposits(
       final BeaconState state, final Eth1Data eth1Data) {
     final long maxDeposits = spec.getMaxDeposits(state);
     final SszListSchema<Deposit, ?> depositsSchema = depositsSchemaCache.get(maxDeposits);
+    return getDepositsWithIndex(state, eth1Data).stream()
+        .map(DepositWithIndex::deposit)
+        .collect(depositsSchema.collector());
+  }
+
+  public synchronized List<DepositWithIndex> getDepositsWithIndex(
+      final BeaconState state, final Eth1Data eth1Data) {
+    final long maxDeposits = spec.getMaxDeposits(state);
     // no Eth1 deposits needed if already transitioned to the EIP-6110 mechanism
     if (spec.isFormerDepositMechanismDisabled(state)) {
-      return depositsSchema.createFromElements(emptyList());
+      return emptyList();
     }
     final UInt64 eth1DepositCount;
     if (spec.isEnoughVotesToUpdateEth1Data(state, eth1Data, 1)) {
@@ -241,7 +249,7 @@ public synchronized SszList<Deposit> getDeposits(
 
     // No deposits to include
     if (eth1PendingDepositCount.isZero()) {
-      return depositsSchema.createFromElements(emptyList());
+      return emptyList();
     }
 
     // We need to have all the deposits that can be included in the state available to ensure
@@ -250,7 +258,7 @@ public synchronized SszList<Deposit> getDeposits(
 
     final UInt64 toDepositIndex = eth1DepositIndex.plus(eth1PendingDepositCount);
 
-    return getDepositsWithProof(eth1DepositIndex, toDepositIndex, eth1DepositCount, depositsSchema);
+    return getDepositsWithProof(eth1DepositIndex, toDepositIndex, eth1DepositCount);
   }
 
   protected synchronized List<DepositWithIndex> getAvailableDeposits() {
@@ -283,11 +291,8 @@ public synchronized int getDepositMapSize() {
    * @param toDepositIndex exclusive
    * @param eth1DepositCount number of deposits in the merkle tree according to Eth1Data in state
    */
-  private SszList<Deposit> getDepositsWithProof(
-      final UInt64 fromDepositIndex,
-      final UInt64 toDepositIndex,
-      final UInt64 eth1DepositCount,
-      final SszListSchema<Deposit, ?> depositsSchema) {
+  private List<DepositWithIndex> getDepositsWithProof(
+      final UInt64 fromDepositIndex, final UInt64 toDepositIndex, final UInt64 eth1DepositCount) {
     final AtomicReference<UInt64> expectedDepositIndex = new AtomicReference<>(fromDepositIndex);
     final SszBytes32VectorSchema<?> depositProofSchema = Deposit.SSZ_SCHEMA.getProofSchema();
     if (depositMerkleTree.getDepositCount() < eth1DepositCount.intValue()) {
@@ -301,17 +306,19 @@ private SszList<Deposit> getDepositsWithProof(
         .values()
         .stream()
         .map(
-            deposit -> {
-              if (!deposit.getIndex().equals(expectedDepositIndex.get())) {
+            depositWithIndex -> {
+              if (!depositWithIndex.index().equals(expectedDepositIndex.get())) {
                 throw MissingDepositsException.missingRange(
-                    expectedDepositIndex.get(), deposit.getIndex());
+                    expectedDepositIndex.get(), depositWithIndex.index());
               }
-              expectedDepositIndex.set(deposit.getIndex().plus(ONE));
+              expectedDepositIndex.set(depositWithIndex.index().plus(ONE));
               SszBytes32Vector proof =
-                  depositProofSchema.of(merkleTree.getProof(deposit.getIndex().intValue()));
-              return new DepositWithIndex(proof, deposit.getData(), deposit.getIndex());
+                  depositProofSchema.of(merkleTree.getProof(depositWithIndex.index().intValue()));
+              return new DepositWithIndex(
+                  new Deposit(proof, depositWithIndex.deposit().getData()),
+                  depositWithIndex.index());
             })
-        .collect(depositsSchema.collector());
+        .toList();
   }
 
   @Override

beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/DepositProviderTest.java

@@ -46,7 +46,6 @@
 import tech.pegasys.teku.spec.config.SpecConfig;
 import tech.pegasys.teku.spec.datastructures.blocks.Eth1Data;
 import tech.pegasys.teku.spec.datastructures.operations.Deposit;
-import tech.pegasys.teku.spec.datastructures.operations.DepositData;
 import tech.pegasys.teku.spec.datastructures.operations.DepositWithIndex;
 import tech.pegasys.teku.spec.datastructures.state.AnchorPoint;
 import tech.pegasys.teku.spec.datastructures.state.Checkpoint;
@@ -130,7 +129,7 @@ void numberOfDepositsThatCanBeIncludedLessThanMaxDeposits() {
     mockDepositsFromEth1Block(0, 10);
     mockDepositsFromEth1Block(10, 20);
 
-    SszList<Deposit> deposits = depositProvider.getDeposits(state, randomEth1Data);
+    List<DepositWithIndex> deposits = depositProvider.getDepositsWithIndex(state, randomEth1Data);
     assertThat(deposits).hasSize(15);
     checkThatDepositProofIsValid(deposits);
   }
@@ -155,9 +154,11 @@ void numberOfDepositsGetsAdjustedAccordingToOurEth1DataVote() {
             .collect(SszListSchema.create(Eth1Data.SSZ_SCHEMA, 32).collector());
     state = state.updated(mutableState -> mutableState.setEth1DataVotes(et1hDataVotes));
 
-    SszList<Deposit> deposits = depositProvider.getDeposits(state, newEth1Data);
+    List<DepositWithIndex> deposits = depositProvider.getDepositsWithIndex(state, newEth1Data);
     assertThat(deposits).hasSize(25);
     checkThatDepositProofIsValid(deposits);
+    assertThat(deposits.stream().map(DepositWithIndex::deposit))
+        .containsExactlyElementsOf(depositProvider.getDeposits(state, newEth1Data));
   }
 
   @Test
@@ -169,9 +170,11 @@ void numberOfDepositsThatCanBeIncludedMoreThanMaxDeposits() {
     mockDepositsFromEth1Block(0, 10);
     mockDepositsFromEth1Block(10, 20);
 
-    SszList<Deposit> deposits = depositProvider.getDeposits(state, randomEth1Data);
+    List<DepositWithIndex> deposits = depositProvider.getDepositsWithIndex(state, randomEth1Data);
     assertThat(deposits).hasSize(10);
     checkThatDepositProofIsValid(deposits);
+    assertThat(deposits.stream().map(DepositWithIndex::deposit))
+        .containsExactlyElementsOf(depositProvider.getDeposits(state, randomEth1Data));
   }
 
   @Test
@@ -196,13 +199,16 @@ void getsRemainingEth1PendingDepositsIfElectraIsEnabled() {
     mockDepositsFromEth1Block(0, 10);
     mockDepositsFromEth1Block(10, 20);
 
-    final SszList<Deposit> deposits = depositProvider.getDeposits(state, randomEth1Data);
+    final List<DepositWithIndex> deposits =
+        depositProvider.getDepositsWithIndex(state, randomEth1Data);
 
     // the pending Eth1 deposits (deposit_receipt_start_index - eth1_deposit_index)
     // we need to process eth1_deposit_index deposit (5) up to 16 (exclusive) so 11 is the
     // expected size
     assertThat(deposits).hasSize(11);
     checkThatDepositProofIsValid(deposits);
+    assertThat(deposits.stream().map(DepositWithIndex::deposit))
+        .containsExactlyElementsOf(depositProvider.getDeposits(state, randomEth1Data));
   }
 
   @Test
@@ -249,7 +255,11 @@ void shouldNotifyEth1DataCacheOfDepositBlocks() {
     depositProvider.onDepositsFromBlock(event);
 
     depositMerkleTree.add(
-        depositUtil.convertDepositEventToOperationDeposit(deposit).getData().hashTreeRoot());
+        depositUtil
+            .convertDepositEventToOperationDeposit(deposit)
+            .deposit()
+            .getData()
+            .hashTreeRoot());
     verify(eth1DataCache)
         .onBlockWithDeposit(
             event.getBlockNumber(),
@@ -482,18 +492,18 @@ void whenCallingForFinalizedSnapshotAndSnapshotAvailable_SnapshotReturned() {
         .isEqualTo(UInt64.valueOf(30));
   }
 
-  private void checkThatDepositProofIsValid(SszList<Deposit> deposits) {
+  private void checkThatDepositProofIsValid(List<DepositWithIndex> depositsWithIndex) {
     final SpecVersion genesisSpec = spec.getGenesisSpec();
-    deposits.forEach(
-        deposit ->
+    depositsWithIndex.forEach(
+        depositWithIndex ->
             assertThat(
                     genesisSpec
                         .predicates()
                         .isValidMerkleBranch(
-                            deposit.getData().hashTreeRoot(),
-                            deposit.getProof(),
+                            depositWithIndex.deposit().getData().hashTreeRoot(),
+                            depositWithIndex.deposit().getProof(),
                             genesisSpec.getConfig().getDepositContractTreeDepth() + 1,
-                            ((DepositWithIndex) deposit).getIndex().intValue(),
+                            depositWithIndex.index().intValue(),
                             depositMerkleTree.getRoot()))
                 .withFailMessage("Expected proof to be valid but was not")
                 .isTrue());
@@ -509,8 +519,7 @@ private void createDepositEvents(int n) {
   private void mockDepositsFromEth1Block(int startIndex, int n) {
     allSeenDepositsList.subList(startIndex, n).stream()
         .map(depositUtil::convertDepositEventToOperationDeposit)
-        .map(Deposit::getData)
-        .map(DepositData::hashTreeRoot)
+        .map(depositWithIndex -> depositWithIndex.deposit().getData().hashTreeRoot())
         .forEachOrdered(depositMerkleTree::add);
 
     DepositsFromBlockEvent depositsFromBlockEvent = mock(DepositsFromBlockEvent.class);

data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/tekuv1/beacon/GetDeposits.java

@@ -35,9 +35,11 @@ public class GetDeposits extends RestApiEndpoint {
 
   private static final SerializableTypeDefinition<DepositWithIndex> DEPOSIT_WITH_INDEX_TYPE =
       SerializableTypeDefinition.object(DepositWithIndex.class)
-          .withField("index", CoreTypes.UINT64_TYPE, DepositWithIndex::getIndex)
+          .withField("index", CoreTypes.UINT64_TYPE, DepositWithIndex::index)
           .withField(
-              "data", DepositData.SSZ_SCHEMA.getJsonTypeDefinition(), DepositWithIndex::getData)
+              "data",
+              DepositData.SSZ_SCHEMA.getJsonTypeDefinition(),
+              depositWithIndex -> depositWithIndex.deposit().getData())
           .build();
 
   public static final SerializableTypeDefinition<List<DepositWithIndex>> DEPOSITS_RESPONSE_TYPE =

ethereum/spec/src/main/java/tech/pegasys/teku/spec/Spec.java

@@ -295,7 +295,7 @@ public int getSyncCommitteeSize(final UInt64 slot) {
   public BeaconState initializeBeaconStateFromEth1(
       final Bytes32 eth1BlockHash,
       final UInt64 eth1Timestamp,
-      final List<? extends Deposit> deposits,
+      final List<Deposit> deposits,
       final Optional<ExecutionPayloadHeader> payloadHeader) {
     final GenesisGenerator genesisGenerator = createGenesisGenerator();
     genesisGenerator.updateCandidateState(eth1BlockHash, eth1Timestamp, deposits);

ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/operations/DepositWithIndex.java

@@ -13,53 +13,15 @@
 
 package tech.pegasys.teku.spec.datastructures.operations;
 
-import java.util.Objects;
 import org.jetbrains.annotations.NotNull;
-import tech.pegasys.teku.infrastructure.ssz.collections.SszBytes32Vector;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 
-public class DepositWithIndex extends Deposit implements Comparable<DepositWithIndex> {
-
-  private final UInt64 index;
-
-  public DepositWithIndex(
-      final SszBytes32Vector proof, final DepositData data, final UInt64 index) {
-    super(proof, data);
-    this.index = index;
-  }
-
-  public DepositWithIndex(final DepositData data, final UInt64 index) {
-    super(data);
-    this.index = index;
-  }
-
-  public UInt64 getIndex() {
-    return index;
-  }
+public record DepositWithIndex(Deposit deposit, UInt64 index)
+    implements Comparable<DepositWithIndex> {
 
   @Override
   public int compareTo(final @NotNull DepositWithIndex o) {
-    return this.getIndex().compareTo(o.getIndex());
-  }
-
-  @Override
-  public boolean equals(final Object o) {
-    if (this == o) {
-      return true;
-    }
-    if (o == null || getClass() != o.getClass()) {
-      return false;
-    }
-    if (!super.equals(o)) {
-      return false;
-    }
-    final DepositWithIndex that = (DepositWithIndex) o;
-    return index.equals(that.index);
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(super.hashCode(), index);
+    return this.index().compareTo(o.index());
   }
 
   @Override

ethereum/spec/src/main/java/tech/pegasys/teku/spec/datastructures/util/DepositUtil.java

@@ -37,6 +37,8 @@ public DepositWithIndex convertDepositEventToOperationDeposit(final Deposit even
             event.getWithdrawal_credentials(),
             event.getAmount(),
             event.getSignature());
-    return new DepositWithIndex(data, event.getMerkle_tree_index());
+    return new DepositWithIndex(
+        new tech.pegasys.teku.spec.datastructures.operations.Deposit(data),
+        event.getMerkle_tree_index());
   }
 }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/genesis/GenesisGenerator.java

@@ -79,9 +79,7 @@ public void updateExecutionPayloadHeader(final ExecutionPayloadHeader payloadHea
   }
 
   public void updateCandidateState(
-      final Bytes32 eth1BlockHash,
-      final UInt64 eth1Timestamp,
-      final List<? extends Deposit> deposits) {
+      final Bytes32 eth1BlockHash, final UInt64 eth1Timestamp, final List<Deposit> deposits) {
     updateGenesisTime(eth1Timestamp);
 
     state.setEth1Data(

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/block/AbstractBlockProcessor.java

@@ -655,8 +655,7 @@ protected BlockValidationResult verifyAttestationSignatures(
   }
 
   @Override
-  public void processDeposits(
-      final MutableBeaconState state, final SszList<? extends Deposit> deposits)
+  public void processDeposits(final MutableBeaconState state, final SszList<Deposit> deposits)
       throws BlockProcessingException {
     safelyProcess(
         () -> {
@@ -667,7 +666,7 @@ public void processDeposits(
         });
   }
 
-  private boolean batchVerifyDepositSignatures(final SszList<? extends Deposit> deposits) {
+  private boolean batchVerifyDepositSignatures(final SszList<Deposit> deposits) {
     try {
       final List<List<BLSPublicKey>> publicKeys = new ArrayList<>();
       final List<Bytes> messages = new ArrayList<>();

ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/block/BlockProcessor.java

@@ -119,7 +119,7 @@ void processAttestations(
       BLSSignatureVerifier signatureVerifier)
       throws BlockProcessingException;
 
-  void processDeposits(MutableBeaconState state, SszList<? extends Deposit> deposits)
+  void processDeposits(MutableBeaconState state, SszList<Deposit> deposits)
       throws BlockProcessingException;
 
   void processDepositWithoutCheckingMerkleProof(