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

Repercussion of latest Num addition changes #90

Open
tchataigner opened this issue Mar 19, 2024 · 0 comments · May be fixed by argumentcomputer/neptune#275
Open

Repercussion of latest Num addition changes #90

tchataigner opened this issue Mar 19, 2024 · 0 comments · May be fixed by argumentcomputer/neptune#275

Comments

@tchataigner
Copy link
Member

tchataigner commented Mar 19, 2024

Issue

The recent PR #88 introduces a new behavior for Num::add, favouring None to Some(v) as a result in (Some(v), None) | (None, Some(v) cases.

This new behavior seems to break the current implementation of PoseidonROCircuit::squeeze in arecibo. In this method we actually call the SpongeCircuit::absorb method with the current state elements, that do not have any values at that stage.

In neptune absorb will call add_rate_element with a Num::add that results on a None variant, transmitted to the set_element method that leverages an unwrap on the value and thus panicking.

I'm not sure about the correct way to fix this, so I'm looking for inputs.

How to reproduce

This bug has been found while leveraging the Bn256EngineKZG engine at PublicParams::setup both for Nova & Supernova. You can reproduce it on your machine by running this test on ethereum-lc/aggpk

I was also able to reproduce it on the aptos-lc if patching both belpepper & bellpepper-core to dev.

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 a pull request may close this issue.

1 participant