Update commons library by martti007 · Pull Request #96 · unicitynetwork/aggregators_net

martti007 · 2025-07-09T11:23:54Z

No description provided.

gemini-code-assist

Summary of Changes

Hello @martti007, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on upgrading the @unicitylabs/commons library, which introduces significant updates to the Sparse Merkle Tree (SMT) implementation and RequestId handling. The changes involve adapting the existing codebase to the new SMT API, streamlining RequestId conversions for SMT paths, and cleaning up various development dependencies. These updates aim to leverage improvements in the commons library and ensure compatibility with its latest features.

Highlights

Dependency Update: The @unicitylabs/commons library has been updated to a newer release candidate version (2.4.0-rc.b5c5ce4).
Sparse Merkle Tree (SMT) API Modernization: The codebase has been adapted to a new SMT API from the updated commons library. This involves replacing SparseMerkleTree with SparseMerkleTreeBuilder and MerkleTreeRootNode, and changing how SMTs are initialized, leaves are added, and root hashes are retrieved (now a synchronous property access instead of an async method call).
RequestId Path Handling Refinement: The conversion of RequestId objects to bigint paths for SMT operations has been updated to use requestId.toBitString().toBigInt(), reflecting changes in the commons library's RequestId representation.
Development Dependency Cleanup: Several unused development dependencies and their transitive dependencies, such as supertest, cookiejar, methods, mime, formidable, asap, component-emitter, dezalgo, and fast-safe-stringify, have been removed, resulting in a leaner package-lock.json.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request updates the @unicitylabs/commons dependency and removes supertest. The code has been adapted to the new APIs, particularly for the Sparse Merkle Tree implementation which now uses a SparseMerkleTreeBuilder. While the application logic appears correct, the review has uncovered several critical issues in the test suite, including type errors and broken test logic that need to be addressed to ensure code quality and correctness.

gemini-code-assist · 2025-07-09T11:27:42Z

