From 53dd361960497485ba68bc86a50beb192aba6dd7 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 13 Sep 2024 13:51:38 +0700 Subject: [PATCH] chore: more code comments --- .../persistent-merkle-tree/src/packedNode.ts | 3 +++ packages/ssz/src/type/listUintNum64.ts | 24 +++++++++++++------ .../byType/listBasic/listUintNum64.test.ts | 6 ++--- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/persistent-merkle-tree/src/packedNode.ts b/packages/persistent-merkle-tree/src/packedNode.ts index 70a0f3ba..459f6444 100644 --- a/packages/persistent-merkle-tree/src/packedNode.ts +++ b/packages/persistent-merkle-tree/src/packedNode.ts @@ -18,6 +18,9 @@ export function packedRootsBytesToNode(depth: number, dataView: DataView, start: * * h0 h1 h2 h3 h4 h5 h6 h7 * |------|------|------|------|------|------|------|------| + * + * @param values list of uint64 numbers + * @param leafNodes optional list of LeafNodes to reuse */ export function packedUintNum64sToLeafNodes(values: number[], leafNodes?: LeafNode[]): LeafNode[] { const nodeCount = Math.ceil(values.length / 4); diff --git a/packages/ssz/src/type/listUintNum64.ts b/packages/ssz/src/type/listUintNum64.ts index c5ad607a..4ee4826b 100644 --- a/packages/ssz/src/type/listUintNum64.ts +++ b/packages/ssz/src/type/listUintNum64.ts @@ -28,8 +28,8 @@ export class ListUintNum64Type extends ListBasicType { /** * Return a ListBasicTreeViewDU with nodes populated - * @param unusedViewDU optional, if provided we'll create ViewDU using the provided rootNode. Consumer should recompute - * parent hashes of the returned ViewDU in this case. + * @param unusedViewDU optional, if provided we'll create ViewDU using the provided rootNode. Need to rehash the whole + * tree in this case to make it clean for consumers. */ toViewDU(value: number[], unusedViewDU?: ListBasicTreeViewDU): ListBasicTreeViewDU { // no need to serialize and deserialize like in the abstract class @@ -58,34 +58,38 @@ export class ListUintNum64Type extends ListBasicType { /** * No need to serialize and deserialize like in the abstract class - * // TODO: modify this? + * This should be conformed to parent's signature so cannot provide an `unusedViewDU` parameter here */ value_toTree(value: number[]): Node { const {treeNode} = this.packedUintNum64sToNode(value); return treeNode; } - private packedUintNum64sToNode(value: number[], oldRootNode?: Node): {treeNode: Node; leafNodes: LeafNode[]} { + private packedUintNum64sToNode(value: number[], unusedRootNode?: Node): {treeNode: Node; leafNodes: LeafNode[]} { if (value.length > this.limit) { throw new Error(`Exceeds limit: ${value.length} > ${this.limit}`); } - if (oldRootNode) { - const oldLength = getLengthFromRootNode(oldRootNode); + if (unusedRootNode) { + // create new tree from unusedRootNode + const oldLength = getLengthFromRootNode(unusedRootNode); if (oldLength > value.length) { throw new Error(`Cannot decrease length: ${oldLength} > ${value.length}`); } + const oldNodeCount = Math.ceil(oldLength / 4); - const oldChunksNode = oldRootNode.left; + const oldChunksNode = unusedRootNode.left; const oldLeafNodes = getNodesAtDepth(oldChunksNode, this.chunkDepth, 0, oldNodeCount) as LeafNode[]; if (oldLeafNodes.length !== oldNodeCount) { throw new Error(`oldLeafNodes.length ${oldLeafNodes.length} !== oldNodeCount ${oldNodeCount}`); } + const newNodeCount = Math.ceil(value.length / 4); const count = newNodeCount - oldNodeCount; const newLeafNodes = Array.from({length: count}, () => new LeafNode(0, 0, 0, 0, 0, 0, 0, 0)); const leafNodes = [...oldLeafNodes, ...newLeafNodes]; packedUintNum64sToLeafNodes(value, leafNodes); + // middle nodes are not changed so consumer must recompute parent hashes const newChunksNode = setNodesAtDepth( oldChunksNode, @@ -94,9 +98,11 @@ export class ListUintNum64Type extends ListBasicType { newLeafNodes ); const treeNode = addLengthNode(newChunksNode, value.length); + return {treeNode, leafNodes}; } + // create new tree from scratch const leafNodes = packedUintNum64sToLeafNodes(value); // subtreeFillToContents mutates the leafNodes array const chunksNode = subtreeFillToContents([...leafNodes], this.chunkDepth); @@ -112,6 +118,10 @@ export class ListUintNum64Type extends ListBasicType { } } +/** + * Consider moving this to persistent-merkle-tree. + * For now this is the only flow to force get hash computations. + */ function forceGetHashComputations( node: Node, nodeDepth: number, diff --git a/packages/ssz/test/unit/byType/listBasic/listUintNum64.test.ts b/packages/ssz/test/unit/byType/listBasic/listUintNum64.test.ts index d53f5e12..ae867a5e 100644 --- a/packages/ssz/test/unit/byType/listBasic/listUintNum64.test.ts +++ b/packages/ssz/test/unit/byType/listBasic/listUintNum64.test.ts @@ -4,7 +4,7 @@ import {ListUintNum64Type} from "../../../../src/type/listUintNum64"; describe("ListUintNum64Type.toViewDU", () => { const type = new ListUintNum64Type(1024); // seed ViewDU contains 16 leaf nodes = 64 uint64 - // we test all cases + // but we test all cases for (const seedLength of [61, 62, 63, 64]) { const value = Array.from({length: seedLength}, (_, i) => i); const unusedViewDU = type.toViewDU(value); @@ -13,8 +13,8 @@ describe("ListUintNum64Type.toViewDU", () => { for (let i = seedLength; i < 1024; i++) { const newValue = Array.from({length: i + 1}, (_, j) => j); const expectedRoot = type.toViewDU(newValue).hashTreeRoot(); - const viewDU = type.toViewDU(newValue, unusedViewDU); - expect(viewDU.hashTreeRoot()).to.deep.equal(expectedRoot); + const viewDUFromExistingTree = type.toViewDU(newValue, unusedViewDU); + expect(viewDUFromExistingTree.hashTreeRoot()).to.deep.equal(expectedRoot); } }); }