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

Rust 1.74 #16416

Draft
wants to merge 29 commits into
base: dw/use-latest-master-proof-systems
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4385e33
Deps/proof-systems: bump up
dannywillems Dec 9, 2024
e27eb17
Submodules/kimchi-vendors: remove temporarily
dannywillems Dec 9, 2024
ce13ec2
kimchi-bindings/wasm: simply reordering deps
dannywillems Dec 9, 2024
f671fae
Kimchi-bindings/stubs: reorganize deps in Cargo.toml
dannywillems Dec 9, 2024
60353ad
Kimchi-bindings/stubs: stop ordering to cargo to use vendored srcs
dannywillems Dec 9, 2024
75b29dc
Kimchi_bindings/wasm: match wasm-binding version with proof-systems
dannywillems Dec 9, 2024
e8014cc
Kimchi-bindings/stubs: enforce 80 characters per line
dannywillems Dec 9, 2024
dfdb0f8
Kimchi_bindings/stubs: make it compilable with latest proof-systems
dannywillems Dec 9, 2024
a1441a6
Kimchi-bindings/wasm: make clippy a bit happier
dannywillems Dec 9, 2024
fb7ac1e
Kimchi-bindings/stubs: make compilable with latest proof-systems
dannywillems Dec 9, 2024
6b98a9d
Kimchi-bindings/stubs: remove --offline parameters to build Rust
dannywillems Dec 9, 2024
5e3c507
Kimchi-bindings/stubs: update Cargo.lock
dannywillems Dec 9, 2024
02bcee2
kimchi-bindings/stubs: make clippy happier
dannywillems Dec 9, 2024
b1f0a26
kimchi-bindings/stubs: enforce 80 characters limit
dannywillems Dec 9, 2024
8255311
Deps/proof-systems: bump up
dannywillems Dec 9, 2024
3593766
Deps/proof-systems: bump up
dannywillems Dec 9, 2024
fe13da1
Kimchi-bindings/stubs: bring back ocaml_str with inherent cache
dannywillems Dec 9, 2024
5c838a5
Pickles: update scalars.ml, result of `dune build src/lib/pickles`
dannywillems Dec 9, 2024
1b76fed
Deps/proof-systems: bump up
dannywillems Dec 9, 2024
e696ce8
Kimchi-bindings/wasm: update Cargo.lock
dannywillems Dec 9, 2024
26a78dc
Kimchi-bindings/stubs: update Cargo.lock
dannywillems Dec 9, 2024
becb09c
Kimchi-bindings/wasm: downgrade wasm-bindgen to 0.2.87
dannywillems Dec 9, 2024
c1c89df
Deps/proof-systems: bump up
dannywillems Dec 9, 2024
2cc26fa
Cargo.toml: enforcing 80 characters limit
dannywillems Dec 9, 2024
2ec9256
rust-impure shell loads with 1.74
martyall Dec 12, 2024
1b667e5
remove refs to 1.72, change to 1.74
martyall Dec 13, 2024
d0b632d
update proof systems
martyall Dec 13, 2024
a4a992f
formatting
martyall Dec 13, 2024
308fa27
don't panic if you need to build wasm bindings
martyall Dec 13, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,4 @@
url = https://github.com/o1-labs/snarky
[submodule "src/lib/crypto/proof-systems"]
path = src/lib/crypto/proof-systems
url = https://github.com/o1-labs/proof-systems.git
[submodule "src/lib/crypto/kimchi_bindings/stubs/kimchi-stubs-vendors"]
path = src/lib/crypto/kimchi_bindings/stubs/kimchi-stubs-vendors
url = https://github.com/MinaProtocol/kimchi-stubs-vendors.git
url = https://github.com/o1-labs/proof-systems.git
6 changes: 2 additions & 4 deletions nix/rust.nix
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ let
# override stdenv.targetPlatform here, if neccesary
};
toolchainHashes = {
"1.72" = "sha256-dxE7lmCFWlq0nl/wKcmYvpP9zqQbBitAQgZ1zx9Ooik=";
"nightly-2023-09-01" =
"sha256-zek9JAnRaoX8V0U2Y5ssXVe9tvoQ0ERGXfUCUGYdrMA=";
"1.74" = "sha256-PjvuouwTsYfNKW5Vi5Ye7y+lL7SsWGBxCtBOOm2z14c=";
# copy the placeholder line with the correct toolchain name when adding a new toolchain
# That is,
# 1. Put the correct version name;
Expand All @@ -19,7 +17,7 @@ let
# error: hash mismatch in fixed-output derivation '/nix/store/XXXXX'
# specified: sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
# got: sha256-Q9UgzzvxLi4x9aWUJTn+/5EXekC98ODRU1TwhUs9RnY=
"placeholder" = "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
# "placeholder" = "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
};
# rust-toolchain.toml -> { rustc, cargo, rust-analyzer, ... }
rustChannelFromToolchainFileOf = file:
Expand Down
8 changes: 1 addition & 7 deletions src/lib/crypto/kimchi_bindings/stubs/.cargo/config
Original file line number Diff line number Diff line change
@@ -1,8 +1,2 @@
[build]
rustflags = ["-C", "link-args=-Wl,-undefined,dynamic_lookup"]

