Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a check to TokenSymbol to match the same assertion as OCaml #465

Closed
MartinMinkov opened this issue Sep 30, 2022 · 2 comments · Fixed by #479
Closed

Add a check to TokenSymbol to match the same assertion as OCaml #465

MartinMinkov opened this issue Sep 30, 2022 · 2 comments · Fixed by #479
Labels
bug Something isn't working

Comments

@MartinMinkov
Copy link
Contributor

Description

In OCaml, we have an assertion to check the length of a token symbol to be a max length of 6. We should additionally do this check-in SnarkyJS as well, to save the user time before sending it off to the network.

Presumably, SnarkyJS is already doing this check but the length seems to be off. If we pass in a very long token symbol, we get the following stack trace:

Error: prefix too long
    at prefixToField (file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/hash.js:56:15)
    at Object.from (file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/hash.js:141:21)
    at Object.set (file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/account_update.js:379:86)
    at TokenContract.init (file:///workspace/deploy2berkeley/build/src/token-minaexplorer-repro.js:27:26)
    at file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/zkapp.js:89:113
    at runWith (file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/global-context.js:55:18)
    at Function.runWith (file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/global-context.js:14:37)
    at file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/zkapp.js:89:61
    at runWith (file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/global-context.js:55:18)
    at Function.runWith (file:///workspace/deploy2berkeley/node_modules/snarkyjs/dist/node/lib/global-context.js:14:37)

We should just do the initial check of length 6 when setting the token symbol.

@MartinMinkov MartinMinkov added the bug Something isn't working label Sep 30, 2022
@garethtdavies
Copy link
Contributor

garethtdavies commented Sep 30, 2022

Error is thrown here https://github.com/o1-labs/snarkyjs/blob/bcf3e8c2f8e90abfbf38a4848d1bf41cf173691f/src/lib/hash.ts#L84

Maybe something as simple as:

from(symbol: string): TokenSymbol {
    if (symbol.length > 6) throw Error('token symbol too long');
    let field = prefixToField(symbol);
    return { symbol, field };
  }

@mitschabaude
Copy link
Collaborator

The existing error is unrelated, it's if the string is too long to fit in one field element.
Yeah a simple error message like that would do it, preferably it should also mention the limit of 6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants