refactor!: using native `u128` type #12213

benesjan · 2025-02-24T09:43:40Z

benesjan · 2025-02-24T09:43:55Z

refactor: renaming Note generic as NOTE #12282
refactor!: using native u128 type #12213 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

benesjan · 2025-02-24T13:05:22Z

docs/docs/migration_notes.md

@@ -63,7 +79,7 @@ await contract.methods
    txHash.hash,
    toBoundedVec(txEffects!.data.noteHashes, MAX_NOTE_HASHES_PER_TX),
    txEffects!.data.nullifiers[0],
-    wallet.getAddress(),


I just ran formatter here

benesjan · 2025-02-25T15:57:38Z

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

-    #[public]
-    fn u128_from_integer_overflow() -> U128 {
-        let should_overflow: Field = 2.pow_32(128); // U128::max() + 1;
-        U128::from_integer(should_overflow)


Removed this because from_integer function does not exist on the native u128 type and when casting from Field to u128 there is no overflow check.

benesjan · 2025-02-25T16:00:30Z

yarn-project/end-to-end/src/e2e_token_contract/minting.test.ts

-        );
+        await expect(
+          asset.methods.mint_to_private(accounts[0].address, accounts[0].address, overflowAmount).simulate(),
+        ).rejects.toThrow('Cannot satisfy constraint');


Since this is a private function this issue does not apply here and we get a proper check on the input arg. The error message is like that as the U128_OVERFLOW_ERROR used to be on the U128 type and it's no longer present on the u128 one.

benesjan · 2025-02-25T16:02:53Z

yarn-project/simulator/src/avm/avm_simulator.test.ts

@@ -389,16 +387,6 @@ describe('AVM simulator: transpiled Noir contracts', () => {
        resolveAvmTestContractAssertionMessage('u128_addition_overflow', results.revertReason!, results.output),
      ).toMatch('attempt to add with overflow');
    });
