TS `root()` isn't compatible with solidity `root()`

[0xbok]

Solidity version returns 0 for an empty tree root but ts code will throw an exception:

https://github.com/privacy-scaling-explorations/zk-kit/blob/950dc5bd5f8f53069c12b56581ed63faa5382515/packages/imt.sol/contracts/internal/InternalLeanIMT.sol#L342-L343

https://github.com/privacy-scaling-explorations/zk-kit/blob/950dc5bd5f8f53069c12b56581ed63faa5382515/packages/imt/src/lean-imt/lean-imt.ts#L54-L55

To compensate for this difference, Semaphore has to do this check:

https://github.com/semaphore-protocol/semaphore/blob/a522fff4483e0409afbeb7ae85de24f4506c9ec0/packages/group/src/index.ts#L33-L35

Would be good to have this check in zk-kit instead of in semaphore.

[sripwoud]

@cedoor I'd like to have a shot at this too. What looks as a trivial change at first, raises a few questions regarding the implementation of a solution. Unlike in semaphore, zk-kit uses generics. While it provides flexibility, it means we can't make any assumption about N or what a possible 0 value is.

The only solution compatible with generic I could think about:

1. somehow "provide" the zero as a parameter (value, getZero function etc...) to the constructor

   
   ...
   constructor(hash, leaves, zero: N) {
     this._zero = zero
   }
   get root() {
     return this._nodes[this.depth][0] ?? this._zero
   }

Discarded ideas

1. Type guarding/checking

    root() {
        if (this._nodes[this.depth][0] !== undefined) {
           return this._nodes[this.depth][0];
       }
      if (typeof this._nodes[this.depth][0] === "bigint") {
           return BigInt(0) as N;
       } 
       if (typeof this._nodes[this.depth][0] === "number") {
           return 0 as N;
       }
       // and then??
       // return undefined again if tree empty?!
     }

   2. restrict the generic type instead. This impacts all the places where we use N generic param in the package. Probably not what we want.

      
      class LeanIMT<N extends bigint> {

   root() { return this._nodes[this.depth][0] ?? BigInt(0) } }

[cedoor]

@sripwoud good point! I had not yet read this issue with attention. I think my idea was to return undefined, and I think it does it. @0xbok it shouldn't throw any exception right?

Returning undefined sounds consistent with generics to me, and the simplest solution. Also, it sounds logically true: if there are no members in the three there's no root. In Solidity there's no undefined type ofc, so 0 looks like the best value.

Re solution 1: One downside I see is that it won't be possible to check if the type of the zero parameter is the right one.

[0xbok]

Returning undefined sounds consistent with generics to me, and the simplest solution. Also, it sounds logically true: if there are no members in the three there's no root. In Solidity there's no undefined type ofc, so 0 looks like the best value.

I agree, let's close this issue. however, then does it make sense to return 0 in semaphore ts?

[cedoor]

I agree, let's close this issue. however, then does it make sense to return 0 in semaphore ts?

Mmh although it looks inconsistent there are no generics there, and it's easier to know what type to use as 1 type only (bigints) is used. So it could make more sense there.

There are different arguments. What do you think?

[0xbok]

yup, makes sense.