feat: Big Integer Implementation for Cryptographic Applications #495

qalisander · 2025-01-15T11:14:56Z

This pr adds Uint<..> bigintegers, and implementation of basic arithmetic operations on integers in Montgomery form. That introduces significant gas cost cut, for finite field arithmetic.
Poseidon hash computation got around 8 times cheaper compare to what was before.
And cheaper than Solidity Assembly poseidon hash.

Resolves #481

PR Checklist

Tests
Documentation
Changelog

netlify · 2025-01-15T11:15:20Z

✅ Deploy Preview for contracts-stylus canceled.

Name	Link
🔨 Latest commit	`e43eb87`
🔍 Latest deploy log	https://app.netlify.com/sites/contracts-stylus/deploys/679bbb726f40b2000852e4c4

codecov · 2025-01-15T11:17:33Z

Codecov Report

Attention: Patch coverage is 70.25393% with 246 lines in your changes missing coverage. Please review.

Project coverage is 84.1%. Comparing base (5d612e6) to head (e43eb87).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/crypto/src/arithmetic/uint.rs	66.2%	182 Missing ⚠️
lib/crypto/src/field/fp.rs	69.5%	64 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
lib/crypto/src/arithmetic/limb.rs	`100.0% <100.0%> (ø)`
lib/crypto/src/field/mod.rs	`100.0% <ø> (ø)`
lib/crypto/src/field/prime.rs	`0.0% <ø> (ø)`
lib/crypto/src/poseidon2/mod.rs	`93.2% <ø> (ø)`
lib/crypto/src/field/fp.rs	`59.6% <69.5%> (+10.9%)`	⬆️
lib/crypto/src/arithmetic/uint.rs	`66.2% <66.2%> (ø)`

bidzyyys · 2025-01-15T15:45:03Z

Cargo.toml

It's great you have done these benchmarks but I am not a fan of maintaining akr-ff or poseidon-renegades. We may want. to have just a separated branch with these benchmarks.

I was about to drop it. It is just for comparison, when I was optimizing it

bidzyyys

Left some comments + many warnings from CI.
Overall great progress @qalisander!

benches/src/main.rs

lib/crypto/src/field/mod.rs

examples/oz-crypto/Cargo.toml

lib/crypto/Cargo.toml

benches/src/oz_crypto.rs

bidzyyys · 2025-01-22T10:56:39Z

lib/crypto/src/arithmetic/mod.rs

