Skip to content

Commit

Permalink
fix: same to _4 but no commit() in the middle of other flows
Browse files Browse the repository at this point in the history
  • Loading branch information
twoeths committed Jun 28, 2024
1 parent ab76885 commit b9f9e99
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 62 deletions.
2 changes: 1 addition & 1 deletion packages/ssz/src/viewDU/abstract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export abstract class TreeViewDU<T extends CompositeType<unknown, unknown, unkno
*/
hashTreeRoot(): Uint8Array {
this.commit();
return this.node.root;
return super.hashTreeRoot();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/ssz/src/viewDU/arrayBasic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ export class ArrayBasicTreeViewDU<ElementType extends BasicType<unknown>> extend
}

commit(): void {
const isOldRootHashed = this._rootNode.h0 !== null;
if (this.nodesChanged.size === 0) {
return;
}
Expand All @@ -165,6 +164,7 @@ export class ArrayBasicTreeViewDU<ElementType extends BasicType<unknown>> extend
}

const chunksNode = this.type.tree_getChunksNode(this._rootNode);
// TODO: Ensure fast setNodesAtDepth() method is correct
const newChunksNode = setNodesAtDepth(chunksNode, this.type.chunkDepth, indexes, nodes);

this._rootNode = this.type.tree_setChunksNode(
Expand Down
11 changes: 3 additions & 8 deletions packages/ssz/src/viewDU/arrayComposite.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
getNodeAtDepth,
getNodesAtDepth,
Node,
setNodesAtDepth,
} from "@chainsafe/persistent-merkle-tree";
import {getNodeAtDepth, getNodesAtDepth, Node, setNodesAtDepth} from "@chainsafe/persistent-merkle-tree";
import {ValueOf} from "../type/abstract";
import {CompositeType, CompositeView, CompositeViewDU} from "../type/composite";
import {ArrayCompositeType} from "../view/arrayComposite";
Expand Down Expand Up @@ -169,7 +164,6 @@ export class ArrayCompositeTreeViewDU<
}

