Skip to content

harvester & unit tests #1331

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 16, 2023
Merged

harvester & unit tests #1331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9602f0c
harvester now considers different oracle decimals. Fixing unit tests
sparrowDom Apr 14, 2023
37009da
dev oracle doesn't need to cache decimals
sparrowDom Apr 16, 2023
fd1af23
prettier
sparrowDom Apr 16, 2023
6380da1
improve code readability
sparrowDom Apr 16, 2023
907f2f2
support different decimal formats in unit tests
sparrowDom Apr 16, 2023
7a02455
prettier & bug fix
sparrowDom Apr 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions contracts/contracts/harvest/Harvester.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,13 +382,13 @@ contract Harvester is Governable {

// This'll revert if there is no price feed
uint256 oraclePrice = IOracle(priceProvider).price(_swapToken);
// Oracle price is 1e8, USDT output is 1e6

// Oracle price is 1e18, USDT output is 1e6
uint256 minExpected = (balanceToSwap *
oraclePrice *
(1e4 - tokenConfig.allowedSlippageBps)).scaleBy( // max allowed slippage
6,
Helpers.getDecimals(_swapToken) + 8
Copy link
Contributor

@DanielVF DanielVF Apr 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scaleby feels wrong here.

I don't think we should be scaling up by some giant amount here. Maybe we should rewrite this line for clarity?

Copy link
Member Author

@sparrowDom sparrowDom Apr 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are scaling down here from (token decimals + 1e18) to 6 decimals (USDT).

But yeah clarity is not the best... Improved it here: 6380da1

) / 1e4; // fix the max slippage decimal position
(1e4 - tokenConfig.allowedSlippageBps) * // max allowed slippage
oraclePrice).scaleBy(6, Helpers.getDecimals(_swapToken)) /
1e4 / // fix the max slippage decimal position
1e18; // and oracle price decimals position

// Uniswap redemption path
address[] memory path = new address[](3);
Expand Down
22 changes: 17 additions & 5 deletions contracts/contracts/oracle/OracleRouter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,7 @@ abstract contract OracleRouterBase is IOracle {
return uint256(_price);
}