[source.crates-io]
replace-with = "vendored-sources"

[source.vendored-sources]
directory = "kimchi-stubs-vendors"
rustflags = ["-C", "link-args=-Wl,-undefined,dynamic_lookup"]
27 changes: 15 additions & 12 deletions src/lib/crypto/kimchi_bindings/stubs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 10 additions & 10 deletions src/lib/crypto/kimchi_bindings/stubs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,31 @@ crate-type = ["lib", "staticlib"]

[dependencies]
array-init = "2.0.0"
rmp-serde = "1.1.2"
libc = "0.2.62"
num-bigint = { version = "0.4.4", features = [ "rand", "serde" ] }
# ocaml-specific
ocaml = { version = "0.22.2", features = ["no-caml-startup"] }
ocaml-gen = "0.1.5"
once_cell = "1.10.0"
paste = "1.0.5"
rand = "0.8.5"
rayon = "1.5.0"
rmp-serde = "1.1.2"
serde = "1.0.130"
serde_json = "1.0.103"
sprs = { version = "0.11.0", features = ["multi_thread"] }
once_cell = "1.10.0"

# arkworks
ark-ff = { version = "0.4.2", features = ["parallel", "asm"] }
ark-serialize = "0.4.2"
ark-ec = { version = "0.4.2", features = ["parallel"] }
ark-ff = { version = "0.4.2", features = ["parallel", "asm"] }
ark-poly = { version = "0.4.2", features = ["parallel"] }
ark-serialize = "0.4.2"

# proof-systems
poly-commitment = { path = "../../proof-systems/poly-commitment", features = ["ocaml_types"] }
groupmap = { path = "../../proof-systems/groupmap" }
kimchi = { path = "../../proof-systems/kimchi", features = ["ocaml_types"] }
mina-curves = { path = "../../proof-systems/curves" }
o1-utils = { path = "../../proof-systems/utils" }
mina-poseidon = { path = "../../proof-systems/poseidon" }
kimchi = { path = "../../proof-systems/kimchi", features = ["ocaml_types"] }
o1-utils = { path = "../../proof-systems/utils" }
poly-commitment = { path = "../../proof-systems/poly-commitment", features = ["ocaml_types"] }

# ocaml-specific
ocaml = { version = "0.22.2", features = ["no-caml-startup"] }
ocaml-gen = "0.1.5"
4 changes: 2 additions & 2 deletions src/lib/crypto/kimchi_bindings/stubs/dune
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
(setenv
RUSTFLAGS
%{read:rustflags.sexp}
(run cargo build --release --offline)))
(run cargo build --release)))
(run
cp
%{read:../dune-build-root}/cargo_kimchi_stubs/release/libwires_15_stubs.a
Expand Down Expand Up @@ -161,7 +161,7 @@
(setenv
CARGO_TARGET_DIR
"%{read:../dune-build-root}/cargo_kimchi_bindgen"
(run cargo run %{targets} --offline))
(run cargo run %{targets}))
(run ocamlformat -i %{targets}))))

;; this is used by nix
Expand Down
Submodule kimchi-stubs-vendors deleted from 2201f3
2 changes: 1 addition & 1 deletion src/lib/crypto/kimchi_bindings/stubs/rust-toolchain.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
# 4. figure out the hashes of the (now obsolete) docker images used in CI rules that are failing, grep for these hashes and replace them with the new hashes

