livepeer / protocol

Livepeer protocol
MIT License
152 stars 45 forks source link

Confluence: Cleanup MathUtils vs. PreciseMathUtils library usage #506

Closed yondonfu closed 2 years ago

yondonfu commented 2 years ago

See https://github.com/livepeer/protocol/pull/496#issuecomment-998235276

yondonfu commented 2 years ago

I was thinking about this and at the moment I am inclined to leave the *MathUtils library usage as-is as the benefit seems to outweigh the costs.

The *MathUtils libraries are currently used in the following manner:

  1. All contracts except for the Minter use MathUtils to handle percentages
  2. The Minter uses MathUtilsV2 to handle percentages as of LIP-40
  3. The BondingManager also uses PreciseMathUtils (in addition to MathUtils) to handle cumulative reward factor and cumulative fee factor calculations

If we switched to PreciseMathUtils for everything, we would need to:

The main benefit I see for switching to PreciseMathUtils everywhere is consistently using a single library instead of multiple libraries with different levels of precision. At the moment, I don't think this benefit outweighs the costs outlined above given the other things we can focus on while preparing for Confluence. This doesn't preclude such a change later on, but is just based where we are at right now.

kautukkundan commented 2 years ago

I don't think this benefit outweighs the costs outlined above

Yeah agreed, the implications of the change and follow up work required is too deep in scope. Let's not do this right now then. However, It will be nice if we revisit this in future.