fix: spec tests

packages/ssz/test/lodestarTypes/phase0/viewDU/listValidator.ts

@@ -57,14 +57,16 @@ export class ListValidatorTreeViewDU extends ListCompositeTreeViewDU<ValidatorNo
     }
 
     // TODO - batch: remove this type cast
-    const viewsChanged = Array.from(this.viewsChanged.values()) as ValidatorTreeViewDU[];
-    const endBatch = viewsChanged.length - (viewsChanged.length % PARALLEL_FACTOR);
+    const indicesChanged = Array.from(this.viewsChanged.keys()).sort((a, b) => a - b);
+    const endBatch = indicesChanged.length - (indicesChanged.length % PARALLEL_FACTOR);
     // nodesChanged is sorted by index
     const nodesChanged: {index: number; node: Node}[] = [];
     // commit every 16 validators in batch
     for (let i = 0; i < endBatch; i++) {
       const indexInBatch = i % PARALLEL_FACTOR;
-      viewsChanged[i].valueToMerkleBytes(this.level3ByteViewsArr[indexInBatch], this.level4BytesArr[indexInBatch]);
+      const viewIndex = indicesChanged[i];
+      const viewChanged = this.viewsChanged.get(viewIndex) as ValidatorTreeViewDU;
+      viewChanged.valueToMerkleBytes(this.level3ByteViewsArr[indexInBatch], this.level4BytesArr[indexInBatch]);
 
       if (indexInBatch === PARALLEL_FACTOR - 1) {
         // hash level 4
@@ -81,19 +83,24 @@ export class ListValidatorTreeViewDU extends ListCompositeTreeViewDU<ValidatorNo
         }
         // commit all validators in this batch
         for (let j = PARALLEL_FACTOR - 1; j >= 0; j--) {
-          const viewIndex = i - j;
-          const indexInBatch = viewIndex % PARALLEL_FACTOR;
+          const viewIndex = indicesChanged[i - j];
+          const indexInBatch = (i - j) % PARALLEL_FACTOR;
           const hashObject = byteArrayToHashObject(validatorRoots.subarray(indexInBatch * 32, (indexInBatch + 1) * 32));
-          viewsChanged[viewIndex].commitToHashObject(hashObject);
-          nodesChanged.push({index: viewIndex, node: viewsChanged[viewIndex].node});
+          const viewChanged = this.viewsChanged.get(viewIndex) as ValidatorTreeViewDU;
+          viewChanged.commitToHashObject(hashObject);
+          nodesChanged.push({index: viewIndex, node: viewChanged.node});
+          // Set new node in nodes array to ensure data represented in the tree and fast nodes access is equal
+          this.nodes[viewIndex] = viewChanged.node;
         }
       }
     }
 
     // commit the remaining validators, we can do in batch too but don't want to create new Uint8Array views
     // it's not much different to commit one by one
