From 05c1d665ccb897b6189fd0de261146ea3f431ce4 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Mon, 4 Dec 2023 20:28:00 -0500 Subject: [PATCH 01/20] fix deployment --- scripts/deployment/phase2-assets/collaterals/deploy_sfrax.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/deployment/phase2-assets/collaterals/deploy_sfrax.ts b/scripts/deployment/phase2-assets/collaterals/deploy_sfrax.ts index 1510377f20..600505d84e 100644 --- a/scripts/deployment/phase2-assets/collaterals/deploy_sfrax.ts +++ b/scripts/deployment/phase2-assets/collaterals/deploy_sfrax.ts @@ -12,7 +12,7 @@ import { getDeploymentFilename, fileExists, } from '../../common' -import { priceTimeout, oracleTimeout } from '../../utils' +import { priceTimeout } from '../../utils' import { SFraxCollateral } from '../../../../typechain' import { ContractFactory } from 'ethers' @@ -53,7 +53,7 @@ async function main() { oracleError: fp('0.01').toString(), // 1% erc20: networkConfig[chainId].tokens.sFRAX, maxTradeVolume: fp('1e6').toString(), // $1m, - oracleTimeout: oracleTimeout(chainId, '3600').toString(), // 1 hr + oracleTimeout: '3600', // 1 hr targetName: hre.ethers.utils.formatBytes32String('USD'), defaultThreshold: fp('0.02').toString(), // 2% = 1% oracleError + 1% buffer delayUntilDefault: bn('86400').toString(), // 24h From 2770b2a22fd7abea0b7e8c7b0c02913bb3b8cf41 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Mon, 4 Dec 2023 20:29:56 -0500 Subject: [PATCH 02/20] fix mainnet integration tests --- .../individual-collateral/frax/SFraxCollateralTestSuite.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/plugins/individual-collateral/frax/SFraxCollateralTestSuite.test.ts b/test/plugins/individual-collateral/frax/SFraxCollateralTestSuite.test.ts index cf4038f9aa..43d09c95be 100644 --- a/test/plugins/individual-collateral/frax/SFraxCollateralTestSuite.test.ts +++ b/test/plugins/individual-collateral/frax/SFraxCollateralTestSuite.test.ts @@ -193,6 +193,7 @@ const opts = { getExpectedPrice, itClaimsRewards: it.skip, itChecksTargetPerRefDefault: it, + itChecksNonZeroDefaultThreshold: it.skip, itChecksRefPerTokDefault: it, itChecksPriceChanges: it, itHasRevenueHiding: it.skip, From 37e05d3ca5a1e4bca0b205d042bc130b684d2b88 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Mon, 4 Dec 2023 20:18:16 -0500 Subject: [PATCH 03/20] init 3.2.0 --- contracts/mixins/Versioned.sol | 2 +- test/fixtures.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/mixins/Versioned.sol b/contracts/mixins/Versioned.sol index c70c7a8857..afc4915e0c 100644 --- a/contracts/mixins/Versioned.sol +++ b/contracts/mixins/Versioned.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.19; import "../interfaces/IVersioned.sol"; // This value should be updated on each release -string constant VERSION = "3.1.0"; +string constant VERSION = "3.2.0"; /** * @title Versioned diff --git a/test/fixtures.ts b/test/fixtures.ts index 6244d68ff3..4b0a8e30fa 100644 --- a/test/fixtures.ts +++ b/test/fixtures.ts @@ -80,7 +80,7 @@ export const ORACLE_ERROR = fp('0.01') // 1% oracle error export const REVENUE_HIDING = fp('0') // no revenue hiding by default; test individually // This will have to be updated on each release -export const VERSION = '3.1.0' +export const VERSION = '3.2.0' export type Collateral = | FiatCollateral From eddf5a3c0a818684b5e21fd1870d8fe69890bb5a Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Mon, 4 Dec 2023 21:43:19 -0500 Subject: [PATCH 04/20] draft implementation --- CHANGELOG.md | 18 ++++++ contracts/interfaces/IBasketHandler.sol | 11 ++++ contracts/p0/BasketHandler.sol | 38 +++++++++--- contracts/p1/BasketHandler.sol | 61 ++++++++++++++----- contracts/p1/RToken.sol | 3 +- .../upgrade-checker-utils/upgrades/3_0_0.ts | 4 +- test/Main.test.ts | 18 +++--- 7 files changed, 116 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fa4815b9e..ec7d50cecf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +# 3.2.0 + +This release adds the ability for each RToken to configure whether the target basket should be revaluable or not. An RToken that is not revaluable can have its target basket changed in terms of ERC20s but not in terms of target weights at the target unit level. + +### Upgrade Steps + +Upgrade BasketHandler + +Optional: For slightly nicer RToken documentation, upgrade RToken as well. No change in functionality + +### Core Protocol Contracts + +- `BasketHandler` [+1 slot] + - Add concept of a revaluable basket: a basket that can have its target amounts (once grouped by target unit) changed + - Add `revaluable` bool and `Revalued` event + - Add `setRevaluable(bool)` + - Require `quoteCustomRedemption` only use constant-value baskets + # 3.1.0 - Unreleased ### Upgrade Steps -- Required diff --git a/contracts/interfaces/IBasketHandler.sol b/contracts/interfaces/IBasketHandler.sol index 2ed829d1b9..5f5d5ed593 100644 --- a/contracts/interfaces/IBasketHandler.sol +++ b/contracts/interfaces/IBasketHandler.sol @@ -48,6 +48,15 @@ interface IBasketHandler is IComponent { /// @param newStatus The new basket status event BasketStatusChanged(CollateralStatus oldStatus, CollateralStatus newStatus); + /// Emitted when the ability to change target weights is changed + /// @param oldVal The old revalueable boolean + /// @param newVal The new revalueable boolean + event RevaluableChanged(bool oldVal, bool newVal); + + /// Emitted when the target basket is revalued (made to be worth different target amounts) + /// @param basketNonce The last nonce available for custom redemption + event Revalued(uint48 basketNonce); + // Initialization function init(IMain main_, uint48 warmupPeriod_) external; @@ -156,4 +165,6 @@ interface TestIBasketHandler is IBasketHandler { function warmupPeriod() external view returns (uint48); function setWarmupPeriod(uint48 val) external; + + function setRevaluable(bool val) external; } diff --git a/contracts/p0/BasketHandler.sol b/contracts/p0/BasketHandler.sol index 998c25e65f..88f81eb0f4 100644 --- a/contracts/p0/BasketHandler.sol +++ b/contracts/p0/BasketHandler.sol @@ -148,6 +148,10 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // A history of baskets by basket nonce; includes current basket mapping(uint48 => Basket) private basketHistory; + bool public revaluable; // whether the weights of the target basket be changed + + uint48 private lastRevalued; // {basketNonce} the earliest basket nonce of same target weights + // ==== Invariants ==== // basket is a valid Basket: // basket.erc20s is a valid collateral array and basket.erc20s == keys(basket.refAmts) @@ -168,6 +172,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { lastStatusTimestamp = uint48(block.timestamp); disabled = true; + // revaluable = false; } /// Disable the basket in order to schedule a basket refresh @@ -244,8 +249,13 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { require(erc20s.length == targetAmts.length, "len mismatch"); requireValidCollArray(erc20s); - // If this isn't initial setup, require targets remain constant - if (config.erc20s.length > 0) requireConstantConfigTargets(erc20s, targetAmts); + // Track constant config targets if revaluable + bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); + require(revaluable || isConstant, "config targets not constant"); + if (revaluable && !isConstant) { + lastRevalued = nonce + 1; // next nonce + emit Revalued(lastRevalued); + } // Clean up previous basket config for (uint256 i = 0; i < config.erc20s.length; ++i) { @@ -466,7 +476,10 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // Calculate the linear combination basket for (uint48 i = 0; i < basketNonces.length; ++i) { - require(basketNonces[i] <= nonce, "invalid basketNonce"); + require( + basketNonces[i] >= lastRevalued && basketNonces[i] <= nonce, + "invalid basketNonce" + ); Basket storage b = basketHistory[basketNonces[i]]; // Add-in refAmts contribution from historical basket @@ -566,6 +579,12 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { warmupPeriod = val; } + /// @custom:governance + function setRevaluable(bool val) public governance { + emit RevaluableChanged(revaluable, val); + revaluable = val; + } + /* _switchBasket computes basket' from three inputs: - the basket configuration (config: BasketConfig) - the function (isGood: erc20 -> bool), implemented here by goodCollateral() @@ -760,10 +779,11 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { } /// Require that newERC20s and newTargetAmts preserve the current config targets - function requireConstantConfigTargets( - IERC20[] calldata newERC20s, - uint192[] calldata newTargetAmts - ) private { + /// @return true iff the target amounts are preserved on a per-target-name basis + function hasConstantConfigTargets(IERC20[] calldata newERC20s, uint192[] calldata newTargetAmts) + private + returns (bool) + { // Empty _targetAmts mapping while (_targetAmts.length() > 0) { (bytes32 key, ) = _targetAmts.at(0); @@ -783,11 +803,11 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { for (uint256 i = 0; i < newERC20s.length; i++) { bytes32 targetName = main.assetRegistry().toColl(newERC20s[i]).targetName(); (bool contains, uint256 amt) = _targetAmts.tryGet(targetName); - require(contains && amt >= newTargetAmts[i], "new target weights"); + if (!contains || amt < newTargetAmts[i]) return false; if (amt == newTargetAmts[i]) _targetAmts.remove(targetName); else _targetAmts.set(targetName, amt - newTargetAmts[i]); } - require(_targetAmts.length() == 0, "missing target weights"); + return (_targetAmts.length() == 0); } /// Good collateral is registered, collateral, SOUND, has the expected targetName, diff --git a/contracts/p1/BasketHandler.sol b/contracts/p1/BasketHandler.sol index fa076253bd..b1b40cee31 100644 --- a/contracts/p1/BasketHandler.sol +++ b/contracts/p1/BasketHandler.sol @@ -79,6 +79,13 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // Effectively local variable of `requireConstantConfigTargets()` EnumerableMap.Bytes32ToUintMap private _targetAmts; // targetName -> {target/BU} + // === + // Added in 3.2.0 + + bool public revaluable; // whether the weights of the target basket be changed + + uint48 private lastRevalued; // {basketNonce} the earliest basket nonce of same target weights + // === // ==== Invariants ==== @@ -107,6 +114,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { lastStatusTimestamp = uint48(block.timestamp); disabled = true; + // revaluable = false; } /// Disable the basket in order to schedule a basket refresh @@ -171,19 +179,23 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // for all i, 0 < targetAmts[i] <= MAX_TARGET_AMT == 1000 // // effects: + // lastRevalued = nonce + 1, if revaluable // config'.erc20s = erc20s // config'.targetAmts[erc20s[i]] = targetAmts[i], for i from 0 to erc20s.length-1 // config'.targetNames[e] = assetRegistry.toColl(e).targetName, for e in erc20s - function setPrimeBasket(IERC20[] calldata erc20s, uint192[] calldata targetAmts) - external - governance - { + function setPrimeBasket(IERC20[] calldata erc20s, uint192[] calldata targetAmts) external { + requireGovernanceOnly(); require(erc20s.length > 0, "empty basket"); require(erc20s.length == targetAmts.length, "len mismatch"); requireValidCollArray(erc20s); - // If this isn't initial setup, require targets remain constant - if (config.erc20s.length > 0) requireConstantConfigTargets(erc20s, targetAmts); + // Track constant config targets if revaluable + bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); + require(revaluable || isConstant, "config targets not constant"); + if (revaluable && !isConstant) { + emit Revalued(nonce + 1); + lastRevalued = nonce + 1; // next nonce + } // Clean up previous basket config for (uint256 i = 0; i < config.erc20s.length; ++i) { @@ -224,7 +236,8 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { bytes32 targetName, uint256 max, IERC20[] calldata erc20s - ) external governance { + ) external { + requireGovernanceOnly(); requireValidCollArray(erc20s); BackupConfig storage conf = config.backups[targetName]; conf.max = max; @@ -410,7 +423,10 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // Calculate the linear combination basket for (uint48 i = 0; i < basketNonces.length; ++i) { - require(basketNonces[i] <= nonce, "invalid basketNonce"); + require( + basketNonces[i] >= lastRevalued && basketNonces[i] <= nonce, + "invalid basketNonce" + ); Basket storage b = basketHistory[basketNonces[i]]; // Add-in refAmts contribution from historical basket @@ -507,14 +523,26 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // === Governance Setters === /// @custom:governance - function setWarmupPeriod(uint48 val) public governance { + function setWarmupPeriod(uint48 val) public { + requireGovernanceOnly(); require(val >= MIN_WARMUP_PERIOD && val <= MAX_WARMUP_PERIOD, "invalid warmupPeriod"); emit WarmupPeriodSet(warmupPeriod, val); warmupPeriod = val; } + /// @custom:governance + function setRevaluable(bool val) public { + requireGovernanceOnly(); + emit RevaluableChanged(revaluable, val); + revaluable = val; + } + // === Private === + // contract-size-saver + // solhint-disable-next-line no-empty-blocks + function requireGovernanceOnly() private governance {} + /// Select and save the next basket, based on the BasketConfig and Collateral statuses function _switchBasket() private { // Mark basket disabled. Pause most protocol functions unless there is a next basket @@ -545,10 +573,11 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { } /// Require that newERC20s and newTargetAmts preserve the current config targets - function requireConstantConfigTargets( - IERC20[] calldata newERC20s, - uint192[] calldata newTargetAmts - ) private { + /// @return true iff the target amounts are preserved on a per-target-name basis + function hasConstantConfigTargets(IERC20[] calldata newERC20s, uint192[] calldata newTargetAmts) + private + returns (bool) + { // Populate _targetAmts mapping with old basket config uint256 len = config.erc20s.length; for (uint256 i = 0; i < len; ++i) { @@ -566,11 +595,11 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { for (uint256 i = 0; i < len; ++i) { bytes32 targetName = assetRegistry.toColl(newERC20s[i]).targetName(); (bool contains, uint256 amt) = _targetAmts.tryGet(targetName); - require(contains && amt >= newTargetAmts[i], "new target weights"); + if (!contains || amt < newTargetAmts[i]) return false; if (amt > newTargetAmts[i]) _targetAmts.set(targetName, amt - newTargetAmts[i]); else _targetAmts.remove(targetName); } - require(_targetAmts.length() == 0, "missing target weights"); + return (_targetAmts.length() == 0); } /// Require that erc20s is a valid collateral array @@ -673,5 +702,5 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { * variables without shifting down storage in the inheritance chain. * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps */ - uint256[37] private __gap; + uint256[36] private __gap; } diff --git a/contracts/p1/RToken.sol b/contracts/p1/RToken.sol index cfbbdcd923..ce0384b455 100644 --- a/contracts/p1/RToken.sol +++ b/contracts/p1/RToken.sol @@ -220,7 +220,7 @@ contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken { // amount > 0 // amount <= balanceOf(caller) // sum(portions) == FIX_ONE - // nonce >= basketHandler.primeNonce() for nonce in basketNonces + // require nonce within [bh.lastRevalued, bh.nonce()] for nonce in basketNonces // // effects: // (so totalSupply -= amount and balanceOf(caller) -= amount) @@ -234,6 +234,7 @@ contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken { // do token.transferFrom(backingManager, caller, min(tokenAmt, prorataAmt)) // BU exchange rate cannot decrease, and it can only increase when < FIX_ONE. /// @dev Allows partial redemptions up to the minAmounts + /// @dev Redemption is unavailable after setPrimeBasket() but before switchBasket() /// @param recipient The address to receive the backing collateral tokens /// @param amount {qRTok} The quantity {qRToken} of RToken to redeem /// @param basketNonces An array of basket nonces to do redemption from diff --git a/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts b/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts index 82e7892e64..8850a6f68f 100644 --- a/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts +++ b/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts @@ -72,7 +72,7 @@ export default async ( await whileImpersonating(hre, timelockAddress, async (tl) => { await expect( basketHandler.connect(tl).setPrimeBasket([usdc.address], [fp('20')]) - ).to.be.revertedWith('new target weights') + ).to.be.revertedWith('config targets not constant') }) // Attempt to change target unit in basket @@ -103,7 +103,7 @@ export default async ( await assetRegistry.connect(tl).register(eurFiatCollateral.address) await expect( basketHandler.connect(tl).setPrimeBasket([eurt.address], [fp('1')]) - ).to.be.revertedWith('new target weights') + ).to.be.revertedWith('config targets not constant') await assetRegistry.connect(tl).unregister(eurFiatCollateral.address) }) diff --git a/test/Main.test.ts b/test/Main.test.ts index 9d00c3b0a6..a96db94fbb 100644 --- a/test/Main.test.ts +++ b/test/Main.test.ts @@ -1802,14 +1802,14 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { // not possible on freshBasketHandler await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) - ).to.be.revertedWith('new target weights') + ).to.be.revertedWith('config targets not constant') }) it('Should not allow to decrease prime Basket weights', async () => { // not possible on freshBasketHandler await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) - ).to.be.revertedWith('missing target weights') + ).to.be.revertedWith('config targets not constant') }) it('Should not allow to set prime Basket with an empty basket', async () => { @@ -1827,7 +1827,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ).to.be.revertedWith('invalid target amount; must be nonzero') await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) - ).to.be.revertedWith('missing target weights') + ).to.be.revertedWith('config targets not constant') }) it('Should be able to set exactly same basket', async () => { @@ -1870,7 +1870,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address, backupToken1.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) - ).to.be.revertedWith('new target weights') + ).to.be.revertedWith('config targets not constant') await expect( basketHandler @@ -1879,7 +1879,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address, eurToken.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) - ).to.be.revertedWith('new target weights') + ).to.be.revertedWith('config targets not constant') }) it('Should not allow to set prime Basket as subset of old basket', async () => { @@ -1890,7 +1890,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.24')] ) - ).to.be.revertedWith('missing target weights') + ).to.be.revertedWith('config targets not constant') await expect( basketHandler .connect(owner) @@ -1898,7 +1898,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address], [fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('missing target weights') + ).to.be.revertedWith('config targets not constant') }) it('Should not allow to change target unit in old basket', async () => { @@ -1909,7 +1909,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, eurToken.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('new target weights') + ).to.be.revertedWith('config targets not constant') }) it('Should not allow to set prime Basket with RSR/RToken', async () => { @@ -1963,7 +1963,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('new target weights') + ).to.be.revertedWith('config targets not constant') }) describe('Custom Redemption', () => { From a8f22dea28b1065eda93ea1105ca3dc895b2a36c Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Mon, 4 Dec 2023 21:51:53 -0500 Subject: [PATCH 05/20] get under size limit --- contracts/p0/BasketHandler.sol | 7 +++--- contracts/p1/BasketHandler.sol | 5 ++--- .../upgrade-checker-utils/upgrades/3_0_0.ts | 4 ++-- test/Main.test.ts | 22 +++++++++---------- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/contracts/p0/BasketHandler.sol b/contracts/p0/BasketHandler.sol index 88f81eb0f4..46007196e6 100644 --- a/contracts/p0/BasketHandler.sol +++ b/contracts/p0/BasketHandler.sol @@ -116,7 +116,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { uint192 public constant MAX_TARGET_AMT = 1e3 * FIX_ONE; // {target/BU} max basket weight // config is the basket configuration, from which basket will be computed in a basket-switch - // event. config is only modified by governance through setPrimeBakset and setBackupConfig + // event. config is only modified by governance through setPrimeBasket and setBackupConfig BasketConfig private config; // basket, disabled, nonce, and timestamp are only ever set by `_switchBasket()` @@ -251,7 +251,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // Track constant config targets if revaluable bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); - require(revaluable || isConstant, "config targets not constant"); + require(revaluable || isConstant, "targets not constant"); if (revaluable && !isConstant) { lastRevalued = nonce + 1; // next nonce emit Revalued(lastRevalued); @@ -272,8 +272,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // This is a nice catch to have, but in general it is possible for // an ERC20 in the prime basket to have its asset unregistered. require(reg.toAsset(erc20s[i]).isCollateral(), "erc20 is not collateral"); - require(0 < targetAmts[i], "invalid target amount; must be nonzero"); - require(targetAmts[i] <= MAX_TARGET_AMT, "invalid target amount; too large"); + require(0 < targetAmts[i] && targetAmts[i] <= MAX_TARGET_AMT, "invalid target amt"); config.erc20s.push(erc20s[i]); config.targetAmts[erc20s[i]] = targetAmts[i]; diff --git a/contracts/p1/BasketHandler.sol b/contracts/p1/BasketHandler.sol index b1b40cee31..cec026c23a 100644 --- a/contracts/p1/BasketHandler.sol +++ b/contracts/p1/BasketHandler.sol @@ -191,7 +191,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // Track constant config targets if revaluable bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); - require(revaluable || isConstant, "config targets not constant"); + require(revaluable || isConstant, "targets not constant"); if (revaluable && !isConstant) { emit Revalued(nonce + 1); lastRevalued = nonce + 1; // next nonce @@ -211,8 +211,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // This is a nice catch to have, but in general it is possible for // an ERC20 in the prime basket to have its asset unregistered. require(assetRegistry.toAsset(erc20s[i]).isCollateral(), "erc20 is not collateral"); - require(0 < targetAmts[i], "invalid target amount; must be nonzero"); - require(targetAmts[i] <= MAX_TARGET_AMT, "invalid target amount; too large"); + require(0 < targetAmts[i] && targetAmts[i] <= MAX_TARGET_AMT, "invalid target amt"); config.erc20s.push(erc20s[i]); config.targetAmts[erc20s[i]] = targetAmts[i]; diff --git a/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts b/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts index 8850a6f68f..a8762833b2 100644 --- a/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts +++ b/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts @@ -72,7 +72,7 @@ export default async ( await whileImpersonating(hre, timelockAddress, async (tl) => { await expect( basketHandler.connect(tl).setPrimeBasket([usdc.address], [fp('20')]) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') }) // Attempt to change target unit in basket @@ -103,7 +103,7 @@ export default async ( await assetRegistry.connect(tl).register(eurFiatCollateral.address) await expect( basketHandler.connect(tl).setPrimeBasket([eurt.address], [fp('1')]) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') await assetRegistry.connect(tl).unregister(eurFiatCollateral.address) }) diff --git a/test/Main.test.ts b/test/Main.test.ts index a96db94fbb..f0c1301c92 100644 --- a/test/Main.test.ts +++ b/test/Main.test.ts @@ -1795,21 +1795,21 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { // not possible on non-fresh basketHandler await expect( freshBasketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) - ).to.be.revertedWith('invalid target amount; too large') + ).to.be.revertedWith('invalid target amt') }) it('Should not allow to increase prime Basket weights', async () => { // not possible on freshBasketHandler await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') }) it('Should not allow to decrease prime Basket weights', async () => { // not possible on freshBasketHandler await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') }) it('Should not allow to set prime Basket with an empty basket', async () => { @@ -1824,10 +1824,10 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { it('Should not allow to set prime Basket with a zero amount', async () => { await expect( freshBasketHandler.connect(owner).setPrimeBasket([token0.address], [0]) - ).to.be.revertedWith('invalid target amount; must be nonzero') + ).to.be.revertedWith('invalid target amt') await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') }) it('Should be able to set exactly same basket', async () => { @@ -1870,7 +1870,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address, backupToken1.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') await expect( basketHandler @@ -1879,7 +1879,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address, eurToken.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') }) it('Should not allow to set prime Basket as subset of old basket', async () => { @@ -1890,7 +1890,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.24')] ) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') await expect( basketHandler .connect(owner) @@ -1898,7 +1898,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address], [fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') }) it('Should not allow to change target unit in old basket', async () => { @@ -1909,7 +1909,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, eurToken.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') }) it('Should not allow to set prime Basket with RSR/RToken', async () => { @@ -1963,7 +1963,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('config targets not constant') + ).to.be.revertedWith('targets not constant') }) describe('Custom Redemption', () => { From a1e2184a710439e7e0b2cb3f93b7f5cdd3c453c0 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Mon, 4 Dec 2023 22:06:37 -0500 Subject: [PATCH 06/20] fix init case --- contracts/p0/BasketHandler.sol | 12 +++++++----- contracts/p1/BasketHandler.sol | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/contracts/p0/BasketHandler.sol b/contracts/p0/BasketHandler.sol index 46007196e6..9a1db43a05 100644 --- a/contracts/p0/BasketHandler.sol +++ b/contracts/p0/BasketHandler.sol @@ -250,11 +250,13 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { requireValidCollArray(erc20s); // Track constant config targets if revaluable - bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); - require(revaluable || isConstant, "targets not constant"); - if (revaluable && !isConstant) { - lastRevalued = nonce + 1; // next nonce - emit Revalued(lastRevalued); + if (config.erc20s.length > 0) { + bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); + require(revaluable || isConstant, "targets not constant"); + if (revaluable && !isConstant) { + lastRevalued = nonce + 1; // next nonce + emit Revalued(lastRevalued); + } } // Clean up previous basket config diff --git a/contracts/p1/BasketHandler.sol b/contracts/p1/BasketHandler.sol index cec026c23a..514e3d5cdb 100644 --- a/contracts/p1/BasketHandler.sol +++ b/contracts/p1/BasketHandler.sol @@ -190,11 +190,13 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { requireValidCollArray(erc20s); // Track constant config targets if revaluable - bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); - require(revaluable || isConstant, "targets not constant"); - if (revaluable && !isConstant) { - emit Revalued(nonce + 1); - lastRevalued = nonce + 1; // next nonce + if (config.erc20s.length > 0) { + bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); + require(revaluable || isConstant, "targets not constant"); + if (revaluable && !isConstant) { + emit Revalued(nonce + 1); + lastRevalued = nonce + 1; // next nonce + } } // Clean up previous basket config From b2450218ab73f93c3b689b9a3b89804ecfa0af08 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Tue, 5 Dec 2023 11:35:45 -0500 Subject: [PATCH 07/20] bump VersionedAsset --- contracts/plugins/assets/VersionedAsset.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/plugins/assets/VersionedAsset.sol b/contracts/plugins/assets/VersionedAsset.sol index b36945769d..4b241f6ef3 100644 --- a/contracts/plugins/assets/VersionedAsset.sol +++ b/contracts/plugins/assets/VersionedAsset.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.19; import "../../interfaces/IVersioned.sol"; // This value should be updated on each release -string constant ASSET_VERSION = "3.1.0"; +string constant ASSET_VERSION = "3.2.0"; /** * @title VersionedAsset From bf67182b26b62ae988d0b8e3ef7e9fb76f3039be Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Tue, 5 Dec 2023 18:52:56 -0500 Subject: [PATCH 08/20] remove earliest nonce boundary requirement --- CHANGELOG.md | 4 +-- contracts/interfaces/IBasketHandler.sol | 4 --- contracts/p0/BasketHandler.sol | 34 +++++++----------- contracts/p1/BasketHandler.sol | 35 +++++++------------ contracts/p1/RToken.sol | 5 ++- .../upgrade-checker-utils/upgrades/3_0_0.ts | 4 +-- test/Main.test.ts | 22 ++++++------ 7 files changed, 40 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec7d50cecf..f792116625 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +14,7 @@ Optional: For slightly nicer RToken documentation, upgrade RToken as well. No ch - `BasketHandler` [+1 slot] - Add concept of a revaluable basket: a basket that can have its target amounts (once grouped by target unit) changed - - Add `revaluable` bool and `Revalued` event - - Add `setRevaluable(bool)` - - Require `quoteCustomRedemption` only use constant-value baskets + - Add `revaluable` bool and `setRevaluable(bool)` # 3.1.0 - Unreleased diff --git a/contracts/interfaces/IBasketHandler.sol b/contracts/interfaces/IBasketHandler.sol index 5f5d5ed593..fd343b353c 100644 --- a/contracts/interfaces/IBasketHandler.sol +++ b/contracts/interfaces/IBasketHandler.sol @@ -53,10 +53,6 @@ interface IBasketHandler is IComponent { /// @param newVal The new revalueable boolean event RevaluableChanged(bool oldVal, bool newVal); - /// Emitted when the target basket is revalued (made to be worth different target amounts) - /// @param basketNonce The last nonce available for custom redemption - event Revalued(uint48 basketNonce); - // Initialization function init(IMain main_, uint48 warmupPeriod_) external; diff --git a/contracts/p0/BasketHandler.sol b/contracts/p0/BasketHandler.sol index 9a1db43a05..8f8c468f38 100644 --- a/contracts/p0/BasketHandler.sol +++ b/contracts/p0/BasketHandler.sol @@ -150,8 +150,6 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { bool public revaluable; // whether the weights of the target basket be changed - uint48 private lastRevalued; // {basketNonce} the earliest basket nonce of same target weights - // ==== Invariants ==== // basket is a valid Basket: // basket.erc20s is a valid collateral array and basket.erc20s == keys(basket.refAmts) @@ -249,14 +247,9 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { require(erc20s.length == targetAmts.length, "len mismatch"); requireValidCollArray(erc20s); - // Track constant config targets if revaluable - if (config.erc20s.length > 0) { - bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); - require(revaluable || isConstant, "targets not constant"); - if (revaluable && !isConstant) { - lastRevalued = nonce + 1; // next nonce - emit Revalued(lastRevalued); - } + // If this isn't initial setup, require targets remain constant + if (revaluable && config.erc20s.length > 0) { + requireConstantConfigTargets(erc20s, targetAmts); } // Clean up previous basket config @@ -274,7 +267,8 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // This is a nice catch to have, but in general it is possible for // an ERC20 in the prime basket to have its asset unregistered. require(reg.toAsset(erc20s[i]).isCollateral(), "erc20 is not collateral"); - require(0 < targetAmts[i] && targetAmts[i] <= MAX_TARGET_AMT, "invalid target amt"); + require(0 < targetAmts[i], "invalid target amount; must be nonzero"); + require(targetAmts[i] <= MAX_TARGET_AMT, "invalid target amount; too large"); config.erc20s.push(erc20s[i]); config.targetAmts[erc20s[i]] = targetAmts[i]; @@ -477,10 +471,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // Calculate the linear combination basket for (uint48 i = 0; i < basketNonces.length; ++i) { - require( - basketNonces[i] >= lastRevalued && basketNonces[i] <= nonce, - "invalid basketNonce" - ); + require(basketNonces[i] <= nonce, "invalid basketNonce"); Basket storage b = basketHistory[basketNonces[i]]; // Add-in refAmts contribution from historical basket @@ -780,11 +771,10 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { } /// Require that newERC20s and newTargetAmts preserve the current config targets - /// @return true iff the target amounts are preserved on a per-target-name basis - function hasConstantConfigTargets(IERC20[] calldata newERC20s, uint192[] calldata newTargetAmts) - private - returns (bool) - { + function requireConstantConfigTargets( + IERC20[] calldata newERC20s, + uint192[] calldata newTargetAmts + ) private { // Empty _targetAmts mapping while (_targetAmts.length() > 0) { (bytes32 key, ) = _targetAmts.at(0); @@ -804,11 +794,11 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { for (uint256 i = 0; i < newERC20s.length; i++) { bytes32 targetName = main.assetRegistry().toColl(newERC20s[i]).targetName(); (bool contains, uint256 amt) = _targetAmts.tryGet(targetName); - if (!contains || amt < newTargetAmts[i]) return false; + require(contains && amt >= newTargetAmts[i], "new target weights"); if (amt == newTargetAmts[i]) _targetAmts.remove(targetName); else _targetAmts.set(targetName, amt - newTargetAmts[i]); } - return (_targetAmts.length() == 0); + require(_targetAmts.length() == 0, "missing target weights"); } /// Good collateral is registered, collateral, SOUND, has the expected targetName, diff --git a/contracts/p1/BasketHandler.sol b/contracts/p1/BasketHandler.sol index 514e3d5cdb..e58b0f1b6e 100644 --- a/contracts/p1/BasketHandler.sol +++ b/contracts/p1/BasketHandler.sol @@ -84,8 +84,6 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { bool public revaluable; // whether the weights of the target basket be changed - uint48 private lastRevalued; // {basketNonce} the earliest basket nonce of same target weights - // === // ==== Invariants ==== @@ -179,7 +177,6 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // for all i, 0 < targetAmts[i] <= MAX_TARGET_AMT == 1000 // // effects: - // lastRevalued = nonce + 1, if revaluable // config'.erc20s = erc20s // config'.targetAmts[erc20s[i]] = targetAmts[i], for i from 0 to erc20s.length-1 // config'.targetNames[e] = assetRegistry.toColl(e).targetName, for e in erc20s @@ -189,14 +186,9 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { require(erc20s.length == targetAmts.length, "len mismatch"); requireValidCollArray(erc20s); - // Track constant config targets if revaluable - if (config.erc20s.length > 0) { - bool isConstant = hasConstantConfigTargets(erc20s, targetAmts); - require(revaluable || isConstant, "targets not constant"); - if (revaluable && !isConstant) { - emit Revalued(nonce + 1); - lastRevalued = nonce + 1; // next nonce - } + // If this isn't initial setup, require targets remain constant + if (revaluable && config.erc20s.length > 0) { + requireConstantConfigTargets(erc20s, targetAmts); } // Clean up previous basket config @@ -213,7 +205,8 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // This is a nice catch to have, but in general it is possible for // an ERC20 in the prime basket to have its asset unregistered. require(assetRegistry.toAsset(erc20s[i]).isCollateral(), "erc20 is not collateral"); - require(0 < targetAmts[i] && targetAmts[i] <= MAX_TARGET_AMT, "invalid target amt"); + require(0 < targetAmts[i], "invalid target amount; must be nonzero"); + require(targetAmts[i] <= MAX_TARGET_AMT, "invalid target amount; too large"); config.erc20s.push(erc20s[i]); config.targetAmts[erc20s[i]] = targetAmts[i]; @@ -424,10 +417,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // Calculate the linear combination basket for (uint48 i = 0; i < basketNonces.length; ++i) { - require( - basketNonces[i] >= lastRevalued && basketNonces[i] <= nonce, - "invalid basketNonce" - ); + require(basketNonces[i] <= nonce, "invalid basketNonce"); Basket storage b = basketHistory[basketNonces[i]]; // Add-in refAmts contribution from historical basket @@ -574,11 +564,10 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { } /// Require that newERC20s and newTargetAmts preserve the current config targets - /// @return true iff the target amounts are preserved on a per-target-name basis - function hasConstantConfigTargets(IERC20[] calldata newERC20s, uint192[] calldata newTargetAmts) - private - returns (bool) - { + function requireConstantConfigTargets( + IERC20[] calldata newERC20s, + uint192[] calldata newTargetAmts + ) private { // Populate _targetAmts mapping with old basket config uint256 len = config.erc20s.length; for (uint256 i = 0; i < len; ++i) { @@ -596,11 +585,11 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { for (uint256 i = 0; i < len; ++i) { bytes32 targetName = assetRegistry.toColl(newERC20s[i]).targetName(); (bool contains, uint256 amt) = _targetAmts.tryGet(targetName); - if (!contains || amt < newTargetAmts[i]) return false; + require(contains && amt >= newTargetAmts[i], "new target weights"); if (amt > newTargetAmts[i]) _targetAmts.set(targetName, amt - newTargetAmts[i]); else _targetAmts.remove(targetName); } - return (_targetAmts.length() == 0); + require(_targetAmts.length() == 0, "missing target weights"); } /// Require that erc20s is a valid collateral array diff --git a/contracts/p1/RToken.sol b/contracts/p1/RToken.sol index ce0384b455..e6793144e8 100644 --- a/contracts/p1/RToken.sol +++ b/contracts/p1/RToken.sol @@ -220,8 +220,8 @@ contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken { // amount > 0 // amount <= balanceOf(caller) // sum(portions) == FIX_ONE - // require nonce within [bh.lastRevalued, bh.nonce()] for nonce in basketNonces - // + // nonce >= basketHandler.primeNonce() for nonce in basketNonces + // effects: // (so totalSupply -= amount and balanceOf(caller) -= amount) // basketsNeeded' / totalSupply' >== basketsNeeded / totalSupply @@ -234,7 +234,6 @@ contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken { // do token.transferFrom(backingManager, caller, min(tokenAmt, prorataAmt)) // BU exchange rate cannot decrease, and it can only increase when < FIX_ONE. /// @dev Allows partial redemptions up to the minAmounts - /// @dev Redemption is unavailable after setPrimeBasket() but before switchBasket() /// @param recipient The address to receive the backing collateral tokens /// @param amount {qRTok} The quantity {qRToken} of RToken to redeem /// @param basketNonces An array of basket nonces to do redemption from diff --git a/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts b/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts index a8762833b2..82e7892e64 100644 --- a/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts +++ b/tasks/testing/upgrade-checker-utils/upgrades/3_0_0.ts @@ -72,7 +72,7 @@ export default async ( await whileImpersonating(hre, timelockAddress, async (tl) => { await expect( basketHandler.connect(tl).setPrimeBasket([usdc.address], [fp('20')]) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('new target weights') }) // Attempt to change target unit in basket @@ -103,7 +103,7 @@ export default async ( await assetRegistry.connect(tl).register(eurFiatCollateral.address) await expect( basketHandler.connect(tl).setPrimeBasket([eurt.address], [fp('1')]) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('new target weights') await assetRegistry.connect(tl).unregister(eurFiatCollateral.address) }) diff --git a/test/Main.test.ts b/test/Main.test.ts index f0c1301c92..9d00c3b0a6 100644 --- a/test/Main.test.ts +++ b/test/Main.test.ts @@ -1795,21 +1795,21 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { // not possible on non-fresh basketHandler await expect( freshBasketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) - ).to.be.revertedWith('invalid target amt') + ).to.be.revertedWith('invalid target amount; too large') }) it('Should not allow to increase prime Basket weights', async () => { // not possible on freshBasketHandler await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('new target weights') }) it('Should not allow to decrease prime Basket weights', async () => { // not possible on freshBasketHandler await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('missing target weights') }) it('Should not allow to set prime Basket with an empty basket', async () => { @@ -1824,10 +1824,10 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { it('Should not allow to set prime Basket with a zero amount', async () => { await expect( freshBasketHandler.connect(owner).setPrimeBasket([token0.address], [0]) - ).to.be.revertedWith('invalid target amt') + ).to.be.revertedWith('invalid target amount; must be nonzero') await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('missing target weights') }) it('Should be able to set exactly same basket', async () => { @@ -1870,7 +1870,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address, backupToken1.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('new target weights') await expect( basketHandler @@ -1879,7 +1879,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address, eurToken.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('new target weights') }) it('Should not allow to set prime Basket as subset of old basket', async () => { @@ -1890,7 +1890,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.24')] ) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('missing target weights') await expect( basketHandler .connect(owner) @@ -1898,7 +1898,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address], [fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('missing target weights') }) it('Should not allow to change target unit in old basket', async () => { @@ -1909,7 +1909,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, eurToken.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('new target weights') }) it('Should not allow to set prime Basket with RSR/RToken', async () => { @@ -1963,7 +1963,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) - ).to.be.revertedWith('targets not constant') + ).to.be.revertedWith('new target weights') }) describe('Custom Redemption', () => { From eacec36aab20f948a4a35ab55c54d8c0cee00aba Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Tue, 5 Dec 2023 19:00:58 -0500 Subject: [PATCH 09/20] floor yearn V2 refPerTok() to be consistent with rest of plugins --- contracts/plugins/assets/yearnv2/YearnV2CurveFiatCollateral.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/plugins/assets/yearnv2/YearnV2CurveFiatCollateral.sol b/contracts/plugins/assets/yearnv2/YearnV2CurveFiatCollateral.sol index 9121982562..8e967f865d 100644 --- a/contracts/plugins/assets/yearnv2/YearnV2CurveFiatCollateral.sol +++ b/contracts/plugins/assets/yearnv2/YearnV2CurveFiatCollateral.sol @@ -91,7 +91,7 @@ contract YearnV2CurveFiatCollateral is CurveStableCollateral { /// @return {ref/tok} Actual quantity of whole reference units per whole collateral tokens function _underlyingRefPerTok() internal view virtual override returns (uint192) { // {ref/tok} = {ref/LP token} * {LP token/tok} - return _safeWrap(curvePool.get_virtual_price()).mul(_pricePerShare()); + return _safeWrap(curvePool.get_virtual_price()).mul(_pricePerShare(), FLOOR); } /// @return {LP token/tok} From f0d3093ad3bf2141ecafc26a2ff664e06cf52f72 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Tue, 5 Dec 2023 19:09:38 -0500 Subject: [PATCH 10/20] remove final leftovers --- CHANGELOG.md | 2 -- contracts/p1/RToken.sol | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f792116625..b5fea123f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,6 @@ This release adds the ability for each RToken to configure whether the target ba Upgrade BasketHandler -Optional: For slightly nicer RToken documentation, upgrade RToken as well. No change in functionality - ### Core Protocol Contracts - `BasketHandler` [+1 slot] diff --git a/contracts/p1/RToken.sol b/contracts/p1/RToken.sol index e6793144e8..cfbbdcd923 100644 --- a/contracts/p1/RToken.sol +++ b/contracts/p1/RToken.sol @@ -221,7 +221,7 @@ contract RTokenP1 is ComponentP1, ERC20PermitUpgradeable, IRToken { // amount <= balanceOf(caller) // sum(portions) == FIX_ONE // nonce >= basketHandler.primeNonce() for nonce in basketNonces - + // // effects: // (so totalSupply -= amount and balanceOf(caller) -= amount) // basketsNeeded' / totalSupply' >== basketsNeeded / totalSupply From 7e5883bdc748631f126f94d93448031cf05168f8 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Tue, 5 Dec 2023 19:41:54 -0500 Subject: [PATCH 11/20] add tests --- contracts/p0/BasketHandler.sol | 2 +- contracts/p1/BasketHandler.sol | 2 +- test/Main.test.ts | 161 ++++++++++++++++++++------------- 3 files changed, 98 insertions(+), 67 deletions(-) diff --git a/contracts/p0/BasketHandler.sol b/contracts/p0/BasketHandler.sol index 8f8c468f38..3c57fdff66 100644 --- a/contracts/p0/BasketHandler.sol +++ b/contracts/p0/BasketHandler.sol @@ -248,7 +248,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { requireValidCollArray(erc20s); // If this isn't initial setup, require targets remain constant - if (revaluable && config.erc20s.length > 0) { + if (!revaluable && config.erc20s.length > 0) { requireConstantConfigTargets(erc20s, targetAmts); } diff --git a/contracts/p1/BasketHandler.sol b/contracts/p1/BasketHandler.sol index e58b0f1b6e..2aee788b4d 100644 --- a/contracts/p1/BasketHandler.sol +++ b/contracts/p1/BasketHandler.sol @@ -187,7 +187,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { requireValidCollArray(erc20s); // If this isn't initial setup, require targets remain constant - if (revaluable && config.erc20s.length > 0) { + if (!revaluable && config.erc20s.length > 0) { requireConstantConfigTargets(erc20s, targetAmts); } diff --git a/test/Main.test.ts b/test/Main.test.ts index 9d00c3b0a6..b3729584f0 100644 --- a/test/Main.test.ts +++ b/test/Main.test.ts @@ -1689,32 +1689,9 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { }) describe('Basket Handling', () => { - let freshBasketHandler: TestIBasketHandler // need to have both this and regular basketHandler around let eurToken: ERC20Mock beforeEach(async () => { - if (IMPLEMENTATION == Implementation.P0) { - const BasketHandlerFactory = await ethers.getContractFactory('BasketHandlerP0') - freshBasketHandler = ((await BasketHandlerFactory.deploy()) as unknown) - } else if (IMPLEMENTATION == Implementation.P1) { - const basketLib = await (await ethers.getContractFactory('BasketLibP1')).deploy() - const BasketHandlerFactory = await ethers.getContractFactory('BasketHandlerP1', { - libraries: { BasketLibP1: basketLib.address }, - }) - freshBasketHandler = await upgrades.deployProxy( - BasketHandlerFactory, - [], - { - kind: 'uups', - unsafeAllow: ['external-library-linking'], // BasketLibP1 - } - ) - } else { - throw new Error('PROTO_IMPL must be set to either `0` or `1`') - } - - await freshBasketHandler.init(main.address, config.warmupPeriod) - eurToken = await (await ethers.getContractFactory('ERC20Mock')).deploy('EURO Token', 'EUR') const FiatCollateralFactory: ContractFactory = await ethers.getContractFactory( 'FiatCollateral' @@ -1734,18 +1711,12 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { }) it('Should not allow to set prime Basket if not OWNER', async () => { - await expect( - freshBasketHandler.connect(other).setPrimeBasket([token0.address], [fp('1')]) - ).to.be.revertedWith('governance only') await expect( basketHandler.connect(other).setPrimeBasket([token0.address], [fp('1')]) ).to.be.revertedWith('governance only') }) it('Should not allow to set prime Basket with invalid length', async () => { - await expect( - freshBasketHandler.connect(owner).setPrimeBasket([token0.address], []) - ).to.be.revertedWith('len mismatch') await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], []) ).to.be.revertedWith('len mismatch') @@ -1755,17 +1726,9 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { await expect( basketHandler.connect(owner).setPrimeBasket([compToken.address], [fp('1')]) ).to.be.revertedWith('erc20 is not collateral') - await expect( - freshBasketHandler.connect(owner).setPrimeBasket([compToken.address], [fp('1')]) - ).to.be.revertedWith('erc20 is not collateral') }) it('Should not allow to set prime Basket with duplicate ERC20s', async () => { - await expect( - freshBasketHandler - .connect(owner) - .setPrimeBasket([token0.address, token0.address], [fp('1'), fp('1')]) - ).to.be.revertedWith('contains duplicates') await expect( basketHandler .connect(owner) @@ -1774,60 +1737,80 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { }) it('Should not allow to set prime Basket with 0 address tokens', async () => { - await expect( - freshBasketHandler.connect(owner).setPrimeBasket([ZERO_ADDRESS], [fp('1')]) - ).to.be.revertedWith('invalid collateral') await expect( basketHandler.connect(owner).setPrimeBasket([ZERO_ADDRESS], [fp('1')]) ).to.be.revertedWith('invalid collateral') }) it('Should not allow to set prime Basket with stRSR', async () => { - await expect( - freshBasketHandler.connect(owner).setPrimeBasket([stRSR.address], [fp('1')]) - ).to.be.revertedWith('invalid collateral') await expect( basketHandler.connect(owner).setPrimeBasket([stRSR.address], [fp('1')]) ).to.be.revertedWith('invalid collateral') }) it('Should not allow to bypass MAX_TARGET_AMT', async () => { - // not possible on non-fresh basketHandler await expect( - freshBasketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) + basketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) + ).to.be.revertedWith('new target weights') + + // Should fail differently after revaluable + await basketHandler.connect(owner).setRevaluable(true) + await expect( + basketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) ).to.be.revertedWith('invalid target amount; too large') }) - it('Should not allow to increase prime Basket weights', async () => { - // not possible on freshBasketHandler + it('Should allow to make revaluable', async () => { + await expect(basketHandler.connect(owner).setRevaluable(true)) + .to.emit(basketHandler, 'RevaluableChanged') + .withArgs(false, true) + await expect(basketHandler.connect(owner).setRevaluable(false)) + .to.emit(basketHandler, 'RevaluableChanged') + .withArgs(true, false) + }) + + it('Should not allow to increase prime Basket weights unless revaluable', async () => { await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) ).to.be.revertedWith('new target weights') + + // Should work once revaluable + await basketHandler.connect(owner).setRevaluable(true) + await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) }) - it('Should not allow to decrease prime Basket weights', async () => { - // not possible on freshBasketHandler + it('Should not allow to decrease prime Basket weights unless revaluable', async () => { await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) ).to.be.revertedWith('missing target weights') + + // Should work once revaluable + await basketHandler.connect(owner).setRevaluable(true) + await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) }) it('Should not allow to set prime Basket with an empty basket', async () => { - await expect(freshBasketHandler.connect(owner).setPrimeBasket([], [])).to.be.revertedWith( + await expect(basketHandler.connect(owner).setPrimeBasket([], [])).to.be.revertedWith( 'empty basket' ) + + // Should still fail after revaluable + await basketHandler.connect(owner).setRevaluable(true) await expect(basketHandler.connect(owner).setPrimeBasket([], [])).to.be.revertedWith( 'empty basket' ) }) it('Should not allow to set prime Basket with a zero amount', async () => { - await expect( - freshBasketHandler.connect(owner).setPrimeBasket([token0.address], [0]) - ).to.be.revertedWith('invalid target amount; must be nonzero') await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) ).to.be.revertedWith('missing target weights') + + // Should still fail after revaluable + await basketHandler.connect(owner).setRevaluable(true) + await expect( + basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) + ).to.be.revertedWith('invalid target amount; must be nonzero') }) it('Should be able to set exactly same basket', async () => { @@ -1837,6 +1820,15 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [token0.address, token1.address, token2.address, token3.address], [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) + + // Should still work once revaluable + await basketHandler.connect(owner).setRevaluable(true) + await basketHandler + .connect(owner) + .setPrimeBasket( + [token0.address, token1.address, token2.address, token3.address], + [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] + ) }) it('Should be able to set prime basket multiple times', async () => { @@ -1871,7 +1863,6 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) ).to.be.revertedWith('new target weights') - await expect( basketHandler .connect(owner) @@ -1880,6 +1871,15 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) ).to.be.revertedWith('new target weights') + + // Should work once revaluable + await basketHandler.connect(owner).setRevaluable(true) + await basketHandler + .connect(owner) + .setPrimeBasket( + [token0.address, token1.address, token2.address, token3.address, backupToken1.address], + [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] + ) }) it('Should not allow to set prime Basket as subset of old basket', async () => { @@ -1899,6 +1899,15 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25')] ) ).to.be.revertedWith('missing target weights') + + // Should work once revaluable + await basketHandler.connect(owner).setRevaluable(true) + await basketHandler + .connect(owner) + .setPrimeBasket( + [token0.address, token1.address, token2.address, token3.address], + [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.24')] + ) }) it('Should not allow to change target unit in old basket', async () => { @@ -1910,21 +1919,32 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) ).to.be.revertedWith('new target weights') + + // Should work once revaluable + await basketHandler.connect(owner).setRevaluable(true) + await basketHandler + .connect(owner) + .setPrimeBasket( + [token0.address, token1.address, token2.address, eurToken.address], + [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] + ) }) it('Should not allow to set prime Basket with RSR/RToken', async () => { - await expect( - freshBasketHandler.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) - ).to.be.revertedWith('invalid collateral') await expect( basketHandler.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) ).to.be.revertedWith('invalid collateral') - await expect( - freshBasketHandler + basketHandler .connect(owner) .setPrimeBasket([token0.address, rToken.address], [fp('0.5'), fp('0.5')]) ).to.be.revertedWith('invalid collateral') + + // Should still fail once revaluable + await basketHandler.connect(owner).setRevaluable(true) + await expect( + basketHandler.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) + ).to.be.revertedWith('invalid collateral') await expect( basketHandler .connect(owner) @@ -1964,6 +1984,15 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) ).to.be.revertedWith('new target weights') + + // Should not revert once revaluable + await basketHandler.connect(owner).setRevaluable(true) + await basketHandler + .connect(owner) + .setPrimeBasket( + [token0.address, token1.address, token2.address, token3.address], + [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] + ) }) describe('Custom Redemption', () => { @@ -2876,10 +2905,11 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { await newColl.refresh() // Set basket with single collateral - await freshBasketHandler.connect(owner).setPrimeBasket([token2.address], [fp('1000')]) + await basketHandler.setRevaluable(true) + await basketHandler.connect(owner).setPrimeBasket([token2.address], [fp('1000')]) // Change basket - valid at this point - await freshBasketHandler.connect(owner).refreshBasket() + await basketHandler.connect(owner).refreshBasket() // Set refPerTok = 1 await newColl.setRate(bn(1)) @@ -2887,20 +2917,21 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { const newPrice: BigNumber = MAX_UINT192.div(bn('1e10')) await setOraclePrice(collateral2.address, newPrice.sub(newPrice.div(100))) // oracle error - const [lowPrice, highPrice] = await freshBasketHandler.price() + const [lowPrice, highPrice] = await basketHandler.price() expect(lowPrice).to.equal(MAX_UINT192) expect(highPrice).to.equal(MAX_UINT192) }) it('Should handle overflow in price calculation and return [FIX_MAX, FIX_MAX] - case 2', async () => { // Set basket with single collateral - await freshBasketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1.1')]) - await freshBasketHandler.refreshBasket() + await basketHandler.setRevaluable(true) + await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1.1')]) + await basketHandler.refreshBasket() const newPrice: BigNumber = MAX_UINT192.div(bn('1e10')) await setOraclePrice(collateral0.address, newPrice.sub(newPrice.div(100))) // oracle error - const [lowPrice, highPrice] = await freshBasketHandler.price() + const [lowPrice, highPrice] = await basketHandler.price() expect(lowPrice).to.equal(MAX_UINT192) expect(highPrice).to.equal(MAX_UINT192) }) From c8e8918462ae6148a45d0f59c935b03d5a1e80d6 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Tue, 5 Dec 2023 19:44:10 -0500 Subject: [PATCH 12/20] comment nits --- contracts/interfaces/IBasketHandler.sol | 2 +- contracts/p0/BasketHandler.sol | 2 +- contracts/p1/BasketHandler.sol | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/interfaces/IBasketHandler.sol b/contracts/interfaces/IBasketHandler.sol index fd343b353c..c200b4cd79 100644 --- a/contracts/interfaces/IBasketHandler.sol +++ b/contracts/interfaces/IBasketHandler.sol @@ -48,7 +48,7 @@ interface IBasketHandler is IComponent { /// @param newStatus The new basket status event BasketStatusChanged(CollateralStatus oldStatus, CollateralStatus newStatus); - /// Emitted when the ability to change target weights is changed + /// Emitted when the ability to change total target weights is changed /// @param oldVal The old revalueable boolean /// @param newVal The new revalueable boolean event RevaluableChanged(bool oldVal, bool newVal); diff --git a/contracts/p0/BasketHandler.sol b/contracts/p0/BasketHandler.sol index 3c57fdff66..a8e6c9bb67 100644 --- a/contracts/p0/BasketHandler.sol +++ b/contracts/p0/BasketHandler.sol @@ -148,7 +148,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // A history of baskets by basket nonce; includes current basket mapping(uint48 => Basket) private basketHistory; - bool public revaluable; // whether the weights of the target basket be changed + bool public revaluable; // whether the total weights of the target basket can be changed // ==== Invariants ==== // basket is a valid Basket: diff --git a/contracts/p1/BasketHandler.sol b/contracts/p1/BasketHandler.sol index 2aee788b4d..a571565505 100644 --- a/contracts/p1/BasketHandler.sol +++ b/contracts/p1/BasketHandler.sol @@ -82,7 +82,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // === // Added in 3.2.0 - bool public revaluable; // whether the weights of the target basket be changed + bool public revaluable; // whether the total weights of the target basket can be changed // === From a6a89c517e86694694bc1779cf0b5988c10eb6ba Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Tue, 5 Dec 2023 20:59:44 -0500 Subject: [PATCH 13/20] revaluable -> reweightable --- CHANGELOG.md | 6 +-- contracts/interfaces/IBasketHandler.sol | 8 ++-- contracts/p0/BasketHandler.sol | 12 ++--- contracts/p1/BasketHandler.sol | 12 ++--- test/Main.test.ts | 62 ++++++++++++------------- 5 files changed, 50 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5fea123f7..fa35d7b36a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ # 3.2.0 -This release adds the ability for each RToken to configure whether the target basket should be revaluable or not. An RToken that is not revaluable can have its target basket changed in terms of ERC20s but not in terms of target weights at the target unit level. +This release adds the ability for each RToken to configure whether the target basket should be reweightable or not. An RToken that is not reweightable can have its target basket changed in terms of ERC20s but not in terms of target weights at the target unit level. ### Upgrade Steps @@ -11,8 +11,8 @@ Upgrade BasketHandler ### Core Protocol Contracts - `BasketHandler` [+1 slot] - - Add concept of a revaluable basket: a basket that can have its target amounts (once grouped by target unit) changed - - Add `revaluable` bool and `setRevaluable(bool)` + - Add concept of a reweightable basket: a basket that can have its target amounts (once grouped by target unit) changed + - Add `reweightable` bool and `setReweightable(bool)` # 3.1.0 - Unreleased diff --git a/contracts/interfaces/IBasketHandler.sol b/contracts/interfaces/IBasketHandler.sol index c200b4cd79..5ac7417726 100644 --- a/contracts/interfaces/IBasketHandler.sol +++ b/contracts/interfaces/IBasketHandler.sol @@ -49,9 +49,9 @@ interface IBasketHandler is IComponent { event BasketStatusChanged(CollateralStatus oldStatus, CollateralStatus newStatus); /// Emitted when the ability to change total target weights is changed - /// @param oldVal The old revalueable boolean - /// @param newVal The new revalueable boolean - event RevaluableChanged(bool oldVal, bool newVal); + /// @param oldVal The old reweightable boolean + /// @param newVal The new reweightable boolean + event ReweightableChanged(bool oldVal, bool newVal); // Initialization function init(IMain main_, uint48 warmupPeriod_) external; @@ -162,5 +162,5 @@ interface TestIBasketHandler is IBasketHandler { function setWarmupPeriod(uint48 val) external; - function setRevaluable(bool val) external; + function setReweightable(bool val) external; } diff --git a/contracts/p0/BasketHandler.sol b/contracts/p0/BasketHandler.sol index a8e6c9bb67..112a37f051 100644 --- a/contracts/p0/BasketHandler.sol +++ b/contracts/p0/BasketHandler.sol @@ -148,7 +148,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // A history of baskets by basket nonce; includes current basket mapping(uint48 => Basket) private basketHistory; - bool public revaluable; // whether the total weights of the target basket can be changed + bool public reweightable; // whether the total weights of the target basket can be changed // ==== Invariants ==== // basket is a valid Basket: @@ -170,7 +170,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { lastStatusTimestamp = uint48(block.timestamp); disabled = true; - // revaluable = false; + // reweightable = false; } /// Disable the basket in order to schedule a basket refresh @@ -248,7 +248,7 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { requireValidCollArray(erc20s); // If this isn't initial setup, require targets remain constant - if (!revaluable && config.erc20s.length > 0) { + if (!reweightable && config.erc20s.length > 0) { requireConstantConfigTargets(erc20s, targetAmts); } @@ -572,9 +572,9 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { } /// @custom:governance - function setRevaluable(bool val) public governance { - emit RevaluableChanged(revaluable, val); - revaluable = val; + function setReweightable(bool val) public governance { + emit ReweightableChanged(reweightable, val); + reweightable = val; } /* _switchBasket computes basket' from three inputs: diff --git a/contracts/p1/BasketHandler.sol b/contracts/p1/BasketHandler.sol index a571565505..dc4bc86701 100644 --- a/contracts/p1/BasketHandler.sol +++ b/contracts/p1/BasketHandler.sol @@ -82,7 +82,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // === // Added in 3.2.0 - bool public revaluable; // whether the total weights of the target basket can be changed + bool public reweightable; // whether the total weights of the target basket can be changed // === @@ -112,7 +112,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { lastStatusTimestamp = uint48(block.timestamp); disabled = true; - // revaluable = false; + // reweightable = false; } /// Disable the basket in order to schedule a basket refresh @@ -187,7 +187,7 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { requireValidCollArray(erc20s); // If this isn't initial setup, require targets remain constant - if (!revaluable && config.erc20s.length > 0) { + if (!reweightable && config.erc20s.length > 0) { requireConstantConfigTargets(erc20s, targetAmts); } @@ -522,10 +522,10 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { } /// @custom:governance - function setRevaluable(bool val) public { + function setReweightable(bool val) public { requireGovernanceOnly(); - emit RevaluableChanged(revaluable, val); - revaluable = val; + emit ReweightableChanged(reweightable, val); + reweightable = val; } // === Private === diff --git a/test/Main.test.ts b/test/Main.test.ts index b3729584f0..ffde383150 100644 --- a/test/Main.test.ts +++ b/test/Main.test.ts @@ -1753,39 +1753,39 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { basketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) ).to.be.revertedWith('new target weights') - // Should fail differently after revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should fail differently after becomes reweightable + await basketHandler.connect(owner).setReweightable(true) await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) ).to.be.revertedWith('invalid target amount; too large') }) - it('Should allow to make revaluable', async () => { - await expect(basketHandler.connect(owner).setRevaluable(true)) - .to.emit(basketHandler, 'RevaluableChanged') + it('Should allow to make reweightable', async () => { + await expect(basketHandler.connect(owner).setReweightable(true)) + .to.emit(basketHandler, 'ReweightableChanged') .withArgs(false, true) - await expect(basketHandler.connect(owner).setRevaluable(false)) - .to.emit(basketHandler, 'RevaluableChanged') + await expect(basketHandler.connect(owner).setReweightable(false)) + .to.emit(basketHandler, 'ReweightableChanged') .withArgs(true, false) }) - it('Should not allow to increase prime Basket weights unless revaluable', async () => { + it('Should not allow to increase prime Basket weights unless reweightable', async () => { await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) ).to.be.revertedWith('new target weights') - // Should work once revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should work once reweightable + await basketHandler.connect(owner).setReweightable(true) await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) }) - it('Should not allow to decrease prime Basket weights unless revaluable', async () => { + it('Should not allow to decrease prime Basket weights unless reweightable', async () => { await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) ).to.be.revertedWith('missing target weights') - // Should work once revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should work once reweightable + await basketHandler.connect(owner).setReweightable(true) await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) }) @@ -1794,8 +1794,8 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { 'empty basket' ) - // Should still fail after revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should still fail after becomes reweightable + await basketHandler.connect(owner).setReweightable(true) await expect(basketHandler.connect(owner).setPrimeBasket([], [])).to.be.revertedWith( 'empty basket' ) @@ -1806,8 +1806,8 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) ).to.be.revertedWith('missing target weights') - // Should still fail after revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should still fail after becomes reweightable + await basketHandler.connect(owner).setReweightable(true) await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) ).to.be.revertedWith('invalid target amount; must be nonzero') @@ -1821,8 +1821,8 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) - // Should still work once revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should still work once reweightable + await basketHandler.connect(owner).setReweightable(true) await basketHandler .connect(owner) .setPrimeBasket( @@ -1872,8 +1872,8 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ) ).to.be.revertedWith('new target weights') - // Should work once revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should work once reweightable + await basketHandler.connect(owner).setReweightable(true) await basketHandler .connect(owner) .setPrimeBasket( @@ -1900,8 +1900,8 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ) ).to.be.revertedWith('missing target weights') - // Should work once revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should work once reweightable + await basketHandler.connect(owner).setReweightable(true) await basketHandler .connect(owner) .setPrimeBasket( @@ -1920,8 +1920,8 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ) ).to.be.revertedWith('new target weights') - // Should work once revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should work once reweightable + await basketHandler.connect(owner).setReweightable(true) await basketHandler .connect(owner) .setPrimeBasket( @@ -1940,8 +1940,8 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { .setPrimeBasket([token0.address, rToken.address], [fp('0.5'), fp('0.5')]) ).to.be.revertedWith('invalid collateral') - // Should still fail once revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should still fail once reweightable + await basketHandler.connect(owner).setReweightable(true) await expect( basketHandler.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) ).to.be.revertedWith('invalid collateral') @@ -1985,8 +1985,8 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ) ).to.be.revertedWith('new target weights') - // Should not revert once revaluable - await basketHandler.connect(owner).setRevaluable(true) + // Should not revert once reweightable + await basketHandler.connect(owner).setReweightable(true) await basketHandler .connect(owner) .setPrimeBasket( @@ -2905,7 +2905,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { await newColl.refresh() // Set basket with single collateral - await basketHandler.setRevaluable(true) + await basketHandler.setReweightable(true) await basketHandler.connect(owner).setPrimeBasket([token2.address], [fp('1000')]) // Change basket - valid at this point @@ -2924,7 +2924,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { it('Should handle overflow in price calculation and return [FIX_MAX, FIX_MAX] - case 2', async () => { // Set basket with single collateral - await basketHandler.setRevaluable(true) + await basketHandler.setReweightable(true) await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1.1')]) await basketHandler.refreshBasket() From da40fdf21985637932325f0df7c14a1b0a771ec8 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Thu, 7 Dec 2023 16:42:13 -0500 Subject: [PATCH 14/20] make reweightable immutable --- CHANGELOG.md | 4 ++-- common/configuration.ts | 1 + contracts/interfaces/IBasketHandler.sol | 13 +++++-------- contracts/interfaces/IDeployer.sol | 1 + contracts/p0/BasketHandler.sol | 17 ++++++++--------- contracts/p0/Deployer.sol | 2 +- contracts/p1/BasketHandler.sol | 18 ++++++++---------- contracts/p1/Deployer.sol | 2 +- test/fixtures.ts | 1 + test/integration/fixtures.ts | 1 + 10 files changed, 29 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa35d7b36a..e69568eb92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ # 3.2.0 -This release adds the ability for each RToken to configure whether the target basket should be reweightable or not. An RToken that is not reweightable can have its target basket changed in terms of ERC20s but not in terms of target weights at the target unit level. +This release gives new RTokens being deployed the option to enable a variable target basket, or to be "reweightable". An RToken that is not reweightable cannot have its target basket changed in terms of quantities of target units. ### Upgrade Steps @@ -12,7 +12,7 @@ Upgrade BasketHandler - `BasketHandler` [+1 slot] - Add concept of a reweightable basket: a basket that can have its target amounts (once grouped by target unit) changed - - Add `reweightable` bool and `setReweightable(bool)` + - Add immutable-after-init `reweightable` bool # 3.1.0 - Unreleased diff --git a/common/configuration.ts b/common/configuration.ts index 0bd5d8ff2e..8aec5837da 100644 --- a/common/configuration.ts +++ b/common/configuration.ts @@ -592,6 +592,7 @@ export interface IConfig { unstakingDelay: BigNumber withdrawalLeak: BigNumber warmupPeriod: BigNumber + reweightable: boolean tradingDelay: BigNumber batchAuctionLength: BigNumber dutchAuctionLength: BigNumber diff --git a/contracts/interfaces/IBasketHandler.sol b/contracts/interfaces/IBasketHandler.sol index 5ac7417726..b835ddc683 100644 --- a/contracts/interfaces/IBasketHandler.sol +++ b/contracts/interfaces/IBasketHandler.sol @@ -48,13 +48,12 @@ interface IBasketHandler is IComponent { /// @param newStatus The new basket status event BasketStatusChanged(CollateralStatus oldStatus, CollateralStatus newStatus); - /// Emitted when the ability to change total target weights is changed - /// @param oldVal The old reweightable boolean - /// @param newVal The new reweightable boolean - event ReweightableChanged(bool oldVal, bool newVal); - // Initialization - function init(IMain main_, uint48 warmupPeriod_) external; + function init( + IMain main_, + uint48 warmupPeriod_, + bool reweightable_ + ) external; /// Set the prime basket /// @param erc20s The collateral tokens for the new prime basket @@ -161,6 +160,4 @@ interface TestIBasketHandler is IBasketHandler { function warmupPeriod() external view returns (uint48); function setWarmupPeriod(uint48 val) external; - - function setReweightable(bool val) external; } diff --git a/contracts/interfaces/IDeployer.sol b/contracts/interfaces/IDeployer.sol index 18c1cc4ddc..d811aef2ec 100644 --- a/contracts/interfaces/IDeployer.sol +++ b/contracts/interfaces/IDeployer.sol @@ -38,6 +38,7 @@ struct DeploymentParams { // // === BasketHandler === uint48 warmupPeriod; // {s} how long to wait until issuance/trading after regaining SOUND + bool reweightable; // whether the basket can change in value // // === BackingManager === uint48 tradingDelay; // {s} how long to wait until starting auctions after switching basket diff --git a/contracts/p0/BasketHandler.sol b/contracts/p0/BasketHandler.sol index 112a37f051..bb32ffad23 100644 --- a/contracts/p0/BasketHandler.sol +++ b/contracts/p0/BasketHandler.sol @@ -148,7 +148,8 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // A history of baskets by basket nonce; includes current basket mapping(uint48 => Basket) private basketHistory; - bool public reweightable; // whether the total weights of the target basket can be changed + // Whether the total weights of the target basket can be changed + bool public reweightable; // immutable after init // ==== Invariants ==== // basket is a valid Basket: @@ -160,17 +161,21 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { // if basket.erc20s is empty then disabled == true // BasketHandler.init() just leaves the BasketHandler state zeroed - function init(IMain main_, uint48 warmupPeriod_) external initializer { + function init( + IMain main_, + uint48 warmupPeriod_, + bool reweightable_ + ) external initializer { __Component_init(main_); setWarmupPeriod(warmupPeriod_); + reweightable = reweightable_; // immutable thereafter // Set last status to DISABLED (default) lastStatus = CollateralStatus.DISABLED; lastStatusTimestamp = uint48(block.timestamp); disabled = true; - // reweightable = false; } /// Disable the basket in order to schedule a basket refresh @@ -571,12 +576,6 @@ contract BasketHandlerP0 is ComponentP0, IBasketHandler { warmupPeriod = val; } - /// @custom:governance - function setReweightable(bool val) public governance { - emit ReweightableChanged(reweightable, val); - reweightable = val; - } - /* _switchBasket computes basket' from three inputs: - the basket configuration (config: BasketConfig) - the function (isGood: erc20 -> bool), implemented here by goodCollateral() diff --git a/contracts/p0/Deployer.sol b/contracts/p0/Deployer.sol index c143ec9d62..b44e11a94d 100644 --- a/contracts/p0/Deployer.sol +++ b/contracts/p0/Deployer.sol @@ -95,7 +95,7 @@ contract DeployerP0 is IDeployer, Versioned { ); // Init Basket Handler - main.basketHandler().init(main, params.warmupPeriod); + main.basketHandler().init(main, params.warmupPeriod, params.reweightable); // Init Revenue Traders main.rsrTrader().init(main, rsr, params.maxTradeSlippage, params.minTradeVolume); diff --git a/contracts/p1/BasketHandler.sol b/contracts/p1/BasketHandler.sol index dc4bc86701..db22162a74 100644 --- a/contracts/p1/BasketHandler.sol +++ b/contracts/p1/BasketHandler.sol @@ -82,7 +82,8 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // === // Added in 3.2.0 - bool public reweightable; // whether the total weights of the target basket can be changed + // Whether the total weights of the target basket can be changed + bool public reweightable; // immutable after init // === @@ -96,7 +97,11 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { // if basket.erc20s is empty then disabled == true // BasketHandler.init() just leaves the BasketHandler state zeroed - function init(IMain main_, uint48 warmupPeriod_) external initializer { + function init( + IMain main_, + uint48 warmupPeriod_, + bool reweightable_ + ) external initializer { __Component_init(main_); assetRegistry = main_.assetRegistry(); @@ -106,13 +111,13 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { stRSR = main_.stRSR(); setWarmupPeriod(warmupPeriod_); + reweightable = reweightable_; // immutable thereafter // Set last status to DISABLED (default) lastStatus = CollateralStatus.DISABLED; lastStatusTimestamp = uint48(block.timestamp); disabled = true; - // reweightable = false; } /// Disable the basket in order to schedule a basket refresh @@ -521,13 +526,6 @@ contract BasketHandlerP1 is ComponentP1, IBasketHandler { warmupPeriod = val; } - /// @custom:governance - function setReweightable(bool val) public { - requireGovernanceOnly(); - emit ReweightableChanged(reweightable, val); - reweightable = val; - } - // === Private === // contract-size-saver diff --git a/contracts/p1/Deployer.sol b/contracts/p1/Deployer.sol index 2119420e43..a962b2efe8 100644 --- a/contracts/p1/Deployer.sol +++ b/contracts/p1/Deployer.sol @@ -183,7 +183,7 @@ contract DeployerP1 is IDeployer, Versioned { ); // Init Basket Handler - components.basketHandler.init(main, params.warmupPeriod); + components.basketHandler.init(main, params.warmupPeriod, params.reweightable); // Init Revenue Traders components.rsrTrader.init(main, rsr, params.maxTradeSlippage, params.minTradeVolume); diff --git a/test/fixtures.ts b/test/fixtures.ts index 4b0a8e30fa..72d6eb4339 100644 --- a/test/fixtures.ts +++ b/test/fixtures.ts @@ -453,6 +453,7 @@ const makeDefaultFixture = async (setBasket: boolean): Promise = unstakingDelay: bn('1209600'), // 2 weeks withdrawalLeak: fp('0'), // 0%; always refresh warmupPeriod: bn('60'), // (the delay _after_ SOUND was regained) + reweightable: false, tradingDelay: bn('0'), // (the delay _after_ default has been confirmed) batchAuctionLength: bn('900'), // 15 minutes dutchAuctionLength: bn('1800'), // 30 minutes diff --git a/test/integration/fixtures.ts b/test/integration/fixtures.ts index 206cfb2afe..ae65eaa450 100644 --- a/test/integration/fixtures.ts +++ b/test/integration/fixtures.ts @@ -648,6 +648,7 @@ const makeDefaultFixture = async (setBasket: boolean): Promise = unstakingDelay: bn('1209600'), // 2 weeks withdrawalLeak: fp('0'), // 0%; always refresh warmupPeriod: bn('60'), // (the delay _after_ SOUND was regained) + reweightable: false, tradingDelay: bn('0'), // (the delay _after_ default has been confirmed) batchAuctionLength: bn('900'), // 15 minutes dutchAuctionLength: bn('1800'), // 30 minutes From 544efc890c14758862ffae7c1402bc75939aca5f Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Thu, 7 Dec 2023 16:49:46 -0500 Subject: [PATCH 15/20] adjust tests --- test/Main.test.ts | 133 ++++++++++++++++++++++------------------------ 1 file changed, 65 insertions(+), 68 deletions(-) diff --git a/test/Main.test.ts b/test/Main.test.ts index ffde383150..09a1c45969 100644 --- a/test/Main.test.ts +++ b/test/Main.test.ts @@ -370,9 +370,9 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ).to.be.revertedWith('Initializable: contract is already initialized') // Attempt to reinitialize - Basket Handler - await expect(basketHandler.init(main.address, config.warmupPeriod)).to.be.revertedWith( - 'Initializable: contract is already initialized' - ) + await expect( + basketHandler.init(main.address, config.warmupPeriod, config.reweightable) + ).to.be.revertedWith('Initializable: contract is already initialized') // Attempt to reinitialize - Distributor await expect(distributor.init(main.address, config.dist)).to.be.revertedWith( @@ -1689,9 +1689,28 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { }) describe('Basket Handling', () => { + let reweightableBH: TestIBasketHandler // a BasketHandler with reweightable = true let eurToken: ERC20Mock beforeEach(async () => { + if (IMPLEMENTATION == Implementation.P0) { + const BasketHandlerFactory = await ethers.getContractFactory('BasketHandlerP0') + reweightableBH = ((await BasketHandlerFactory.deploy()) as unknown) + } else if (IMPLEMENTATION == Implementation.P1) { + const basketLib = await (await ethers.getContractFactory('BasketLibP1')).deploy() + const BasketHandlerFactory = await ethers.getContractFactory('BasketHandlerP1', { + libraries: { BasketLibP1: basketLib.address }, + }) + reweightableBH = await upgrades.deployProxy(BasketHandlerFactory, [], { + kind: 'uups', + unsafeAllow: ['external-library-linking'], // BasketLibP1 + }) + } else { + throw new Error('PROTO_IMPL must be set to either `0` or `1`') + } + + await reweightableBH.init(main.address, config.warmupPeriod, config.reweightable) + eurToken = await (await ethers.getContractFactory('ERC20Mock')).deploy('EURO Token', 'EUR') const FiatCollateralFactory: ContractFactory = await ethers.getContractFactory( 'FiatCollateral' @@ -1711,12 +1730,18 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { }) it('Should not allow to set prime Basket if not OWNER', async () => { + await expect( + reweightableBH.connect(other).setPrimeBasket([token0.address], [fp('1')]) + ).to.be.revertedWith('governance only') await expect( basketHandler.connect(other).setPrimeBasket([token0.address], [fp('1')]) ).to.be.revertedWith('governance only') }) it('Should not allow to set prime Basket with invalid length', async () => { + await expect( + reweightableBH.connect(owner).setPrimeBasket([token0.address], []) + ).to.be.revertedWith('len mismatch') await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], []) ).to.be.revertedWith('len mismatch') @@ -1726,9 +1751,17 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { await expect( basketHandler.connect(owner).setPrimeBasket([compToken.address], [fp('1')]) ).to.be.revertedWith('erc20 is not collateral') + await expect( + reweightableBH.connect(owner).setPrimeBasket([compToken.address], [fp('1')]) + ).to.be.revertedWith('erc20 is not collateral') }) it('Should not allow to set prime Basket with duplicate ERC20s', async () => { + await expect( + reweightableBH + .connect(owner) + .setPrimeBasket([token0.address, token0.address], [fp('1'), fp('1')]) + ).to.be.revertedWith('contains duplicates') await expect( basketHandler .connect(owner) @@ -1737,65 +1770,48 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { }) it('Should not allow to set prime Basket with 0 address tokens', async () => { + await expect( + reweightableBH.connect(owner).setPrimeBasket([ZERO_ADDRESS], [fp('1')]) + ).to.be.revertedWith('invalid collateral') await expect( basketHandler.connect(owner).setPrimeBasket([ZERO_ADDRESS], [fp('1')]) ).to.be.revertedWith('invalid collateral') }) it('Should not allow to set prime Basket with stRSR', async () => { + await expect( + reweightableBH.connect(owner).setPrimeBasket([stRSR.address], [fp('1')]) + ).to.be.revertedWith('invalid collateral') await expect( basketHandler.connect(owner).setPrimeBasket([stRSR.address], [fp('1')]) ).to.be.revertedWith('invalid collateral') }) it('Should not allow to bypass MAX_TARGET_AMT', async () => { + // not possible on non-fresh basketHandler await expect( - basketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) - ).to.be.revertedWith('new target weights') - - // Should fail differently after becomes reweightable - await basketHandler.connect(owner).setReweightable(true) - await expect( - basketHandler.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) + reweightableBH.connect(owner).setPrimeBasket([token0.address], [MAX_TARGET_AMT.add(1)]) ).to.be.revertedWith('invalid target amount; too large') }) - it('Should allow to make reweightable', async () => { - await expect(basketHandler.connect(owner).setReweightable(true)) - .to.emit(basketHandler, 'ReweightableChanged') - .withArgs(false, true) - await expect(basketHandler.connect(owner).setReweightable(false)) - .to.emit(basketHandler, 'ReweightableChanged') - .withArgs(true, false) - }) - - it('Should not allow to increase prime Basket weights unless reweightable', async () => { + it('Should not allow to increase prime Basket weights', async () => { + // not possible on reweightableBH await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) ).to.be.revertedWith('new target weights') - - // Should work once reweightable - await basketHandler.connect(owner).setReweightable(true) - await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').add(1)]) }) - it('Should not allow to decrease prime Basket weights unless reweightable', async () => { + it('Should not allow to decrease prime Basket weights', async () => { + // not possible on reweightableBH await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) ).to.be.revertedWith('missing target weights') - - // Should work once reweightable - await basketHandler.connect(owner).setReweightable(true) - await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1').sub(1)]) }) it('Should not allow to set prime Basket with an empty basket', async () => { - await expect(basketHandler.connect(owner).setPrimeBasket([], [])).to.be.revertedWith( + await expect(reweightableBH.connect(owner).setPrimeBasket([], [])).to.be.revertedWith( 'empty basket' ) - - // Should still fail after becomes reweightable - await basketHandler.connect(owner).setReweightable(true) await expect(basketHandler.connect(owner).setPrimeBasket([], [])).to.be.revertedWith( 'empty basket' ) @@ -1803,27 +1819,15 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { it('Should not allow to set prime Basket with a zero amount', async () => { await expect( - basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) - ).to.be.revertedWith('missing target weights') - - // Should still fail after becomes reweightable - await basketHandler.connect(owner).setReweightable(true) + reweightableBH.connect(owner).setPrimeBasket([token0.address], [0]) + ).to.be.revertedWith('invalid target amount; must be nonzero') await expect( basketHandler.connect(owner).setPrimeBasket([token0.address], [0]) - ).to.be.revertedWith('invalid target amount; must be nonzero') + ).to.be.revertedWith('missing target weights') }) it('Should be able to set exactly same basket', async () => { - await basketHandler - .connect(owner) - .setPrimeBasket( - [token0.address, token1.address, token2.address, token3.address], - [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] - ) - - // Should still work once reweightable - await basketHandler.connect(owner).setReweightable(true) - await basketHandler + await reweightableBH .connect(owner) .setPrimeBasket( [token0.address, token1.address, token2.address, token3.address], @@ -1873,8 +1877,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ).to.be.revertedWith('new target weights') // Should work once reweightable - await basketHandler.connect(owner).setReweightable(true) - await basketHandler + await reweightableBH .connect(owner) .setPrimeBasket( [token0.address, token1.address, token2.address, token3.address, backupToken1.address], @@ -1901,8 +1904,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ).to.be.revertedWith('missing target weights') // Should work once reweightable - await basketHandler.connect(owner).setReweightable(true) - await basketHandler + await reweightableBH .connect(owner) .setPrimeBasket( [token0.address, token1.address, token2.address, token3.address], @@ -1921,8 +1923,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ).to.be.revertedWith('new target weights') // Should work once reweightable - await basketHandler.connect(owner).setReweightable(true) - await basketHandler + await reweightableBH .connect(owner) .setPrimeBasket( [token0.address, token1.address, token2.address, eurToken.address], @@ -1941,12 +1942,11 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ).to.be.revertedWith('invalid collateral') // Should still fail once reweightable - await basketHandler.connect(owner).setReweightable(true) await expect( - basketHandler.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) + reweightableBH.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) ).to.be.revertedWith('invalid collateral') await expect( - basketHandler + reweightableBH .connect(owner) .setPrimeBasket([token0.address, rToken.address], [fp('0.5'), fp('0.5')]) ).to.be.revertedWith('invalid collateral') @@ -1986,8 +1986,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { ).to.be.revertedWith('new target weights') // Should not revert once reweightable - await basketHandler.connect(owner).setReweightable(true) - await basketHandler + await reweightableBH .connect(owner) .setPrimeBasket( [token0.address, token1.address, token2.address, token3.address], @@ -2905,11 +2904,10 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { await newColl.refresh() // Set basket with single collateral - await basketHandler.setReweightable(true) - await basketHandler.connect(owner).setPrimeBasket([token2.address], [fp('1000')]) + await reweightableBH.connect(owner).setPrimeBasket([token2.address], [fp('1000')]) // Change basket - valid at this point - await basketHandler.connect(owner).refreshBasket() + await reweightableBH.connect(owner).refreshBasket() // Set refPerTok = 1 await newColl.setRate(bn(1)) @@ -2917,21 +2915,20 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { const newPrice: BigNumber = MAX_UINT192.div(bn('1e10')) await setOraclePrice(collateral2.address, newPrice.sub(newPrice.div(100))) // oracle error - const [lowPrice, highPrice] = await basketHandler.price() + const [lowPrice, highPrice] = await reweightableBH.price() expect(lowPrice).to.equal(MAX_UINT192) expect(highPrice).to.equal(MAX_UINT192) }) it('Should handle overflow in price calculation and return [FIX_MAX, FIX_MAX] - case 2', async () => { // Set basket with single collateral - await basketHandler.setReweightable(true) - await basketHandler.connect(owner).setPrimeBasket([token0.address], [fp('1.1')]) - await basketHandler.refreshBasket() + await reweightableBH.connect(owner).setPrimeBasket([token0.address], [fp('1.1')]) + await reweightableBH.refreshBasket() const newPrice: BigNumber = MAX_UINT192.div(bn('1e10')) await setOraclePrice(collateral0.address, newPrice.sub(newPrice.div(100))) // oracle error - const [lowPrice, highPrice] = await basketHandler.price() + const [lowPrice, highPrice] = await reweightableBH.price() expect(lowPrice).to.equal(MAX_UINT192) expect(highPrice).to.equal(MAX_UINT192) }) From a8fac29a814ad6e925dc285f8eb0a11e2ac10709 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Thu, 7 Dec 2023 16:58:46 -0500 Subject: [PATCH 16/20] cleanup diff --- test/Main.test.ts | 50 +++++++++-------------------------------------- 1 file changed, 9 insertions(+), 41 deletions(-) diff --git a/test/Main.test.ts b/test/Main.test.ts index 09a1c45969..452f7bc1be 100644 --- a/test/Main.test.ts +++ b/test/Main.test.ts @@ -1689,7 +1689,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { }) describe('Basket Handling', () => { - let reweightableBH: TestIBasketHandler // a BasketHandler with reweightable = true + let reweightableBH: TestIBasketHandler // need to have both this and regular basketHandler around let eurToken: ERC20Mock beforeEach(async () => { @@ -1827,7 +1827,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { }) it('Should be able to set exactly same basket', async () => { - await reweightableBH + await basketHandler .connect(owner) .setPrimeBasket( [token0.address, token1.address, token2.address, token3.address], @@ -1867,6 +1867,7 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) ).to.be.revertedWith('new target weights') + await expect( basketHandler .connect(owner) @@ -1875,14 +1876,6 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] ) ).to.be.revertedWith('new target weights') - - // Should work once reweightable - await reweightableBH - .connect(owner) - .setPrimeBasket( - [token0.address, token1.address, token2.address, token3.address, backupToken1.address], - [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25'), fp('0.01')] - ) }) it('Should not allow to set prime Basket as subset of old basket', async () => { @@ -1902,14 +1895,6 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25')] ) ).to.be.revertedWith('missing target weights') - - // Should work once reweightable - await reweightableBH - .connect(owner) - .setPrimeBasket( - [token0.address, token1.address, token2.address, token3.address], - [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.24')] - ) }) it('Should not allow to change target unit in old basket', async () => { @@ -1921,32 +1906,23 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) ).to.be.revertedWith('new target weights') - - // Should work once reweightable - await reweightableBH - .connect(owner) - .setPrimeBasket( - [token0.address, token1.address, token2.address, eurToken.address], - [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] - ) }) it('Should not allow to set prime Basket with RSR/RToken', async () => { await expect( - basketHandler.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) + reweightableBH.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) ).to.be.revertedWith('invalid collateral') await expect( - basketHandler - .connect(owner) - .setPrimeBasket([token0.address, rToken.address], [fp('0.5'), fp('0.5')]) + basketHandler.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) ).to.be.revertedWith('invalid collateral') - // Should still fail once reweightable await expect( - reweightableBH.connect(owner).setPrimeBasket([rsr.address], [fp('1')]) + reweightableBH + .connect(owner) + .setPrimeBasket([token0.address, rToken.address], [fp('0.5'), fp('0.5')]) ).to.be.revertedWith('invalid collateral') await expect( - reweightableBH + basketHandler .connect(owner) .setPrimeBasket([token0.address, rToken.address], [fp('0.5'), fp('0.5')]) ).to.be.revertedWith('invalid collateral') @@ -1984,14 +1960,6 @@ describe(`MainP${IMPLEMENTATION} contract`, () => { [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] ) ).to.be.revertedWith('new target weights') - - // Should not revert once reweightable - await reweightableBH - .connect(owner) - .setPrimeBasket( - [token0.address, token1.address, token2.address, token3.address], - [fp('0.25'), fp('0.25'), fp('0.25'), fp('0.25')] - ) }) describe('Custom Redemption', () => { From 6923d231fa93ba450d9554d9bdc539b17c37d2a9 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Thu, 7 Dec 2023 17:36:01 -0500 Subject: [PATCH 17/20] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e69568eb92..85e47bec94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,13 @@ Upgrade BasketHandler ### Core Protocol Contracts +New governance param added to `DeploymentParams`: `reweightable` + - `BasketHandler` [+1 slot] - Add concept of a reweightable basket: a basket that can have its target amounts (once grouped by target unit) changed - Add immutable-after-init `reweightable` bool +- `Deployer` + - New boolean field `reweightable` added to `IDeployer.DeploymentParams` # 3.1.0 - Unreleased From 1db878c14974e711e663547d7044e44df431d2b6 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Thu, 7 Dec 2023 17:36:35 -0500 Subject: [PATCH 18/20] test/Upgradeability.test.ts --- test/Upgradeability.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Upgradeability.test.ts b/test/Upgradeability.test.ts index 0c94eec20c..bacd80f602 100644 --- a/test/Upgradeability.test.ts +++ b/test/Upgradeability.test.ts @@ -248,7 +248,7 @@ describeP1(`Upgradeability - P${IMPLEMENTATION}`, () => { it('Should deploy valid implementation - BasketHandler', async () => { const newBasketHandler: BasketHandlerP1 = await upgrades.deployProxy( BasketHandlerFactory, - [main.address, config.warmupPeriod], + [main.address, config.warmupPeriod, config.reweightable], { initializer: 'init', kind: 'uups', From 6a853c365d50643b2924377ef46741b88840bd37 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Fri, 8 Dec 2023 21:06:18 -0500 Subject: [PATCH 19/20] remove unnecessary main.stRSR() + main.furnace() fetches --- contracts/p1/Distributor.sol | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/contracts/p1/Distributor.sol b/contracts/p1/Distributor.sol index 776e19fe5a..6b1835bc9f 100644 --- a/contracts/p1/Distributor.sol +++ b/contracts/p1/Distributor.sol @@ -35,8 +35,8 @@ contract DistributorP1 is ComponentP1, IDistributor { IERC20 private rsr; IERC20 private rToken; - address private furnace; - address private stRSR; + IFurnace private furnace; + IStRSR private stRSR; address private rTokenTrader; address private rsrTrader; @@ -108,8 +108,8 @@ contract DistributorP1 is ComponentP1, IDistributor { Transfer[] memory transfers = new Transfer[](destinations.length()); uint256 numTransfers; - address furnaceAddr = furnace; // gas-saver - address stRSRAddr = stRSR; // gas-saver + address furnaceAddr = address(furnace); // gas-saver + address stRSRAddr = address(stRSR); // gas-saver bool accountRewards = false; @@ -144,9 +144,9 @@ contract DistributorP1 is ComponentP1, IDistributor { // Perform reward accounting if (accountRewards) { if (isRSR) { - main.stRSR().payoutRewards(); + stRSR.payoutRewards(); } else { - main.furnace().melt(); + furnace.melt(); } } } @@ -176,7 +176,7 @@ contract DistributorP1 is ComponentP1, IDistributor { function _setDistribution(address dest, RevenueShare memory share) internal { require(dest != address(0), "dest cannot be zero"); require( - dest != furnace && dest != stRSR, + dest != address(furnace) && dest != address(stRSR), "destination can not be furnace or strsr directly" ); if (dest == FURNACE) require(share.rsrDist == 0, "Furnace must get 0% of RSR"); @@ -205,8 +205,8 @@ contract DistributorP1 is ComponentP1, IDistributor { function cacheComponents() public { rsr = main.rsr(); rToken = IERC20(address(main.rToken())); - furnace = address(main.furnace()); - stRSR = address(main.stRSR()); + furnace = main.furnace(); + stRSR = main.stRSR(); rTokenTrader = address(main.rTokenTrader()); rsrTrader = address(main.rsrTrader()); } From a617468b406cde61fdf773e6747953a2bdcbb4dc Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Fri, 8 Dec 2023 21:08:05 -0500 Subject: [PATCH 20/20] CHANGELOG --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85e47bec94..3f2edd990b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ This release gives new RTokens being deployed the option to enable a variable ta ### Upgrade Steps -Upgrade BasketHandler +Upgrade BasketHandler and Distributor + +Call `Distributor.cacheComponents()` if this is the first upgrade to a >=3.0.0 token. ### Core Protocol Contracts @@ -17,6 +19,8 @@ New governance param added to `DeploymentParams`: `reweightable` - Add immutable-after-init `reweightable` bool - `Deployer` - New boolean field `reweightable` added to `IDeployer.DeploymentParams` +- `Distributor` + - Minor gas-optimization # 3.1.0 - Unreleased