+  public addLeaves(leaves: Array<{ path: bigint; value: Uint8Array }>): Promise<void> {
    return this.withSmtLock(async () => {
-      for (const leaf of leaves) {
-        try {
-          this.smt.addLeaf(leaf.path, leaf.value);
-        } catch (error) {
-          // Check if the error is "Cannot add leaf inside branch" which indicates
-          // the leaf is already in the tree - this is not a fatal error
-          if (error instanceof Error && error.message.includes('Cannot add leaf inside branch')) {
-            logger.warn(`Leaf already exists in tree for path ${leaf.path} - skipping`);
-          } else {
-            throw error;
-          }
-        }
-      }
+      await Promise.all(
+        leaves.map((leaf) =>
+          this.smt.addLeaf(leaf.path, leaf.value).catch((error) => {
+            if (error instanceof Error && error.message.includes('Cannot add leaf inside branch')) {
+              logger.warn(`Leaf already exists in tree for path ${leaf.path} - skipping`);
+            } else {
+              throw error;
+            }
+          }),
+        ),
+      );
+
+      this._tree = await this.smt.calculateRoot();
    });


The implementation of addLeaves has been changed from a sequential for...of loop to Promise.all. While this is a great improvement for performance, the error handling inside the .catch() block only logs a warning for a specific error message and then swallows the error. This means that if this.smt.addLeaf fails for any other reason, the failure will be silently ignored, and Promise.all will still resolve successfully. This could lead to an inconsistent SMT state without any indication of failure.

It's recommended to re-throw any unexpected errors to ensure the calling function is aware of the failure.

public addLeaves(leaves: Array<{ path: bigint; value: Uint8Array }>): Promise<void> { return this.withSmtLock(async () => { await Promise.all( leaves.map((leaf) => this.smt.addLeaf(leaf.path, leaf.value).catch((error) => { // Check if the error is "Cannot add leaf inside branch" which indicates // the leaf is already in the tree - this is not a fatal error if (error instanceof Error && error.message.includes('Cannot add leaf inside branch')) { logger.warn(`Leaf already exists in tree for path ${leaf.path} - skipping`); } else { // Re-throw other errors to ensure the Promise.all call rejects throw error; } }), ), ); this._tree = await this.smt.calculateRoot(); }); }

gemini-code-assist · 2025-07-09T11:27:42Z


-        const nodePath = commitment.requestId.toBigInt();
+        const nodePath = commitment.requestId.toBitString().toBigInt();
        const nodeValue = commitment.transactionHash.data;


The SMT leaf value is being set to commitment.transactionHash.data. However, in the production code (RoundManager.ts) and other tests, the leaf value is derived differently, typically using LeafValue.create() or at least transactionHash.imprint to include the hash algorithm identifier.

Using just .data makes this benchmark inconsistent with the actual implementation, which could lead to inaccurate performance measurements. To better align with other parts of the code, consider using .imprint.

Suggested change

const nodeValue = commitment.transactionHash.data;

const nodeValue = commitment.transactionHash.imprint;

gemini-code-assist · 2025-07-09T11:27:42Z


    smt.addLeaf(path, value);
-    const rootHashAfterFirstAddition = await smt.root.calculateHash();
+    const rootHashAfterFirstAddition = await smt.calculateRoot();


The smt.calculateRoot() method on SparseMerkleTreeBuilder returns a MerkleTreeRootNode object, not a DataHash. The subsequent logging on the next line will incorrectly stringify the entire node object instead of just its hash.

To get the hash, you need to access the .hash property of the returned root node.

Suggested change

const rootHashAfterFirstAddition = await smt.calculateRoot();

const rootHashAfterFirstAddition = (await smt.calculateRoot()).hash;

gemini-code-assist · 2025-07-09T11:27:43Z

+        const merkleTreePath = (await smt.calculateRoot()).getPath(path);
        return merkleTreePath;


The smt.calculateRoot() method is called twice in a row. The result of the first call on line 54 is discarded, and the method is immediately called again on line 55. This is inefficient and redundant.

You should call it once, store the result in a variable, and then use that variable.

Suggested change

await smt.calculateRoot();

const merkleTreePath = (await smt.calculateRoot()).getPath(path);

const root = await smt.calculateRoot();

const merkleTreePath = root.getPath(path);

Update commons library

5841018

gemini-code-assist Bot reviewed Jul 9, 2025

View reviewed changes

martti007 added 2 commits July 9, 2025 14:31

Remove old imports

962e431

Fix signingService and SMT leaf adding tests

1d1c5c4

MastaP reviewed Jul 9, 2025

View reviewed changes

Comment thread src/smt/Smt.ts Outdated

martti007 added 2 commits July 9, 2025 20:25

Update naming of sparse merkle tree

bbe241d

Fix block creation benchmark

1023a3e

martti007 requested a review from MastaP July 10, 2025 06:41

Update naming and logging in tests

3472899

MastaP approved these changes Jul 10, 2025

View reviewed changes

Update SMT to check for existing leaves using error

c53933e

martti007 merged commit 8420fd0 into main Jul 10, 2025
1 check passed

martti007 deleted the on-demand-smt branch July 10, 2025 08:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update commons library#96

Update commons library#96
martti007 merged 7 commits into
mainfrom
on-demand-smt

martti007 commented Jul 9, 2025

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Jul 9, 2025

Uh oh!

gemini-code-assist Bot Jul 9, 2025

Uh oh!

gemini-code-assist Bot Jul 9, 2025

Uh oh!

gemini-code-assist Bot Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	const nodeValue = commitment.transactionHash.data;
	const nodeValue = commitment.transactionHash.imprint;

	const rootHashAfterFirstAddition = await smt.calculateRoot();
	const rootHashAfterFirstAddition = (await smt.calculateRoot()).hash;

		const merkleTreePath = (await smt.calculateRoot()).getPath(path);
		return merkleTreePath;

Conversation

martti007 commented Jul 9, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants