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

Hashes - ARC-81 #2588

Draft
wants to merge 4 commits into
base: staging
Choose a base branch
from
Draft

Hashes - ARC-81 #2588

wants to merge 4 commits into from

Conversation

anstylian
Copy link

Motivation

This pull request introduces changes to the Hash instructions in the synthesizer/program/src/logic/instruction/operation/hash.rs file. The main focus is on improving the handling of destination types and adding support for additional array types in hashing operations (sha3 and keccak).

It is a step forward for the implementation of ARC-81.

Test Plan

Let me know how you prefer to test this

@anstylian anstylian changed the title Hashes Hashes - ARC-81 Jan 9, 2025
.fold(0, |acc, (i, bit)| if bit.eject_value() { acc | (1 << i) } else { acc });

let value = console::types::U8::new(value);
circuit::Plaintext::Literal(
Copy link
Collaborator

@d0cd d0cd Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this circuit is constrained sufficiently. You'll need to use the from_bits and to_bits utilities.

@@ -256,14 +462,9 @@ impl<N: Network, const VARIANT: u8> HashInstruction<N, VARIANT> {
(_, PlaintextType::Struct(..)) => bail!("Cannot hash into a struct"),
(_, PlaintextType::Array(..)) => bail!("Cannot hash into an array (yet)"),
};
// Cast the output to the destination type.
let output = match self.destination_type {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, it would nice to have fewer deltas on the variants that are not being changed.

@d0cd
Copy link
Collaborator

d0cd commented Jan 18, 2025

Thanks @anstylian for taking an attempt at ARC-81! I think this approach will need careful consideration before we move forward. Specifically we'd like to:

  • Ensure the approach is consistent in design. In particular, I'd like to be certain that the bespoke variants don't create confusion for users and tech debt in the future.
  • Ensure the circuits are sufficiently constrained.
  • Thoroughly test the implementation. One property that needs testing is that the output arrays are correctly constructed. I imagine there are more.

@anstylian anstylian marked this pull request as draft January 20, 2025 07:45
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.

2 participants