maci icon indicating copy to clipboard operation
maci copied to clipboard

core: store `StateLeaf` objects in `IncrementalQuinTree`

Open baumstern opened this issue 1 year ago • 6 comments

Context

IncrementalQuinTree currently holds only StateLeaf hashes. Suggesting an enhancement to store StateLeaf objects directly, removing the need for the stateLeaves array in MaciState and in Poll object.

Objective

Refactor IncrementalQuinTree to encapsulate StateLeaf objects, simplifying the state management within MaciState.

baumstern avatar Dec 14 '23 03:12 baumstern

Context

IncrementalQuinTree currently holds only StateLeaf hashes. Suggesting an enhancement to store StateLeaf objects directly, removing the need for the stateLeaves array in MaciState and in Poll object.

Objective

Refactor IncrementalQuinTree to encapsulate StateLeaf objects, simplifying the state management within MaciState.

Given some thoughts on this and I think it might be counterproductive to do that, as we use the IMT for other trees aside from the state tree, for instance the message, ballot and vote option tree. Thus accepting a bigint value as a leaf seems more like a general solution. Any thoughts?

ctrlc03 avatar May 14 '24 10:05 ctrlc03

Context

IncrementalQuinTree currently holds only StateLeaf hashes. Suggesting an enhancement to store StateLeaf objects directly, removing the need for the stateLeaves array in MaciState and in Poll object.

Objective

Refactor IncrementalQuinTree to encapsulate StateLeaf objects, simplifying the state management within MaciState.

Given some thoughts on this and I think it might be counterproductive to do that, as we use the IMT for other trees aside from the state tree, for instance the message, ballot and vote option tree. Thus accepting a bigint value as a leaf seems more like a general solution. Any thoughts?

I see IncrementalQuinTree is still used in core package (e.g. see https://github.com/privacy-scaling-explorations/maci//blob/67711d44058831302d4556f12d1972ffacede653/core/ts/Poll.ts#L72). Although this issue is a minor improvement to simplify the code. Not a big deal

baumstern avatar May 26 '24 09:05 baumstern

Context

IncrementalQuinTree currently holds only StateLeaf hashes. Suggesting an enhancement to store StateLeaf objects directly, removing the need for the stateLeaves array in MaciState and in Poll object.

Objective

Refactor IncrementalQuinTree to encapsulate StateLeaf objects, simplifying the state management within MaciState.

Given some thoughts on this and I think it might be counterproductive to do that, as we use the IMT for other trees aside from the state tree, for instance the message, ballot and vote option tree. Thus accepting a bigint value as a leaf seems more like a general solution. Any thoughts?

I see IncrementalQuinTree is still used in core package (e.g. see https://github.com/privacy-scaling-explorations/maci//blob/67711d44058831302d4556f12d1972ffacede653/core/ts/Poll.ts#L72). Although this issue is a minor improvement to simplify the code. Not a big deal

I don't think I properly understood then? you mentioned that we should store the StateLeaf object in the IMT, but then on the last comment there's a link to the IMT that stores the user ballots which are not StateLeaf objects? We also use the tree to store vote options, or just to generate an inclusion proof on the fly

ctrlc03 avatar May 27 '24 13:05 ctrlc03

@baumstern wondering if you had a look at my latest reply on this

ctrlc03 avatar Jul 15 '24 08:07 ctrlc03

Context

IncrementalQuinTree currently holds only StateLeaf hashes. Suggesting an enhancement to store StateLeaf objects directly, removing the need for the stateLeaves array in MaciState and in Poll object.

Objective

Refactor IncrementalQuinTree to encapsulate StateLeaf objects, simplifying the state management within MaciState.

Given some thoughts on this and I think it might be counterproductive to do that, as we use the IMT for other trees aside from the state tree, for instance the message, ballot and vote option tree. Thus accepting a bigint value as a leaf seems more like a general solution. Any thoughts?

I see IncrementalQuinTree is still used in core package (e.g. see https://github.com/privacy-scaling-explorations/maci//blob/67711d44058831302d4556f12d1972ffacede653/core/ts/Poll.ts#L72). Although this issue is a minor improvement to simplify the code. Not a big deal

I don't think I properly understood then? you mentioned that we should store the StateLeaf object in the IMT, but then on the last comment there's a link to the IMT that stores the user ballots which are not StateLeaf objects? We also use the tree to store vote options, or just to generate an inclusion proof on the fly

Sorry, I wrote a reply but didn't posted it 🤦

so..currently, the IncrementalQuinTree can only hold Leaf values as bigint. This limitation forces us to maintain separate object arrays like Ballot[] or Message[] in the Poll class. I suggest allowing IncrementalQuinTree to handle flexible data types, such as Ballot or Message objects. This way, we can eliminate the need for these separate arrays and directly refer to tree members from IncrementalQuinTree.

baumstern avatar Jul 17 '24 11:07 baumstern

Context

IncrementalQuinTree currently holds only StateLeaf hashes. Suggesting an enhancement to store StateLeaf objects directly, removing the need for the stateLeaves array in MaciState and in Poll object.

Objective

Refactor IncrementalQuinTree to encapsulate StateLeaf objects, simplifying the state management within MaciState.

Given some thoughts on this and I think it might be counterproductive to do that, as we use the IMT for other trees aside from the state tree, for instance the message, ballot and vote option tree. Thus accepting a bigint value as a leaf seems more like a general solution. Any thoughts?

I see IncrementalQuinTree is still used in core package (e.g. see https://github.com/privacy-scaling-explorations/maci//blob/67711d44058831302d4556f12d1972ffacede653/core/ts/Poll.ts#L72). Although this issue is a minor improvement to simplify the code. Not a big deal

I don't think I properly understood then? you mentioned that we should store the StateLeaf object in the IMT, but then on the last comment there's a link to the IMT that stores the user ballots which are not StateLeaf objects? We also use the tree to store vote options, or just to generate an inclusion proof on the fly

Sorry, I wrote a reply but didn't posted it 🤦

so..currently, the IncrementalQuinTree can only hold Leaf values as bigint. This limitation forces us to maintain separate object arrays like Ballot[] or Message[] in the Poll class. I suggest allowing IncrementalQuinTree to handle flexible data types, such as Ballot or Message objects. This way, we can eliminate the need for these separate arrays and directly refer to tree members from IncrementalQuinTree.

My bad now for a late reply! So the difference would just be that the arrays instead of being in the Poll instance they will be inside the IncrementalQuinTree? either way those values need to be hashed to be able to generate/update the tree so we will still end up having both the StateLeaf/Ballot/Message + the hashes - unless I'm missing something?

ctrlc03 avatar Aug 07 '24 17:08 ctrlc03