Ergonomic MerkleTreeGadget

[tessico]

Description

closes: ##88
as well as some indirect progress towards #142.

Note that non membership proofs are out of scope of this PR, but we can still close the referenced issue #88, if approved, since we already have a separate issue for non-membership proof constraints: #99.

Regarding the design, the initial refactor I had started doing was based on the API proposed by @alxiong in #88 - thanks a lot for this! You will see that it has changed in that methods don't accept native field elements/structs, but rather variables representing these items in the circuit. This was done for two reasons:

• There would otherwise be no way to (efficiently) enforce multiple constraints with the same proof. E.g. given a Merkle path, it would be impossible (without allocating the path variables twice) to enforce that e.g. one element is a member and another is not.
• API consistency. Most of our other APIs follow the format of create_<>_variable -> Result<SomeVariable, ()> and then using SomeVariable in enforcing constraints. I feel that this is a clean abstraction, and we should probably stick to not enforcing constraints "directly" from native items.

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (main)
• Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
• Wrote unit tests
• Updated relevant documentation in the code
• Added a relevant changelog entry to the Pending section in CHANGELOG.md
• Re-reviewed Files changed in the GitHub PR explorer

[alxiong]

I agree with @tessico's choice of accepting Var instead of the original struct.

1. I wonder if we should name rescue_merkle_tree.rs as append_only.rs for consistency with their non-circuit implementations?

[alxiong]

Do you think we should make this an associated type of trait MerkleTreeGadget instead, just like MerklePathVar?
The main rationale being that some MT's leaf (e.g. SMT) doesn't have to care about or even have uid. So it may suggest leaving the concrete instantiation to specific gadgets.

[alxiong]

on this note, I noticed our current RescueSMT's digest_leaf is actually hashing [0, pos, elem], which I guess doesn't hurt, but when we treat it as a set (as we do when using SMT), we don't need that pos, no? @mrain

[mrain]

No. In case of set, elem is of type (). Information is stored in pos actually.

[alxiong]

oh, right!

my original comment on making it an associated type might still be worth considering.

[tessico]

Yes, good suggestion. Done in 7288912.

[tessico]

I wonder if we should name rescue_merkle_tree.rs as append_only.rs for consistency with their non-circuit implementations?

Yes, but after #142 lands. Currently the Rescue hash is "hardcoded" in the gadget implementation, so I think for now we should keep the name as is.
Later we can have append_only.rs and then instantiate a concrete one in the prelude of the circuit, like the native version does.

@alxiong

[alxiong]

LGTM!

[mrain]

LGTM