+        const_for!((i in 0..N) {
+            let a = self.limbs[N - i - 1];
+            let b = other.limbs[N - i - 1];
+            if a < b {


Can't you have jus return a < b?

What if a == b?

If they're equal go to next. As I remember it is code for geq comparison

bidzyyys · 2025-01-22T10:57:58Z

lib/crypto/src/arithmetic/mod.rs

+        while self.const_is_even() {
+            self = self.const_shr();
+        }
+        assert!(self.const_is_odd());


I do not like these asserts but I believe it is the only way to assure proper execution :(

I think I've already dropped this code:D

bidzyyys · 2025-01-22T11:01:41Z

lib/crypto/src/arithmetic/mod.rs

+        }
+    }
+
+    // TODO#q: rename to checked_add?


If you are about checked_add you should implement the proper trait then.

I've thought about about names, and constant ct_checked_add and checked_add_assign that returns boolean seems rational to me

lib/crypto/src/arithmetic/mod.rs

bidzyyys · 2025-01-22T11:07:04Z

lib/crypto/src/arithmetic/mod.rs

Fuzzing here is super needed 😅 @0xNeshi

Proptests would be a bare minimum I'd say 😃

Yeah. We will try to add more proptests, but currently: the best warranty gives poseidon, that computes valid hash on each instance

Just one wrong addition/multiplication or messed up index and hash will be totally different from the other implementations

I'd say there's still value in testing the primitives, not just the upper-layer stuff. There can always be an edge case that might cause an issue, but would rarely pop up in the upper-layer test (e.g. for poseidon)

qalisander · 2025-01-29T09:11:32Z

lib/crypto/src/arithmetic/uint.rs

+        !self.ct_is_odd()
+    }
+
+    const fn ct_geq(&self, rhs: &Self) -> bool {


The whole point of ct_* prefix is to distinguish constant function from runtime function.
E.g. we can have geq for runtime and ct_geq for compile time.

Also same convention applied to ct_for! macro, that adds convenience of for but in const context.
p.s. for cycle is not available in const context

bidzyyys

Great progress, left some comments.

benches/src/report.rs

examples/oz-crypto/src/lib.rs

lib/crypto/src/arithmetic/limb.rs

lib/crypto/src/arithmetic/mod.rs

bidzyyys · 2025-01-29T09:04:23Z

lib/crypto/src/field/fp.rs

+            let (_, mut carry) = arithmetic::limb::mac(lo.limbs[i], tmp, P::MODULUS.limbs[0]);
+
+            ct_for_unroll6!((j in 1..N) {
+                let k = i + j;


Nit: I do not like k, i, l for loop iterations, prefer more self-descriptive names to not mess them.

In this (complex) case I have to agree

lib/crypto/src/arithmetic/uint.rs

qalisander · 2025-01-29T10:06:11Z

lib/crypto/src/field/fp.rs

+    /// reduction for efficient implementation.
+    #[inline(always)]
+    fn mul_assign(a: &mut Fp<Self, N>, b: &Fp<Self, N>) {
+        // Implements CIOS.


Attach link

lib/crypto/src/arithmetic/uint.rs

bidzyyys

Only few comments left.

Please:

update CHANGELOG.md
add GH Issue describing how we should add prop tests and fuzzing to this Big Integer implementation

benches/src/PoseidonT3_asm.sol

benches/src/report.rs

lib/crypto/src/arithmetic/limb.rs

lib/crypto/src/arithmetic/uint.rs

0xNeshi

My one big critique is terse/missing doc comments.

I think for the sake of our future selves, and for the sake of our audit team, we should have clear doc comments for each function.

This is especially important if we plan to make this bigint implementation into a separate crate that we want to share with the world

lib/crypto/src/arithmetic/uint.rs

lib/crypto/src/field/fp.rs

0xNeshi · 2025-01-29T20:09:02Z

lib/crypto/src/field/fp.rs

    }
 }

+/// Compute `-M^{-1} mod 2^64`.


This is a very terse doc comment.

See LLM's suggestion

Suggested change

/// Compute `-M^{-1} mod 2^64`.

/// Computes the Montgomery modular inverse `-MODULUS^{-1} mod 2^64` for the field modulus.

///

/// This is a constant function that computes the modular multiplicative inverse of

/// the negative field modulus, modulo 2^64. This value is crucial for Montgomery arithmetic

/// operations in the field.

///

/// The calculation uses the fact that for a 64-bit value:

/// - We only need the lowest limb of the modulus

/// - The Euler totient φ(2^64) = 2^63

/// - Therefore we can compute the inverse by raising to power (2^63 - 1)

I think the audit team will thank us

0xNeshi · 2025-01-29T20:12:24Z

lib/crypto/src/field/fp.rs

+            let (_, mut carry) = arithmetic::limb::mac(lo.limbs[i], tmp, P::MODULUS.limbs[0]);
+
+            ct_for_unroll6!((j in 1..N) {
+                let k = i + j;


In this (complex) case I have to agree

0xNeshi · 2025-01-29T20:12:56Z

lib/crypto/src/field/fp.rs

+    /// Algorithm 14.32 in Handbook of Applied Cryptography [reference].
+    ///
+    /// [reference]: https://cacr.uwaterloo.ca/hac/about/chap14.pdf


See previous comment on how to link to a paper in a cleaner way

0xNeshi · 2025-01-29T20:17:08Z

lib/crypto/src/poseidon2/mod.rs

@@ -42,6 +42,7 @@ impl<P: PoseidonParams<F>, F: PrimeField> Default for Poseidon2<P, F> {
 impl<P: PoseidonParams<F>, F: PrimeField> Poseidon2<P, F> {
    /// Create a new Poseidon sponge.
    #[must_use]
+    #[inline]


Inlining everything is the way to reducing gas cost in all cases? 🤔
I just noticed that many lib/crypto functions are inlined, but not all. Shouldn't we then be inlining everything?

Did you measure the impact of not using #[inline]?
Shouldn't link-time optimizations perform the same optimization? Stylus projects have this option ON by default, and so do we.

Increased binary size is also an impact of aggressive inlines #[inline(always)]. It is usually a tradeoff
Yeah, measured. Impact with inlines seems significant

Actually I've checked most of them and also #[inline(always)] vs normal #[inline]. The first one gives more impact sometimes

Did you measure in Release mode or in Debug mode?

qalisander · 2025-01-30T12:22:31Z

@bidzyyys this issue for proptests

bidzyyys

Looks good to me!

0xNeshi

*clicks tongue* Noice!

bidzyyys reviewed Jan 15, 2025

View reviewed changes

bidzyyys reviewed Jan 22, 2025

View reviewed changes

bidzyyys self-requested a review January 29, 2025 08:33

qalisander requested a review from 0xNeshi January 29, 2025 08:57

qalisander commented Jan 29, 2025

View reviewed changes

bidzyyys reviewed Jan 29, 2025

View reviewed changes

qalisander commented Jan 29, 2025

View reviewed changes

lib/crypto/src/arithmetic/uint.rs Show resolved Hide resolved

qalisander marked this pull request as ready for review January 29, 2025 13:08

bidzyyys reviewed Jan 29, 2025

View reviewed changes

bidzyyys changed the title ~~feat: biginteger implementation~~ feat: Big Integer implementation Jan 29, 2025

0xNeshi reviewed Jan 29, 2025

View reviewed changes

qalisander changed the title ~~feat: Big Integer implementation~~ feat: Big Integer Implementation for Cryptographic Applications Jan 30, 2025

bidzyyys approved these changes Jan 30, 2025

View reviewed changes

qalisander added 13 commits January 30, 2025 21:40

++

f42e39f

++

be0753d

++

3fbd3e8

++

357a227

++

c70e723

++

3e95966

++

73072af

++

f81552c

++

c469c49

++

554f547

++

792a9b1

++

f670a53

++

70fe8f9

qalisander added 22 commits January 30, 2025 21:40

++

54a65b8

rename Example -> Examples

668c5c2

review fixes

56dda77

clippy fixes

0351386

don't use !ct_is_odd for possible optimization reasons

9d69e94

remove #[allow(unused_qualifications)]

49f7dff

++

708609d

++

b51bb68

add docs

4afb3b0

++

93ea81a

remove String from hex formatting

ee40c75

++

b0ae8c5

drop shr and shl operators (will add them later)

eb7ff67

++

0696659

update CHANGELOG.md

b361cc0

add links and drop *.sol files from the benchmarks

6d6169f

++

7ec8608

remove ct_shr, add it later

9f9ae1d

add other ct_le, ct_lt, ct_gt, ct_ne operators and tests

66cf5ab

++

1cfd2af

++

aa45ffe

++

97063af

qalisander force-pushed the benchmark-experiments/biginteger branch from 318781f to 97063af Compare January 30, 2025 17:45

add docs

e43eb87

qalisander requested a review from 0xNeshi January 30, 2025 18:14

0xNeshi approved these changes Jan 30, 2025

View reviewed changes

0xNeshi merged commit d142e88 into main Jan 30, 2025
26 checks passed

0xNeshi deleted the benchmark-experiments/biginteger branch January 30, 2025 19:04

This was referenced Jan 30, 2025

[Feature]: Document openzeppelin-crypto Functions #519

Open

[Feature]: Add Unit/Prop Tests to openzeppelin-crypto #520

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Big Integer Implementation for Cryptographic Applications #495

feat: Big Integer Implementation for Cryptographic Applications #495

qalisander commented Jan 15, 2025 •

edited

Loading

netlify bot commented Jan 15, 2025 •

edited

Loading

codecov bot commented Jan 15, 2025 •

edited

Loading

bidzyyys Jan 15, 2025

qalisander Jan 15, 2025

bidzyyys left a comment

bidzyyys Jan 22, 2025

qalisander Jan 29, 2025

bidzyyys Jan 22, 2025

qalisander Jan 29, 2025

bidzyyys Jan 22, 2025

qalisander Jan 29, 2025

bidzyyys Jan 22, 2025

0xNeshi Jan 22, 2025

qalisander Jan 29, 2025

qalisander Jan 29, 2025

0xNeshi Jan 30, 2025 •

edited

Loading

qalisander Jan 29, 2025

bidzyyys left a comment

bidzyyys Jan 29, 2025

0xNeshi Jan 29, 2025

qalisander Jan 29, 2025

bidzyyys left a comment •

edited by qalisander

Loading

0xNeshi left a comment •

edited

Loading

0xNeshi Jan 29, 2025

0xNeshi Jan 29, 2025

0xNeshi Jan 29, 2025

0xNeshi Jan 29, 2025 •

edited

Loading

0xNeshi Jan 29, 2025 •

edited

Loading

qalisander Jan 30, 2025

qalisander Jan 30, 2025

0xNeshi Jan 30, 2025

qalisander commented Jan 30, 2025

bidzyyys left a comment

0xNeshi left a comment •

edited

Loading

-/// Compute `-M^{-1} mod 2^64`.
+/// Computes the Montgomery modular inverse `-MODULUS^{-1} mod 2^64` for the field modulus.
+///
+/// This is a constant function that computes the modular multiplicative inverse of
+/// the negative field modulus, modulo 2^64. This value is crucial for Montgomery arithmetic
+/// operations in the field.
+///
+/// The calculation uses the fact that for a 64-bit value:
+/// - We only need the lowest limb of the modulus
+/// - The Euler totient φ(2^64) = 2^63
+/// - Therefore we can compute the inverse by raising to power (2^63 - 1)

feat: Big Integer Implementation for Cryptographic Applications #495

feat: Big Integer Implementation for Cryptographic Applications #495

Conversation

qalisander commented Jan 15, 2025 • edited Loading

PR Checklist

netlify bot commented Jan 15, 2025 • edited Loading

✅ Deploy Preview for contracts-stylus canceled.

codecov bot commented Jan 15, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bidzyyys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xNeshi Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bidzyyys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bidzyyys left a comment • edited by qalisander Loading

Choose a reason for hiding this comment

0xNeshi left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xNeshi Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

0xNeshi Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qalisander commented Jan 30, 2025

bidzyyys left a comment

Choose a reason for hiding this comment

0xNeshi left a comment • edited Loading

Choose a reason for hiding this comment

qalisander commented Jan 15, 2025 •

edited

Loading

netlify bot commented Jan 15, 2025 •

edited

Loading

codecov bot commented Jan 15, 2025 •

edited

Loading

0xNeshi Jan 30, 2025 •

edited

Loading

bidzyyys left a comment •

edited by qalisander

Loading

0xNeshi left a comment •

edited

Loading

0xNeshi Jan 29, 2025 •

edited

Loading

0xNeshi Jan 29, 2025 •

edited

Loading

0xNeshi left a comment •

edited

Loading