Skip to content

Support bls12 381 temporarilly#87

Closed
CPerezz wants to merge 8 commits into
mainfrom
support_bls12-381
Closed

Support bls12 381 temporarilly#87
CPerezz wants to merge 8 commits into
mainfrom
support_bls12-381

Conversation

@CPerezz

@CPerezz CPerezz commented Sep 11, 2023

Copy link
Copy Markdown
Contributor

This is an attempt to temporarily providing support for Bls12-381 in halo2curves.

This will require first to complete: privacy-ethereum/bls12_381#4 so that CurveExt is implemented for G1Projective so that the trait requirements for CruveEndo are met.

@CPerezz

CPerezz commented Sep 14, 2023

Copy link
Copy Markdown
Contributor Author

See: privacy-ethereum/bls12_381#4 (comment)

Somehow we still fail endo tests for bls although all the constants for it are correct (at least tests for them pass). Note that the tests don't include ZETA-related tests which are exactly the ones we need to pass here. But is weird as the way to derive Z does not seem crazy and yet does not work. I'm definitely missing something here.

And also, the endo params have been derived twice resulting on the exact same thing.

@CPerezz

CPerezz commented Sep 14, 2023

Copy link
Copy Markdown
Contributor Author

This was the test used in Gnark to derive the parameters:

func TestGenGLV(t *testing.T) {

	var lambda, r big.Int
	var l Lattice

	// // bls
	r.SetString("73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001", 16)
	lambda.SetString("ac45a4010001a40200000000ffffffff", 16)

	// // bn
	// r.SetString("30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001", 16)
	// lambda.SetString("30644e72e131a029048b6e193fd84104cc37a73fec2bc5e9b8ca0b2d36636f23", 16)

	// //  grumpkin
	// r.SetString("30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47", 16)
	// lambda.SetString("30644e72e131a0295e6dd9e7e0acccb0c28f069fbb966e3de4bd44e5607cfd48", 16)

	// // book
	// r.SetString("1461501637330902918203687013445034429194588307251", 10)
	// lambda.SetString("903860042511079968555273866340564498116022318806", 10)

	// // secp
	// r.SetString("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", 16)
	// lambda.SetString("5363ad4cc05c30e0a5261c028812645a122e22ea20816678df02967c1b23bd72", 16)

	// // eq
	// r.SetString("40000000000000000000000000000000224698fc094cf91b992d30ed00000001", 16)
	// lambda.SetString("12ccca834acdba712caad5dc57aab1b01d1f8bd237ad31491dad5ebdfdfe4ab9", 16)

	// // ep
	// r.SetString("40000000000000000000000000000000224698fc0994a8dd8c46eb2100000001", 16)
	// lambda.SetString("06819a58283e528e511db4d81cf70f5a0fed467d47c033af2aa9d2e050aa0e4f", 16)

	PrecomputeLattice(&r, &lambda, &l)

	fmt.Printf("a1 0x%x %d\n", l.V1[0].Bytes(), l.V1[0].Sign())
	fmt.Printf("b1 0x%x %d\n", l.V1[1].Bytes(), l.V1[1].Sign())
	fmt.Printf("a2 0x%x %d\n", l.V2[0].Bytes(), l.V2[0].Sign())
	fmt.Printf("b2 0x%x %d\n", l.V2[1].Bytes(), l.V2[1].Sign())

	fmt.Printf("g1 0x%x\n", l.b1.Bytes())
	fmt.Printf("g2 0x%x\n", l.b2.Bytes())

}

Unless lambda is wrong, I'm not sure what could be the issue anymore. Maybe @kilic has a hint?

@mratsim

mratsim commented Sep 19, 2023

Copy link
Copy Markdown
Contributor

You might want to try my parameter generators here: https://github.com/mratsim/constantine/blob/master/sage/derive_endomorphisms.sage

Even when taking the formulats from Guide to pairing-based cryptography, I had an issue.

And for easier to modify/dump test vectors, you can use this old self-contained file: https://github.com/mratsim/constantine/blob/2613356281ed33626bd9cc8b2cbaf68ae11a0867/sage/lattice_decomposition_bls12_381_g1.sage

@kilic

kilic commented Sep 19, 2023

Copy link
Copy Markdown
Contributor

@CPerezz in the script you send parameters are generated are good however there is a misnaming issue that g1 should be gamma2 and g2 should be gamma1. So can you try it with these parameters which are working in my end:

const ENDO_PARAMS_BLS: EndoParameters = EndoParameters {
    // round(b2/n)
    gamma2: [0x63f6e522f6cfee30u64, 0x7c6becf1e01faadd, 0x01, 0x0],
    // round(-b1/n)
    gamma1: [0x02u64, 0x0, 0x0, 0x0],
    b1: [0x01u64, 0x0, 0x0, 0x0],
    b2: [0x0000000100000000, 0xac45a4010001a402, 0x0, 0x0],
};

@CPerezz
CPerezz marked this pull request as ready for review September 27, 2023 11:16
@CPerezz

CPerezz commented Sep 27, 2023

Copy link
Copy Markdown
Contributor Author

The PR on which this relies in order to be merged is all finalized. So this can start to be reviewed too considering that I'll need to get ridd from the git_dependencies.

@CPerezz
CPerezz requested review from han0110 and kilic and removed request for han0110 September 27, 2023 11:17
Comment thread Cargo.toml Outdated
serde_arrays = { version = "0.1.0", optional = true }
hex = { version = "0.4", optional = true, default-features = false, features = ["alloc", "serde"] }
blake2b_simd = "1"
bls12_381 = { git = "https://github.com/privacy-scaling-explorations/bls12_381", branch = "tmp_curve_ext", features = ["groups", "basefield"] }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey Carlos, depending on a branch would be problematic here.

Every time people install Halo2curve or the CI runs the test, they pull the branch head. So if an incompatible commit is introduced to tmp_curve_ext branch then all the packages break.

We should release a tag on bls12_381 repo and then depends on the tag here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to release a tag and update it here as well as rebasing this!
Will do it within the next days! 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed the release was done already back in October. See: https://github.com/privacy-scaling-explorations/bls12_381/releases/tag/v2023_10_26

Will update this in main

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in 9329af1

@CPerezz

CPerezz commented Dec 18, 2023

Copy link
Copy Markdown
Contributor Author

Updated @ChihChengLiang 🥳

@davidnevadoc davidnevadoc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is finally ready! Thanks for taking care of this :)
LGTM

@CPerezz

CPerezz commented Dec 20, 2023

Copy link
Copy Markdown
Contributor Author

I think this is finally ready! Thanks for taking care of this :) LGTM

Notice that this branch will never get merged (at least until we have a crates.io dep instead of a git dep).
This passes by refactoring all the stuff we have with the RFC to Zcash.

@ChihChengLiang ChihChengLiang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I didn't review the rest of the parts.
LGTM for the dependency update.

@CPerezz

CPerezz commented Feb 21, 2024

Copy link
Copy Markdown
Contributor Author

This should now be updated after privacy-ethereum/bls12_381#6 is merged.
And we should also consider what to do with this crate. If we don't publish it, we will never be able to merge it.

cc: @ed255 @han0110 @kilic any opinions?
I think this is relevant as after 4844 and Verkle, BLS will be probably the next standard.

@davidnevadoc

Copy link
Copy Markdown
Collaborator

Superseded by #162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants