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

Balancer fixes #1734

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ abstract contract BaseAuraStrategy is BaseBalancerStrategy {
true // also claim reward tokens
);
}

/**
* @dev Withdraw all Balancer Pool Tokens (BPT) from
* the Aura rewards pool to this strategy contract.
Expand Down
41 changes: 17 additions & 24 deletions contracts/contracts/strategies/balancer/BaseBalancerStrategy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,13 @@ abstract contract BaseBalancerStrategy is InitializableAbstractStrategy {
* This is not denominated in OUSD/ETH value of the assets in the Balancer pool.
* @param _asset Address of the Vault collateral asset
* @return amount the amount of vault collateral assets
*
*
* IMPORTANT if this function is overridden it needs to have a whenNotInVaultContext
* modifier on it or it is susceptible to read-only re-entrancy attack
* modifier on it or it is susceptible to read-only re-entrancy attack
*
* @dev it is important that this function is not affected by reporting inflated
* values of assets in case of any pool manipulation. Such a manipulation could easily
* exploit the protocol by:
* exploit the protocol by:
* - minting OETH
* - tilting Balancer pool to report higher balances of assets
* - rebasing() -> all that extra token balances get distributed to OETH holders
Expand All @@ -128,21 +128,6 @@ abstract contract BaseBalancerStrategy is InitializableAbstractStrategy {

uint256 bptBalance = _getBalancerPoolTokens();

// sum of all of the token's inverted rates
uint256 invertedRateAccumulator = 0;
// queried asset inverted rate
uint256 assetInvertedRate = 0;
uint256 assetRate = 0;
for (uint256 i = 0; i < tokens.length; ++i) {
uint256 rate = getRateProviderRate(address(tokens[i]));
uint256 rateInverted = uint256(1e18).divPrecisely(rate);
invertedRateAccumulator += rateInverted;
if (toPoolAsset(_asset) == address(tokens[i])) {
assetInvertedRate = rateInverted;
assetRate = rate;
}
}

/* To calculate the worth of queried asset in accordance with pool token
* rates (provided by asset rateProvider)
* - convert complete balance of BPT to underlying tokens ETH denominated amount
Expand All @@ -153,21 +138,29 @@ abstract contract BaseBalancerStrategy is InitializableAbstractStrategy {
* - divide the amount of the previous step with assetRate to convert the ETH
* denominated representation to asset denominated
*/
amount = ((bptBalance.mulTruncate(
amount = (bptBalance.mulTruncate(
IRateProvider(platformAddress).getRate()
) * assetInvertedRate) / invertedRateAccumulator).divPrecisely(
assetRate
);
) / tokens.length);

/* If pool asset is equals _asset it means a rate provider for that asset
* exists and that asset is not necessarily pegged to a unit (ETH).
*
* Because this function returns the balance of the asset not denominated in
* ETH units we need to convert the amount to asset amount.
*/
if (toPoolAsset(_asset) == _asset) {
amount = amount.divPrecisely(getRateProviderRate(_asset));
}
}

/**
* @notice Returns the value of all assets managed by this strategy.
* Uses the Balancer pool's rate (virtual price) to convert the strategy's
* Balancer Pool Tokens (BPT) to ETH value.
* @return value The ETH value
*
*
* IMPORTANT if this function is overridden it needs to have a whenNotInVaultContext
* modifier on it or it is susceptible to read-only re-entrancy attack
* modifier on it or it is susceptible to read-only re-entrancy attack
*/
function checkBalance()
external
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ forkOnlyDescribe(
);
log(`Aura BPTs before: ${formatUnits(bptBefore)}`);

const withdrawAmount = 29800;
const withdrawAmount = 29700;
const withdrawAmountUnits = oethUnits(withdrawAmount.toString(), 18);

await balancerREthStrategy
Expand Down
11 changes: 9 additions & 2 deletions contracts/utils/funding.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,15 @@ const fundAccounts = async () => {

const ousdCoins = [dai, usdc, usdt, tusd, ogn];
const oethCoins = [weth, rETH, stETH, frxETH];
const allCoins = [...ousdCoins, ...oethCoins];

const skipOUSDCoins = !!process.env.SKIP_OUSD_COINS;
const skipOETHCoins = !!process.env.SKIP_OETH_COINS;
let allCoins = [];
if (!skipOUSDCoins) {
allCoins = [...allCoins, ...ousdCoins];
}
if (!skipOETHCoins) {
allCoins = [...allCoins, ...oethCoins];
}
const signers = await hre.ethers.getSigners();

const addressPromises = new Array(10)
Expand Down