From 3e562fef68dd7a20feb16ef7e43c87a5f2866d8b Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Mon, 19 Jun 2023 14:43:49 -0300 Subject: [PATCH] registry: implement stateless property --- packages/deployer/contracts/Deployer.sol | 16 ++-- packages/deployer/test/Deployer.test.ts | 12 +-- packages/registry/contracts/Registry.sol | 19 ++-- .../contracts/interfaces/IRegistry.sol | 14 ++- packages/registry/test/Registry.test.ts | 86 ++++++++++++++----- packages/smart-vault/contracts/SmartVault.sol | 8 +- packages/smart-vault/test/SmartVaults.test.ts | 40 ++++++--- packages/tasks/test/BaseTask.test.ts | 2 +- 8 files changed, 139 insertions(+), 58 deletions(-) diff --git a/packages/deployer/contracts/Deployer.sol b/packages/deployer/contracts/Deployer.sol index da4c6bed..88682d62 100644 --- a/packages/deployer/contracts/Deployer.sol +++ b/packages/deployer/contracts/Deployer.sol @@ -76,10 +76,12 @@ contract Deployer { /** * @dev Task params + * @param custom Whether the implementation is custom or not, if it is it won't be checked with Mimic's Registry * @param impl Address of the task implementation to be used * @param initializeData Call-data to initialize the new task instance */ struct TaskParams { + bool custom; address impl; bytes initializeData; } @@ -102,7 +104,7 @@ contract Deployer { * @dev Deploys a new authorizer instance */ function deployAuthorizer(string memory namespace, string memory name, AuthorizerParams memory params) external { - address instance = _deployClone(namespace, name, params.impl); + address instance = _deployClone(namespace, name, params.impl, true); Authorizer(instance).initialize(params.owners); emit AuthorizerDeployed(namespace, name, instance, params.impl); } @@ -111,7 +113,7 @@ contract Deployer { * @dev Deploys a new smart vault instance */ function deploySmartVault(string memory namespace, string memory name, SmartVaultParams memory params) external { - address payable instance = payable(_deployClone(namespace, name, params.impl)); + address payable instance = payable(_deployClone(namespace, name, params.impl, true)); SmartVault(instance).initialize(params.authorizer, params.priceOracle, params.priceFeedParams); emit SmartVaultDeployed(namespace, name, instance, params.impl); } @@ -120,7 +122,7 @@ contract Deployer { * @dev Deploys a new task instance */ function deployTask(string memory namespace, string memory name, TaskParams memory params) external { - address instance = _deployClone(namespace, name, params.impl); + address instance = _deployClone(namespace, name, params.impl, !params.custom); if (params.initializeData.length > 0) instance.functionCall(params.initializeData, 'DEPLOYER_TASK_INIT_FAILED'); emit TaskDeployed(namespace, name, instance, params.impl); } @@ -128,12 +130,14 @@ contract Deployer { /** * @dev Deploys a new clone using CREATE3 */ - function _deployClone(string memory namespace, string memory name, address implementation) + function _deployClone(string memory namespace, string memory name, address implementation, bool check) internal returns (address) { - require(registry.isRegistered(implementation), 'DEPLOYER_IMPL_NOT_REGISTERED'); - require(!registry.isDeprecated(implementation), 'DEPLOYER_IMPL_DEPRECATED'); + if (check) { + require(registry.isRegistered(implementation), 'DEPLOYER_IMPL_NOT_REGISTERED'); + require(!registry.isDeprecated(implementation), 'DEPLOYER_IMPL_DEPRECATED'); + } bytes memory bytecode = abi.encodePacked( hex'3d602d80600a3d3981f3363d3d373d3d3d363d73', diff --git a/packages/deployer/test/Deployer.test.ts b/packages/deployer/test/Deployer.test.ts index 38461113..90bb8788 100644 --- a/packages/deployer/test/Deployer.test.ts +++ b/packages/deployer/test/Deployer.test.ts @@ -41,7 +41,7 @@ describe('Deployer', () => { context('when the implementation is registered', () => { beforeEach('register implementation', async () => { - await registry.connect(mimic).register('authorizer@0.0.1', authorizer.address) + await registry.connect(mimic).register('authorizer@0.0.1', authorizer.address, false) }) context('when the implementation is not deprecated', () => { @@ -172,8 +172,8 @@ describe('Deployer', () => { context('when the implementation is registered', () => { beforeEach('register implementations', async () => { - await registry.connect(mimic).register('smart-vault@0.0.1', smartVault.address) - await registry.connect(mimic).register('price-oracle@0.0.1', PRICE_ORACLE) + await registry.connect(mimic).register('smart-vault@0.0.1', smartVault.address, false) + await registry.connect(mimic).register('price-oracle@0.0.1', PRICE_ORACLE, true) }) context('when the implementation is not deprecated', () => { @@ -292,10 +292,10 @@ describe('Deployer', () => { const FEE_CONTROLLER = '0x0000000000000000000000000000000000000001' const WNT = '0x0000000000000000000000000000000000000002' const smartVaultImpl = await deploy(ARTIFACTS.SMART_VAULT, [registry.address, FEE_CONTROLLER, WNT]) - await registry.connect(mimic).register('smart-vault@0.0.1', smartVaultImpl.address) + await registry.connect(mimic).register('smart-vault@0.0.1', smartVaultImpl.address, false) const PRICE_ORACLE = '0x0000000000000000000000000000000000000004' - await registry.connect(mimic).register('price-oracle@0.0.1', PRICE_ORACLE) + await registry.connect(mimic).register('price-oracle@0.0.1', PRICE_ORACLE, true) const tx = await deployer.deploySmartVault(namespace, 'smart-vault', { impl: smartVaultImpl.address, @@ -316,7 +316,7 @@ describe('Deployer', () => { context('when the implementation is registered', () => { beforeEach('register implementations', async () => { - await registry.connect(mimic).register('task-mock@0.0.1', task.address) + await registry.connect(mimic).register('task-mock@0.0.1', task.address, false) }) context('when the implementation is not deprecated', () => { diff --git a/packages/registry/contracts/Registry.sol b/packages/registry/contracts/Registry.sol index 295bd2a8..db28dd34 100644 --- a/packages/registry/contracts/Registry.sol +++ b/packages/registry/contracts/Registry.sol @@ -27,6 +27,9 @@ contract Registry is IRegistry, Ownable { // List of registered implementations mapping (address => bool) public override isRegistered; + // List of stateless implementations + mapping (address => bool) public override isStateless; + // List of deprecated implementations mapping (address => bool) public override isDeprecated; @@ -42,19 +45,21 @@ contract Registry is IRegistry, Ownable { * @dev Creates and registers an implementation * @param name Name of the implementation * @param code Code of the implementation to create and register + * @param stateless Whether the new implementation is considered stateless or not */ - function create(string memory name, bytes memory code) external override onlyOwner { + function create(string memory name, bytes memory code, bool stateless) external override onlyOwner { address implementation = CREATE3.deploy(keccak256(abi.encode(name)), code, 0); - _register(name, implementation); + _register(name, implementation, stateless); } /** * @dev Registers an implementation * @param name Name logged for the implementation * @param implementation Address of the implementation to be registered + * @param stateless Whether the given implementation is considered stateless or not */ - function register(string memory name, address implementation) external override onlyOwner { - _register(name, implementation); + function register(string memory name, address implementation, bool stateless) external override onlyOwner { + _register(name, implementation, stateless); } /** @@ -74,12 +79,14 @@ contract Registry is IRegistry, Ownable { * @dev Registers an implementation * @param name Name of the implementation * @param implementation Address of the implementation to be registered + * @param stateless Whether the given implementation is considered stateless or not */ - function _register(string memory name, address implementation) internal { + function _register(string memory name, address implementation, bool stateless) internal { require(implementation != address(0), 'REGISTRY_IMPL_ADDRESS_ZERO'); require(!isRegistered[implementation], 'REGISTRY_IMPL_ALREADY_REGISTERED'); isRegistered[implementation] = true; - emit Registered(implementation, name); + isStateless[implementation] = stateless; + emit Registered(implementation, name, stateless); } } diff --git a/packages/registry/contracts/interfaces/IRegistry.sol b/packages/registry/contracts/interfaces/IRegistry.sol index 4cbe028f..7ef5ae04 100644 --- a/packages/registry/contracts/interfaces/IRegistry.sol +++ b/packages/registry/contracts/interfaces/IRegistry.sol @@ -25,7 +25,7 @@ interface IRegistry { /** * @dev Emitted every time an implementation is registered */ - event Registered(address indexed implementation, string name); + event Registered(address indexed implementation, string name, bool stateless); /** * @dev Emitted every time an implementation is deprecated @@ -38,6 +38,12 @@ interface IRegistry { */ function isRegistered(address implementation) external view returns (bool); + /** + * @dev Tells whether an implementation is stateless or not + * @param implementation Address of the implementation being queried + */ + function isStateless(address implementation) external view returns (bool); + /** * @dev Tells whether an implementation is deprecated * @param implementation Address of the implementation being queried @@ -48,15 +54,17 @@ interface IRegistry { * @dev Creates and registers an implementation * @param name Name of the implementation * @param code Code of the implementation to create and register + * @param stateless Whether the new implementation is considered stateless or not */ - function create(string memory name, bytes memory code) external; + function create(string memory name, bytes memory code, bool stateless) external; /** * @dev Registers an implementation * @param name Name of the implementation * @param implementation Address of the implementation to be registered + * @param stateless Whether the given implementation is considered stateless or not */ - function register(string memory name, address implementation) external; + function register(string memory name, address implementation, bool stateless) external; /** * @dev Deprecates an implementation diff --git a/packages/registry/test/Registry.test.ts b/packages/registry/test/Registry.test.ts index d13c816b..ec3a94e9 100644 --- a/packages/registry/test/Registry.test.ts +++ b/packages/registry/test/Registry.test.ts @@ -44,35 +44,57 @@ describe('Registry', () => { context('when the requested implementation was not created', () => { context('when the requested implementation is not registered', () => { - it('creates and registers the requested implementation', async () => { - const tx = await registry.create(name, bytecode) + context('when the requested implementation is considered stateless', () => { + const stateless = true - const event = await assertEvent(tx, 'Registered', { name }) + it('creates and registers the requested implementation', async () => { + const tx = await registry.create(name, bytecode, stateless) - const implementation = event.args.implementation - expect(await registry.isRegistered(implementation)).to.be.true - expect(await registry.isDeprecated(implementation)).to.be.false + const event = await assertEvent(tx, 'Registered', { name, stateless }) + + const implementation = event.args.implementation + expect(await registry.isRegistered(implementation)).to.be.true + expect(await registry.isStateless(implementation)).to.be.true + expect(await registry.isDeprecated(implementation)).to.be.false + }) + }) + + context('when the requested implementation is not considered stateless', () => { + const stateless = false + + it('creates and registers the requested implementation', async () => { + const tx = await registry.create(name, bytecode, stateless) + + const event = await assertEvent(tx, 'Registered', { name, stateless }) + + const implementation = event.args.implementation + expect(await registry.isRegistered(implementation)).to.be.true + expect(await registry.isStateless(implementation)).to.be.false + expect(await registry.isDeprecated(implementation)).to.be.false + }) }) }) context('when the requested implementation is registered', () => { beforeEach('register', async () => { - await registry.register(name, implementation) + await registry.register(name, implementation, true) }) it('reverts', async () => { - await expect(registry.register(name, implementation)).to.be.revertedWith('REGISTRY_IMPL_ALREADY_REGISTERED') + await expect(registry.register(name, implementation, true)).to.be.revertedWith( + 'REGISTRY_IMPL_ALREADY_REGISTERED' + ) }) }) }) context('when the requested implementation was created', () => { beforeEach('create', async () => { - await registry.create(name, bytecode) + await registry.create(name, bytecode, true) }) it('reverts', async () => { - await expect(registry.create(name, bytecode)).to.be.revertedWith('DEPLOYMENT_FAILED') + await expect(registry.create(name, bytecode, true)).to.be.revertedWith('DEPLOYMENT_FAILED') }) }) }) @@ -83,7 +105,7 @@ describe('Registry', () => { }) it('reverts', async () => { - await expect(registry.create(name, implementation)).to.be.revertedWith('Ownable: caller is not the owner') + await expect(registry.create(name, implementation, true)).to.be.revertedWith('Ownable: caller is not the owner') }) }) }) @@ -97,27 +119,44 @@ describe('Registry', () => { }) context('when the requested implementation is not registered', () => { - it('registers the requested implementation', async () => { - await registry.register(name, implementation) + context('when the requested implementation is considered stateless', () => { + const stateless = true + + it('registers the requested implementation', async () => { + const tx = await registry.register(name, implementation, stateless) - expect(await registry.isRegistered(implementation)).to.be.true - expect(await registry.isDeprecated(implementation)).to.be.false + await assertEvent(tx, 'Registered', { name, implementation, stateless }) + + expect(await registry.isRegistered(implementation)).to.be.true + expect(await registry.isStateless(implementation)).to.be.true + expect(await registry.isDeprecated(implementation)).to.be.false + }) }) - it('emits an event', async () => { - const tx = await registry.register(name, implementation) + context('when the requested implementation is not considered stateless', () => { + const stateless = false + + it('registers the requested implementation', async () => { + const tx = await registry.register(name, implementation, stateless) + + await assertEvent(tx, 'Registered', { name, implementation, stateless }) - await assertEvent(tx, 'Registered', { name, implementation }) + expect(await registry.isRegistered(implementation)).to.be.true + expect(await registry.isStateless(implementation)).to.be.false + expect(await registry.isDeprecated(implementation)).to.be.false + }) }) }) context('when the requested implementation is registered', () => { beforeEach('register', async () => { - await registry.register(name, implementation) + await registry.register(name, implementation, true) }) it('reverts', async () => { - await expect(registry.register(name, implementation)).to.be.revertedWith('REGISTRY_IMPL_ALREADY_REGISTERED') + await expect(registry.register(name, implementation, true)).to.be.revertedWith( + 'REGISTRY_IMPL_ALREADY_REGISTERED' + ) }) }) }) @@ -128,7 +167,9 @@ describe('Registry', () => { }) it('reverts', async () => { - await expect(registry.register(name, implementation)).to.be.revertedWith('Ownable: caller is not the owner') + await expect(registry.register(name, implementation, true)).to.be.revertedWith( + 'Ownable: caller is not the owner' + ) }) }) }) @@ -147,7 +188,7 @@ describe('Registry', () => { context('when the requested implementation is registered', () => { beforeEach('register', async () => { - await registry.register('implementation@0.0.1', implementation) + await registry.register('implementation@0.0.1', implementation, true) }) context('when the requested implementation is not deprecated', () => { @@ -155,6 +196,7 @@ describe('Registry', () => { await registry.deprecate(implementation) expect(await registry.isRegistered(implementation)).to.be.true + expect(await registry.isStateless(implementation)).to.be.true expect(await registry.isDeprecated(implementation)).to.be.true }) diff --git a/packages/smart-vault/contracts/SmartVault.sol b/packages/smart-vault/contracts/SmartVault.sol index 62a9d472..0679745a 100644 --- a/packages/smart-vault/contracts/SmartVault.sol +++ b/packages/smart-vault/contracts/SmartVault.sol @@ -158,7 +158,7 @@ contract SmartVault is ISmartVault, Authorized, ReentrancyGuardUpgradeable { authP(authParams(connector)) returns (bytes memory result) { - _validateDependency(connector); + _validateDependency(connector, true); result = Address.functionDelegateCall(connector, data, 'SMART_VAULT_EXECUTE_FAILED'); emit Executed(connector, data, result); } @@ -260,7 +260,7 @@ contract SmartVault is ISmartVault, Authorized, ReentrancyGuardUpgradeable { */ function _setPriceOracle(address newPriceOracle) internal { require(newPriceOracle != address(0), 'SMART_VAULT_ORACLE_ZERO'); - _validateDependency(newPriceOracle); + _validateDependency(newPriceOracle, true); priceOracle = newPriceOracle; emit PriceOracleSet(newPriceOracle); } @@ -279,10 +279,12 @@ contract SmartVault is ISmartVault, Authorized, ReentrancyGuardUpgradeable { /** * @dev Validates a dependency against the Mimic Registry * @param dependency Address of the dependency to validate + * @param stateless Whether the given dependency must be stateless or not */ - function _validateDependency(address dependency) private view { + function _validateDependency(address dependency, bool stateless) private view { if (isDependencyCheckIgnored[dependency]) return; require(IRegistry(registry).isRegistered(dependency), 'SMART_VAULT_DEP_NOT_REGISTERED'); + require(IRegistry(registry).isStateless(dependency) == stateless, 'SMART_VAULT_DEP_BAD_STATE_COND'); require(!IRegistry(registry).isDeprecated(dependency), 'SMART_VAULT_DEP_DEPRECATED'); } } diff --git a/packages/smart-vault/test/SmartVaults.test.ts b/packages/smart-vault/test/SmartVaults.test.ts index 9e56d56d..23551b18 100644 --- a/packages/smart-vault/test/SmartVaults.test.ts +++ b/packages/smart-vault/test/SmartVaults.test.ts @@ -38,7 +38,7 @@ describe('SmartVault', () => { priceOracle = await deploy('@mimic-fi/v3-price-oracle/artifacts/contracts/PriceOracle.sol/PriceOracle', [ wrappedNT.address, ]) - await registry.connect(mimic).register('price-oracle@0.0.1', priceOracle.address) + await registry.connect(mimic).register('price-oracle@0.0.1', priceOracle.address, true) }) beforeEach('create smart vault', async () => { @@ -112,7 +112,7 @@ describe('SmartVault', () => { context('when the implementation is registered', async () => { beforeEach('deploy implementation', async () => { - await registry.connect(mimic).register('price-oracle@0.0.2', newPriceOracle.address) + await registry.connect(mimic).register('price-oracle@0.0.2', newPriceOracle.address, true) }) context('when the implementation is not deprecated', async () => { @@ -327,21 +327,39 @@ describe('SmartVault', () => { } context('when the connector is registered', async () => { - beforeEach('deploy connector', async () => { - await registry.connect(mimic).register('connector@0.0.1', connector.address) - }) + context('when the connector is stateless', async () => { + const stateless = true - context('when the connector is not deprecated', async () => { - itExecutesTheConnector() + beforeEach('deploy connector', async () => { + await registry.connect(mimic).register('connector@0.0.1', connector.address, stateless) + }) + + context('when the connector is not deprecated', async () => { + itExecutesTheConnector() + }) + + context('when the connector is deprecated', async () => { + beforeEach('deprecate connector', async () => { + await registry.connect(mimic).deprecate(connector.address) + }) + + it('reverts', async () => { + await expect(smartVault.execute(connector.address, data)).to.be.revertedWith('SMART_VAULT_DEP_DEPRECATED') + }) + }) }) - context('when the connector is deprecated', async () => { - beforeEach('deprecate connector', async () => { - await registry.connect(mimic).deprecate(connector.address) + context('when the connector is stateful', async () => { + const stateless = false + + beforeEach('deploy connector', async () => { + await registry.connect(mimic).register('connector@0.0.1', connector.address, stateless) }) it('reverts', async () => { - await expect(smartVault.execute(connector.address, data)).to.be.revertedWith('SMART_VAULT_DEP_DEPRECATED') + await expect(smartVault.execute(connector.address, data)).to.be.revertedWith( + 'SMART_VAULT_DEP_BAD_STATE_COND' + ) }) }) }) diff --git a/packages/tasks/test/BaseTask.test.ts b/packages/tasks/test/BaseTask.test.ts index d70b6354..e182b930 100644 --- a/packages/tasks/test/BaseTask.test.ts +++ b/packages/tasks/test/BaseTask.test.ts @@ -24,7 +24,7 @@ describe('BaseTask', () => { priceOracle = await deploy('@mimic-fi/v3-price-oracle/artifacts/contracts/PriceOracle.sol/PriceOracle', [ wrappedNT.address, ]) - await registry.connect(mimic).register('price-oracle@0.0.1', priceOracle.address) + await registry.connect(mimic).register('price-oracle@0.0.1', priceOracle.address, true) }) beforeEach('create smart vault', async () => {