Skip to content

Conversation

@Eduardogbg
Copy link

i need to add more tests, particularly about the consistent rounding charles mentioned. also i think we need to merge json ast first?

Copy link
Member

@xrchz xrchz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, pending tests!

@xrchz
Copy link
Member

xrchz commented Dec 26, 2025

You could update the readme too to say that isqrt is done. Also maybe the readme should be more clear about what it's talking about - it was kind of a snapshot at the time of submission for a grant (hence it fixes particular HOL and Verifereum commits), but maybe we want to now make it living again (can be in this PR or separately).

| BlockHash
| Env env_item
| Acc account_item
| Isqrt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it would be cleaner for builtins to go inside of a separate datatype (maybe even further broken down by builtin class, e.g. BUILTIN_Math, BUILTIN_Chain, BUILTIN_Env, etc)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already the built-in datatype right? Maybe we can refactor it later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants