Skip to content

Commit

Permalink
fix: ListCompositeTreeViewDU to getAllReadonly() without commit (#456)
Browse files Browse the repository at this point in the history
* fix: ListCompositeTreeViewDU to getAllReadonly() without commit()

* fix: lint

---------

Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com>
  • Loading branch information
twoeths and twoeths authored Jan 27, 2025
1 parent a07548a commit 94510c9
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 4 deletions.
20 changes: 17 additions & 3 deletions packages/ssz/src/viewDU/arrayComposite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,16 @@ export class ArrayCompositeTreeViewDU<
}

/**
* WARNING: Returns all commited changes, if there are any pending changes commit them beforehand
* Returns all elements at every index, if an index is modified it will return the modified view.
* No need to commit() before calling this function.
*/
getAllReadonly(): CompositeViewDU<ElementType>[] {
this.populateAllNodes();
this.populateAllOldNodes();

const views = new Array<CompositeViewDU<ElementType>>(this._length);
for (let i = 0; i < this._length; i++) {
views[i] = this.type.elementType.getViewDU(this.nodes[i], this.caches[i]);
// this will get pending change first, if not it will get from the `this.nodes` array
views[i] = this.getReadonly(i);
}
return views;
}
Expand Down Expand Up @@ -256,4 +258,16 @@ export class ArrayCompositeTreeViewDU<
this.nodesPopulated = true;
}
}

/**
* Similar to `populateAllNodes` but this does not require a commit() before reading all nodes.
* If there are pendingChanges, they will NOT be included in the `nodes` array.
*/
protected populateAllOldNodes(): void {
if (!this.nodesPopulated) {
const originalLength = this.dirtyLength ? this.type.tree_getLength(this._rootNode) : this._length;
this.nodes = getNodesAtDepth(this._rootNode, this.type.depth, 0, originalLength);
this.nodesPopulated = true;
}
}
}
24 changes: 23 additions & 1 deletion packages/ssz/test/unit/byType/listComposite/tree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ describe("ListCompositeType tree reads", () => {

// Only for viewDU
if (view instanceof ArrayCompositeTreeViewDU) {
expect(() => view.getAllReadonly()).toThrow("Must commit changes before reading all nodes");
expect(() => view.getAllReadonly()).not.toThrow();
expect(() => view.getAllReadonlyValues()).toThrow("Must commit changes before reading all nodes");
view.commit();
}

Expand Down Expand Up @@ -337,3 +338,24 @@ describe("ListCompositeType batchHashTreeRoot", () => {
});
}
});

describe("ListCompositeType getAllReadOnly - no commit", () => {
it("getAllReadOnly() without commit", () => {
const listType = new ListCompositeType(ssz.Root, 1024);
const listLength = 2;
const list = Array.from({length: listLength}, (_, i) => Buffer.alloc(32, i));
const listView = listType.toViewDU(list);
expect(listView.getAllReadonly()).to.deep.equal(list);

// modify
listView.set(0, Buffer.alloc(32, 1));
// push
listView.push(Buffer.alloc(32, 1));

// getAllReadOnly() without commit, now all items should be the same
expect(listView.getAllReadonly()).to.deep.equal(Array.from({length: 3}, () => Buffer.alloc(32, 1)));

// getAllReadOnlyValues() will throw
expect(() => listView.getAllReadonlyValues()).toThrow("Must commit changes before reading all nodes");
});
});

0 comments on commit 94510c9

Please sign in to comment.