privacy-scaling-explorations / zk-kit

A monorepo of reusable libraries for zero-knowledge technologies.
https://zkkit.pse.dev
MIT License
290 stars 76 forks source link

SMT add error with poseidon2 hashing #303

Closed ashpect closed 4 months ago

ashpect commented 4 months ago

Describe the bug https://zkkit.pse.dev/modules/_zk_kit_smt.html Following this article , if you create a tree2 using poseidon2 and try adding big ints using tree2.add(BigInt(2),BigInt(2)) , the addition fails with the following error :

 throw new Error(`poseidon-lite: Incorrect M length, expected ${t} got ${M.length}`);
          ^
Error: poseidon-lite: Incorrect M length, expected 4 got 3

To Reproduce Here is a sample code you can try to see the error :

import { ChildNodes, SMT } from "@zk-kit/smt"
import { poseidon2 } from "poseidon-lite"

const hash2 = (childNodes: ChildNodes) => poseidon2(childNodes)
const tree2 = new SMT(hash2, true)
console.log(tree2.root)
tree2.add(BigInt("2"),BigInt("2"))
console.log(tree2.root)

Expected behavior Should add the node to the smt

Screenshots If applicable, add screenshots to help explain your problem.

Technologies (please complete the following information):

Additional context Upon digging into it, this error from Poseidon-lite occurs when you try to pack more stuff using a specific Poseidon algo.

This is where the Poseidon-lite throws error, where M is the Poseidon type+1 like 3 in case of Poseidon2 hashing used, so inputs.length +1 = M (input length should be 2 in case of posedion2)

Screenshot 2024-07-02 at 5 22 22 PM

So, if I use any other than Poseidon2 to hash, it would throw error as it expects an algo that can hash2 Bigint's which is as expected :

Screenshot 2024-07-02 at 5 27 19 PM

Now the issue arises when I call add, where it tries to hash 3 inputs using poseidon2 hashing as clearly stated here.

Screenshot 2024-07-02 at 5 29 41 PM

This is all which I can deduce from the call-stack. If it is something I am missing from my side, pls let me know. If not, how can I resolve this.

cedoor commented 4 months ago

Hi @ashpect, thanks for pointing this out!

I see why we haven't caught it. The README file was updated with poseidon-lite but tests are still using with circomlibjs, which works because it exports a Poseidon function that supports different numbers of inputs. Instead, poseidon2 only supports two parameters.

This should fix it:

import { SMT } from "@zk-kit/smt"
import { poseidon2, poseidon3 } from "poseidon-lite"

const hash2 = (childNodes) => (childNodes.length === 2 ? poseidon2(childNodes) : poseidon3(childNodes))
const tree2 = new SMT(hash2, true)

console.log(tree2.root)

tree2.add(BigInt("2"), BigInt("2"))

console.log(tree2.root)

poseidon-lite doesn't have a poseidon function like circomlibjs, so you can either use circomlibjs or the previous code.

I think the code above is better because it only imports the constants Poseidon needs for 2 and 3 inputs. circomlibjs-poseidon import all of them: https://github.com/iden3/circomlibjs/blob/43cc582b100fc3459cf78d903a6f538e5d7f38ee/src/poseidon_opt.js#L13.

The main improvement of poseidon-lite in comparison with circomlibjs-poseidon is precisely the possibility of importing only a portion of constants, saving a lot of memory.

ashpect commented 4 months ago

Hi, Thanks for the quick response, it got resolved. I'll close the issue.

cedoor commented 4 months ago

@ashpect no worries! I'll update the README 👍🏽

https://github.com/privacy-scaling-explorations/zk-kit/pull/304