[toolchain]
channel = "1.72"
channel = "1.74"
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ use crate::arkworks::CamlBigInteger256;
use crate::caml::caml_bytes_string::CamlBytesString;
use ark_ff::{FftField, Field, One, PrimeField, UniformRand, Zero};
use ark_poly::{EvaluationDomain, Radix2EvaluationDomain as Domain};
use ark_serialize::{CanonicalSerialize, CanonicalDeserialize};
use mina_curves::pasta::fields::{fp::{Fp, FpParameters as Fp_params}, fft::FpParameters};
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
use mina_curves::pasta::fields::{
fft::FpParameters,
fp::{Fp, FpParameters as Fp_params},
};
use num_bigint::BigUint;
use rand::rngs::StdRng;
use std::{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ use crate::arkworks::CamlBigInteger256;
use crate::caml::caml_bytes_string::CamlBytesString;
use ark_ff::{FftField, Field, One, PrimeField, UniformRand, Zero};
use ark_poly::{EvaluationDomain, Radix2EvaluationDomain as Domain};
use ark_serialize::{CanonicalSerialize, CanonicalDeserialize};
use mina_curves::pasta::{fields::{fq::FqParameters as Fq_params, fft::FpParameters}, Fq};
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
use mina_curves::pasta::{
fields::{fft::FpParameters, fq::FqParameters as Fq_params},
Fq,
};
use num_bigint::BigUint;
use rand::rngs::StdRng;
use std::{
Expand Down
27 changes: 15 additions & 12 deletions src/lib/crypto/kimchi_bindings/stubs/src/caml/caml_pointer.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
macro_rules! impl_caml_pointer {
($name: ident => $typ: ty) => {
#[derive(std::fmt::Debug, Clone, ::ocaml_gen::CustomType)]
pub struct $name(pub ::std::rc::Rc<$typ>);
pub struct $name(pub ::std::rc::Rc<std::cell::UnsafeCell<$typ>>);
Copy link
Member

Choose a reason for hiding this comment

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

Note as it is important for the Caml binding: UnsafeCell is implemented as:

#[lang = "unsafe_cell"]
#[stable(feature = "rust1", since = "1.0.0")]
#[repr(transparent)]
#[rustc_pub_transparent]
pub struct UnsafeCell<T: ?Sized> {
    value: T,
}

Copy link
Member

Choose a reason for hiding this comment

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

I guess we're adding a new indirection? We might lose some performances?

Copy link
Member Author

@martyall martyall Dec 13, 2024

Choose a reason for hiding this comment

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

Note as it is important for the Caml binding: UnsafeCell is implemented as:

The nightly tests were passing with this change, does this mean that that the ocaml bindings lib was able to resolve this on its own or is it just coincidence because maybe the runtime rep is the same

Copy link
Member Author

@martyall martyall Dec 15, 2024

Choose a reason for hiding this comment

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

I guess we're adding a new indirection? We might lose some performances?

My intuition is that this UnsafeCell should be optimized away. We can discuss


impl $name {
extern "C" fn caml_pointer_finalize(v: ocaml::Raw) {
Expand All @@ -12,8 +12,8 @@ macro_rules! impl_caml_pointer {
}

extern "C" fn caml_pointer_compare(_: ocaml::Raw, _: ocaml::Raw) -> i32 {
// Always return equal. We can use this for sanity checks, and anything else using this
// would be broken anyway.
// Always return equal. We can use this for sanity checks, and
// anything else using this would be broken anyway.
0
}
}
Expand All @@ -32,15 +32,17 @@ macro_rules! impl_caml_pointer {

impl $name {
pub fn create(x: $typ) -> $name {
$name(::std::rc::Rc::new(x))
$name(::std::rc::Rc::new(std::cell::UnsafeCell::new(x)))
}
}

impl ::std::ops::Deref for $name {
type Target = $typ;

fn deref(&self) -> &Self::Target {
&*self.0
unsafe {
martyall marked this conversation as resolved.
Show resolved Hide resolved
&*self.0.get()
}
}
}

Expand All @@ -49,14 +51,15 @@ macro_rules! impl_caml_pointer {
unsafe {
// Wholely unsafe, Batman!
// We would use [`get_mut_unchecked`] here, but it is nightly-only.
// Instead, we get coerce our constant pointer to a mutable pointer, in the knowledge
// that
// * all of our mutations called from OCaml are blocking, so we won't have multiple
// live mutable references live simultaneously, and
// * the underlying pointer is in the correct state to be mutable, since we can call
// [`get_mut_unchecked`] in nightly, or can call [`get_mut`] and unwrap if this is
// Instead, we use UnsafeCell in the knowledge that
// * all of our mutations called from OCaml are blocking, so
// we won't have multiple live mutable references
// simultaneously, and
// * the underlying pointer is in the correct state to be
// mutable, since we can call [`get_mut_unchecked`] in
// nightly, or can call [`get_mut`] and unwrap if this is
// the only live reference.
&mut *(((&*self.0) as *const Self::Target) as *mut Self::Target)
Copy link
Member Author

Choose a reason for hiding this comment

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

1.74 throws a compiler error here. There might be some other way to get it to stop complaining, but I think this is equivalent without the illegal cast

&mut *self.0.get()
Copy link
Member

Choose a reason for hiding this comment

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

Note: implementation of get.

    #[inline(always)]
    #[stable(feature = "rust1", since = "1.0.0")]
    #[rustc_const_stable(feature = "const_unsafecell_get", since = "1.32.0")]
    #[rustc_never_returns_null_ptr]
    pub const fn get(&self) -> *mut T {
        // We can just cast the pointer from `UnsafeCell<T>` to `T` because of
        // #[repr(transparent)]. This exploits std's special status, there is
        // no guarantee for user code that this will work in future versions of the compiler!
        self as *const UnsafeCell<T> as *const T as *mut T
    }

Copy link
Member

Choose a reason for hiding this comment

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

Nit: cargo fmt reformats this line. It seems we do not run in the Mina CI clippy nor fmt.

Copy link
Member Author

@martyall martyall Dec 13, 2024

Choose a reason for hiding this comment

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

Note: implementation of get.

This makes me think someone was trying to get around using UnsafeCell when they wrote this the first time. I would hope that this would be optimized away

}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib/crypto/kimchi_bindings/stubs/src/field_vector.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//! We implement a custom type for field vectors in order to quickly build field vectors from the OCaml side and avoid large vector clones.
//! We implement a custom type for field vectors in order to quickly build field
//! vectors from the OCaml side and avoid large vector clones.

use paste::paste;

Expand Down
Loading