feat: export pre-defined bignum types #125
Conversation
src/fields/ed25519Fr.nr
Outdated
| fn get_params() -> BigNumParams<3, 255> { | ||
| pub type ED25519_Fr = BigNum<3, 253, ED25519_Fr_Params>; | ||
|
|
||
| impl BigNumParamsGetter<3, 253> for ED25519_Fr_Params { |
There was a problem hiding this comment.
Error. Originates in paramgen tool.
There was a problem hiding this comment.
cc @kashbrti any idea why the paramgen tool might be broken here? I generated from your PR (although of course the bug might have originated earlier)
There was a problem hiding this comment.
I've manually fixed it, for now, so that this PR can be merged.
| // Re-export the pre-defined bignum types, for easier access: | ||
| pub use fields::bls12_377Fq::BLS12_377_Fq; | ||
| pub use fields::bls12_377Fr::BLS12_377_Fr; | ||
| pub use fields::bls12_381Fq::BLS12_381_Fq; | ||
| pub use fields::bls12_381Fr::BLS12_381_Fr; | ||
| pub use fields::bn254Fq::BN254_Fq; | ||
| pub use fields::ed25519Fq::ED25519_Fq; | ||
| pub use fields::ed25519Fr::ED25519_Fr; | ||
| pub use fields::mnt4_753Fq::MNT4_753_Fq; | ||
| pub use fields::mnt4_753Fr::MNT4_753_Fr; | ||
| pub use fields::mnt6_753Fq::MNT6_753_Fq; | ||
| pub use fields::mnt6_753Fr::MNT6_753_Fr; | ||
| pub use fields::pallasFq::Pallas_Fq; | ||
| pub use fields::pallasFr::Pallas_Fr; | ||
| pub use fields::secp256k1Fq::Secp256k1_Fq; | ||
| pub use fields::secp256k1Fr::Secp256k1_Fr; | ||
| pub use fields::secp256r1Fq::Secp256r1_Fq; | ||
| pub use fields::secp256r1Fr::Secp256r1_Fr; | ||
| pub use fields::secp384r1Fq::Secp384r1_Fq; | ||
| pub use fields::secp384r1Fr::Secp384r1_Fr; | ||
| pub use fields::U1024::U1024; | ||
| pub use fields::U2048::U2048; | ||
| pub use fields::U256::U256; | ||
| pub use fields::U384::U384; | ||
| pub use fields::U4096::U4096; | ||
| pub use fields::U512::U512; | ||
| pub use fields::U768::U768; | ||
| pub use fields::U8192::U8192; | ||
| pub use fields::vestaFq::Vesta_Fq; | ||
| pub use fields::vestaFr::Vesta_Fr; |
There was a problem hiding this comment.
I don't think that we should be exporting these from the root, maybe from fields.
There was a problem hiding this comment.
Why not? The names won’t collide with the other things being exported. It’s more convenient / cleaner to import them with ‘use dep::bignum::U256’.
‘use dep::bignum::fields::U256‘ is tautological, plus using fields as a synonym for bignum is a bit ugly here.
There was a problem hiding this comment.
Bumping this debate :)
There was a problem hiding this comment.
I went looking for a good "Fight me" gif. There was a pretty good one from the movie Troy. Remember that movie? But when I went to the site it seemed broken, so I can't use it. Please imagine a gif of the actor Brian Cox (not the physicist) shouting "Fight me!", dressed in ancient greek armour, holding a sword, standing in front of an army. I think he's shouting at Brad Pitt. You're Brad Pitt in this scenario. Please don't pass up on this opportunity to portray Brad Pitt. You know Brian Cox - he's in Succession - that HBO TV series. It's a pretty good show. Very good acting. Possibly too good, because all of the characters are despicable and unlikeable. One comes away from the show quite depressed, actually.
There was a problem hiding this comment.
I don't have strong feelings about this. One thing that annoys me is calling the module fields, but it contains things like U1024 which is a ring. I think having a separate module for the predefined types would be neat but would maybe change the name.
There was a problem hiding this comment.
I WON THE FIGHT. It was the gif description that scared you off, right?
|
@iAmMichaelConnor how urgent is this? merging this will break my branch (we would need to redo it) because the bignum parameters' format is changing with the |
I don't see how a minor change such as this would require you to "redo" your entire branch. It introduces two lines of code to each field file, and those lines don't conflict with the parameters definitions? I imagine it'll merge seamlessly. 
From your feature banch, if this were to be merged first, just do:
and it'll all be sorted. |
|
I don't see any significant conflicts between the two PRs. They should be able to either resolved automatically by vscode or in a minute or two manually. |
* main: chore: address warnings (#137)
resolves #101
Exports all of the pre-defined bignum types as top-level types, so that users don't have to construct them. Users were rightly complaining that the process of defining bignums was cumbersome.
I used noir-lang/noir-bignum-paramgen#6 to generate the new lines of code.