transmissions11/libcompound

cUSDC Compliance

JTraversa opened this issue · 5 comments

Revealed to me in a recent audit of our upcoming codebase, cUSDC doesnt actually follow the same interface (and perhaps other tokens) as the interface expected in LibCompound.sol.

Specifically the return of InterestModel.getBorrowRate returns two variables rather than the expected 1 variable stored in borrowRateMantissa.

As you can see, getBorrowRate() here returns a tuple (uint256, uint256), where a single value representing the borrowRateMantissa is expected. The first value is used to know if the borrowRate is valid, and the other is the borrow rate.
This library is then not compliant with all Compound tokens.

The transition happened on this commit in Nov 2019, so id imagine any CTokens launched before that date return the tuple.

My suggested remediation would be a try/catch? Though honestly I'm not familiar with the interactions there and am unsure if calling an interface incorrectly would be caught and allow you to revert to the old interface

hm interesting

in general im very uneasy about this lib these days, as relying on the internals of the comptroller is not at all safe and they could be upgraded at any time

would be happy to merge a PR to fix for cUSDC and then archive this

I believe that should cover it, but how uneasy is very uneasy hah.

Wouldnt that be the same implication for pretty much any upgradable contracts? Integrating interfaces that can be upgraded can be breaking?

very uneasy.

the difference here is we're not only relying on the existence of an interface, but that the computation we're doing based on the interface replicates the internal logic of an opaque function. what if compound added an extra free of some sort? we'd be screwed