function getDecimals(address _asset)
internal
view
returns (uint8)
{
function getDecimals(address _asset) internal view virtual returns (uint8) {
uint8 decimals = decimalsCache[_asset];
require(decimals > 0, "Oracle: Decimals not cached");
return decimals;
Expand Down Expand Up @@ -164,6 +160,22 @@ contract OracleRouterDev is OracleRouterBase {
assetToFeed[_asset] = _feed;
}

/*
* The dev version of the Oracle doesn't need to gas optimize and cache the decimals
*/
function getDecimals(address _asset)
internal
view
override
returns (uint8)
{
address _feed = feed(_asset);
require(_feed != address(0), "Asset not available");
require(_feed != FIXED_PRICE, "Fixed price feeds not supported");

return AggregatorV3Interface(_feed).decimals();
}

/**
* @dev The price feed contract to use for a particular asset.
* @param asset address of the asset
Expand Down
1 change: 1 addition & 0 deletions contracts/contracts/utils/StableMath.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ library StableMath {
if (to > from) {
x = x.mul(10**(to - from));
} else if (to < from) {
// slither-disable-next-line divide-before-multiply
x = x.div(10**(from - to));
}
return x;
Expand Down
20 changes: 10 additions & 10 deletions contracts/deploy/000_mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,47 +140,47 @@ const deployMocks = async ({ getNamedAccounts, deployments }) => {
await deploy("MockChainlinkOracleFeedDAI", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 DAI = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 DAI = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedUSDT", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 USDT = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 USDT = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedUSDC", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 USDC = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 USDC = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedTUSD", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 TUSD = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 TUSD = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedCOMP", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 COMP = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 COMP = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedAAVE", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 AAVE = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 AAVE = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedCRV", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 CRV = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 CRV = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedCVX", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 CVX = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 CVX = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedNonStandardToken", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1", 8).toString(), 18], // 1 = 1 USD, 8 digits decimal.
args: [parseUnits("1", 8).toString(), 8], // 1 = 1 USD, 8 digits decimal.
});
await deploy("MockChainlinkOracleFeedETH", {
from: deployerAddr,
Expand All @@ -195,7 +195,7 @@ const deployMocks = async ({ getNamedAccounts, deployments }) => {
await deploy("MockChainlinkOracleFeedRETHETH", {
from: deployerAddr,
contract: "MockChainlinkOracleFeed",
args: [parseUnits("1.2", 8).toString(), 18], // 1 RETH = 1.2 ETH , 8 digits decimal.
Copy link
Contributor

@DanielVF DanielVF Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this.

args: [parseUnits("1.2", 18).toString(), 18], // 1 RETH = 1.2 ETH , 18 digits decimal.
});

// Deploy mock Uniswap router
Expand Down
47 changes: 37 additions & 10 deletions contracts/deploy/001_core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,52 +677,79 @@ const deployOracles = async () => {
// Not needed in production
const oracleAddresses = await getOracleAddresses(deployments);
const assetAddresses = await getAssetAddresses(deployments);
withConfirmation(
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.DAI, oracleAddresses.chainlink.DAI_USD)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.DAI)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.USDC, oracleAddresses.chainlink.USDC_USD)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.USDC)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.USDT, oracleAddresses.chainlink.USDT_USD)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.USDT)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.TUSD, oracleAddresses.chainlink.TUSD_USD)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.TUSD)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.COMP, oracleAddresses.chainlink.COMP_USD)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.COMP)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.AAVE, oracleAddresses.chainlink.AAVE_USD)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.AAVE)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.CRV, oracleAddresses.chainlink.CRV_USD)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.CRV)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.CVX, oracleAddresses.chainlink.CVX_USD)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.CVX)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(assetAddresses.RETH, oracleAddresses.chainlink.RETH_ETH)
);
withConfirmation(
await withConfirmation(
oracleRouter.connect(sDeployer).cacheDecimals(assetAddresses.RETH)
);
await withConfirmation(
oracleRouter
.connect(sDeployer)
.setFeed(
Expand Down
11 changes: 11 additions & 0 deletions contracts/deploy/052_decimal_cache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ module.exports = deploymentWithGovernanceProposal(
await cOracleRouter.cacheDecimals(addresses.mainnet.CRV);
await cOracleRouter.cacheDecimals(addresses.mainnet.CVX);

const cHarvesterProxy = await ethers.getContract("HarvesterProxy");
const dHarvester = await deployWithConfirmation("Harvester", [
cVaultProxy.address,
assetAddresses.USDT,
]);

const cVaultAdmin = new ethers.Contract(cVaultProxy.address, [
{
inputs: [
Expand Down Expand Up @@ -93,6 +99,11 @@ module.exports = deploymentWithGovernanceProposal(
signature: "cacheDecimals(address)",
args: [assetAddresses.USDC],
},
{
contract: cHarvesterProxy,
signature: "upgradeTo(address)",
args: [dHarvester.address],
},
],
};
}
Expand Down
2 changes: 2 additions & 0 deletions contracts/test/_fixture.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1200,6 +1200,8 @@ async function hackedVaultFixture() {
evilDAI.address,
oracleAddresses.chainlink.DAI_USD
);
await oracleRouter.cacheDecimals(evilDAI.address);

await fixture.vault.connect(sGovernor).supportAsset(evilDAI.address, 0);

fixture.evilDAI = evilDAI;
Expand Down
17 changes: 15 additions & 2 deletions contracts/test/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,15 @@ const getOracleAddress = async (deployments) => {
* @returns {Promise<void>}
*/
const setOracleTokenPriceUsd = async (tokenSymbol, usdPrice) => {
const symbolMap = {
USDC: 6,
USDT: 6,
DAI: 6,
COMP: 6,
CVX: 6,
CRV: 6,
};

if (isMainnetOrFork) {
throw new Error(
`setOracleTokenPriceUsd not supported on network ${hre.network.name}`
Expand All @@ -223,8 +232,12 @@ const setOracleTokenPriceUsd = async (tokenSymbol, usdPrice) => {
const tokenFeed = await ethers.getContract(
"MockChainlinkOracleFeed" + tokenSymbol
);
await tokenFeed.setDecimals(8);
await tokenFeed.setPrice(parseUnits(usdPrice, 8));

const decimals = Object.keys(symbolMap).includes(tokenSymbol)
? symbolMap[tokenSymbol]
: 18;
await tokenFeed.setDecimals(decimals);
await tokenFeed.setPrice(parseUnits(usdPrice, decimals));
};

const getOracleAddresses = async (deployments) => {
Expand Down
6 changes: 3 additions & 3 deletions contracts/test/vault/compound.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,10 @@ describe("Vault with Compound strategy", function () {
});

it("Should handle non-standard token deposits", async () => {
let { ousd, vault, matt, nonStandardToken, governor } = await loadFixture(
compoundVaultFixture
);
let { ousd, vault, matt, nonStandardToken, oracleRouter, governor } =
await loadFixture(compoundVaultFixture);

await oracleRouter.cacheDecimals(nonStandardToken.address);
if (nonStandardToken) {
await vault.connect(governor).supportAsset(nonStandardToken.address, 0);
}
Expand Down
6 changes: 5 additions & 1 deletion contracts/test/vault/exchangeRate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ describe("Vault Redeem", function () {
});

it("Should mint at a positive exchange rate", async () => {
const { ousd, vault, reth, anna } = fixture;
const { ousd, vault, reth, oracleRouter, anna } = fixture;

console.log(
"ORACLE PRICE",
(await oracleRouter.price(reth.address)).toString()
);
await reth.connect(anna).mint(daiUnits("4.0"));
await reth.connect(anna).approve(vault.address, daiUnits("4.0"));
await vault.connect(anna).mint(reth.address, daiUnits("4.0"), 0);
Expand Down
14 changes: 7 additions & 7 deletions contracts/test/vault/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ describe("Vault", function () {
const origAssetCount = await vault.connect(governor).getAssetCount();
expect(await vault.isSupportedAsset(ousd.address)).to.be.false;
await oracleRouter.setFeed(ousd.address, oracleAddresses.chainlink.DAI_USD);
await oracleRouter.cacheDecimals(ousd.address);
await expect(vault.connect(governor).supportAsset(ousd.address, 0)).to.emit(
vault,
"AssetSupported"
Expand Down Expand Up @@ -123,12 +124,11 @@ describe("Vault", function () {
});

it("Should correctly handle a deposit failure of Non-Standard ERC20 Token", async function () {
const { ousd, vault, anna, nonStandardToken, governor } = await loadFixture(
defaultFixture
);
const { ousd, vault, anna, nonStandardToken, oracleRouter, governor } =
await loadFixture(defaultFixture);

await oracleRouter.cacheDecimals(nonStandardToken.address);
await vault.connect(governor).supportAsset(nonStandardToken.address, 0);

await expect(anna).has.a.balanceOf("1000.00", nonStandardToken);
await setOracleTokenPriceUsd("NonStandardToken", "1.30");
await nonStandardToken
Expand Down Expand Up @@ -156,9 +156,9 @@ describe("Vault", function () {
});

it("Should correctly handle a deposit of Non-Standard ERC20 Token", async function () {
const { ousd, vault, anna, nonStandardToken, governor } = await loadFixture(
defaultFixture
);
const { ousd, vault, anna, nonStandardToken, oracleRouter, governor } =
await loadFixture(defaultFixture);
await oracleRouter.cacheDecimals(nonStandardToken.address);
await vault.connect(governor).supportAsset(nonStandardToken.address, 0);

await expect(anna).has.a.balanceOf("1000.00", nonStandardToken);
Expand Down
6 changes: 3 additions & 3 deletions contracts/test/vault/redeem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ describe("Vault Redeem", function () {
});

it("Should allow redeems of non-standard tokens", async () => {
const { ousd, vault, anna, governor, nonStandardToken } = await loadFixture(
defaultFixture
);
const { ousd, vault, anna, governor, oracleRouter, nonStandardToken } =
await loadFixture(defaultFixture);

await oracleRouter.cacheDecimals(nonStandardToken.address);
await vault.connect(governor).supportAsset(nonStandardToken.address, 0);

await setOracleTokenPriceUsd("NonStandardToken", "1.00");
Expand Down