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

Optimize multiplication operation on Vyper #965

Closed
hskang9 opened this issue Jul 27, 2018 · 5 comments
Closed

Optimize multiplication operation on Vyper #965

hskang9 opened this issue Jul 27, 2018 · 5 comments

Comments

@hskang9
Copy link
Contributor

hskang9 commented Jul 27, 2018

Simple Summary

Optimize multiplication check as recent openzeppelin safemath implementation.

Abstract

One of the things I like Vyper is that it supports safemath from the language.
Its logic is on expr.py.
However, I found a typo in the comment:
from
# Checks that: a == 0 || a / b == b
to
# Checks that: a == 0 || c(which is a*b) / a == b

and a space for gas optimization which was approved in Openzeppelin's SafeMath implementation.

Motivation

If this change is not approved, comment typo is not fixed and vyper uses more gas than solidity with safemath contract.

Specification

Here is the difference of Opcode to be pushed to LLL parse tree:

Before

assert(a==0 || (a*b / a) == b)

['assert', ['or', ['iszero', left],
['eq', ['div', ['mul', left, right], left], right]]],
['mul', left, right]

After

if (a == 0) { return 0; } // Without verifying a*b / a == b
assert(a*b/a==b);

[  'if', 
   ['eq', left, 0],
   [0], 
  ['seq',
     ['assert', ['eq', ['div', ['mul', left, right], left], right]],
     ['mul', left, right]
  ]
]

Reasons and verification is described on the link here

Backwards Compatibility

This version is compatible with the others but it uses less gas for an arithmetic operation.

Copyright

Copyright and related rights waived via CC0

hskang9 added a commit to hskang9/vyper that referenced this issue Jul 27, 2018
@jacqueswww
Copy link
Contributor

jacqueswww commented Jul 27, 2018

Wow good catch picking that up from SafeMath! 😄

@fubuloubu
Copy link
Member

I'm missing something about this comment in context of the code. How does checking if and only if a == 0 in order to proceed with any multiplication help?

@hskang9 hskang9 closed this as completed Jul 27, 2018
@hskang9 hskang9 reopened this Jul 27, 2018
@hskang9
Copy link
Contributor Author

hskang9 commented Jul 27, 2018

Oops. I passed out 3 hours ago and checked this. I will implement it right.

@jacqueswww
Copy link
Contributor

jacqueswww commented Jul 27, 2018

@fubuloubu so basically you save gas because you immediately return 0 for 0/x instead of doing all the checks and the mul and return 0 anyways.

@fubuloubu
Copy link
Member

Okay, that makes way more sense now lol

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

No branches or pull requests

3 participants