-
Notifications
You must be signed in to change notification settings - Fork 504
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
Move Poseidon primitive into halo2_poseidon
#831
Conversation
0192b97
to
0d88368
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 0d88368 with two minor questions.
self.mode = Absorbing::init_empty(); | ||
self.mode.absorb(value).expect("state is not full"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this suggest that 0d88368#diff-76142ee2fee95e1ba7ebdff8392dfb7442517333aaf489d5cf92d6dcebee37a4R190-R194 should be public?
/// Attempts to absorb a value into the sponge state. | ||
/// | ||
/// Returns the value if it was not absorbed because the sponge is full. | ||
pub fn absorb(&mut self, value: F) -> Result<(), F> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Result
the right type to return here? Or should it be a bespoke enum? It looks to me like the kind of thing that I would ordinarily use Either
for rather than Result
, but a bespoke enum might be better.
This is to make the next commit correctly detect a move.
0d88368
to
5893850
Compare
Rebased on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-utACK 5893850
No description provided.