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

[feat] Utreexo: removing item from the accumulator #218

Merged
merged 12 commits into from
Sep 27, 2024

Conversation

TAdev0
Copy link
Collaborator

@TAdev0 TAdev0 commented Sep 22, 2024

Copy link

vercel bot commented Sep 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
raito ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 27, 2024 11:21am

@TAdev0
Copy link
Collaborator Author

TAdev0 commented Sep 22, 2024

@m-kus first draft.

Need to add more tests to it, not 100% sure of my implementation.
Works well for a simple test where i remove one leave to a 2 leaves - 1 root forest merkle tree

@TAdev0
Copy link
Collaborator Author

TAdev0 commented Sep 22, 2024

can you confirm that desired behaviour is that once an UtreexoState has contained n roots, then even if we remove all leaves, we should still have n roots, all set to Option::None ?

I think there is no choice actually. Because we need to ''copy paste'' all the remaining roots that are not concerned by the removal, we need to append residual roots to the utreexostate. Hence, in case where we remove a lot of leaves, we'll have multiple Option::None appended. I dont see any way (that is not costly and doesnt require to loop over all the remaining roots) to check whether we should append or not remaining roots to stick to only one Option::None at the end of our array of roots.
not sure if i'm totally clear

@TAdev0
Copy link
Collaborator Author

TAdev0 commented Sep 22, 2024

Also, can you confirm that the deletion algorithm doesnt require to use leaf_index binary representation in any step? just to make sure

@m-kus
Copy link
Collaborator

m-kus commented Sep 23, 2024

can you confirm that desired behaviour is that once an UtreexoState has contained n roots, then even if we remove all leaves, we should still have n roots, all set to Option::None ?

I think there is no choice actually. Because we need to ''copy paste'' all the remaining roots that are not concerned by the removal, we need to append residual roots to the utreexostate. Hence, in case where we remove a lot of leaves, we'll have multiple Option::None appended. I dont see any way (that is not costly and doesnt require to loop over all the remaining roots) to check whether we should append or not remaining roots to stick to only one Option::None at the end of our array of roots. not sure if i'm totally clear

There always has to be at least one root (Option::None if the forest is empty).
It's ok to have multiple None roots, the total number will remain pretty small (e.g. in ZeroSync they have a fixed array)

@m-kus
Copy link
Collaborator

m-kus commented Sep 23, 2024

Also, can you confirm that the deletion algorithm doesnt require to use leaf_index binary representation in any step? just to make sure

It requires leaf index (which is part of the proof) to determine the position of the sibling node from the proof when you compute parent hash.

@TAdev0
Copy link
Collaborator Author

TAdev0 commented Sep 27, 2024

@m-kus ready for final review

@m-kus m-kus merged commit e1380ce into keep-starknet-strange:main Sep 27, 2024
7 checks passed
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

Successfully merging this pull request may close these issues.

[feat] Utreexo: removing item from the accumulator
2 participants