diff --git a/contracts/p1/AssetRegistry.sol b/contracts/p1/AssetRegistry.sol index 82e1911c0..8398654e7 100644 --- a/contracts/p1/AssetRegistry.sol +++ b/contracts/p1/AssetRegistry.sol @@ -47,7 +47,7 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry { uint256 length = assets_.length; for (uint256 i = 0; i < length; ++i) { - _register(assets_[i]); + _register(assets_[i], assets_[i].erc20()); } } @@ -70,6 +70,8 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry { } /// Register a new JIT-deployed RTokenAsset instance + /// NOT parallel with register()/swapRegistered(): + /// Can be used for initial registration OR rotation /// @param maxTradeVolume {UoA} The maximum trade volume for the RTokenAsset /// @return swapped If the asset was swapped for a previously-registered asset /// @custom:governance @@ -78,7 +80,10 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry { governance returns (bool swapped) { - swapped = _registerIgnoringCollisions(new RTokenAsset(main.rToken(), maxTradeVolume)); + IRToken rToken = main.rToken(); + RTokenAsset asset = new RTokenAsset(rToken, maxTradeVolume); + + swapped = _registerIgnoringCollisions(asset, IERC20Metadata(address(rToken))); } /// Register `asset` @@ -89,8 +94,9 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry { // effects: assets' = assets.set(asset.erc20(), asset) // returns: (asset.erc20 not in keys(assets)) function register(IAsset asset) external governance returns (bool) { - require(address(asset.erc20()) != address(main.rToken()), "cannot register RToken"); - return _register(asset); + IERC20Metadata erc20 = asset.erc20(); + + return _register(asset, erc20); } /// Register `asset` if and only if its erc20 address is already registered. @@ -102,16 +108,18 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry { // effects: assets' = assets + {asset.erc20(): asset} // actions: if asset.erc20() is in basketHandler's basket then basketHandler.disableBasket() function swapRegistered(IAsset asset) external governance returns (bool swapped) { - require(address(asset.erc20()) != address(main.rToken()), "cannot swap RToken"); - require(_erc20s.contains(address(asset.erc20())), "no ERC20 collision"); + IERC20Metadata erc20 = asset.erc20(); + + require(address(erc20) != address(main.rToken()), "cannot swap RToken"); + require(_erc20s.contains(address(erc20)), "no ERC20 collision"); - try basketHandler.quantity{ gas: _reserveGas() }(asset.erc20()) returns (uint192 quantity) { + try basketHandler.quantity{ gas: _reserveGas() }(erc20) returns (uint192 quantity) { if (quantity != 0) basketHandler.disableBasket(); // not an interaction } catch { basketHandler.disableBasket(); } - swapped = _registerIgnoringCollisions(asset); + swapped = _registerIgnoringCollisions(asset, erc20); } /// Unregister an asset, requiring that it is already registered @@ -121,19 +129,21 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry { // checks: assets[asset.erc20()] == asset // effects: assets' = assets - {asset.erc20():_} + {asset.erc20(), asset} function unregister(IAsset asset) external governance { - require(address(asset.erc20()) != address(main.rToken()), "cannot unregister RToken"); - require(_erc20s.contains(address(asset.erc20())), "no asset to unregister"); - require(assets[asset.erc20()] == asset, "asset not found"); + IERC20Metadata erc20 = asset.erc20(); + + require(address(erc20) != address(main.rToken()), "cannot unregister RToken"); + require(_erc20s.contains(address(erc20)), "no asset to unregister"); + require(assets[erc20] == asset, "asset not found"); - try basketHandler.quantity{ gas: _reserveGas() }(asset.erc20()) returns (uint192 quantity) { + try basketHandler.quantity{ gas: _reserveGas() }(erc20) returns (uint192 quantity) { if (quantity != 0) basketHandler.disableBasket(); // not an interaction } catch { basketHandler.disableBasket(); } - _erc20s.remove(address(asset.erc20())); - assets[asset.erc20()] = IAsset(address(0)); - emit AssetUnregistered(asset.erc20(), asset); + _erc20s.remove(address(erc20)); + assets[erc20] = IAsset(address(0)); + emit AssetUnregistered(erc20, asset); } /// Return the Asset registered for erc20; revert if erc20 is not registered. @@ -218,19 +228,23 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry { // checks: (asset.erc20() not in assets) or (assets[asset.erc20()] == asset) // effects: assets' = assets.set(asset.erc20(), asset) // returns: assets.erc20() not in assets - function _register(IAsset asset) internal returns (bool registered) { + function _register(IAsset asset, IERC20Metadata erc20) internal returns (bool registered) { + require(address(erc20) != address(main.rToken()), "cannot register RToken"); require( - !_erc20s.contains(address(asset.erc20())) || assets[asset.erc20()] == asset, + !_erc20s.contains(address(erc20)) || assets[erc20] == asset, "duplicate ERC20 detected" ); - registered = _registerIgnoringCollisions(asset); + registered = _registerIgnoringCollisions(asset, erc20); } /// Register an asset, unregistering any previous asset with the same ERC20. // effects: assets' = assets.set(asset.erc20(), asset) // returns: assets[asset.erc20()] != asset - function _registerIgnoringCollisions(IAsset asset) private returns (bool swapped) { + function _registerIgnoringCollisions(IAsset asset, IERC20Metadata erc20) + private + returns (bool swapped) + { if (asset.isCollateral()) { require( ICollateral(address(asset)).status() == CollateralStatus.SOUND, @@ -238,20 +252,17 @@ contract AssetRegistryP1 is ComponentP1, IAssetRegistry { ); } - IERC20Metadata erc20 = asset.erc20(); - - if (address(erc20) != address(main.rToken())) { - AssetPluginRegistry assetPluginRegistry = main.assetPluginRegistry(); + AssetPluginRegistry assetPluginRegistry = main.assetPluginRegistry(); - if (address(assetPluginRegistry) != address(0)) { - require( + if (address(assetPluginRegistry) != address(0)) { + require( + address(erc20) == address(main.rToken()) || assetPluginRegistry.isValidAsset( keccak256(abi.encodePacked(this.version())), address(asset) ), - "unsupported asset" - ); - } + "unsupported asset" + ); } if (_erc20s.contains(address(erc20))) { diff --git a/test/Upgradeability.test.ts b/test/Upgradeability.test.ts index 34c6af81a..6dbacaab9 100644 --- a/test/Upgradeability.test.ts +++ b/test/Upgradeability.test.ts @@ -216,7 +216,7 @@ describeP1(`Upgradeability - P${IMPLEMENTATION}`, () => { it('Should deploy valid implementation - AssetRegistry', async () => { const newAssetRegistry: AssetRegistryP1 = await upgrades.deployProxy( AssetRegistryFactory, - [main.address, [rsrAsset.address, rTokenAsset.address]], + [main.address, [rsrAsset.address]], { initializer: 'init', kind: 'uups', @@ -226,7 +226,7 @@ describeP1(`Upgradeability - P${IMPLEMENTATION}`, () => { await newAssetRegistry.deployed() expect(await newAssetRegistry.isRegistered(rsr.address)).to.equal(true) - expect(await newAssetRegistry.isRegistered(rToken.address)).to.equal(true) + expect(await newAssetRegistry.isRegistered(rToken.address)).to.equal(false) expect(await newAssetRegistry.main()).to.equal(main.address) })