thedavidmeister
commented
1 year ago As noted staking contract does not use scaleN
Worth also noting that ERC4626 that the staking contract is based on dedicates a lot of the spec to rounding/dust handling, so if scaleN would be hypothetically used in the future it would still need to be 4626 compliant (which means always leaving dust from the underlying asset in the vault in the case of rounding issues)
that's largely what the mulDiv is handling in open zeppelin's implementation and we wrap in the other fixed point math functions
as scaleN is a library contract it has no storage of its own so there's nowhere for it to save information about the lost precision directly, the best it could do is return two values, one representing the scaled value and one representing the lost precision
currently scaleN is only used in expressions in the interpreter as a provided opcode, so i'm not sure if it's something on the average expression author's radar to be worrying about or ready to handle (juggling 2 values on the stack to avoid some dust)
it's probably worth documenting all this though as it's worth pointing out as something to consider for anyone who does care about it
File: stake/Stake.sol#L15 Description: In contracts/math/FixedPointMath.sol the function scaleN will lead to precision loss if a number is scaled down to 18. There may arise a scenario in future development where a deposited token having higher decimal (i.e > 18) may need to scale down to 18 decimals for logic consistency. The conversion either from or to 18 will lead to loss in precision which may result in dust amount being locked in the contract. However, this function is not used in the current implementation of the staking contract to either scale up or down. Recommendation: Introduce a mechanism to manage the di erence that was lost during the conversion. One way could be by storing the di erence & using it to convert back.