search

dforce-network / dToken

dForce yield token
MIT License
30 stars 10 forks source link

Rebalancing will fail if the token being rebalanced is deflationary #16

Open mgcolburn opened 4 years ago

mgcolburn commented 4 years ago

Severity: Informational

Description

Deflationary tokens don't send the full expected amount to the destination on calls to transfer/transferFrom, which can result in incorrect bookkeeping. The DToken contract may be affected if this type of token were used due to the following sanity check:

 require( 
     IHandler(_withdraw[i]).withdraw(_token, _withdrawAmount[i]) == 
         _realWithdrawAmount[i], 
     "rebalance: actual withdrown amount does not match the wanted" 
 );

It's not a severe issue right now, but it could result in the system not holding enough tokens for users' positions and they may not receive the expected amount of tokens out.

Note that some high-profile tokens are (or can become) deflationary. For example, USDT could take a fee on transfers, though it is not currently enabled.

This behavior may be intentional, or this check might just be a sanity check slapped on there without really thinking about the implications. Given the recent fiasco revolving around deflationary tokens though, might as well mention it. Tether is technically deflationary, after all (although currently the fee is set to zero)

Recommendation

Check for this type of behavior when considering supporting new assets and avoid the use of deflationary tokens.

Donald-Nobel commented 4 years ago

Known issue, no further plan to fix this right now. If token such as USDT would take a fee then this asset will be disabled.