Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: store StateLeaf objects in IncrementalQuinTree #917

Open
baumstern opened this issue Dec 14, 2023 · 6 comments
Open

core: store StateLeaf objects in IncrementalQuinTree #917

baumstern opened this issue Dec 14, 2023 · 6 comments

Comments

@baumstern
Copy link
Member

baumstern commented Dec 14, 2023

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
Copy link
Collaborator

ctrlc03 commented May 14, 2024

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
Copy link
Member Author

baumstern commented May 26, 2024

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
Copy link
Collaborator

ctrlc03 commented May 27, 2024

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
Copy link
Collaborator

ctrlc03 commented Jul 15, 2024

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

@baumstern
Copy link
Member Author

baumstern commented Jul 17, 2024

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
Copy link
Collaborator

ctrlc03 commented Aug 7, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants