Repository navigation
Make circuit crate generic over BitzIntRing - #135
Conversation
circuit crate generic over BitzIntRingcircuit crate generic over BitzIntRing
zkfriendly
left a comment
There was a problem hiding this comment.
LGTM, thanks. Left a minor comment
|
|
||
| impl<T> BitzSemiring for T where T: Semiring + From<u64> {} | ||
|
|
||
| // Since BigInt does not support CheckedNeg and CheckedRem, we can't use Ring here |
There was a problem hiding this comment.
Interesting workaround of crypto-primitives interface. I explored this a bit, and I was wondering if we can relax the CheckedNeg bound upstream? Also since SemiRing already requires checked_sub one could implement checked_neg like:
fn checked_neg(&self, x: &R) -> Option<R> {
R::zero().checked_sub(x)
}
There was a problem hiding this comment.
crypto-primitives strives to be generic, and having checked sub/rem sounds like a reasonable thing for a ring consumer to use. Also, we can't use this workaround as a blanket for semirings since it would interfere with upstream types that provide actual CheckedNeg.
What we can do upstream though is to make a BigInt wrapper similarly to crypto_bigint::Int one, but that doesn't seem necessary in this case.
Btw here's the PR I opened to BigInt to implement missing traits: rust-num/num-bigint#357
Goes toward #136
BitzSemiringandBitzRingtraits (with blanket implementations) and abstractcircuitcrate over it as much as was viable.BigUintandBigIntrespectively, but they aren't hardcoded anymore (except forp256, there we merely alias them)BitsandIntoWordstraits to help this abstraction.Fromtrait,LazyLock,thiserrorcrate.Nothing in this PR changes observable behaviour.
This is made into a separate self-contained PR in order to make the overall rework more granular.