-    for (let i = endBatch; i < viewsChanged.length; i++) {
-      viewsChanged[i].valueToMerkleBytes(this.singleLevel3ByteView, this.singleLevel4Bytes);
+    for (let i = endBatch; i < indicesChanged.length; i++) {
+      const viewIndex = indicesChanged[i];
+      const viewChanged = this.viewsChanged.get(viewIndex) as ValidatorTreeViewDU;
+      viewChanged.valueToMerkleBytes(this.singleLevel3ByteView, this.singleLevel4Bytes);
       // level 4 hash
       const pubkeyRoot = digestNLevelUnsafe(this.singleLevel4Bytes, 1);
       if (pubkeyRoot.length !== 32) {
@@ -106,8 +113,10 @@ export class ListValidatorTreeViewDU extends ListCompositeTreeViewDU<ValidatorNo
         throw new Error(`Invalid validatorRoot length, expect 32, got ${validatorRoot.length}`);
       }
       const hashObject = byteArrayToHashObject(validatorRoot);
-      viewsChanged[i].commitToHashObject(hashObject);
-      nodesChanged.push({index: i, node: viewsChanged[i].node});
+      viewChanged.commitToHashObject(hashObject);
+      nodesChanged.push({index: viewIndex, node: viewChanged.node});
+      // Set new node in nodes array to ensure data represented in the tree and fast nodes access is equal
+      this.nodes[viewIndex] = viewChanged.node;
     }
 
     // do the remaining commit step the same to parent (ArrayCompositeTreeViewDU)

packages/ssz/test/lodestarTypes/phase0/viewDU/validator.ts

@@ -201,12 +201,9 @@ export class ValidatorTreeViewDU extends TreeViewDU<ContainerTypeGeneric<typeof
    * The HashObject is computed by parent so that we don't need to create a tree from scratch.
    */
   commitToHashObject(ho: HashObject): void {
-    if (this.valueChanged === null) {
-      return;
-    }
-
     const oldRoot = this._rootNode;
-    const value = this.valueChanged;
+    // in case pushing a new validator to array, valueChanged could be null
+    const value = this.valueChanged ?? this._rootNode.value;
     this._rootNode = new BranchNodeStruct(oldRoot["valueToNode"], value);
     this._rootNode.applyHash(ho);
     this.valueChanged = null;

packages/ssz/test/unit/lodestarTypes/phase0/listValidator.test.ts

@@ -1,10 +1,12 @@
 import { ListCompositeType } from "../../../../src/type/listComposite";
-import { ValidatorNodeStruct } from "../../../lodestarTypes/phase0/validator";
+import { ValidatorType } from "../../../lodestarTypes/phase0/validator";
 import {
   preset,
 } from "../../../lodestarTypes/params";
 import { ssz } from "../../../lodestarTypes";
 import { expect } from "chai";
+import { ContainerType } from "../../../../src/type/container";
+import { Validator } from "../../../lodestarTypes/phase0";
 const {VALIDATOR_REGISTRY_LIMIT} = preset;
 
 describe("ListValidator ssz type", function () {
@@ -20,7 +22,8 @@ describe("ListValidator ssz type", function () {
   };
 
   const testCases = [32, 33, 34, 35];
-  const oldValidatorsType = new ListCompositeType(ValidatorNodeStruct, VALIDATOR_REGISTRY_LIMIT);
+  const ValidatorContainer = new ContainerType(ValidatorType, {typeName: "Validator", jsonCase: "eth2"});
+  const oldValidatorsType = new ListCompositeType(ValidatorContainer, VALIDATOR_REGISTRY_LIMIT);
   for (const numValidators of testCases) {
     it (`should commit ${numValidators} validators`, () => {
       const validators = Array.from({length: numValidators}, (_, i) => ({...seedValidator, withdrawableEpoch: seedValidator.withdrawableEpoch + i}));
@@ -35,4 +38,45 @@ describe("ListValidator ssz type", function () {
       expect(newViewDU.serialize()).to.be.deep.equal(oldViewDU.serialize());
     });
   }
+
+  const testCases2 = [[1], [3, 5], [1,9, 7]];
+  const numValidator = 33;
+  for (const modifiedIndices of testCases2) {
+    it(`should modify ${modifiedIndices.length} validators`, () => {
+      const validators = Array.from({length: numValidator}, (_, i) => ({...seedValidator, withdrawableEpoch: seedValidator.withdrawableEpoch + i}));
+      const oldViewDU = oldValidatorsType.toViewDU(validators);
+      const newViewDU = ssz.phase0.Validators.toViewDU(validators);
+      for (const index of modifiedIndices) {
+        oldViewDU.get(index).activationEpoch = 2024;
+        newViewDU.get(index).activationEpoch = 2024;
+      }
+      expect(newViewDU.hashTreeRoot()).to.be.deep.equal(oldViewDU.hashTreeRoot());
+      expect(newViewDU.serialize()).to.be.deep.equal(oldViewDU.serialize());
+    })
+  }
+
+  const testCases3 = [1, 3, 5, 7];
+  for (const numPush of testCases3) {
+    it(`should push ${numPush} validators`, () => {
+      const validators = Array.from({length: numValidator}, (_, i) => ({...seedValidator, withdrawableEpoch: seedValidator.withdrawableEpoch + i}));
+      const oldViewDU = oldValidatorsType.toViewDU(validators);
+      const newViewDU = ssz.phase0.Validators.toViewDU(validators);
+      const newValidators: Validator[] = [];
+      // this ensure the commit() should update nodes array
+      newViewDU.getAllReadonlyValues();
+      for (let i = 0; i < numPush; i++) {
+        const validator = {...seedValidator, withdrawableEpoch: seedValidator.withdrawableEpoch + numValidator + i};
+        newValidators.push(validator);
+        oldViewDU.push(ValidatorContainer.toViewDU(validator));
+        newViewDU.push(ssz.phase0.Validator.toViewDU(validator));
+      }
+      oldViewDU.commit();
+      expect(newViewDU.hashTreeRoot()).to.be.deep.equal(oldViewDU.node.root);
+      expect(newViewDU.serialize()).to.be.deep.equal(oldViewDU.serialize());
+      const allValidators = newViewDU.getAllReadonlyValues();
+      for (let i = 0; i < numPush; i++) {
+        expect(allValidators[numValidator + i]).to.be.deep.equal(newValidators[i]);
+      }
+    })
+  }
 });