`Mst` efficiency improvement #188

enricobottazzi · 2023-11-21T14:32:04Z

#166

Major breaking changes in this PR:

Logic of building the middle nodes of the MerkleSumTree is modified. Before the pre-image of the hash was equal to balance_left, hash_left, balance_right, hash_right now it is balance_left + balance_right, hash_left, hash_right
Further constraints added to the circuit. Before the circuit would just take sibling_sums and sibling_hashes to be used inside the witness. Now we need to open the hash of each sibling. Because of that the MerkleProof now has fieldssibling_leaf_node_hash_preimage and sibling_middle_node_hash_preimages rather than sibling_hashes and sibling_sums. The logic of the circuit is modified to build each sibling_hash inside the circuit and constraint that the balances associated to a sibling are the same used as input to build the hash

To do:

As I modified the Tree trait, it might also have an impact on AggregationMerkleSumTree

Benchmarks:

Reduced Merkle Sum Tree generation time (2^20 users, 1 asset) -> 73.695 s vs 88.182s
Increased Proof of Inclusion generation time -> 517.98 ms vs 465.61 ms

Given that the focus of V1 is fast commitment time. I think that this is a reasonable tradeoff.

…reimage` methods

This reverts commit e3ba8e6.

alxkzmn · 2023-11-23T15:13:01Z

zk_prover/src/merkle_sum_tree/tree.rs

+            .try_into()
+            .unwrap();
+
+        let sibling_leaf_node_hash = poseidon_entry::<N_ASSETS>(


poseidon_entry as well as poseidon_node are specific to Node creation and are always used in that context, so their implementations can be placed directly in the Node (no need to have them inside a separate file in utils). For example, here you first create the hash of sibling_leaf_node and then create the sibling_leaf_node Node using this hash, not using the hash for anything else. Meanwhile, Node already has the ::leaf constructor that takes username and balances as args. Moving both hashing operations into the Node would remove confusion over how to create the node.

Thanks for the suggestion! Changes applied in fc44e01. I hope that now it looks cleaner

sifnoc

This is a good improvement, especially for large entry sets.
Also, thank you for the minor updates in the backend.

zk_prover/src/merkle_sum_tree/mod.rs

backend/src/apis/round.rs

zk_prover/src/circuits/merkle_sum_tree.rs

enricobottazzi · 2023-11-27T09:19:47Z

I updated the code base according to your comments. A further thing that I noticed while reviewing the codebase is that there was an API generate_leaf_hash in zk_prover that was only used inside backend/examples. I therefore decided to move it back to backend as part of our effort to reduce the number of utils API inside zk_prover.

…improvement

alxkzmn · 2023-11-28T08:19:11Z

zk_prover/src/merkle_sum_tree/node.rs

+    /// Builds a leaf-level node of the MST
+    /// The leaf node hash is equal to `H(username, balance[0], balance[1], ... balance[N_ASSETS - 1])`
+    /// The balances are equal to `balance[0], balance[1], ... balance[N_ASSETS - 1]`
+    pub fn leaf(username: &BigUint, balances: &[BigUint; N_ASSETS]) -> Node<N_ASSETS>


In the true spirit of DRY this could just call the Node::leaf_node_from_preimage() after converting the args :) Same with the middle calling Node::middle_from_preimage inside itself. Won't request changes, we can do this minor enhancement later.

Let's DRY!

Gonna apply the changes now (:

alxkzmn · 2023-11-28T09:05:19Z

zk_prover/src/merkle_sum_tree/utils/build_tree.rs

+
+            hash_preimage[N_ASSETS] = tree[level - 1][index].hash;
+            hash_preimage[N_ASSETS + 1] = tree[level - 1][index + 1].hash;
+            Node::middle_node_from_preimage(&hash_preimage)


Actually that's why we could have both constructors. I just meant that one could call the other inside itself.

Oh I see, I misunderstood your point

Applied changes in last commit!

This reverts commit ae539a2.

* feat: tree building rules * feat: add `get_middle_node_hash_preimage` and `get_middle_node_hash_preimage` methods * feat: update `generate_proof` * fix: `generate_proof` * fix: modify middle nodes hashing logic in circuit * feat: update `MerkleProof` * feat: add further constraint on `MstInclusionCircuit` * feat: rm `test_balance_not_in_range` * refactor: update `MerkleProof` * chore: fix `backend` * test: fix `backend` * fix: testing * fix: modify `MstInclusionCircuit` struct * Revert "fix: modify `MstInclusionCircuit` struct" This reverts commit e3ba8e6. * chore: update `benches` * fix: move poseidon hasher APIs to `Node` * chore: add guide on `MerkleProof ` * chore: update `where` clause in backend * chore: update comments * chore: move `generate_leaf_hash` outside of `zk_prover` * chore: add `ethers` legacy feature * chore: update verifier contract * fix: removed minting erc20 on the backend test * chore: rm unused imports * feat: remove `Node::leaf()` * feat: rm `Node::middle()` * Revert "feat: remove `Node::leaf()`" This reverts commit ae539a2. * chore: DRY code --------- Co-authored-by: sifnoc <[email protected]>

enricobottazzi added 10 commits November 21, 2023 09:08

feat: tree building rules

e0e8ce2

feat: add get_middle_node_hash_preimage and `get_middle_node_hash_p…

