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.

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

Unit test fixes #1317

Merged
merged 1 commit into from
Apr 12, 2023
Merged

Unit test fixes #1317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion contracts/contracts/vault/VaultAdmin.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,10 @@ contract VaultAdmin is VaultStorage {
*/
function priceUSDMint(address asset) external view returns (uint256) {
uint256 price = IOracle(priceProvider).price(asset);
require(price >= MINT_MINIMUM_ORACLE, "Asset price below peg");
require(
price >= uint256(MINT_MINIMUM_UNIT_PRICE).scaleBy(8, 18),
"Asset price below peg"
);
if (price > 1e8) {
price = 1e8;
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/vault/VaultCore.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ contract VaultCore is VaultStorage {
if (unitPrice > 1e18) {
unitPrice = 1e18;
}
require(unitPrice >= MINT_MINIMUM_ORACLE, "Asset price below peg");
require(unitPrice >= MINT_MINIMUM_UNIT_PRICE, "Asset price below peg");
uint256 priceAdjustedDeposit = (units * unitPrice) / 1e18;

if (_minimumOusdAmount > 0) {
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/vault/VaultStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ contract VaultStorage is Initializable, Governable {
// Deprecated: Tokens that should be swapped for stablecoins
address[] private _deprecated_swapTokens;

uint256 constant MINT_MINIMUM_ORACLE = 99800000;
uint256 constant MINT_MINIMUM_UNIT_PRICE = 0.998e18;

// Meta strategy that is allowed to mint/burn OUSD without changing collateral
address public ousdMetaStrategy = address(0);
Expand Down
1 change: 1 addition & 0 deletions contracts/test/vault/exchangeRate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ describe("Vault Redeem", function () {
await vault.connect(anna).mint(reth.address, daiUnits("4.0"), 0);
await expect(anna).has.a.approxBalanceOf("4.796", ousd);
});

it("Should revert mint at too low oracle, positive exchange rate", async () => {
const { vault, reth, anna } = fixture;

Expand Down
113 changes: 63 additions & 50 deletions contracts/test/vault/harvester.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,37 @@ describe("Harvester", function () {
await comp.connect(governor).transfer(compoundStrategy.address, compAmount);
};

let fixture;

beforeEach(async function () {
fixture = await loadFixture(compoundVaultFixture);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: We aren't using the fixtures the way it's supposed to be. So, using loadFixture has zero effect on the performance. We should switch to use deployments.createFixture instead. I already started doing that on the Uniswap V3 branch. It'd be nice if we can do that for all new tests as well: https://github.com/OriginProtocol/origin-dollar/pull/1258/files#diff-42bf690234c6c24cf49e6eee681c1d54e88995355d811b66366a640e03e935dbR1253

Related issues: #1259


/* Ethereum Waffle caches fixtures and uses evm snapshot and evm revert:
* https://github.com/TrueFiEng/Waffle/blob/f0d78cd5529684f2f377aaa0025c33aed52e268e/waffle-provider/src/fixtures.ts#L18-L32
*
* to optimize the speed of test execution. Somewhere in the caching
* there is a bug where Harvester tests fail if they are ran within the whole
* unit test suite and succeed if they are ran by themselves. This is a bit
* of a nasty workaround.
*/
const { governorAddr } = await getNamedAccounts();
const sGovernor = await ethers.provider.getSigner(governorAddr);

try {
await fixture.vault
.connect(sGovernor)
.approveStrategy(fixture.compoundStrategy.address);
} catch (e) {
// ignore the strategy already approved exception
}

await fixture.harvester
.connect(sGovernor)
.setSupportedStrategy(fixture.compoundStrategy.address, true);
});

it("Should correctly set reward token config and have correct allowances set for Uniswap like routers", async () => {
const { harvester, governor, comp } = await loadFixture(
compoundVaultFixture
);
const { harvester, governor, comp } = fixture;
const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");

await harvester
Expand Down Expand Up @@ -87,9 +114,7 @@ describe("Harvester", function () {
});

it("Should fail when calling harvest or harvestAndSwap with the non valid strategy address", async () => {
const { harvester, governor, anna } = await loadFixture(
compoundVaultFixture
);
const { harvester, governor, anna } = fixture;
const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");

// prettier-ignore
Expand All @@ -113,7 +138,7 @@ describe("Harvester", function () {
});

it("Should not allow adding reward token config without price feed", async () => {
const { harvester, governor } = await loadFixture(compoundVaultFixture);
const { harvester, governor } = fixture;

await expect(
harvester
Expand All @@ -130,7 +155,7 @@ describe("Harvester", function () {
});

it("Should not allow non-Governor to set reward token config", async () => {
const { harvester, anna, comp } = await loadFixture(compoundVaultFixture);
const { harvester, anna, comp } = fixture;

await expect(
// Use the vault address for an address that definitely won't have a price
Expand All @@ -149,9 +174,7 @@ describe("Harvester", function () {
});

it("Should allow Governor to set reward token config", async () => {
const { harvester, governor, comp } = await loadFixture(
compoundVaultFixture
);
const { harvester, governor, comp } = fixture;

await harvester
.connect(governor)
Expand All @@ -167,7 +190,7 @@ describe("Harvester", function () {

it("Should skip swapping when token configuration is missing and leave harvested funds on harvester", async () => {
const { harvester, governor, comp, compoundStrategy, anna, usdt, vault } =
await loadFixture(compoundVaultFixture);
fixture;

await sendRewardsToCompStrategy("100", governor, compoundStrategy, comp);

Expand Down Expand Up @@ -200,12 +223,12 @@ describe("Harvester", function () {
josh,
usdt,
vault,
} = await loadFixture(compoundVaultFixture);
} = fixture;

await sendRewardsToCompStrategy("10", governor, compoundStrategy, comp);

const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");
mockUniswapRouter.initialize([comp.address], [usdt.address]);
await mockUniswapRouter.initialize([comp.address], [usdt.address]);
await usdt
.connect(josh)
.transfer(mockUniswapRouter.address, usdtUnits("100"));
Expand All @@ -226,7 +249,7 @@ describe("Harvester", function () {
// prettier-ignore
const annaBalanceChange = await changeInBalance(
async () => {
harvester
await harvester
.connect(anna)["harvestAndSwap(address)"](compoundStrategy.address);
},
usdt,
Expand All @@ -246,7 +269,7 @@ describe("Harvester", function () {

it("Should fail when slippage is just over threshold", async () => {
const { harvester, governor, comp, compoundStrategy, anna, josh, usdt } =
await loadFixture(compoundVaultFixture);
fixture;

await sendRewardsToCompStrategy("10", governor, compoundStrategy, comp);

Expand Down Expand Up @@ -286,12 +309,12 @@ describe("Harvester", function () {
josh,
usdt,
vault,
} = await loadFixture(compoundVaultFixture);
} = fixture;

await sendRewardsToCompStrategy("10", governor, compoundStrategy, comp);

const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");
mockUniswapRouter.initialize([comp.address], [usdt.address]);
await mockUniswapRouter.initialize([comp.address], [usdt.address]);
await usdt
.connect(josh)
.transfer(mockUniswapRouter.address, usdtUnits("100"));
Expand All @@ -310,7 +333,7 @@ describe("Harvester", function () {
// prettier-ignore
const annaBalanceChange = await changeInBalance(
async () => {
harvester
await harvester
.connect(anna)["harvestAndSwap(address)"](compoundStrategy.address);
},
usdt,
Expand All @@ -329,9 +352,7 @@ describe("Harvester", function () {
});

it("Should fail setting rewards percentage to 11%", async () => {
const { harvester, governor, comp } = await loadFixture(
compoundVaultFixture
);
const { harvester, governor, comp } = fixture;

const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");
await expect(
Expand All @@ -349,9 +370,7 @@ describe("Harvester", function () {
});

it("Should fail setting rewards percentage to a negative value", async () => {
const { harvester, governor, comp } = await loadFixture(
compoundVaultFixture
);
const { harvester, governor, comp } = fixture;

const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");

Expand Down Expand Up @@ -386,7 +405,7 @@ describe("Harvester", function () {
josh,
usdt,
vault,
} = await loadFixture(compoundVaultFixture);
} = fixture;

await sendRewardsToCompStrategy("10", governor, compoundStrategy, comp);

Expand All @@ -410,7 +429,7 @@ describe("Harvester", function () {
// prettier-ignore
const annaBalanceChange = await changeInBalance(
async () => {
harvester
await harvester
.connect(anna)["harvestAndSwap(address)"](compoundStrategy.address);
},
usdt,
Expand All @@ -429,9 +448,7 @@ describe("Harvester", function () {
});

it("Should fail when setting setSupportedStrategy from a non vault/governor address", async () => {
const { harvester, anna, compoundStrategy } = await loadFixture(
compoundVaultFixture
);
const { harvester, anna, compoundStrategy } = fixture;

// prettier-ignore
await expect(
Expand All @@ -441,9 +458,7 @@ describe("Harvester", function () {
});

it("Should succeed when governor sets a supported strategy address", async () => {
const { harvester, governor, compoundStrategy } = await loadFixture(
compoundVaultFixture
);
const { harvester, governor, compoundStrategy } = fixture;

// prettier-ignore
await harvester
Expand All @@ -463,7 +478,7 @@ describe("Harvester", function () {
vault,
dai,
threePoolStrategy,
} = await loadFixture(compoundVaultFixture);
} = fixture;
// load another strategy to override default asset strategies to lift restriction of removing compound strategy
await vault.connect(governor).approveStrategy(threePoolStrategy.address);

Expand Down Expand Up @@ -500,7 +515,7 @@ describe("Harvester", function () {
// prettier-ignore
const annaBalanceChange = await changeInBalance(
async () => {
harvester
await harvester
.connect(anna)["harvestAndSwap(address)"](compoundStrategy.address);
},
usdt,
Expand All @@ -519,9 +534,7 @@ describe("Harvester", function () {
});

it("Should fail harvestAndSwap when governor sets a strategy as not supported one", async () => {
const { harvester, governor, anna, compoundStrategy } = await loadFixture(
compoundVaultFixture
);
const { harvester, governor, anna, compoundStrategy } = fixture;

// prettier-ignore
await harvester
Expand All @@ -544,12 +557,12 @@ describe("Harvester", function () {
josh,
usdt,
vault,
} = await loadFixture(compoundVaultFixture);
} = fixture;

await sendRewardsToCompStrategy("10", governor, compoundStrategy, comp);

const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");
mockUniswapRouter.initialize([comp.address], [usdt.address]);
await mockUniswapRouter.initialize([comp.address], [usdt.address]);
await usdt
.connect(josh)
.transfer(mockUniswapRouter.address, usdtUnits("100"));
Expand All @@ -572,7 +585,7 @@ describe("Harvester", function () {
async () => {
annaBalanceChange = await changeInBalance(
async () => {
harvester
await harvester
.connect(anna)["harvestAndSwap(address)"](compoundStrategy.address);
},
usdt,
Expand Down Expand Up @@ -604,12 +617,12 @@ describe("Harvester", function () {
josh,
usdt,
vault,
} = await loadFixture(compoundVaultFixture);
} = fixture;

await sendRewardsToCompStrategy("10", governor, compoundStrategy, comp);

const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");
mockUniswapRouter.initialize([comp.address], [usdt.address]);
await mockUniswapRouter.initialize([comp.address], [usdt.address]);
await usdt
.connect(josh)
.transfer(mockUniswapRouter.address, usdtUnits("100"));
Expand All @@ -632,7 +645,7 @@ describe("Harvester", function () {
async () => {
annaBalanceChange = await changeInBalance(
async () => {
harvester
await harvester
.connect(anna)["harvestAndSwap(address,address)"](compoundStrategy.address, anna.address);
},
usdt,
Expand Down Expand Up @@ -664,12 +677,12 @@ describe("Harvester", function () {
josh,
usdt,
vault,
} = await loadFixture(compoundVaultFixture);
} = fixture;

await sendRewardsToCompStrategy("10", governor, compoundStrategy, comp);

const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");
mockUniswapRouter.initialize([comp.address], [usdt.address]);
await mockUniswapRouter.initialize([comp.address], [usdt.address]);
await usdt
.connect(josh)
.transfer(mockUniswapRouter.address, usdtUnits("100"));
Expand All @@ -692,7 +705,7 @@ describe("Harvester", function () {
async () => {
joshBalanceChange = await changeInBalance(
async () => {
harvester
await harvester
.connect(anna)["harvestAndSwap(address,address)"](compoundStrategy.address, josh.address);
},
usdt,
Expand All @@ -716,12 +729,12 @@ describe("Harvester", function () {

it("Should correctly distribute rewards to a changed proceeds address", async () => {
const { harvester, governor, comp, compoundStrategy, anna, josh, usdt } =
await loadFixture(compoundVaultFixture);
fixture;

await sendRewardsToCompStrategy("10", governor, compoundStrategy, comp);

const mockUniswapRouter = await ethers.getContract("MockUniswapRouter");
mockUniswapRouter.initialize([comp.address], [usdt.address]);
await mockUniswapRouter.initialize([comp.address], [usdt.address]);
await usdt
.connect(josh)
.transfer(mockUniswapRouter.address, usdtUnits("100"));
Expand All @@ -745,7 +758,7 @@ describe("Harvester", function () {
async () => {
annaBalanceChange = await changeInBalance(
async () => {
harvester
await harvester
.connect(anna)["harvestAndSwap(address)"](compoundStrategy.address);
},
usdt,
Expand Down