Stakedllc / robo-advisor-yield

The Robo-Advisor for Yield
https://staking.staked.us/ray
Other
45 stars 15 forks source link

Fails calls to get current balance for some portfolios #17

Closed pash7ka closed 4 years ago

pash7ka commented 4 years ago

During my tests I've found the problem, that if my smart-contract needs to get its current balance from RAY and then write something to storage, it fails with "evm: write protection" error. Here is an example of such transaction. I've found the reason for this: in CompoundOpportunity you have

  function getBalance(address principalToken) external view returns(uint) {

      address compoundMarket = markets[principalToken];
      uint currCompoundBalance = CompoundInterface(compoundMarket).balanceOf(address(this)) * CompoundInterface(compoundMarket).exchangeRateCurrent() / COMPOUND_BALANCE_DENOM; // put it in wei

      return currCompoundBalance;

  }

This function is defined as view function, but it uses CompoundInterface.exchangeRateCurrent() which is not view. And during this call it executes CToken.accrueInterest(), which actually writes current exchange rate to storage.

All this stuff is working if you are trying to get balance from frontend, because no transaction is actually executed in this case. Probably for perfomance reasons such calls do not check state modification inside STATICCALL. But as soon as you do this in a state-changing transaction, this fails.

To fix this you may just change exchangeRateCurrent() to exchangeRateStored(), that's what i did in my test deployment, and it works. The only drawback - balance is not updated as fast as before, i need to wait untill some other transaction calls exchangeRateCurrent() and stores actual exchange rate.

dpurhar27 commented 4 years ago

Hi @pash7ka ,

Thanks for raising this. Your diagnosis is correct. It's a known issue and we're modifying our Opportunity interface in the future to remove the view functionality, as not all (ie. Compound) Opportunities will conform to it.

This has been an issue for you because I'm assuming you're using Solidity 0.5.0 or greater, and these versions started enforcing the view and pure (or constant) behavior on functions as it started using the STATICCALL opcode to execute them which reverts if any state modification is made, while previous versions didn't.

The short-term solution for you is to remove view from the function in the interface you're using to interact with RAY (ie. getTokenValue()?). This way, your contracts will no longer use theSTATICCALL opcode to execute the function, and when it modifies state, will no longer revert.

Please let me know if you have any questions!

pash7ka commented 4 years ago

Hi @dpurhar27 , I've tested your short-term solution and it works in my rinkeby test. Hope that will also work on mainnet. Thank you!

dpurhar27 commented 4 years ago

Glad to hear @pash7ka! It will also work on Mainnet.

I've made an internal note to change the interfaces displayed in this repo 👍.