134020c

…reimage` methods

feat: update generate_proof

b236bfa

fix: generate_proof

698dce6

fix: modify middle nodes hashing logic in circuit

d62f231

feat: update MerkleProof

63f89d0

feat: add further constraint on MstInclusionCircuit

d95feef

feat: rm test_balance_not_in_range

68ab601

refactor: update MerkleProof

eda4ad2

chore: fix backend

41f2257

enricobottazzi linked an issue Nov 21, 2023 that may be closed by this pull request

Merkle Sum Tree Efficiency Improvement #166

Closed

1 task

enricobottazzi added 5 commits November 21, 2023 15:57

test: fix backend

efe6b87

fix: testing

ac0cecd

fix: modify MstInclusionCircuit struct

e3ba8e6

Revert "fix: modify MstInclusionCircuit struct"

8d9ee86

This reverts commit e3ba8e6.

chore: update benches

11f533f

enricobottazzi requested review from alxkzmn and sifnoc November 21, 2023 18:46

enricobottazzi marked this pull request as ready for review November 21, 2023 18:46

alxkzmn reviewed Nov 23, 2023

View reviewed changes

sifnoc requested changes Nov 25, 2023

View reviewed changes

zk_prover/src/merkle_sum_tree/mod.rs Show resolved Hide resolved

backend/src/apis/round.rs Outdated Show resolved Hide resolved

backend/src/apis/round.rs Show resolved Hide resolved

zk_prover/src/circuits/merkle_sum_tree.rs Outdated Show resolved Hide resolved

enricobottazzi mentioned this pull request Dec 1, 2023

Update AggregationMerkleSumTree summa-dev/summa-aggregation#5

Closed

enricobottazzi added 5 commits November 27, 2023 09:39

fix: move poseidon hasher APIs to Node

fc44e01

chore: add guide on MerkleProof

94404ed

chore: update where clause in backend

fe9a72f

chore: update comments

1775ca7

chore: move generate_leaf_hash outside of zk_prover

06a15e4

enricobottazzi requested review from alxkzmn and sifnoc November 27, 2023 09:19

chore: add ethers legacy feature

559ac3a

sifnoc approved these changes Nov 27, 2023

View reviewed changes

enricobottazzi and others added 4 commits November 27, 2023 17:51

Merge branch 'v1-improvements-and-consolidation' into mst-efficiency-…

cc7d02e

…improvement

chore: update verifier contract

79cbe2c

fix: removed minting erc20 on the backend test

d84aa36

chore: rm unused imports

be15a56

alxkzmn reviewed Nov 28, 2023

View reviewed changes

alxkzmn approved these changes Nov 28, 2023

View reviewed changes

enricobottazzi added 2 commits November 28, 2023 09:34

feat: remove Node::leaf()

ae539a2

feat: rm Node::middle()

ac396ce

alxkzmn reviewed Nov 28, 2023

View reviewed changes

enricobottazzi added 2 commits November 28, 2023 11:22

Revert "feat: remove Node::leaf()"

69d049c

This reverts commit ae539a2.

chore: DRY code

5dea9ed

enricobottazzi merged commit a734d94 into v1-improvements-and-consolidation Nov 28, 2023
4 checks passed

enricobottazzi deleted the mst-efficiency-improvement branch November 28, 2023 11:13

enricobottazzi mentioned this pull request Jan 21, 2024

fix: replace swap_balances_per_level with sum_balances_per_level #255

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Mst` efficiency improvement #188

`Mst` efficiency improvement #188

enricobottazzi commented Nov 21, 2023 •

edited

Loading

alxkzmn Nov 23, 2023

enricobottazzi Nov 27, 2023 •

edited

Loading

sifnoc left a comment

enricobottazzi commented Nov 27, 2023 •

edited

Loading

alxkzmn Nov 28, 2023

enricobottazzi Nov 28, 2023

alxkzmn Nov 28, 2023

enricobottazzi Nov 28, 2023

enricobottazzi Nov 28, 2023

Mst efficiency improvement #188

Mst efficiency improvement #188

Conversation

enricobottazzi commented Nov 21, 2023 • edited Loading

alxkzmn Nov 23, 2023

Choose a reason for hiding this comment

enricobottazzi Nov 27, 2023 • edited Loading

Choose a reason for hiding this comment

sifnoc left a comment

Choose a reason for hiding this comment

enricobottazzi commented Nov 27, 2023 • edited Loading

alxkzmn Nov 28, 2023

Choose a reason for hiding this comment

enricobottazzi Nov 28, 2023

Choose a reason for hiding this comment

alxkzmn Nov 28, 2023

Choose a reason for hiding this comment

enricobottazzi Nov 28, 2023

Choose a reason for hiding this comment

enricobottazzi Nov 28, 2023

Choose a reason for hiding this comment

`Mst` efficiency improvement #188

`Mst` efficiency improvement #188

enricobottazzi commented Nov 21, 2023 •

edited

Loading

enricobottazzi Nov 27, 2023 •

edited

Loading

enricobottazzi commented Nov 27, 2023 •

edited

Loading