Supporting variants of SHA2 #1957

querolita · 2024-12-18T16:41:53Z

Closes #1941

…of lines

…re/eddsa/sha512

dannywillems · 2025-02-07T16:31:24Z

src/lib/provable/test/sha2.unit-test.ts

+  });
+
+  for (let { preimage, hash } of testVectors()) {
+    let actual = Gadgets.SHA2.hash(384, Bytes.fromString(preimage));


Nit: not sure about the convention, but as a user, reviewing the PR and running the tests, I would like to see some logs here, as it seems it is taking a bit of time to run. I was looking at the stdout for some seconds and stop it as I didn't know what it was running.

(again: not blocking at all)

dannywillems · 2025-02-07T16:32:39Z

src/lib/provable/test/sha2.unit-test.ts

+    { runs: RUNS }
+  )(nobleSha384, async (x) => {
+    const { proof } = await Sha2_384Program.sha384(x);
+    await Sha2_384Program.verify(proof);


Nit: is there a convention to check it also verifies? I added on my side.

const is_verified = await Sha2_384Program.verify(proof); expect(is_verified);

I don't know if there is another place where we actually test the fact it verifies.

Good point.. we are not doing that currently, anyway, but we probably should start doing that for completness altought I am not completely sure if Pickles or Kimchi checks if a proof is valid after generating it as a sanity check already

dannywillems · 2025-02-07T16:35:24Z

src/lib/provable/gadgets/sha2.ts

+  compression,
+  messageSchedule,
+  padding,
+  initialState<T extends UInt32 | UInt64>(length: Length): T[] {


Unrelated to the PR: is there a convention in o1js to order the methods? As an engineer not working often in TS, I was surprised to see the method initialState defined after a place it is used (hash above).

Not really a convention, TS doesn't care - I personally try to order methods by factors such as importance or level of abstraction

dannywillems · 2025-02-07T16:37:18Z

src/lib/provable/gadgets/sha2.ts

+    // padding the message $5.1.1 into blocks that are a multiple of 512
+    let messageBlocks = padding<Type>(length, data);
+
+    let H = SHA2.initialState<Type>(length);


Nit: it seems we're doing the Uint32/Uint64 transformations every time we execute hash. A direct optimization would be to do it once and for all.

Not blocking.

dannywillems · 2025-02-07T16:46:54Z

src/lib/provable/gadgets/sha2.ts

+    '0'.repeat(k) + // append k zero bits
+    '0'.repeat(lengthChunk - lBinary.length) + // append 64|128-bit containing the length of the original message
+    lBinary
+  ).match(/.{1,8}/g)!; // this should always be divisible by 8


Oh. Is it a common practice in TS? :o

Not common, but it's something I had to do in the other SHA implementation a while ago - cant remember why tho

tests/vk-regression/plain-constraint-system.ts

dannywillems · 2025-02-07T17:16:28Z

src/lib/provable/gadgets/sha2.ts

+// Helper functions
+
+// Helper function to check if it is a short SHA2 (SHA2-224 or SHA2-256) or not
+function isShort(length: Length): boolean {


Nit: I would inline this check as isShort does not have a clear semantics for the primitive.

Yeah I didn't like it either... At first it was is256 but then it was true also in 224. Any suggestion?

src/lib/provable/test/sha2.unit-test.ts

dannywillems

Mostly comments at the moment.
I am not approving (sorry for being picky) because I would love to see :

the origin of the test vectors
a regression test for the number of constraints

I also think we should therefore get rid of sha256.ts if the primitive is implemented twice.

I have not fully verified that the circuits are not under-constrained.

Trivo25 · 2025-02-11T07:52:05Z

src/lib/provable/gadgets/gadgets.ts

@@ -973,6 +974,27 @@ const Gadgets = {
   */
  SHA256: SHA256,


let's deprecate it!

Trivo25 · 2025-02-11T08:05:17Z

src/lib/provable/gadgets/sha2.ts

+   */
+  hash<T extends Length>(length: T, data: FlexibleBytes): Bytes {
+    // Infer the type T based on the value of `length` (conditional type)
+    type Type = T extends 224 | 256 ? UInt32 : UInt64;


damn, some serious typescript! :D

Trivo25 · 2025-02-11T08:07:16Z

src/lib/provable/gadgets/sha2.ts

+  compression,
+  messageSchedule,
+  padding,
+  initialState<T extends UInt32 | UInt64>(length: Length): T[] {


Not really a convention, TS doesn't care - I personally try to order methods by factors such as importance or level of abstraction

Trivo25 · 2025-02-11T08:12:03Z

src/lib/provable/gadgets/sha2.ts

+    '0'.repeat(k) + // append k zero bits
+    '0'.repeat(lengthChunk - lBinary.length) + // append 64|128-bit containing the length of the original message
+    lBinary
+  ).match(/.{1,8}/g)!; // this should always be divisible by 8


Not common, but it's something I had to do in the other SHA implementation a while ago - cant remember why tho

Trivo25 · 2025-02-11T08:41:40Z

src/lib/provable/gadgets/sha2.ts

+
+// Subroutines for SHA2
+
+function Ch<T extends UInt32 | UInt64>(length: Length, x: T, y: T, z: T): T {


This works well, but if you are interested you could take a look at type guards that might make things in certain cases a bit cleaner

https://www.typescriptlang.org/docs/handbook/advanced-types.html#user-defined-type-guards

Trivo25 · 2025-02-11T08:52:19Z

src/lib/provable/test/sha2.unit-test.ts

+    { runs: RUNS }
+  )(nobleSha384, async (x) => {
+    const { proof } = await Sha2_384Program.sha384(x);
+    await Sha2_384Program.verify(proof);


Good point.. we are not doing that currently, anyway, but we probably should start doing that for completness altought I am not completely sure if Pickles or Kimchi checks if a proof is valid after generating it as a sanity check already

querolita added 30 commits December 3, 2024 16:35

clarify sha2 or sha3 in examples

eb5fa81

bytes conversion for UInt64

ff313c6

constants for sha2 variants

d040fdc

preliminary sha2 interface

ff1ff4d

padding function for short and long sha2

b31edda

generic message schedule function

8847397

infer flag from instanceof generic T to avoid passing is_short

a72e52c

compression function and auxiliary subroutines from sha256 module

9c5e988

different parameters for sigma function depending on variant

98be5f2

generic compression function

3b6c837

auxiliary functions for reduction modulo 32 64 bits to reduce number …

9f001b1

…of lines

adapt auxiliary functions for generic case

1bde951

prettify and sigma for 256 and 512

62c4ae5

truncate for 224 and 384 variants

5f4527a

remove length property

854cb19

unit tests for sha2

086f927

undo name change

4f40f97

Merge branch 'main' into feature/eddsa/sha512

9dd0ff6

Merge branch 'main' into feature/eddsa/sha512

d18716d

Merge branch 'main' into feature/eddsa/sha512

a9a3668

remove unused web3 type import

924bb8f

pass flag because ts cannot infer types from runtime values

0b934e7

refactor constants

d2fef76

pass length to functions to improve readability

47e12da

fix switch syntax

06afc51

fix aliases for sha3 vs sha2

40482ee

update changelog

b885ddc

Merge branch 'main' into feature/eddsa/sha512

86da703

Merge remote-tracking branch 'origin/feature/eddsa/sha512' into featu…

a2bc7ec

…re/eddsa/sha512

update bindings commit

a6163d7

dannywillems reviewed Feb 7, 2025

View reviewed changes

tests/vk-regression/plain-constraint-system.ts Show resolved Hide resolved

dannywillems reviewed Feb 7, 2025

View reviewed changes

src/lib/provable/test/sha2.unit-test.ts Show resolved Hide resolved

dannywillems reviewed Feb 7, 2025

View reviewed changes

src/lib/provable/test/sha2.unit-test.ts Show resolved Hide resolved

dannywillems reviewed Feb 7, 2025

View reviewed changes

typo

bf788f3

querolita force-pushed the feature/eddsa/sha512 branch from 900b89a to bf788f3 Compare February 10, 2025 15:24

Merge branch 'main' into feature/eddsa/sha512

00be305

Trivo25 approved these changes Feb 11, 2025

View reviewed changes

querolita added 6 commits February 12, 2025 16:29

deprecate sha256 and regression tests

a103890

Merge branch 'main' into feature/eddsa/sha512

ef9a1b4

fix lint

cc0ff82

apply prettier fixes

27dddf2

use type guards in some places

38b4210

update bindings

b6b032c

querolita mentioned this pull request Feb 12, 2025

[easy] Rebuild bindings for SHA2-512 o1-labs/o1js-bindings#337

Merged

Trivo25 and others added 5 commits February 13, 2025 01:33

add sha variants to regression tests

689d962

update bindings to aa980e5

6b2b3c9

remove old test counting rows in favor of regression tests

22f90c4

Merge branch 'main' into feature/eddsa/sha512

08f5809

update bindings

645e294

querolita mentioned this pull request Feb 13, 2025

[easy] Apply patch for 1957 o1-labs/o1js-bindings#339

Merged

update bindings commit

215fb34

querolita merged commit 61970ca into main Feb 13, 2025
31 of 32 checks passed

querolita deleted the feature/eddsa/sha512 branch February 13, 2025 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Supporting variants of SHA2 #1957

Supporting variants of SHA2 #1957

querolita commented Dec 18, 2024

dannywillems Feb 7, 2025

dannywillems Feb 7, 2025

Trivo25 Feb 11, 2025

dannywillems Feb 7, 2025

Trivo25 Feb 11, 2025

dannywillems Feb 7, 2025

dannywillems Feb 7, 2025

Trivo25 Feb 11, 2025

dannywillems Feb 7, 2025

querolita Feb 7, 2025

dannywillems left a comment

Trivo25 Feb 11, 2025

Trivo25 Feb 11, 2025

Trivo25 Feb 11, 2025

Trivo25 Feb 11, 2025

Trivo25 Feb 11, 2025

Trivo25 Feb 11, 2025


		// Subroutines for SHA2

		function Ch<T extends UInt32 \| UInt64>(length: Length, x: T, y: T, z: T): T {

Supporting variants of SHA2 #1957

Supporting variants of SHA2 #1957

Conversation

querolita commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dannywillems left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment