Closed andreivladbrg closed 4 months ago
Also, I think we should start using
unchecked
wherever its safe to do so inspite of it reducing readability. We should save gas on mathematical operations.
I changed my mind, and I added the unchecked
blocks in the calculation functions. It wasn’t about readability; rather, I was concerned that something might break, so I thought it was safer not to use unchecked
. Also, the gas savings are not high.
Unfortunately, we cannot unchecked
the whole block. (amount * (10 ** normalizingFactor)).toUint128()
can overflow.
@smol-ninja updated the PR with not allowing assets with more than 18 decimals
Linked issues
Changelog
Src
deposit
totransferAmount
ERC20
assets with more than 18 decimalsHelpers
library_calculateTransferAmount
and_safeAssetDecimals
inHelpers
calculateNormalizedAmount
functioncheckAndCalculateBrokerFee
function inHelpers
and use it in_depositViaBroker
Test
create
deposit
Question: should we add independent tests for
Helpers.calculateNormalizedAmount
andHelpers.calculateTransferAmount
?