search

privacy-scaling-explorations / maci

Minimal Anti-Collusion Infrastructure (MACI)
https://maci.pse.dev/
Other
534 stars 148 forks source link

core: store `StateLeaf` objects in `IncrementalQuinTree` #917

Open baumstern opened 11 months ago

baumstern commented 11 months ago

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.

ctrlc03 commented 6 months ago

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?

baumstern commented 6 months ago

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

ctrlc03 commented 6 months ago

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 commented 4 months ago

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

baumstern commented 4 months ago

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.

ctrlc03 commented 3 months ago

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?