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

Deterministic IntegerMath #481

Closed
wants to merge 25 commits into from
Closed

Deterministic IntegerMath #481

wants to merge 25 commits into from

Conversation

MaxGraey
Copy link
Member

@MaxGraey MaxGraey commented Feb 10, 2019

@vgrichina Currently there are pretty basic concept of IntegerMath without overflow checking yet. All others functions will be stubs with "not supported" exceptions. WDYT?

Relate to #465

DONE

  • tests

сс @Jannis

Copy link

@vgrichina vgrichina 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 overall

std/assembly/math.ts Show resolved Hide resolved
std/assembly/math.ts Outdated Show resolved Hide resolved
std/assembly/math.ts Show resolved Hide resolved
std/assembly/math.ts Outdated Show resolved Hide resolved
@MaxGraey MaxGraey changed the title [WIP] Deterministic SafeMath [WIP] Deterministic IntegerMath Feb 17, 2019
@MaxGraey MaxGraey changed the title [WIP] Deterministic IntegerMath Deterministic IntegerMath Feb 20, 2019
@MaxGraey MaxGraey requested a review from dcodeIO February 20, 2019 00:55
Copy link
Member

@dcodeIO dcodeIO left a comment

Choose a reason for hiding this comment

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

LGTM, but wondering whether IntegerMath does quite cut it compared to let's name it SafeMath that also works with floats but does NaN canonicalization (or whatever is necessary to make it deterministic). IntegerMath feels a bit like it's most useful for games, which is great, but then it might or might not cross the line for what should be included with the standard library and what should become a userland module? Other than that I'd absolutely use it if I had a use case, OK to merge for me, but mentioning. Any thoughts?

@MaxGraey
Copy link
Member Author

MaxGraey commented Feb 20, 2019

@dcodeIO NaN canonicalization should be done on virtual machine side because this would be faster, safer and don't increase binary size.

@vgrichina
Copy link

@dcodeIO NaN canonicalization should be done on virtual machine side because this would be faster, safer and don't increase binary size.

@MaxGraey this is also better for us cause e.g. in distributed consensus context we cannot trust that contract is compiled to canonicalize NaNs, unless it happens either on VM or post-processing stage (vs on compile stage).

@MaxGraey
Copy link
Member Author

I will implement this in different way. But current road block is #1137.

Closing for now due to general cleanup described here #1277.

@MaxGraey MaxGraey closed this May 16, 2020
@MaxGraey MaxGraey deleted the deterministic-math branch May 16, 2020 17:12
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