Ensure safe Uint64/32 instanciations, with new Unsafe.fromField()

[julio4]

Related to #1286
Wanted to explore the o1js repository and tried to tackle this issue.

• Changed Field to FieldVar in constructors
• Removed Field from from() functions
• Added Unsafe.fromField()

The sha256 gadget still rely on a lots of integers directly from field, so this may be a follow up issue as well.

[mitschabaude]

Awesome!

[mitschabaude]

The sha256 gadget still rely on a lots of integers directly from field, so this may be a follow up issue as well.

not needed, sha256 is a low-level gadget and has to reach beyond the safe abstraction of a UInt32

[julio4]

The sha256 gadget still rely on a lots of integers directly from field, so this may be a follow up issue as well.

not needed, sha256 is a low-level gadget and has to reach beyond the safe abstraction of a UInt32

Ok, then I can safely use the FieldVar value of the Field (or the new Unsafe.fromField).
Could you please review the changes inside the methods of integers to be sure that there is nothing unsafe (As I only used Field.value internally)?

[mitschabaude]

Hey @julio4 this is really valuable work that we'd love to include in a release soon. Any way we can help land this?
If you don't have time to finish it, I'm offering to find someone on our team to do so!

[julio4]

Hey @julio4 this is really valuable work that we'd love to include in a release soon. Any way we can help land this? If you don't have time to finish it, I'm offering to find someone on our team to do so!

Hello! I planned to work on it this week. But I think it should be ready, I would just need some help to run pipelines and build locally. I followed the local dev env setup but I encountered some issues.

[mitschabaude]

I would just need some help to run pipelines and build locally.

@julio4 sure, try to run the following steps. does it fail somewhere?

# ensure packages are up to date
npm i

# pull in dependencies in git submodules
git submodule update --init --recursive

# ts build of the main code base
npm run build

# check that all examples still build
npm run build:examples

# check that integration tests work (takes long, feel free to stop after a minute or so if it seems to work)
npm run test:integration

# run unit tests (this takes super long unfortunately, feel free to let ci do this if you got this far)
npm run test:unit

[julio4]

@julio4 sure, try to run the following steps. does it fail somewhere?

# ensure packages are up to date
npm i

# pull in dependencies in git submodules
git submodule update --init --recursive

# ts build of the main code base
npm run build

# check that all examples still build
npm run build:examples

# check that integration tests work (takes long, feel free to stop after a minute or so if it seems to work)
npm run test:integration

# run unit tests (this takes super long unfortunately, feel free to let ci do this if you got this far)
npm run test:unit

Thank you! But I already followed the README-dev and it fail at the main ts build:

building cjs version of ./src/index.ts
using bindings from ./src/bindings/compiled/node_bindings/
▲ [WARNING] "import.meta" is not available with the "cjs" output format and will be empty [empty-import-meta]

    dist/node/bindings/js/node/node-backend.js:6:10:
      6 │ let url = import.meta.url;
        ╵           ~~~~~~~~~~~

  You need to set the output format to "esm" for "import.meta" to work correctly.

Do you know what node version is used?

[mitschabaude]

Thank you! But I already followed the README-dev and it fail at the main ts build:

@julio4 the command line output you show isn't a failure. It's a warning, which does not point to an actual problem

[mitschabaude]

you can also see in the CI tests that the build works fine, but something about UInt construction still fails in every single test, at runtime

[julio4]

@julio4 the command line output you show isn't a failure. It's a warning, which does not point to an actual problem

Oh yes, I didn't noticed but all builds are working. The error comes from account-update, but I'm having some troubles finding where's the UInt are created.

[mitschabaude]

@julio4 it seems that every construction of uints is failing. So if the account update file is hard to parse, I recommend to just create a trivial test or run the integer test with ./jest src/lib/int.test.ts

[mitschabaude]

@julio4 thanks so much for doing most of this heavy change! I pushed the last remaining fixes, it's ready to go!

[julio4]

@julio4 thanks so much for doing most of this heavy change! I pushed the last remaining fixes, it's ready to go!

Thank you for fixing the issue. I tried to dig a little bit on it and didn't found what was causing the error.