morpho-org / morpho-utils

Repository gathering useful libraries and contracts.
GNU Affero General Public License v3.0
64 stars 1 forks source link

Rename compound math functions #21

Closed pakim249CAL closed 1 year ago

pakim249CAL commented 2 years ago

There can be potential confusion with the function names of CompoundMath imo. This lib does fixed point mul and div, which is not the behavior of most past usage of mul and div in solidity when both the inputs and outputs are uint256. This can be confusing imo if contracts use using CompoundMath for uint256. For readability and to remove any possible confusion, I propose renaming the fns. Some of my ideas:

MathisGD commented 1 year ago

I think that the reason why it has been let like this, is that we wanted this lib to be able to replace without any change Compound's lib. If we want to change the naming now (which might be a good idea), I would go for wadMul and wadDiv. And I would change the name of the lib for WadMath.

MathisGD commented 1 year ago

What do you think about switching everything to rMul.. wMul... etc ?

Rubilmax commented 1 year ago

I find wad & ray prefixes clearer and not that long Also, we'd be introducing a breaking change

MathisGD commented 1 year ago

okok.

note that there is also the rounding at the end: rayMulDown ...

Tristan22400 commented 1 year ago

So, What convention do we choose for the renaming of compound functions?

pakim249CAL commented 1 year ago

Maybe this issue can be closed, as I don't anticipate having to do anything for compound in the future.

MerlinEgalite commented 1 year ago

Yep