PaulRBerg / PaulRBerg/prb-math
overflow classification and error is inconsistent on divide by zero in mulDiv
Nobody has claimed this yet.
- Dominant language
- Solidity
- Stars
- 1k
- Forks
- 145
- PR merge metrics
- No merged PRs in 30d
Description
What version of PRBMath are you using?
5959ef59f906d689c2472ed08797872a1cc00644
What version of Solidity are you using?
0.8.19
Describe the bug
the unchecked division at the start of mulDiv actually jumps into a divide by zero check even though it is "unchecked".
this is probably a good thing because mulDiv(0, 1e18, 0) and mulDiv(1, 1e18, 0) and other examples cause prod1 to be 0 which is treated as a "non overflow case" according to the comments, but we still don't want to allow dividing by zero here.
further down in the same fn, divide by 0 triggers an "overflow" error if prod1 >= denominator and the denominator is 0.
i don't think this is dangerous but it leads to inconsistent errors which makes fuzzing downstream quite fiddly if we want to assert on the error selector with foundry.
the same error should be thrown for "divide by 0" regardless of the internal prod0 and prod1 logic.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the mulDiv function and reproduce the divide-by-zero examples mulDiv(0, 1e18, 0) and mulDiv(1, 1e18, 0). Trace how prod0 and prod1 affect the checks, then add or update coverage so every divide-by-zero path produces the same error selector.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- solidity
- Domain
- blockchain
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100