Skip to content

Commit

Permalink
refactor(crypto): change hash_to_point algorithm (BFT-404) (#80)
Browse files Browse the repository at this point in the history
## What ❔

Changing the existing `hash_to_point` algorithm to the standard
try-and-increment algorithm:
* hash msg into 32 uniform random bytes
* reduce the 32 bytes to base field element for the `x` coordinate
* try to get the `y` coordinate by computing the square root of `x^3 +
b`
* if failed, iterate again with `x + 1`

## Why ❔

The existing hash to curve algorithm, which turns 32 bytes to
`G1::compressed` and maps it into `G1::uncompressed` isn’t valid; it
doesn’t produce a point with an unknown discrete log.

## Notes
* The function changed to return the successful nonce, in addition to
the point. It’s currently unused, but suppose to be used for onchain
verification, so that it won't require a loop. From the benchmarks I've
run, the avg number of iteration is around 2. (nonce=1)
* Need to review the crypto security of this approach, along with the
expected onchain gas usage, mainly for the square root. I already
prepared a compatible Solidity implementation, but I haven’t checked gas
yet.
* Need to review whether 32 uniform random bytes, to be reduced to `x`,
are sufficient, or that it should be extended to 64 bytes, given the
onchain verification gas usage implications.
* Need to review whether just picking [only] the positive square root is
good enough.
* The prime field modulus
(`0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47` /
`21888242871839275222246405745257275088696311157297823662689037894645226208583`)
is defined in the base field type macro definition, but I found no way
to access it, hence it is currently redefined in the implementation.
This should be fixed.
* `get_point_from_x` is already implemented for `G1Affine`, but it's
private and unaccessible. This should be fixed.

---------

Co-authored-by: Bruno França <[email protected]>
  • Loading branch information
moshababo and brunoffranca authored Apr 9, 2024
1 parent d17c018 commit b77d70c
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 23 deletions.
2 changes: 2 additions & 0 deletions node/Cargo.lock

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

2 changes: 2 additions & 0 deletions node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ kube = { version = "0.88.1", features = ["runtime", "derive"] }
k8s-openapi = { version = "0.21.0", features = ["latest"] }
jsonrpsee = { version = "0.21.0", features = ["server", "http-client"] }
tower = { version = "0.4.13" }
num-bigint = "0.4.4"
num-traits = "0.2.18"

# Note that "bench" profile inherits from "release" profile and
# "test" profile inherits from "dev" profile.
Expand Down
3 changes: 1 addition & 2 deletions node/actors/network/src/consensus/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Consensus network is a full graph of connections between all validators.
//! BFT consensus messages are exchanged over this network.
use crate::rpc::Rpc as _;
use crate::{config, gossip, io, noise, pool::PoolWatch, preface, rpc};
use crate::{config, gossip, io, noise, pool::PoolWatch, preface, rpc, rpc::Rpc as _};
use anyhow::Context as _;
use rand::seq::SliceRandom;
use std::{collections::HashSet, sync::Arc};
Expand Down
2 changes: 2 additions & 0 deletions node/libs/crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ rand04.workspace = true
thiserror.workspace = true
tracing.workspace = true
ff_ce.workspace = true
num-bigint.workspace = true
num-traits.workspace = true

[dev-dependencies]
criterion.workspace = true
Expand Down
47 changes: 33 additions & 14 deletions node/libs/crypto/src/bn254/hash.rs
Original file line number Diff line number Diff line change
@@ -1,27 +1,46 @@
//! Hash operations.
use ff_ce::{Field, PrimeField, SqrtField};
use num_bigint::BigUint;
use num_traits::Num;
use pairing::{
bn256::{G1Affine, G1Compressed},
EncodedPoint,
bn256::{fq, Fq, FqRepr, G1Affine},
CurveAffine,
};
use sha3::Digest as _;

/// Hashes an arbitrary message and maps it to an elliptic curve point in G1.
pub(crate) fn hash_to_g1(msg: &[u8]) -> G1Affine {
for i in 0..256 {
// Hash the message with the index as suffix.
let bytes: [u8; 32] = sha3::Keccak256::new()
.chain_update(msg)
.chain_update((i as u32).to_be_bytes())
.finalize()
.into();
pub(crate) fn hash_to_point(msg: &[u8]) -> (G1Affine, u8) {
let hash: [u8; 32] = sha3::Keccak256::new().chain_update(msg).finalize().into();

// Try to get a G1 point from the hash. The probability that this works is around 1/8.
let p = G1Compressed::from_fixed_bytes(bytes).into_affine();
if let Ok(p) = p {
return p;
let hash_num = BigUint::from_bytes_be(&hash);
let prime_field_modulus = BigUint::from_str_radix(
"30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47",
16,
)
.unwrap();
let x_num = hash_num % prime_field_modulus;
let x_arr: [u64; 4] = x_num.to_u64_digits().try_into().unwrap();
let mut x = Fq::from_repr(FqRepr(x_arr)).unwrap();

for i in 0..255 {
let p = get_point_from_x(x);
if let Some(p) = p {
return (p, i);
}
x.add_assign(&Fq::one());
}

// It should be statistically infeasible to finish the loop without finding a point.
unreachable!()
}

fn get_point_from_x(x: Fq) -> Option<G1Affine> {
// Compute x^3 + b.
let mut x3b = x;
x3b.square();
x3b.mul_assign(&x);
x3b.add_assign(&fq::B_COEFF);
// Try find the square root.
x3b.sqrt().map(|y| G1Affine::from_xy_unchecked(x, y))
}
11 changes: 6 additions & 5 deletions node/libs/crypto/src/bn254/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ impl SecretKey {

/// Produces a signature using this [`SecretKey`]
pub fn sign(&self, msg: &[u8]) -> Signature {
let hash_point = hash::hash_to_g1(msg);
let sig = hash_point.mul(self.0);
let (msg_point, _) = hash::hash_to_point(msg);
let sig = msg_point.mul(self.0);
Signature(sig)
}
}
Expand Down Expand Up @@ -166,10 +166,10 @@ impl Signature {
// public key when constructing it, this should never fail (in theory).
pk.verify().unwrap();

let hash_point = hash::hash_to_g1(msg);
let (msg_point, _) = hash::hash_to_point(msg);

// First pair: e(H(m): G1, pk: G2)
let a = Bn256::pairing(hash_point, pk.0);
let a = Bn256::pairing(msg_point, pk.0);
// Second pair: e(sig: G1, generator: G2)
let b = Bn256::pairing(self.0, G2Affine::one());

Expand Down Expand Up @@ -246,7 +246,8 @@ impl AggregateSignature {
// Second pair: e(H(m1): G1, pk1: G2) * ... * e(H(m1000): G1, pk1000: G2)
let mut b = Fq12::one();
for (msg, pk) in pairs {
b.mul_assign(&Bn256::pairing(hash::hash_to_g1(msg), pk.0))
let (msg_point, _) = hash::hash_to_point(msg);
b.mul_assign(&Bn256::pairing(msg_point, pk.0))
}

anyhow::ensure!(a == b, "Aggregate signature verification failure");
Expand Down
6 changes: 4 additions & 2 deletions node/libs/crypto/src/ed25519/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::ed25519::{PublicKey, SecretKey, Signature};
use crate::ByteFmt;
use crate::{
ed25519::{PublicKey, SecretKey, Signature},
ByteFmt,
};

#[test]
fn test_ed25519() -> anyhow::Result<()> {
Expand Down

0 comments on commit b77d70c

Please sign in to comment.