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

chore: bump curve25519-dalek from 3.2.1 to 4.1.3 #2252

Merged
merged 7 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .github/scripts/downstream-project-spl-common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,6 @@ fi

yihau marked this conversation as resolved.
Show resolved Hide resolved
# anza migration stopgap. can be removed when agave is fully recommended for public usage.
sed -i 's/solana-geyser-plugin-interface/agave-geyser-plugin-interface/g' ./Cargo.toml

# should be removed when spl bump their curve25519-dalek
sed -i "s/^curve25519-dalek =.*/curve25519-dalek = \"4.1.3\"/" token/confidential-transfer/proof-generation/Cargo.toml
88 changes: 52 additions & 36 deletions Cargo.lock

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

44 changes: 11 additions & 33 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ criterion-stats = "0.3.0"
crossbeam-channel = "0.5.13"
csv = "1.3.0"
ctrlc = "3.4.5"
curve25519-dalek = "3.2.1"
curve25519-dalek = { version = "4.1.3", features = ["digest", "rand_core"] }
dashmap = "5.5.3"
derivation-path = { version = "0.2.0", default-features = false }
derivative = "2.2.0"
Expand Down Expand Up @@ -543,38 +543,16 @@ solana-program = { path = "sdk/program" }
solana-zk-sdk = { path = "zk-sdk" }
solana-zk-token-sdk = { path = "zk-token-sdk" }

yihau marked this conversation as resolved.
Show resolved Hide resolved
# Our dependency tree has `curve25519-dalek` v3.2.1. They have removed the
# constraint in the next major release. The commit that removes the `zeroize`
# constraint was added to multiple release branches, but not to the 3.2 branch.
#
# `curve25519-dalek` maintainers are saying they do not want to invest any more
# time in the 3.2 release:
#
# https://github.com/dalek-cryptography/curve25519-dalek/issues/452#issuecomment-1749809428
#
# So we have to fork and create our own release, based on v3.2.1, with the
# commit that removed `zeroize` constraint on the `main` branch cherry-picked on
# top.
#
# `curve25519-dalek` v3.2.1 release:
#
# https://github.com/dalek-cryptography/curve25519-dalek/releases/tag/3.2.1
#
# Corresponds to commit
#
# https://github.com/dalek-cryptography/curve25519-dalek/commit/29e5c29b0e5c6821e4586af58b0d0891dd2ec639
#
# Comparison with `b500cdc2a920cd5bff9e2dd974d7b97349d61464`:
#
# https://github.com/dalek-cryptography/curve25519-dalek/compare/3.2.1...solana-labs:curve25519-dalek:b500cdc2a920cd5bff9e2dd974d7b97349d61464
#
# Or, using the branch name instead of the hash:
#
# https://github.com/dalek-cryptography/curve25519-dalek/compare/3.2.1...solana-labs:curve25519-dalek:3.2.1-unpin-zeroize
#
[patch.crates-io.curve25519-dalek]
git = "https://github.com/anza-xyz/curve25519-dalek.git"
rev = "b500cdc2a920cd5bff9e2dd974d7b97349d61464"
# curve25519-dalek uses the simd backend by default in v4 if possible,
# which has very slow performance on some platforms with opt-level 0,
# which is the default for dev and test builds.
# This slowdown causes certain interactions in the solana-test-validator,
# such as verifying ZK proofs in transactions, to take much more than 400ms,
# creating problems in the testing environment.
# To enable better performance in solana-test-validator during tests and dev builds,
# we override the opt-level to 3 for the crate.
[profile.dev.package.curve25519-dalek]
opt-level = 3
Comment on lines +554 to +555

Choose a reason for hiding this comment

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

Can you give more of an explanation about why this is added? I suggested this language in the SPL PR, but feel free to improve or clarify it:

curve25519-dalek uses the simd backend by default in v4 if possible, which has very slow performance on some platforms with opt-level 0, which is the default for dev and test builds. This slowdown causes certain interactions in the solana-test-validator, such as verifying ZK proofs in transactions, to take much more than 400ms, creating problems in the testing environment.
To enable better performance in solana-test-validator during tests and dev builds, we override the opt-level to 3 for the crate.

