From 3022675aee42475fe74370b8a5f3dfcc2024263f Mon Sep 17 00:00:00 2001 From: Bruno Deferrari Date: Tue, 21 Nov 2023 23:01:47 -0300 Subject: [PATCH 1/4] When comparing `Field.t` values, avoid the conversion to bigint --- src/lib/crypto/kimchi_backend/common/field.ml | 4 +++- src/lib/crypto/kimchi_backend/kimchi_backend.mli | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lib/crypto/kimchi_backend/common/field.ml b/src/lib/crypto/kimchi_backend/common/field.ml index 91d76aa71ffd..77af0a352b12 100644 --- a/src/lib/crypto/kimchi_backend/common/field.ml +++ b/src/lib/crypto/kimchi_backend/common/field.ml @@ -37,6 +37,8 @@ module type Input_intf = sig val is_square : t -> bool + val compare : t -> t -> int + val equal : t -> t -> bool val print : t -> unit @@ -194,7 +196,7 @@ module Make (F : Input_intf) : let hash = Hash.of_fold hash_fold_t - let compare t1 t2 = Bigint.compare (to_bigint t1) (to_bigint t2) + let compare = compare let equal = equal diff --git a/src/lib/crypto/kimchi_backend/kimchi_backend.mli b/src/lib/crypto/kimchi_backend/kimchi_backend.mli index b2fb42a082bb..eb9baab3a9d8 100644 --- a/src/lib/crypto/kimchi_backend/kimchi_backend.mli +++ b/src/lib/crypto/kimchi_backend/kimchi_backend.mli @@ -11,8 +11,6 @@ module Kimchi_backend_common : sig val sexp_of_t : t -> Sexplib0.Sexp.t - val compare : t -> t -> int - val bin_size_t : t Bin_prot.Size.sizer val bin_write_t : t Bin_prot.Write.writer @@ -56,6 +54,8 @@ module Kimchi_backend_common : sig val is_square : t -> bool + val compare : t -> t -> int + val equal : t -> t -> bool val print : t -> unit From 5651bced7ab425022727ebefdd43bd5d609a272a Mon Sep 17 00:00:00 2001 From: Bruno Deferrari Date: Wed, 22 Nov 2023 12:46:35 -0300 Subject: [PATCH 2/4] Use `into_repr()` when comparing fields in Rust to ensure compatibility with protocol and o1js --- src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fp.rs | 4 ++-- src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fq.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fp.rs b/src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fp.rs index 3d9144f13033..08264921088f 100644 --- a/src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fp.rs +++ b/src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fp.rs @@ -31,7 +31,7 @@ impl CamlFp { unsafe extern "C" fn ocaml_compare(x: ocaml::Raw, y: ocaml::Raw) -> i32 { let x = x.as_pointer::(); let y = y.as_pointer::(); - match x.as_ref().0.cmp(&y.as_ref().0) { + match x.as_ref().0.into_repr().cmp(&y.as_ref().0.into_repr()) { core::cmp::Ordering::Less => -1, core::cmp::Ordering::Equal => 0, core::cmp::Ordering::Greater => 1, @@ -240,7 +240,7 @@ pub fn caml_pasta_fp_mut_square(mut x: ocaml::Pointer) { #[ocaml_gen::func] #[ocaml::func] pub fn caml_pasta_fp_compare(x: ocaml::Pointer, y: ocaml::Pointer) -> ocaml::Int { - match x.as_ref().0.cmp(&y.as_ref().0) { + match x.as_ref().0.into_repr().cmp(&y.as_ref().0.into_repr()) { Less => -1, Equal => 0, Greater => 1, diff --git a/src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fq.rs b/src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fq.rs index 8fbca9da595b..bc81f5962a67 100644 --- a/src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fq.rs +++ b/src/lib/crypto/kimchi_bindings/stubs/src/arkworks/pasta_fq.rs @@ -36,7 +36,7 @@ impl CamlFq { unsafe extern "C" fn ocaml_compare(x: ocaml::Raw, y: ocaml::Raw) -> i32 { let x = x.as_pointer::(); let y = y.as_pointer::(); - match x.as_ref().0.cmp(&y.as_ref().0) { + match x.as_ref().0.into_repr().cmp(&y.as_ref().0.into_repr()) { core::cmp::Ordering::Less => -1, core::cmp::Ordering::Equal => 0, core::cmp::Ordering::Greater => 1, @@ -241,7 +241,7 @@ pub fn caml_pasta_fq_mut_square(mut x: ocaml::Pointer) { #[ocaml_gen::func] #[ocaml::func] pub fn caml_pasta_fq_compare(x: ocaml::Pointer, y: ocaml::Pointer) -> ocaml::Int { - match x.as_ref().0.cmp(&y.as_ref().0) { + match x.as_ref().0.into_repr().cmp(&y.as_ref().0.into_repr()) { Less => -1, Equal => 0, Greater => 1, From 6fcd9395c5643bad6e799233b96929633d8d88fc Mon Sep 17 00:00:00 2001 From: Bruno Deferrari Date: Wed, 22 Nov 2023 18:05:14 -0300 Subject: [PATCH 3/4] Field: More efficient conversion of into Bignum_bigint.t --- src/lib/crypto/kimchi_backend/common/field.ml | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/lib/crypto/kimchi_backend/common/field.ml b/src/lib/crypto/kimchi_backend/common/field.ml index 77af0a352b12..173bdc7feb15 100644 --- a/src/lib/crypto/kimchi_backend/common/field.ml +++ b/src/lib/crypto/kimchi_backend/common/field.ml @@ -179,20 +179,30 @@ module Make (F : Input_intf) : let of_sexpable = of_bigint end) + let zero = of_int 0 + let to_bignum_bigint n = - let rec go i two_to_the_i acc = - if Int.equal i size_in_bits then acc - else - let acc' = - if Bigint.test_bit n i then Bignum_bigint.(acc + two_to_the_i) - else acc - in - go (i + 1) Bignum_bigint.(two_to_the_i + two_to_the_i) acc' - in - go 0 Bignum_bigint.one Bignum_bigint.zero - - let hash_fold_t s x = - Bignum_bigint.hash_fold_t s (to_bignum_bigint (to_bigint x)) + (* For non-zero values, conversion is done by creating the bin_prot representation + of the [Bignum_bigit.t] value, and then parsing it with bin_prot. *) + match compare n zero with + | 0 -> + Bignum_bigint.zero + | c -> + (* Tag for positive values is 1, negative is 2: + https://github.com/janestreet/bignum/blob/6c63419787a4e209e85befd3d823fff2790677e0/bigint/src/bigint.ml#L27-L30 *) + let tag_byte = if c > 0 then '\x01' else '\x02' in + let bytes = to_bytes n in + let len = Bytes.length bytes in + let size_byte = Char.of_int_exn len in + let buf = Bytes.create (2 + len) in + (* First byte is the tag, second the amount of bytes *) + Bytes.unsafe_set buf 0 tag_byte ; + Bytes.unsafe_set buf 1 size_byte ; + (* Copy the bytes representation of the value, skip the tag and size bytes *) + Bytes.unsafe_blit ~src:bytes ~src_pos:0 ~dst_pos:2 ~len ~dst:buf ; + Bin_prot.Reader.of_bytes Bignum_bigint.Stable.V1.bin_reader_t buf + + let hash_fold_t s x = Bignum_bigint.hash_fold_t s (to_bignum_bigint x) let hash = Hash.of_fold hash_fold_t From b570c8f03b73e032a45ceb5bffb3e848bd4e0160 Mon Sep 17 00:00:00 2001 From: Bruno Deferrari Date: Mon, 27 Nov 2023 19:06:16 -0300 Subject: [PATCH 4/4] Do not use binprot serialization when converting bigint fields, use Zarith directly --- src/lib/crypto/kimchi_backend/common/dune | 1 + src/lib/crypto/kimchi_backend/common/field.ml | 32 ++++++------------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/src/lib/crypto/kimchi_backend/common/dune b/src/lib/crypto/kimchi_backend/common/dune index 3d75a75b7ca5..9a7cb8b8c919 100644 --- a/src/lib/crypto/kimchi_backend/common/dune +++ b/src/lib/crypto/kimchi_backend/common/dune @@ -25,6 +25,7 @@ base.caml ppx_inline_test.config bignum.bigint + zarith base.base_internalhash_types ;; local libraries tuple_lib diff --git a/src/lib/crypto/kimchi_backend/common/field.ml b/src/lib/crypto/kimchi_backend/common/field.ml index 173bdc7feb15..12261d7b9f52 100644 --- a/src/lib/crypto/kimchi_backend/common/field.ml +++ b/src/lib/crypto/kimchi_backend/common/field.ml @@ -179,28 +179,16 @@ module Make (F : Input_intf) : let of_sexpable = of_bigint end) - let zero = of_int 0 - - let to_bignum_bigint n = - (* For non-zero values, conversion is done by creating the bin_prot representation - of the [Bignum_bigit.t] value, and then parsing it with bin_prot. *) - match compare n zero with - | 0 -> - Bignum_bigint.zero - | c -> - (* Tag for positive values is 1, negative is 2: - https://github.com/janestreet/bignum/blob/6c63419787a4e209e85befd3d823fff2790677e0/bigint/src/bigint.ml#L27-L30 *) - let tag_byte = if c > 0 then '\x01' else '\x02' in - let bytes = to_bytes n in - let len = Bytes.length bytes in - let size_byte = Char.of_int_exn len in - let buf = Bytes.create (2 + len) in - (* First byte is the tag, second the amount of bytes *) - Bytes.unsafe_set buf 0 tag_byte ; - Bytes.unsafe_set buf 1 size_byte ; - (* Copy the bytes representation of the value, skip the tag and size bytes *) - Bytes.unsafe_blit ~src:bytes ~src_pos:0 ~dst_pos:2 ~len ~dst:buf ; - Bin_prot.Reader.of_bytes Bignum_bigint.Stable.V1.bin_reader_t buf + let to_bignum_bigint = + let zero = of_int 0 in + let one = of_int 1 in + fun n -> + if equal n zero then Bignum_bigint.zero + else if equal n one then Bignum_bigint.one + else + Bytes.unsafe_to_string + ~no_mutation_while_string_reachable:(to_bytes n) + |> Z.of_bits |> Bignum_bigint.of_zarith_bigint let hash_fold_t s x = Bignum_bigint.hash_fold_t s (to_bignum_bigint x)