-
-    it('Expect failure on U128::from_integer() overflow', async () => {


Removed this test because from_integer function does not exist on the native u128 type and when casting from Field to u128 there is no overflow check.

benesjan · 2025-02-26T01:08:33Z

noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr

@@ -588,13 +574,3 @@ fn fancier_test() {
    let deserialized = Fancier::deserialize(serialized);
    assert(deserialized == fancier);
 }
-
-#[test]
-fn contains_u128_test() {


This test became pointless as now u128 occupies only 1 field.

sklppy88

Honestly such a great change ! 🫡 I have some tiny nits, and a broader open question.

If we have disparity between U128::from_integer(field) and field as u128, are we sure that all of these conversions now are safe ? I highlighted one such example in the value_note utils.

sklppy88 · 2025-02-26T13:15:40Z

noir-projects/aztec-nr/value-note/src/filter.nr

            let retrieved_note = notes[i].unwrap_unchecked();
            selected[i] = Option::some(retrieved_note);
-            sum += U128::from_integer(retrieved_note.note.value);
+            sum += retrieved_note.note.value as u128;


Just curious, unrelated to your PR, what happens if note.value > what's storeable in a u128, as we are reading from a full field. Does it just overflow ? It's a bit weird to me.

This is indeed a problem because before the size was checked. I will go through all the occurrences of U128::from_integer( on master and fix it when it's needed. Wrote this before I realized that we lose the size check with this change. Thanks

Fixed it in this commit. The size is still not checked when the u128 value is an argument of a public function but that is covered by this issue.

You can do retrieved_note.note.value.assert_max_bit_size::<128>() before you cast it to a u128 to ensure that it fits.

noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr

noir-projects/noir-protocol-circuits/crates/bug-collecting-crate/devex-integers.nr

noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr

noir-projects/noir-protocol-circuits/crates/types/src/traits.nr

noir-projects/noir-protocol-circuits/crates/types/src/type_serialization.nr

yarn-project/end-to-end/src/e2e_token_contract/minting.test.ts

yarn-project/noir-bb-bench/src/index.ts

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch from 5d46e61 to db93615 Compare February 24, 2025 09:52

benesjan added the e2e-all CI: Deprecated, use ci-full. Enable all master checks. label Feb 24, 2025

benesjan marked this pull request as ready for review February 24, 2025 09:52

benesjan changed the title ~~refactor: using native u128 type~~ refactor!: using native u128 type Feb 24, 2025

benesjan marked this pull request as draft February 24, 2025 11:02

benesjan marked this pull request as ready for review February 24, 2025 11:48

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch from 4a41fe7 to 49aa800 Compare February 24, 2025 13:02

benesjan commented Feb 24, 2025

View reviewed changes

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch from f8914a4 to f54fbd9 Compare February 24, 2025 18:01

benesjan requested review from fcarreiro and dbanks12 as code owners February 24, 2025 18:01

benesjan marked this pull request as draft February 24, 2025 18:02

benesjan marked this pull request as ready for review February 24, 2025 18:27

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch from f54fbd9 to 12259a6 Compare February 24, 2025 19:01

fcarreiro removed request for dbanks12 and fcarreiro February 24, 2025 20:26

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch 3 times, most recently from 51c4068 to 2a6b583 Compare February 25, 2025 15:22

benesjan commented Feb 25, 2025

View reviewed changes

benesjan requested review from nventuro, sklppy88 and Thunkar February 25, 2025 16:04

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch from 2a6b583 to f6949f1 Compare February 25, 2025 18:16

benesjan removed the e2e-all CI: Deprecated, use ci-full. Enable all master checks. label Feb 26, 2025

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch from f6949f1 to 1fa2b77 Compare February 26, 2025 01:06

benesjan commented Feb 26, 2025

View reviewed changes

sklppy88 approved these changes Feb 26, 2025

View reviewed changes

benesjan marked this pull request as draft February 26, 2025 13:53

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch from 5915e27 to ca92e98 Compare February 26, 2025 13:54

benesjan marked this pull request as ready for review February 26, 2025 14:45

benesjan enabled auto-merge (squash) February 26, 2025 14:47

benesjan disabled auto-merge February 26, 2025 15:22

benesjan added 20 commits February 26, 2025 17:00

refactor: using native u128 type

0436d4d

fix

cb558cf

WIP

487d54d

WIP

4810249

fixes

4063f7a

migration notes

f5d6350

more tests

9e0c3e9

fixes

8bd5ca6

fixes

b76440b

fix

5a45a0f

fixes

36035ca

fixes

35cb196

fixes

4fb8571

removing workaround

e4aa38c

fix

f860e7a

fmt

8f5164d

nuking stale comment

0480cd4

nuking redundant test

18488c8

fix: u128 casting issue

d1c3841

fix

30141fc

benesjan force-pushed the 02-24-refactor_using_native_u128_type branch from cc9de73 to 30141fc Compare February 26, 2025 17:00

benesjan enabled auto-merge (squash) February 26, 2025 17:01

benesjan merged commit 52ae8cc into master Feb 26, 2025
10 checks passed

benesjan deleted the 02-24-refactor_using_native_u128_type branch February 26, 2025 17:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: using native `u128` type #12213

refactor!: using native `u128` type #12213

benesjan commented Feb 24, 2025 •

edited

Loading

benesjan commented Feb 24, 2025 •

edited

Loading

benesjan Feb 24, 2025 •

edited

Loading

benesjan Feb 25, 2025

benesjan Feb 25, 2025 •

edited

Loading

benesjan Feb 25, 2025

benesjan Feb 26, 2025

sklppy88 left a comment

sklppy88 Feb 26, 2025

benesjan Feb 26, 2025

benesjan Feb 26, 2025

TomAFrench Feb 26, 2025

refactor!: using native u128 type #12213

refactor!: using native u128 type #12213

Conversation

benesjan commented Feb 24, 2025 • edited Loading

benesjan commented Feb 24, 2025 • edited Loading

benesjan Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

benesjan Feb 25, 2025

Choose a reason for hiding this comment

benesjan Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

benesjan Feb 25, 2025

Choose a reason for hiding this comment

benesjan Feb 26, 2025

Choose a reason for hiding this comment

sklppy88 left a comment

Choose a reason for hiding this comment

sklppy88 Feb 26, 2025

Choose a reason for hiding this comment

benesjan Feb 26, 2025

Choose a reason for hiding this comment

benesjan Feb 26, 2025

Choose a reason for hiding this comment

TomAFrench Feb 26, 2025

Choose a reason for hiding this comment

refactor!: using native `u128` type #12213

refactor!: using native `u128` type #12213

benesjan commented Feb 24, 2025 •

edited

Loading

benesjan commented Feb 24, 2025 •

edited

Loading

benesjan Feb 24, 2025 •

edited

Loading

benesjan Feb 25, 2025 •

edited

Loading