Skip to content

overflow classification and error is inconsistent on divide by zero in mulDiv #201

@thedavidmeister

Description

@thedavidmeister

What version of PRBMath are you using?

5959ef5

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    effort: mediumDefault level of effort.priority: 2We will do our best to deal with this.type: refactorChange that neither fixes a bug nor adds a feature.work: complicatedSense-analyze-respond. The relationship between cause and effect requires analysis or expertise.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions