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

SW <-> TE map for Bandersnatch #804

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- [\#772](https://github.com/arkworks-rs/algebra/pull/772) (`ark-ff`) Implementation of `mul` method for `BigInteger`.
- [\#794](https://github.com/arkworks-rs/algebra/pull/794) (`ark-ff`) Fix `wasm` compilation.
- [\#XXX](https://github.com/arkworks-rs/algebra/pull/XXX) (`ark-curves`) Implement TE <-> SW map for Bandersnatch.
Copy link
Member

@weikengchen weikengchen Mar 8, 2024

Choose a reason for hiding this comment

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

Suggested change
- [\#XXX](https://github.com/arkworks-rs/algebra/pull/XXX) (`ark-curves`) Implement TE <-> SW map for Bandersnatch.
- [\#804](https://github.com/arkworks-rs/algebra/pull/XXX) (`ark-curves`) Implement TE <-> SW map for Bandersnatch.

(since the PR is from an organization, I cannot edit directly)


### Breaking changes

Expand Down
136 changes: 133 additions & 3 deletions curves/ed_on_bls12_381_bandersnatch/src/curves/mod.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use crate::{Fq, Fr};
use ark_ec::{
hashing::curve_maps::elligator2::Elligator2Config,
models::CurveConfig,
short_weierstrass::{self, SWCurveConfig},
twisted_edwards::{Affine, MontCurveConfig, Projective, TECurveConfig},
};
use ark_ff::{AdditiveGroup, MontFp};

use crate::{Fq, Fr};
use ark_ff::{AdditiveGroup, Field, MontFp, One};

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -163,6 +162,77 @@ impl Elligator2Config for BandersnatchConfig {
MontFp!("22511181562295907836254750456843438087744031914659733450388350895537307167857");
}

// Constants used in mapping TE form to SW form and vice versa
//
// sage: q = 52435875175126190479447740508185965837690552500527637822603658699938581184513
// sage: Fq = GF(q)
// sage: MONT_A = 29978822694968839326280996386011761570173833766074948509196803838190355340952
// sage: MONT_B = 25465760566081946422412445027709227188579564747101592991722834452325077642517
// sage: MONT_A/Fq(3)
// 9992940898322946442093665462003920523391277922024982836398934612730118446984
// sage: Fq(1)/MONT_B
// 41180284393978236561320365279764246793818536543197771097409483252169927600582
const MONT_A_OVER_THREE: Fq =
MontFp!("9992940898322946442093665462003920523391277922024982836398934612730118446984");
const MONT_B_INV: Fq =
MontFp!("41180284393978236561320365279764246793818536543197771097409483252169927600582");

impl BandersnatchConfig {
/// Map an point in TE form to its corresponding point on SW curve
Copy link

Choose a reason for hiding this comment

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

Suggested change
/// Map an point in TE form to its corresponding point on SW curve
/// Map a point in TE form to its corresponding point on SW curve

pub fn map_te_to_sw(point: EdwardsAffine) -> Option<SWAffine> {
//First we map the point from the TE to Montgamory
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//First we map the point from the TE to Montgamory
if point.is_zero() {
return SWAffine::identity()
}
//First we map the point from the TE to Montgamory

//edward to mont ((1+y)/(1-y), (1+y)/(x(1-y)))
let v_denom = <<BandersnatchConfig as CurveConfig>::BaseField as One>::one() - point.y;
let w_denom = point.x - point.x * point.y;

let v_denom_inv = v_denom.inverse()?;
let w_denom_inv = w_denom.inverse()?;

let v_w_num = <<BandersnatchConfig as CurveConfig>::BaseField as One>::one() + point.y;

let v = v_w_num * v_denom_inv;
let w = v_w_num * w_denom_inv;

//now we are mappyng the montgamory point to SW.
Copy link

@dvdplm dvdplm Apr 17, 2024

Choose a reason for hiding this comment

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

Suggested change
//now we are mappyng the montgamory point to SW.
//now we are mapping the montgomery point to SW.

//Mont->SW ((x+A/3)/B,y/B)

let x = MONT_B_INV * (v + MONT_A_OVER_THREE);
let y = MONT_B_INV * w;

let point_on_sw_curve = SWAffine::new_unchecked(x, y);
debug_assert!(
point_on_sw_curve.is_on_curve(),
"TE point mapped off the SW curve"
);
Comment on lines +203 to +206
Copy link

Choose a reason for hiding this comment

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

I'd probably play it safe here and use SWAffine::new here and remove the debug asserts. That way you'd run both checks. Then consider adding a te_to_sw_unchecked() if/when there's proof that it's too slow to run the checks.


return Some(point_on_sw_curve);
}

pub fn map_sw_to_te(point: SWAffine) -> Option<EdwardsAffine> {
//first we are mappyng the sw point to montgamory point
Copy link

Choose a reason for hiding this comment

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

Suggested change
//first we are mappyng the sw point to montgamory point
//first we are mappng the sw point to its montgomery point

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//first we are mappyng the sw point to montgamory point
if point.is_zero() {
return EdwardsAffine::zero()
}
// First we are mappyng the sw point to montgamory point

//SW->Mont by (Bx−A/3,By)
let mx = <BandersnatchConfig as MontCurveConfig>::COEFF_B * point.x - MONT_A_OVER_THREE;
let my = <BandersnatchConfig as MontCurveConfig>::COEFF_B * point.y;

//Then we map the TE point to Montgamory
// (x,y)↦(x/y,(x−1)/(x+1))
let v_denom = my.inverse()?;
let x_p_1 = mx + <<BandersnatchConfig as CurveConfig>::BaseField as One>::one();
let w_denom = x_p_1.inverse()?;

let v = mx * v_denom;
let w = (mx - <<BandersnatchConfig as CurveConfig>::BaseField as One>::one()) * w_denom;

let point_on_te_curve = EdwardsAffine::new_unchecked(v, w);
debug_assert!(
point_on_te_curve.is_on_curve(),
"TE point mapped off the SW curve"
);
Comment on lines +226 to +230
Copy link

Choose a reason for hiding this comment

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

Same comment as above: consider using EdwartdsAffine::new instead and ditch the debug assert.


return Some(point_on_te_curve);
}
}

#[cfg(test)]
mod test {
use super::*;
Expand All @@ -189,4 +259,64 @@ mod test {
"hash results into a point off the curve"
);
}

#[test]
fn test_mapping_from_te_to_sw_curve() {
let test_elligator2_to_curve_hasher = MapToCurveBasedHasher::<
Projective<BandersnatchConfig>,
DefaultFieldHasher<Sha512, 128>,
Elligator2Map<BandersnatchConfig>,
>::new(&[1])
.unwrap();

let hash_result = test_elligator2_to_curve_hasher.hash(b"if you stick a Babel fish in your ear you can instantly understand anything said to you in any form of language.").expect("fail to hash the string to curve");

assert!(
hash_result.is_on_curve(),
"hash results into a point off the curve"
);

assert!(
BandersnatchConfig::map_te_to_sw(hash_result)
.expect("could not map the te point to sw form")
.is_on_curve(),
"hash results into a point off the curve"
);
}

#[test]
fn test_mapping_from_te_to_sw_to_te_curve() {
let test_elligator2_to_curve_hasher = MapToCurveBasedHasher::<
Projective<BandersnatchConfig>,
DefaultFieldHasher<Sha512, 128>,
Elligator2Map<BandersnatchConfig>,
>::new(&[1])
.unwrap();

let hash_result = test_elligator2_to_curve_hasher.hash(b"if you stick a Babel fish in your ear you can instantly understand anything said to you in any form of language.").expect("fail to hash the string to curve");

assert!(
hash_result.is_on_curve(),
"hash results into a point off the curve"
);

let sw_point = BandersnatchConfig::map_te_to_sw(hash_result)
.expect("could not map the te point to sw form");
assert!(
sw_point.is_on_curve(),
"hash results into a point off the curve"
);

let te_point = BandersnatchConfig::map_sw_to_te(sw_point)
.expect("could not map the te point to sw form");
assert!(
te_point.is_on_curve(),
"hash results into a point off the curve"
);

assert!(
te_point == hash_result,
"point mapped to sw form was re-mapped to a different point on te form"
);
}
}
Loading