Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add barnbridge strategy #82

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Add barnbridge strategy #82

wants to merge 19 commits into from

Conversation

thashimoto1998
Copy link
Contributor

No description provided.

@Dominator008
Copy link
Member

Dominator008 commented Apr 9, 2021

@thashimoto1998 FYI I'm fixing the Compound implementation to be compatible with a view only IStrategy.getBalance() requirement.

@thashimoto1998
Copy link
Contributor Author

I encountered below compatible error when compile barnbridge strategy.

contracts/strategies/StrategyBarnBridgeJcUSDC.sol:80:31: TypeError: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
        uint256 jcUsdcPrice = IISmartYield(smartYield).price();
                              ^------------------------------^

Error HH600: Compilation failed

@thashimoto1998
Copy link
Contributor Author

I see, there is need to also modify interface.

@Dominator008
Copy link
Member

I see, there is need to also modify interface.

Yeah my bad, it looks like there is no way to enforce getBalance() to be view-only for anything Compound related. In that case I'll rename IStrategy.getBalance() to syncBalance() and remove the view-only modifier.

@Dominator008
Copy link
Member

@thashimoto1998 FYI, I changed getAssetAmount() and getPrice() back to view in the V2 contract interfaces. I think we can cache ISmartYield(smartYield).price() periodically and use the cached price in the view functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants