Submitted by hyh
Both contracts treat meta assets as if they have fixed decimals of 18. Minting logic breaks when its not the case. However, meta tokens decimals arent controlled.
If actual meta assets have any other decimals, minting slippage control logic of both contracts will break up as total is calculated as a plain sum of token amounts.
In the higher token decimals case minTotalAmount will be magnitudes higher than actual amount Curve can provide and minting becomes unavailable.
In the lower token decimals case minTotalAmount will lack value and slippage control will be rendered void, which opens up a possibility of a fund loss from the excess slippage.
Setting severity to medium as the contract can be used with various meta tokens (_metaPoolAssetCache  can be filled with any assets) and, whenever decimals differ from 18 add_liquidity uses, its logic be broken: the inability to mint violates the contract purpose, the lack of slippage control can lead to fund losses.
I.e. this is system breaking impact conditional on a low probability assumption of different meta token decimals.
Meta tokens decimals are de facto hard coded into the contract as plain amounts are used (L. 905):
ThreePoolAssetManager.sol#L896-L905
ThreePoolAssetManager.sol#L915-L919
The same plain sum approach is used in EthAssetManager._mintMetaPoolTokens:
EthAssetManager.sol#L566-L573
When this decimals assumption doesnt hold, the slippage logic will not hold too: either the mint be blocked or slippage control disabled.
Notice, that ThreePoolAssetManager.calculateRebalance do query alUSD decimals (which is inconsistent with the above as its either fix and control on inception or do not fix and accommodate the logic):
ThreePoolAssetManager.sol#L338-L338
If meta assets are always supposed to have fixed decimals of 18, consider controlling it at the construction time.
I.e. the decimals can be controlled in constructors:
EthAssetManager.sol#L214-L219
ThreePoolAssetManager.sol#L254-L256
In this case further decimals reading as its done in calculateRebalance() is redundant.
Otherwise (which is less recommended as fixed decimals assumption is viable and simplify the logic) the meta token decimals can be added to calculations similarly to stables:
ThreePoolAssetManager.sol#L779-L779
0xfoobar (Alchemix) confirmed
0xleastwood (judge) commented:
