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

Store the root of a branch seperately to the levels #189

Closed
moCello opened this issue Sep 21, 2022 · 1 comment · Fixed by #190
Closed

Store the root of a branch seperately to the levels #189

moCello opened this issue Sep 21, 2022 · 1 comment · Fixed by #190
Labels
fix:bug Something isn't working type:tech-debt the issue is a tech debt that needs fixing

Comments

@moCello
Copy link
Member

moCello commented Sep 21, 2022

Describe what you want implemented
Rethink the way we store the root in the PoseidonBranch.

Describe "Why" this is needed
At the moment the root of the PoseidonBranch is stored in the last level of the branch at the index 1. Ideally we want the root stored separately to the levels and not as an additional level.
This is also an issue due to the rkyv implementations. Since it is now a Vec, PoseidonBranch<17> would parse in the exact same way as PoseidonBranch<16> for example.

Describe alternatives you've considered
We could also want to store the levels not as a Vec but as an array instead.

Additional context
N/A

@ureeves
Copy link
Member

ureeves commented Sep 21, 2022

This is also an issue due to the rkyv implementations. Since it is now a Vec, PoseidonBranch<17> would parse in the exact same way as PoseidonBranch<16> for example.

@ureeves ureeves added type:tech-debt the issue is a tech debt that needs fixing fix:bug Something isn't working labels Sep 21, 2022
ureeves added a commit that referenced this issue Sep 22, 2022
This also required a slight change in the algorithm computing the
branch, to handle the root not being stored at the last position of the
path.

Resolves #189
ureeves added a commit that referenced this issue Sep 22, 2022
This also required a slight change in the algorithm computing the
branch, to handle the root not being stored at the last position of the
path.

Co-authored-by: moana <moana@dusk.network>
Resolves #189
ureeves added a commit that referenced this issue Sep 22, 2022
This also required a slight change in the algorithm computing the
branch, to handle the root not being stored at the last position of the
path.

Co-authored-by: moCello <moana@dusk.network>
Resolves #189
ureeves added a commit that referenced this issue Sep 22, 2022
This also required a slight change in the algorithm computing the
branch, to handle the root not being stored at the last position of the
path.

Resolves: #189
Co-authored-by: moCello <moana@dusk.network>
ureeves added a commit that referenced this issue Sep 23, 2022
This also required a slight change in the algorithm computing the
branch, to handle the root not being stored at the last position of the
path.

Resolves: #189
Co-authored-by: moCello <moana@dusk.network>
ureeves added a commit that referenced this issue Sep 23, 2022
This also required a slight change in the algorithm computing the
branch, to handle the root not being stored at the last position of the
path.

Resolves: #189
Co-authored-by: moCello <moana@dusk.network>
ureeves added a commit that referenced this issue Sep 23, 2022
This also required a slight change in the algorithm computing the
branch, to handle the root not being stored at the last position of the
path.

Resolves: #189
Co-authored-by: moCello <moana@dusk.network>
ureeves added a commit that referenced this issue Sep 29, 2022
This also required a slight change in the algorithm computing the
branch, to handle the root not being stored at the last position of the
path.

Resolves: #189
Co-authored-by: moCello <moana@dusk.network>
This was referenced Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix:bug Something isn't working type:tech-debt the issue is a tech debt that needs fixing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants