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

Why doesn't mul() check for b == 0 in the first if? #930

Closed
1 of 3 tasks
jeffreydwalter opened this issue May 4, 2018 · 3 comments
Closed
1 of 3 tasks

Why doesn't mul() check for b == 0 in the first if? #930

jeffreydwalter opened this issue May 4, 2018 · 3 comments

Comments

@jeffreydwalter
Copy link

🎉 Description

I'm just curious to know why this function:

  function mul(uint256 a, uint256 b) internal pure returns (uint256) {
    if (a == 0) {
      return 0;
    }
    uint256 c = a * b;
    assert(c / a == b);
    return c;
  }

isn't written like this:

  function mul(uint256 a, uint256 b) internal pure returns (uint256) {
    if (a == 0 || b == 0) {
      return 0;
    }
    uint256 c = a * b;
    assert(c / a == b);
    return c;
  }

Why not check for b == 0 in the first if and return?

  • 🐛 This is a bug report.
  • 📈 This is a feature request.
  • ? This is a question.
@come-maiz
Copy link
Contributor

Take a look at the pull request that added this check:
#522

The comments in there discuss exactly your question.

@jeffreydwalter
Copy link
Author

Makes sense. Thanks! Is it really that rare to have b == 0? I'd be curious (but not too curious) to see a comparison of b == 0 with an explicit check and without.

@come-maiz
Copy link
Contributor

@emn178 here is assuming that most cases will be non-zero, which sounds reasonable. So this is about gas cost for this case, not about how rare is b = 0 compared to a = 0.
You can use remix as they did in the PR to satisfy your curiousity :)

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

2 participants