Copy link
Member Author

Choose a reason for hiding this comment

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

added! 7ba52fa


# Solana RPC nodes experience stalls when running with `tokio` containing this
# commit:
Expand Down
12 changes: 8 additions & 4 deletions curves/curve25519/src/edwards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ mod target_arch {
type Error = Curve25519Error;

fn try_from(pod: &PodEdwardsPoint) -> Result<Self, Self::Error> {
CompressedEdwardsY::from_slice(&pod.0)
let Ok(compressed_edwards_y) = CompressedEdwardsY::from_slice(&pod.0) else {
return Err(Curve25519Error::PodConversion);
};
compressed_edwards_y
.decompress()
.ok_or(Curve25519Error::PodConversion)
}
Expand All @@ -73,9 +76,10 @@ mod target_arch {
type Point = Self;

fn validate_point(&self) -> bool {
CompressedEdwardsY::from_slice(&self.0)
.decompress()
.is_some()
let Ok(compressed_edwards_y) = CompressedEdwardsY::from_slice(&self.0) else {
return false;
};
compressed_edwards_y.decompress().is_some()
}
}

Expand Down
12 changes: 8 additions & 4 deletions curves/curve25519/src/ristretto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ mod target_arch {
type Error = Curve25519Error;

fn try_from(pod: &PodRistrettoPoint) -> Result<Self, Self::Error> {
CompressedRistretto::from_slice(&pod.0)
let Ok(compressed_ristretto) = CompressedRistretto::from_slice(&pod.0) else {
return Err(Curve25519Error::PodConversion);
};
compressed_ristretto
.decompress()
.ok_or(Curve25519Error::PodConversion)
}
Expand All @@ -73,9 +76,10 @@ mod target_arch {
type Point = Self;

fn validate_point(&self) -> bool {
CompressedRistretto::from_slice(&self.0)
.decompress()
.is_some()
let Ok(compressed_ristretto) = CompressedRistretto::from_slice(&self.0) else {
return false;
};
compressed_ristretto.decompress().is_some()
}
}

Expand Down
8 changes: 6 additions & 2 deletions curves/curve25519/src/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ mod target_arch {
type Error = Curve25519Error;

fn try_from(pod: &PodScalar) -> Result<Self, Self::Error> {
Scalar::from_canonical_bytes(pod.0).ok_or(Curve25519Error::PodConversion)
Scalar::from_canonical_bytes(pod.0)
.into_option()
.ok_or(Curve25519Error::PodConversion)
}
}

Expand All @@ -32,7 +34,9 @@ mod target_arch {
type Error = Curve25519Error;

fn try_from(pod: PodScalar) -> Result<Self, Self::Error> {
Scalar::from_canonical_bytes(pod.0).ok_or(Curve25519Error::PodConversion)
Scalar::from_canonical_bytes(pod.0)
.into_option()
.ok_or(Curve25519Error::PodConversion)
}
}
}
4 changes: 2 additions & 2 deletions perf/src/sigverify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1276,7 +1276,7 @@ mod tests {
for _ in 0..1_000_000 {
thread_rng().fill(&mut input);
let ans = get_checked_scalar(&input);
let ref_ans = Scalar::from_canonical_bytes(input);
let ref_ans = Scalar::from_canonical_bytes(input).into_option();
if let Some(ref_ans) = ref_ans {
passed += 1;
assert_eq!(ans.unwrap(), ref_ans.to_bytes());
Expand Down Expand Up @@ -1311,7 +1311,7 @@ mod tests {
for _ in 0..1_000_000 {
thread_rng().fill(&mut input);
let ans = check_packed_ge_small_order(&input);
let ref_ge = CompressedEdwardsY::from_slice(&input);
let ref_ge = CompressedEdwardsY::from_slice(&input).unwrap();
if let Some(ref_element) = ref_ge.decompress() {
if ref_element.is_small_order() {
assert!(!ans);
Expand Down
Loading