feat: added to_field method
#99
Conversation
src/tests/bignum_test.nr
Outdated
| #[test] | ||
| fn test_to_field_three_digits() { | ||
| let field: Field = 2330301921655783950764183713945533646391233209687308929386184468126823563744; | ||
| let bn = | ||
| Fq { limbs: [0x862cf8ea69d6c70c9cc8d8871b41e0, 0xe7763528201566c2fc8d93973cf1b4, 0x526] }; | ||
| let result = bn.to_field(); | ||
| assert(result == field); | ||
| } | ||
|
|
||
| #[test(should_fail_with = "BigNum::validate_gt check fails")] | ||
| fn test_to_field_three_digits_overflow() { | ||
| let bn: Fq = BigNum { | ||
| limbs: [0x4e6405505a33bb9b9c0563df2bd59a, 0x48dbe03a9bb4865ba961e41ef9dded, 0x3a36], | ||
| }; | ||
| let result = bn.to_field(); | ||
| } | ||
|
|
||
| #[test(should_fail_with = "BigNum::validate_gt check fails")] | ||
| fn test_to_field_too_many_digits() { | ||
| let bn: BN381 = BN381 { | ||
| limbs: [0xea1742447ee9d92f9f18e1c80a481e, 0x3d89ad3d3ae85f3f482a08435c93ec, 0x1e9f, 0x1], | ||
| }; | ||
| let result = bn.to_field(); | ||
| } |
There was a problem hiding this comment.
instead of failing in runtime, is it possible to implement .to_field() only for bignums that are guaranteed to be convertable to a field.
Imagine, I am writing a portal Aztec contract that computes let hash: U256 = sha256(message) and then I try to convert hash into a Field (hash.to_field()) to store it in Aztec storage. There is a high probability that hash.to_field() will fail IIRC. This will be essentially a DDOS of the portal.
I am thinking of something like:
impl<let N: u32> ToField for BigNum<N> where N: <= 254 {
fn to_field(...) { ... }
}There was a problem hiding this comment.
What I mean, is that this code should not compile:
let a: U256 = BigNum::from_field(123);
a.to_field(); // compilation error
There was a problem hiding this comment.
You can't implement a trait conditionally based on a numeric generic value without enumerating all of them. What can be done is have a compile time check against the modulus of the field. If the field modulus won't fit into a field element then completion can be halted.
There was a problem hiding this comment.
anything that throws a compile time error works. Better throw in compile time than DoS a portal and lock tokens forever due to a runtime throw.
There was a problem hiding this comment.
hmm, but this would mean that we can't even have the method for U256 or even U254.
There was a problem hiding this comment.
What about U253? Also, what about adding an additional .try_to_field that will throw if U256 is too large. Actually, it can be just U253::into and U256::try_into. Is there a TryFrom trait in Noir std?
Changes to circuit sizes
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
src/bignum.nr
Outdated
|
|
||
| // @Brief: Convert a BigNum to a Field | ||
| // we decided to not add this to the BigNumTrait as it is might lead to bad use cases of it | ||
| pub(crate) fn to_field<let N: u32, let MOD_BITS: u32, Params>(input: BigNum<N, MOD_BITS, Params>) -> Field |
There was a problem hiding this comment.
this is a free function now. I wonder how it would be accessed outside the lib though.
There was a problem hiding this comment.
It just needs to be marked pub rather than pub(crate).
There was a problem hiding this comment.
if it's not going to check modulus of the field at compile time, I would recommend renaming it to try_to_field to signal to the users that this function can fail in runtime
Description
added a method that allows casting bignums to
Fieldtype given the bignum is in range.Moreover, this PR fixes issues with the range checks in the
from_fieldmethod.Problem*
Resolves
Summary*
Additional Context
PR Checklist*
cargo fmton default settings.