Support UTF-8 encode/decode for CircuitString

[chiro-hiro]

We're working on zkDatabase project and found an issue with unicode characters, I think this patch could help.

const a = CircuitString.fromString('Xin chào O1js!');

console.log(a.toString(), a.length().toString());

Before this patch:

Xin cho O1js! 13

After this patch:

Xin chào O1js! 15

[mitschabaude]

lgtm!

I just want to mention that this is breaking for circuits that use constant CircuitStrings with non-ASCII characters.
Current handling of those strings could be viewed as a bug, though, in which case you could argue we don't have to preserve these (buggy) circuits

[chiro-hiro]

Actually, our teams also treats all unicode strings as invalid strings and work around with Bytes, which is also problematic. I agree that this code shouldn't be rolled out without the notice for developers.

[45930]

In the past when we made updates to stable APIs, we created new versions targeting the next major o1js version and deprecated the existing version (but didn't modiify it). I wonder if we could do the same thing here and make a new class called RawCircuitStringUTF8 with the new behavior.

I'm not sure how many are currently using circuitString, but breaking VKs is really annoying.

[chiro-hiro]

In the past when we made updates to stable APIs, we created new versions targeting the next major o1js version and deprecated the existing version (but didn't modiify it). I wonder if we could do the same thing here and make a new class called RawCircuitStringUTF8 with the new behavior.

I'm not sure how many are currently using circuitString, but breaking VKs is really annoying.

@45930 What do you think about adding new encoding parameter in .fromString()?

e.g:

type CircuitStringEncoding = 'utf-8' | 'ascii';

fromString(str: string, encoding: CircuitStringEncoding = 'ascii');

toString(str: string, encoding: CircuitStringEncoding = 'ascii');

This change will maintain the backward compatibility.

[45930]

@chiro-hiro I like that as an API, but your current changes modify the toValue and fromValue code in the raw circuit string, so both branches would use your new code, so it would still be a breaking change.

If you can update the fromString and toString methods instead, and leave the value conversion methods unchanged, I think that would work and not be breaking.

[chiro-hiro]

@45930 I've updated my code to support both ASCII and UTF-8 following your suggestion (minimizing the modifications in RawCircuitString). I may need to add several tests to CircuitString to make sure it working properly.

Add .setEncoding() method to help developer set default encoding to prevent misconfiguration.

import { CircuitString } from 'o1js';

const a = CircuitString.fromString('Xin chào O1js!'); // Xin cho O1js! 14

console.log(a.toString(), a.length().toString());

CircuitString.setEncoding('utf-8');

const b = CircuitString.fromString('Xin chào O1js!'); // Xin chào O1js! 15

console.log(b.toString(), b.length().toString());

[chiro-hiro]

@45930 Can you check this PR? is it work for u?.

[45930]

It looks good to me!

Can you add a changelog entry and fix the styling issues (https://github.com/o1-labs/o1js/blob/main/README-dev.md#style-guide)? Then I can merge.

[chiro-hiro]

I updated the changelog following your suggestion.

$ npx oxlint --config=./.oxlintrc.json src/lib/provable/string.ts
Found 0 warnings and 0 errors.
Finished in 4ms on 1 file with 90 rules using 16 threads.
$ npx prettier --config ./.prettierrc.cjs  -c src/lib/provable/string.ts
Checking formatting...
All matched files use Prettier code style!

Checked the lint and format as well, probably a false positive. Github action shown errors and warnings in 250 files.

[boray]

I updated the changelog following your suggestion.

$ npx oxlint --config=./.oxlintrc.json src/lib/provable/string.ts
Found 0 warnings and 0 errors.
Finished in 4ms on 1 file with 90 rules using 16 threads.
$ npx prettier --config ./.prettierrc.cjs  -c src/lib/provable/string.ts
Checking formatting...
All matched files use Prettier code style!

Checked the lint and format as well, probably a false positive. Github action shown errors and warnings in 250 files.

Runningnpm run format src/lib/provable/string.ts should fix the failing check.

[chiro-hiro]

Hi @boray, I updated the given code. Didn't aware that the prettier rules were changed.

[boray]

You can follow README-dev.md#bindings-check-in-ci to fix the failing bindings check.

[chiro-hiro]

@boray My branch is out date and it was generate a huge patch, just sync with main. Can you trigger the action so I can get the binding patch?.

[chiro-hiro]

@boray I can't reproduce the issue in Github action.

Node.js v22.12.0
Ubuntu on ARM64

[45930]

Thanks @chiro-hiro, the change looks good to me! Since there are no bindings diffs, I think it's ok to merge.