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 lotPrice() back everywhere (unused) for backwards compatibility #1004

Merged
merged 4 commits into from
Nov 17, 2023
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
7 changes: 7 additions & 0 deletions contracts/interfaces/IAsset.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ interface IAsset is IRewardable {
/// @return high {UoA/tok} The upper end of the price estimate
function price() external view returns (uint192 low, uint192 high);

/// Should not revert
/// lotLow should be nonzero when the asset might be worth selling
/// @dev Deprecated. Phased out in 3.1.0, but left on interface for backwards compatibility
/// @return lotLow {UoA/tok} The lower end of the lot price estimate
/// @return lotHigh {UoA/tok} The upper end of the lot price estimate
function lotPrice() external view returns (uint192 lotLow, uint192 lotHigh);

/// @return {tok} The balance of the ERC20 in whole tokens
function bal(address account) external view returns (uint192);

Expand Down
7 changes: 7 additions & 0 deletions contracts/interfaces/IBasketHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ interface IBasketHandler is IComponent {
/// @return high {UoA/BU} The upper end of the price estimate
function price() external view returns (uint192 low, uint192 high);

/// Should not revert
/// lotLow should be nonzero if a BU could be worth selling
/// @dev Deprecated. Phased out in 3.1.0, but left on interface for backwards compatibility
/// @return lotLow {UoA/tok} The lower end of the lot price estimate
/// @return lotHigh {UoA/tok} The upper end of the lot price estimate
function lotPrice() external view returns (uint192 lotLow, uint192 lotHigh);

/// @return timestamp The timestamp at which the basket was last set
function timestamp() external view returns (uint48);

Expand Down
22 changes: 20 additions & 2 deletions contracts/p0/BasketHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,27 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler {
}

/// Should not revert
/// low should be nonzero when the asset might be worth selling
/// @return low {UoA/BU} The lower end of the price estimate
/// @return high {UoA/BU} The upper end of the price estimate
// returns sum(quantity(erc20) * price(erc20) for erc20 in basket.erc20s)
function price() external view returns (uint192 low, uint192 high) {
return _price(false);
}

/// Should not revert
/// lowLow should be nonzero when the asset might be worth selling
/// @dev Deprecated. Phased out in 3.1.0, but left on interface for backwards compatibility
/// @return lotLow {UoA/BU} The lower end of the lot price estimate
/// @return lotHigh {UoA/BU} The upper end of the lot price estimate
// returns sum(quantity(erc20) * lotPrice(erc20) for erc20 in basket.erc20s)
function lotPrice() external view returns (uint192 lotLow, uint192 lotHigh) {
return _price(true);
}

/// Returns the price of a BU, using the lot prices if `useLotPrice` is true
/// @return low {UoA/BU} The lower end of the lot price estimate
/// @return high {UoA/BU} The upper end of the lot price estimate
function _price(bool useLotPrice) internal view returns (uint192 low, uint192 high) {
IAssetRegistry reg = main.assetRegistry();

uint256 low256;
Expand All @@ -384,7 +400,9 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler {
uint192 qty = quantity(basket.erc20s[i]);
if (qty == 0) continue;

(uint192 lowP, uint192 highP) = reg.toAsset(basket.erc20s[i]).price();
(uint192 lowP, uint192 highP) = useLotPrice
? reg.toAsset(basket.erc20s[i]).lotPrice()
: reg.toAsset(basket.erc20s[i]).price();

low256 += qty.safeMul(lowP, RoundingMode.FLOOR);

Expand Down
23 changes: 21 additions & 2 deletions contracts/p1/BasketHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,28 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler {
}

/// Should not revert
/// low should be nonzero when BUs are worth selling
/// @return low {UoA/BU} The lower end of the price estimate
/// @return high {UoA/BU} The upper end of the price estimate
// returns sum(quantity(erc20) * price(erc20) for erc20 in basket.erc20s)
function price() external view returns (uint192 low, uint192 high) {
return _price(false);
}

/// Should not revert
/// lowLow should be nonzero when the asset might be worth selling
/// @dev Deprecated. Phased out in 3.1.0, but left on interface for backwards compatibility
/// @return lotLow {UoA/BU} The lower end of the lot price estimate
/// @return lotHigh {UoA/BU} The upper end of the lot price estimate
// returns sum(quantity(erc20) * lotPrice(erc20) for erc20 in basket.erc20s)
function lotPrice() external view returns (uint192 lotLow, uint192 lotHigh) {
return _price(true);
}

/// Returns the price of a BU, using the lot prices if `useLotPrice` is true
/// @param useLotPrice Whether to use lotPrice() or price()
/// @return low {UoA/BU} The lower end of the price estimate
/// @return high {UoA/BU} The upper end of the price estimate
function _price(bool useLotPrice) internal view returns (uint192 low, uint192 high) {
uint256 low256;
uint256 high256;

Expand All @@ -324,7 +341,9 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler {
uint192 qty = quantity(basket.erc20s[i]);
if (qty == 0) continue;

(uint192 lowP, uint192 highP) = assetRegistry.toAsset(basket.erc20s[i]).price();
(uint192 lowP, uint192 highP) = useLotPrice
? assetRegistry.toAsset(basket.erc20s[i]).lotPrice()
: assetRegistry.toAsset(basket.erc20s[i]).price();

low256 += qty.safeMul(lowP, RoundingMode.FLOOR);

Expand Down
9 changes: 9 additions & 0 deletions contracts/plugins/assets/Asset.sol
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,15 @@ contract Asset is IAsset, VersionedAsset {
assert(_low <= _high);
}

/// Should not revert
/// lotLow should be nonzero when the asset might be worth selling
/// @dev Deprecated. Phased out in 3.1.0, but left on interface for backwards compatibility
/// @return lotLow {UoA/tok} The lower end of the lot price estimate
/// @return lotHigh {UoA/tok} The upper end of the lot price estimate
function lotPrice() external view virtual returns (uint192 lotLow, uint192 lotHigh) {
return price();
}

/// @return {tok} The balance of the ERC20 in whole tokens
function bal(address account) external view virtual returns (uint192) {
return shiftl_toFix(erc20.balanceOf(account), -int8(erc20Decimals));
Expand Down
9 changes: 9 additions & 0 deletions contracts/plugins/assets/RTokenAsset.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@ contract RTokenAsset is IAsset, VersionedAsset, IRTokenOracle {
}
}

/// Should not revert
/// lotLow should be nonzero when the asset might be worth selling
/// @dev Deprecated. Phased out in 3.1.0, but left on interface for backwards compatibility
/// @return lotLow {UoA/tok} The lower end of the lot price estimate
/// @return lotHigh {UoA/tok} The upper end of the lot price estimate
function lotPrice() external view virtual returns (uint192 lotLow, uint192 lotHigh) {
return price();
}

/// @return {tok} The balance of the ERC20 in whole tokens
function bal(address account) external view returns (uint192) {
// The RToken has 18 decimals, so there's no reason to waste gas here doing a shiftl_toFix
Expand Down
13 changes: 13 additions & 0 deletions docs/collateral.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ interface IAsset is IRewardable {
/// @return high {UoA/tok} The upper end of the price estimate
function price() external view returns (uint192 low, uint192 high);

/// Should not revert
/// lotLow should be nonzero when the asset might be worth selling
/// @dev Deprecated. Phased out in 3.1.0, but left on interface for backwards compatibility
/// @return lotLow {UoA/tok} The lower end of the lot price estimate
/// @return lotHigh {UoA/tok} The upper end of the lot price estimate
function lotPrice() external view returns (uint192 lotLow, uint192 lotHigh);

/// @return {tok} The balance of the ERC20 in whole tokens
function bal(address account) external view returns (uint192);

Expand Down Expand Up @@ -367,6 +374,12 @@ Should return `(0, FIX_MAX)` if pricing data is _completely_ unavailable or stal

Should be gas-efficient.

### lotPrice() `{UoA/tok}`

Deprecated. Phased out in 3.1.0, but left on interface for backwards compatibility.

Recommend implement `lotPrice()` by calling `price()`. If you are inheriting from any of our existing collateral plugins, this is already done for you. See [Asset.sol](../contracts/plugins/Asset.sol) for the implementation.

### refPerTok() `{ref/tok}`

Should never revert.
Expand Down
9 changes: 9 additions & 0 deletions test/Main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3150,6 +3150,15 @@ describe(`MainP${IMPLEMENTATION} contract`, () => {
await expectPrice(basketHandler.address, fp('0.75'), ORACLE_ERROR, true)
})

it('lotPrice (deprecated) is equal to price()', async () => {
const lotPrice = await basketHandler.lotPrice()
const price = await basketHandler.price()
expect(price.length).to.equal(2)
expect(lotPrice.length).to.equal(price.length)
expect(lotPrice[0]).to.equal(price[0])
expect(lotPrice[1]).to.equal(price[1])
})

it('Should not put backup tokens with different targetName in the basket', async () => {
// Swap out collateral for bad target name
const CollFactory = await ethers.getContractFactory('FiatCollateral')
Expand Down
11 changes: 11 additions & 0 deletions test/plugins/Asset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,17 @@ describe('Assets contracts #fast', () => {
expect(lowPrice5).to.be.equal(bn(0))
expect(highPrice5).to.be.equal(MAX_UINT192)
})

it('lotPrice (deprecated) is equal to price()', async () => {
for (const asset of [rsrAsset, compAsset, aaveAsset, rTokenAsset]) {
const lotPrice = await asset.lotPrice()
const price = await asset.price()
expect(price.length).to.equal(2)
expect(lotPrice.length).to.equal(price.length)
expect(lotPrice[0]).to.equal(price[0])
expect(lotPrice[1]).to.equal(price[1])
}
})
})

describe('Constructor validation', () => {
Expand Down
11 changes: 11 additions & 0 deletions test/plugins/Collateral.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,17 @@ describe('Collateral contracts', () => {
expect(await unpricedAppFiatCollateral.savedHighPrice()).to.equal(highPrice)
expect(await unpricedAppFiatCollateral.lastSave()).to.equal(currBlockTimestamp)
})

it('lotPrice (deprecated) is equal to price()', async () => {
for (const coll of [tokenCollateral, usdcCollateral, aTokenCollateral, cTokenCollateral]) {
const lotPrice = await coll.lotPrice()
const price = await coll.price()
expect(price.length).to.equal(2)
expect(lotPrice.length).to.equal(price.length)
expect(lotPrice[0]).to.equal(price[0])
expect(lotPrice[1]).to.equal(price[1])
}
})
})

describe('Status', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,15 @@ describeFork(`ATokenFiatCollateral - Mainnet Forking P${IMPLEMENTATION}`, functi
await zeropriceCtokenCollateral.refresh()
expect(await zeropriceCtokenCollateral.status()).to.equal(CollateralStatus.IFFY)
})

it('lotPrice (deprecated) is equal to price()', async () => {
const lotPrice = await aDaiCollateral.lotPrice()
const price = await aDaiCollateral.price()
expect(price.length).to.equal(2)
expect(lotPrice.length).to.equal(price.length)
expect(lotPrice[0]).to.equal(price[0])
expect(lotPrice[1]).to.equal(price[1])
})
})

// Note: Here the idea is to test all possible statuses and check all possible paths to default
Expand Down
9 changes: 9 additions & 0 deletions test/plugins/individual-collateral/collateralTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,15 @@ export default function fn<X extends CollateralFixtureContext>(
await advanceTime(priceTimeout / 2)
await expectUnpriced(collateral.address)
})

it('lotPrice (deprecated) is equal to price()', async () => {
const lotPrice = await collateral.lotPrice()
const price = await collateral.price()
expect(price.length).to.equal(2)
expect(lotPrice.length).to.equal(price.length)
expect(lotPrice[0]).to.equal(price[0])
expect(lotPrice[1]).to.equal(price[1])
})
})

describe('status', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,15 @@ describeFork(`CTokenFiatCollateral - Mainnet Forking P${IMPLEMENTATION}`, functi
await zeropriceCtokenCollateral.refresh()
expect(await zeropriceCtokenCollateral.status()).to.equal(CollateralStatus.IFFY)
})

it('lotPrice (deprecated) is equal to price()', async () => {
const lotPrice = await cDaiCollateral.lotPrice()
const price = await cDaiCollateral.price()
expect(price.length).to.equal(2)
expect(lotPrice.length).to.equal(price.length)
expect(lotPrice[0]).to.equal(price[0])
expect(lotPrice[1]).to.equal(price[1])
})
})

// Note: Here the idea is to test all possible statuses and check all possible paths to default
Expand Down
9 changes: 9 additions & 0 deletions test/plugins/individual-collateral/curve/collateralTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,15 @@ export default function fn<X extends CurveCollateralFixtureContext>(
await advanceTime(priceTimeout / 2)
await expectUnpriced(ctx.collateral.address)
})

it('lotPrice (deprecated) is equal to price()', async () => {
const lotPrice = await ctx.collateral.lotPrice()
const price = await ctx.collateral.price()
expect(price.length).to.equal(2)
expect(lotPrice.length).to.equal(price.length)
expect(lotPrice[0]).to.equal(price[0])
expect(lotPrice[1]).to.equal(price[1])
})
})

describe('status', () => {
Expand Down
Loading