diff --git a/networks/ethereum/schnorr/contracts/Schnorr.sol b/networks/ethereum/schnorr/contracts/Schnorr.sol index 7405051a..57269c5f 100644 --- a/networks/ethereum/schnorr/contracts/Schnorr.sol +++ b/networks/ethereum/schnorr/contracts/Schnorr.sol @@ -1,7 +1,13 @@ // SPDX-License-Identifier: AGPL-3.0-only pragma solidity ^0.8.26; -// See https://github.com/noot/schnorr-verify for implementation details +/// @title A library for verifying Schnorr signatures +/// @author Luke Parker +/// @author Elizabeth Binks +/// @notice Verifies a Schnorr signature for a specified public key +/// @dev This contract is not complete. Only certain public keys are compatible +/// @dev See https://github.com/serai-dex/serai/blob/next/networks/ethereum/schnorr/src/tests/premise.rs for implementation details +// TODO: Pin to a specific branch/commit once `next` is merged into `develop` library Schnorr { // secp256k1 group order uint256 private constant Q = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141; @@ -11,31 +17,39 @@ library Schnorr { // 2) An x-coordinate < Q uint8 private constant KEY_PARITY = 27; - // px := public key x-coordinate, where the public key has an even y-coordinate - // message := the message signed - // c := Schnorr signature challenge - // s := Schnorr signature solution - function verify(bytes32 px, bytes32 message, bytes32 c, bytes32 s) internal pure returns (bool) { + /// @notice Verifies a Schnorr signature for the specified public key + /// @dev The y-coordinate of the public key is assumed to be even + /// @dev The x-coordinate of the public key is assumed to be less than the order of secp256k1 + /// @dev The challenge is calculated as `keccak256(abi.encodePacked(address(R), public_key, message))` where `R` is the commitment to the Schnorr signature's nonce + /// @param public_key The x-coordinate of the public key + /// @param message The (hash of the) message signed + /// @param c The challenge for the Schnorr signature + /// @param s The response to the challenge for the Schnorr signature + /// @return If the signature is valid + function verify(bytes32 public_key, bytes32 message, bytes32 c, bytes32 s) + internal + pure + returns (bool) + { // ecrecover = (m, v, r, s) -> key - // We instead pass the following to obtain the nonce (not the key) - // Then we hash it and verify it matches the challenge - bytes32 sa = bytes32(Q - mulmod(uint256(s), uint256(px), Q)); - bytes32 ca = bytes32(Q - mulmod(uint256(c), uint256(px), Q)); + // We instead pass the following to recover the Schnorr signature's nonce (not a public key) + bytes32 sa = bytes32(Q - mulmod(uint256(s), uint256(public_key), Q)); + bytes32 ca = bytes32(Q - mulmod(uint256(c), uint256(public_key), Q)); /* - The ecrecover precompile checks `r` and `s` (`px` and `ca`) are non-zero, - banning the two keys with zero for their x-coordinate and zero challenge. - Each has negligible probability of occuring (assuming zero x-coordinates - are even on-curve in the first place). + The ecrecover precompile checks `r` and `s` (`public_key` and `ca`) are non-zero, banning the + two keys with zero for their x-coordinate and zero challenges. Each already only had a + negligible probability of occuring (assuming zero x-coordinates are even on-curve in the first + place). - `sa` is not checked to be non-zero yet it does not need to be. The inverse - of it is never taken. + `sa` is not checked to be non-zero yet it does not need to be. The inverse of it is never + taken. */ - address R = ecrecover(sa, KEY_PARITY, px, ca); + address R = ecrecover(sa, KEY_PARITY, public_key, ca); // The ecrecover failed if (R == address(0)) return false; // Check the signature is correct by rebuilding the challenge - return c == keccak256(abi.encodePacked(R, px, message)); + return c == keccak256(abi.encodePacked(R, public_key, message)); } } diff --git a/networks/ethereum/schnorr/contracts/tests/Schnorr.sol b/networks/ethereum/schnorr/contracts/tests/Schnorr.sol index 412786a3..92922f5e 100644 --- a/networks/ethereum/schnorr/contracts/tests/Schnorr.sol +++ b/networks/ethereum/schnorr/contracts/tests/Schnorr.sol @@ -3,7 +3,19 @@ pragma solidity ^0.8.26; import "../Schnorr.sol"; +/// @title A thin wrapper around the library for verifying Schnorr signatures to test it with +/// @author Luke Parker +/// @author Elizabeth Binks contract TestSchnorr { + /// @notice Verifies a Schnorr signature for the specified public key + /// @dev The y-coordinate of the public key is assumed to be even + /// @dev The x-coordinate of the public key is assumed to be less than the order of secp256k1 + /// @dev The challenge is calculated as `keccak256(abi.encodePacked(address(R), public_key, message))` where `R` is the commitment to the Schnorr signature's nonce + /// @param public_key The x-coordinate of the public key + /// @param message The (hash of the) message signed + /// @param c The challenge for the Schnorr signature + /// @param s The response to the challenge for the Schnorr signature + /// @return If the signature is valid function verify(bytes32 public_key, bytes calldata message, bytes32 c, bytes32 s) external pure diff --git a/networks/ethereum/schnorr/src/public_key.rs b/networks/ethereum/schnorr/src/public_key.rs index 3c39552f..fbf00584 100644 --- a/networks/ethereum/schnorr/src/public_key.rs +++ b/networks/ethereum/schnorr/src/public_key.rs @@ -31,8 +31,7 @@ impl PublicKey { let x_coordinate = affine.x(); // Return None if the x-coordinate isn't mutual to both fields - // While reductions shouldn't be an issue, it's one less headache/concern to have - // The trivial amount of public keys this makes non-representable aren't a concern + // The trivial amount of public keys this makes non-representable aren't considered a concern if >::reduce_bytes(&x_coordinate).to_repr() != x_coordinate { None?; } diff --git a/networks/ethereum/schnorr/src/signature.rs b/networks/ethereum/schnorr/src/signature.rs index 1af1d60f..105e6d4d 100644 --- a/networks/ethereum/schnorr/src/signature.rs +++ b/networks/ethereum/schnorr/src/signature.rs @@ -20,11 +20,17 @@ pub struct Signature { impl Signature { /// Construct a new `Signature`. #[must_use] - pub fn new(c: Scalar, s: Scalar) -> Signature { - Signature { c, s } + pub fn new(c: Scalar, s: Scalar) -> Option { + if bool::from(c.is_zero()) { + None?; + } + Some(Signature { c, s }) } /// The challenge for a signature. + /// + /// With negligible probability, this MAY return 0 which will create an invalid/unverifiable + /// signature. #[must_use] pub fn challenge(R: ProjectivePoint, key: &PublicKey, message: &[u8]) -> Scalar { // H(R || A || m) diff --git a/networks/ethereum/schnorr/src/tests/mod.rs b/networks/ethereum/schnorr/src/tests/mod.rs index 90774e30..4b23a0ba 100644 --- a/networks/ethereum/schnorr/src/tests/mod.rs +++ b/networks/ethereum/schnorr/src/tests/mod.rs @@ -17,6 +17,9 @@ use alloy_node_bindings::{Anvil, AnvilInstance}; use crate::{PublicKey, Signature}; +mod public_key; +pub(crate) use public_key::test_key; +mod signature; mod premise; #[expect(warnings)] @@ -88,12 +91,7 @@ async fn test_verify() { let (_anvil, provider, address) = setup_test().await; for _ in 0 .. 100 { - let (key, public_key) = loop { - let key = Scalar::random(&mut OsRng); - if let Some(public_key) = PublicKey::new(ProjectivePoint::GENERATOR * key) { - break (key, public_key); - } - }; + let (key, public_key) = test_key(); let nonce = Scalar::random(&mut OsRng); let mut message = vec![0; 1 + usize::try_from(OsRng.next_u32() % 256).unwrap()]; @@ -102,11 +100,37 @@ async fn test_verify() { let c = Signature::challenge(ProjectivePoint::GENERATOR * nonce, &public_key, &message); let s = nonce + (c * key); - let sig = Signature::new(c, s); + let sig = Signature::new(c, s).unwrap(); assert!(sig.verify(&public_key, &message)); assert!(call_verify(&provider, address, &public_key, &message, &sig).await); + + // Test setting `s = 0` doesn't pass verification + { + let zero_s = Signature::new(c, Scalar::ZERO).unwrap(); + assert!(!zero_s.verify(&public_key, &message)); + assert!(!call_verify(&provider, address, &public_key, &message, &zero_s).await); + } + // Mutate the message and make sure the signature now fails to verify - message[0] = message[0].wrapping_add(1); - assert!(!call_verify(&provider, address, &public_key, &message, &sig).await); + { + let mut message = message.clone(); + message[0] = message[0].wrapping_add(1); + assert!(!sig.verify(&public_key, &message)); + assert!(!call_verify(&provider, address, &public_key, &message, &sig).await); + } + + // Mutate c and make sure the signature now fails to verify + { + let mutated_c = Signature::new(c + Scalar::ONE, s).unwrap(); + assert!(!mutated_c.verify(&public_key, &message)); + assert!(!call_verify(&provider, address, &public_key, &message, &mutated_c).await); + } + + // Mutate s and make sure the signature now fails to verify + { + let mutated_s = Signature::new(c, s + Scalar::ONE).unwrap(); + assert!(!mutated_s.verify(&public_key, &message)); + assert!(!call_verify(&provider, address, &public_key, &message, &mutated_s).await); + } } } diff --git a/networks/ethereum/schnorr/src/tests/premise.rs b/networks/ethereum/schnorr/src/tests/premise.rs index 01571a43..28d9135d 100644 --- a/networks/ethereum/schnorr/src/tests/premise.rs +++ b/networks/ethereum/schnorr/src/tests/premise.rs @@ -12,7 +12,7 @@ use k256::{ use alloy_core::primitives::Address; -use crate::{PublicKey, Signature}; +use crate::{Signature, tests::test_key}; // The ecrecover opcode, yet with if the y is odd replacing v fn ecrecover(message: Scalar, odd_y: bool, r: Scalar, s: Scalar) -> Option<[u8; 20]> { @@ -64,12 +64,7 @@ fn test_ecrecover() { // of efficiently verifying Schnorr signatures in an Ethereum contract #[test] fn nonce_recovery_via_ecrecover() { - let (key, public_key) = loop { - let key = Scalar::random(&mut OsRng); - if let Some(public_key) = PublicKey::new(ProjectivePoint::GENERATOR * key) { - break (key, public_key); - } - }; + let (key, public_key) = test_key(); let nonce = Scalar::random(&mut OsRng); let R = ProjectivePoint::GENERATOR * nonce; @@ -81,26 +76,28 @@ fn nonce_recovery_via_ecrecover() { let s = nonce + (c * key); /* - An ECDSA signature is `(r, s)` with `s = (H(m) + rx) / k`, where: - - `m` is the message + An ECDSA signature is `(r, s)` with `s = (m + (r * x)) / k`, where: + - `m` is the hash of the message - `r` is the x-coordinate of the nonce, reduced into a scalar - `x` is the private key - `k` is the nonce We fix the recovery ID to be for the even key with an x-coordinate < the order. Accordingly, - `kG = Point::from(Even, r)`. This enables recovering the public key via - `((s Point::from(Even, r)) - H(m)G) / r`. + `k * G = Point::from(Even, r)`. This enables recovering the public key via + `((s * Point::from(Even, r)) - (m * G)) / r`. We want to calculate `R` from `(c, s)` where `s = r + cx`. That means we need to calculate - `sG - cX`. + `(s * G) - (c * X)`. - We can calculate `sG - cX` with `((s Point::from(Even, r)) - H(m)G) / r` if: - - Latter `r` = `X.x` - - Latter `s` = `c` - - `H(m)` = former `s` - This gets us to `(cX - sG) / X.x`. If we additionally scale the latter's `s, H(m)` values (the - former's `c, s` values) by `X.x`, we get `cX - sG`. This just requires negating each to achieve - `sG - cX`. + We can calculate `(s * G) - (c * X)` with `((s * Point::from(Even, r)) - (m * G)) / r` if: + - ECDSA `r` = `X.x`, the x-coordinate of the Schnorr public key + - ECDSA `s` = `c`, the Schnorr signature's challenge + - ECDSA `m` = Schnorr `s` + This gets us to `((c * X) - (s * G)) / X.x`. If we additionally scale the ECDSA `s, m` values + (the Schnorr `c, s` values) by `X.x`, we get `(c * X) - (s * G)`. This just requires negating + to achieve `(s * G) - (c * X)`. + + With `R`, we can recalculate and compare the challenges to confirm the signature is valid. */ let x_scalar = >::reduce_bytes(&public_key.point().to_affine().x()); let sa = -(s * x_scalar); diff --git a/networks/ethereum/schnorr/src/tests/public_key.rs b/networks/ethereum/schnorr/src/tests/public_key.rs new file mode 100644 index 00000000..9294cbac --- /dev/null +++ b/networks/ethereum/schnorr/src/tests/public_key.rs @@ -0,0 +1,69 @@ +use rand_core::OsRng; + +use subtle::Choice; +use group::ff::{Field, PrimeField}; +use k256::{ + elliptic_curve::{ + FieldBytesEncoding, + ops::Reduce, + point::{AffineCoordinates, DecompressPoint}, + }, + AffinePoint, ProjectivePoint, Scalar, U256 as KU256, +}; + +use crate::PublicKey; + +// Generates a key usable within tests +pub(crate) fn test_key() -> (Scalar, PublicKey) { + loop { + let key = Scalar::random(&mut OsRng); + let point = ProjectivePoint::GENERATOR * key; + if let Some(public_key) = PublicKey::new(point) { + // While here, test `PublicKey::point` and its serialization functions + assert_eq!(point, public_key.point()); + assert_eq!(PublicKey::from_eth_repr(public_key.eth_repr()).unwrap(), public_key); + return (key, public_key); + } + } +} + +#[test] +fn test_odd_key() { + // We generate a valid key to ensure there's not some distinct reason this key is invalid + let (_, key) = test_key(); + // We then take its point and negate it so its y-coordinate is odd + let odd = -key.point(); + assert!(PublicKey::new(odd).is_none()); +} + +#[test] +fn test_non_mutual_key() { + let mut x_coordinate = KU256::from(-(Scalar::ONE)).saturating_add(&KU256::ONE); + + let y_is_odd = Choice::from(0); + let non_mutual = loop { + if let Some(point) = Option::::from(AffinePoint::decompress( + &FieldBytesEncoding::encode_field_bytes(&x_coordinate), + y_is_odd, + )) { + break point; + } + x_coordinate = x_coordinate.saturating_add(&KU256::ONE); + }; + let x_coordinate = non_mutual.x(); + assert!(>::reduce_bytes(&x_coordinate).to_repr() != x_coordinate); + + // Even point whose x-coordinate isn't mutual to both fields (making it non-zero) + assert!(PublicKey::new(non_mutual.into()).is_none()); +} + +#[test] +fn test_zero_key() { + let y_is_odd = Choice::from(0); + if let Some(A_affine) = + Option::::from(AffinePoint::decompress(&[0; 32].into(), y_is_odd)) + { + let A = ProjectivePoint::from(A_affine); + assert!(PublicKey::new(A).is_none()); + } +} diff --git a/networks/ethereum/schnorr/src/tests/signature.rs b/networks/ethereum/schnorr/src/tests/signature.rs new file mode 100644 index 00000000..27c640f8 --- /dev/null +++ b/networks/ethereum/schnorr/src/tests/signature.rs @@ -0,0 +1,33 @@ +use rand_core::OsRng; + +use group::ff::Field; +use k256::Scalar; + +use crate::Signature; + +#[test] +fn test_zero_challenge() { + assert!(Signature::new(Scalar::ZERO, Scalar::random(&mut OsRng)).is_none()); +} + +#[test] +fn test_signature_serialization() { + let c = Scalar::random(&mut OsRng); + let s = Scalar::random(&mut OsRng); + let sig = Signature::new(c, s).unwrap(); + assert_eq!(sig.c(), c); + assert_eq!(sig.s(), s); + + let sig_bytes = sig.to_bytes(); + assert_eq!(Signature::from_bytes(sig_bytes).unwrap(), sig); + + { + let mut sig_written_bytes = vec![]; + sig.write(&mut sig_written_bytes).unwrap(); + assert_eq!(sig_bytes.as_slice(), &sig_written_bytes); + } + + let mut sig_read_slice = sig_bytes.as_slice(); + assert_eq!(Signature::read(&mut sig_read_slice).unwrap(), sig); + assert!(sig_read_slice.is_empty()); +} diff --git a/processor/ethereum/src/primitives/machine.rs b/processor/ethereum/src/primitives/machine.rs index 1762eb28..e3252f30 100644 --- a/processor/ethereum/src/primitives/machine.rs +++ b/processor/ethereum/src/primitives/machine.rs @@ -140,7 +140,7 @@ impl SignatureMachine for ActionSignatureMachine { self.machine.complete(shares).map(|signature| { let s = signature.s; let c = Signature::challenge(signature.R, &self.key, &self.action.message()); - Transaction(self.action, Signature::new(c, s)) + Transaction(self.action, Signature::new(c, s).unwrap()) }) } }