commit(): void {
const isOldRootHashed = this._rootNode.h0 !== null;
if (this.viewsChanged.size === 0) {
return;
}
Expand All @@ -193,7 +187,8 @@ export class ArrayCompositeTreeViewDU<
const nodes = nodesChangedSorted.map((entry) => entry.node);

const chunksNode = this.type.tree_getChunksNode(this._rootNode);
const newChunksNode = setNodesAtDepth(chunksNode, this.type.chunkDepth, indexes, nodes, null);
// TODO: Ensure fast setNodesAtDepth() method is correct
const newChunksNode = setNodesAtDepth(chunksNode, this.type.chunkDepth, indexes, nodes);

this._rootNode = this.type.tree_setChunksNode(
this._rootNode,
Expand Down
15 changes: 2 additions & 13 deletions packages/ssz/src/viewDU/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class ContainerTreeViewDU<Fields extends Record<string, Type<unknown>>> extends
}

commit(): void {
const isOldRootHashed = this._rootNode.h0 !== null;
if (this.nodesChanged.size === 0 && this.viewsChanged.size === 0) {
return;
}
Expand Down Expand Up @@ -97,14 +96,7 @@ class ContainerTreeViewDU<Fields extends Record<string, Type<unknown>>> extends
const indexes = nodesChangedSorted.map((entry) => entry.index);
const nodes = nodesChangedSorted.map((entry) => entry.node);

this._rootNode = setNodesAtDepth(
this._rootNode,
this.type.depth,
indexes,
nodes,
// isOldRootHashed ? hashComps : null
null,
);
this._rootNode = setNodesAtDepth(this._rootNode, this.type.depth, indexes, nodes);

this.nodesChanged.clear();
this.viewsChanged.clear();
Expand All @@ -127,10 +119,7 @@ class ContainerTreeViewDU<Fields extends Record<string, Type<unknown>>> extends
* Same method to `type/container.ts` that call ViewDU.serializeToBytes() of internal fields.
*/
serializeToBytes(output: ByteViews, offset: number): number {
// it's the responsibility of consumer to call commit() before calling this method
if (this.nodesChanged.size !== 0 || this.viewsChanged.size !== 0) {
throw Error(`Must commit changes before serializeToBytes(Uint8Array(${output.uint8Array.length}, ${offset})`);
}
// this.commit();

let fixedIndex = offset;
let variableIndex = offset + this.type.fixedEnd;
Expand Down
4 changes: 0 additions & 4 deletions packages/ssz/src/viewDU/containerNodeStruct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,13 @@ class ContainerTreeViewDU<Fields extends Record<string, Type<unknown>>> extends

commit(): void {
if (this.valueChanged === null) {
// this does not suppor batch hash
// this._rootNode.root;
return;
}

const value = this.valueChanged;
this.valueChanged = null;

this._rootNode = this.type.value_toTree(value) as BranchNodeStruct<ValueOfFields<Fields>>;
// this does not suppor batch hash
// this._rootNode.root;
}

protected clearCache(): void {
Expand Down
13 changes: 4 additions & 9 deletions packages/ssz/src/viewDU/listBasic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,11 @@ export class ListBasicTreeViewDU<ElementType extends BasicType<unknown>> extends
*/
sliceTo(index: number): this {
if (index < 0) {
throw Error(`Does not support sliceTo() with negative index ${index}`);
throw new Error(`Does not support sliceTo() with negative index ${index}`);
}

// it's the responsibility of consumer to call commit() before calling this method
if (this.nodesChanged.size > 0) {
throw Error(`Must commit changes before sliceTo(${index})`);
}
// Commit before getting rootNode to ensure all pending data is in the rootNode
// this.commit();

// All nodes beyond length are already zero
if (index >= this._length - 1) {
Expand Down Expand Up @@ -86,10 +84,7 @@ export class ListBasicTreeViewDU<ElementType extends BasicType<unknown>> extends
* Same method to `type/listBasic.ts` leveraging cached nodes.
*/
serializeToBytes(output: ByteViews, offset: number): number {
// it's the responsibility of consumer to call commit() before calling this method
if (this.nodesChanged.size > 0) {
throw Error(`Must commit changes before serializeToBytes(Uint8Array(${output.uint8Array.length}), ${offset})`);
}
// this.commit();
const {nodes, nodesPopulated} = this.cache;
const chunksNode = this.type.tree_getChunksNode(this._rootNode);
return tree_serializeToBytesArrayBasic(
Expand Down
11 changes: 3 additions & 8 deletions packages/ssz/src/viewDU/listComposite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ export class ListCompositeTreeViewDU<
* Note: Using index = -1, returns an empty list of length 0.
*/
sliceTo(index: number): this {
// it's the responsibility of consumer to call commit() before calling this method
if (this.viewsChanged.size > 0) {
throw Error(`Must commit changes before sliceTo(${index})`);
}
// Commit before getting rootNode to ensure all pending data is in the rootNode
// this.commit();
const rootNode = this._rootNode;
const length = this._length;

Expand Down Expand Up @@ -113,10 +111,7 @@ export class ListCompositeTreeViewDU<
* Same method to `type/listComposite.ts` leveraging cached nodes.
*/
serializeToBytes(output: ByteViews, offset: number): number {
// it's the responsibility of consumer to call commit() before calling this method
if (this.viewsChanged.size > 0) {
throw Error(`Must commit changes before serializeToBytes(Uint8Array(${output.uint8Array.length}, ${offset})`);
}
// this.commit();
this.populateAllNodes();
const chunksNode = this.type.tree_getChunksNode(this._rootNode);
return tree_serializeToBytesArrayComposite(
Expand Down
36 changes: 18 additions & 18 deletions packages/ssz/test/unit/eth2/validators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,22 +147,22 @@ describe.skip("getHashComputations BranchNodeStruct", function () {
},
];

for (const {name, fn} of testCases) {
it(name, () => {
const hashComps: HashComputationGroup = {
byLevel: [],
offset: 0,
};
const validatorViewDU = ValidatorNodeStruct.toViewDU(validator);
// cache all roots
validatorViewDU.hashTreeRoot();
fn(validatorViewDU);
validatorViewDU.commit(hashComps);
expect(hashComps.byLevel.length).to.be.equal(4);
expect(hashComps.byLevel[0].length).to.be.equal(1);
expect(hashComps.byLevel[1].length).to.be.equal(2);
expect(hashComps.byLevel[2].length).to.be.equal(4);
expect(hashComps.byLevel[3].length).to.be.equal(1);
});
}
// for (const {name, fn} of testCases) {
// it(name, () => {
// const hashComps: HashComputationGroup = {
// byLevel: [],
// offset: 0,
// };
// const validatorViewDU = ValidatorNodeStruct.toViewDU(validator);
// // cache all roots
// validatorViewDU.hashTreeRoot();
// fn(validatorViewDU);
// validatorViewDU.commit(hashComps);
// expect(hashComps.byLevel.length).to.be.equal(4);
// expect(hashComps.byLevel[0].length).to.be.equal(1);
// expect(hashComps.byLevel[1].length).to.be.equal(2);
// expect(hashComps.byLevel[2].length).to.be.equal(4);
// expect(hashComps.byLevel[3].length).to.be.equal(1);
// });
// }
});

0 comments on commit b9f9e99

Please sign in to comment.