From 557642b711815c2ac1e1afa3874a05e8a295ba69 Mon Sep 17 00:00:00 2001 From: Artem Chystiakov Date: Tue, 8 Aug 2023 15:39:55 +0300 Subject: [PATCH] minor improvements --- .../AbstractContractsRegistry.sol | 21 ++++++++++++-- .../contracts-registry/AbstractDependant.sol | 2 +- .../pools/AbstractPoolContractsRegistry.sol | 14 +++++++-- .../pool-factory/AbstractPoolFactory.sol | 13 +++++---- .../presets/OwnableContractsRegistry.sol | 8 +++++ .../libs/data-structures/memory/Vector.sol | 29 +++++++++++++++++-- .../mock/contracts-registry/CRDependant.sol | 5 +--- .../mock/contracts-registry/pools/Pool.sol | 5 +--- .../pools/PoolContractsRegistry.sol | 2 +- .../pools/pool-factory/PoolFactory.sol | 2 +- .../data-structures/memory/VectorMock.sol | 25 ++++++++++++++++ contracts/package.json | 2 +- package.json | 2 +- .../ContractsRegistry.test.js | 7 ++++- .../pools/PoolContractsRegistry.test.js | 1 + ...eTree.js => IncrementalMerkleTree.test.js} | 0 .../data-structures/memory/Vector.test.js | 4 +++ 17 files changed, 116 insertions(+), 26 deletions(-) rename test/libs/data-structures/{IncrementalMerkleTree.js => IncrementalMerkleTree.test.js} (100%) diff --git a/contracts/contracts-registry/AbstractContractsRegistry.sol b/contracts/contracts-registry/AbstractContractsRegistry.sol index c0f3ac71..dd874e78 100644 --- a/contracts/contracts-registry/AbstractContractsRegistry.sol +++ b/contracts/contracts-registry/AbstractContractsRegistry.sol @@ -13,12 +13,12 @@ import {AbstractDependant} from "./AbstractDependant.sol"; * The purpose of this module is to provide an organized registry of the project's smartcontracts * together with the upgradeability and dependency injection mechanisms. * - * The ContractsRegistry should be used as the highest level smartcontract that is aware of any other + * The ContractsRegistry should be used as the highest level smart contract that is aware of any other * contract present in the system. The contracts that demand other system's contracts would then inherit * special `AbstractDependant` contract and override `setDependencies()` function to enable ContractsRegistry * to inject dependencies into them. * - * The ContractsRegistry will help with the following usecases: + * The ContractsRegistry will help with the following use cases: * * 1) Making the system upgradeable * 2) Making the system contracts-interchangeable @@ -174,10 +174,25 @@ abstract contract AbstractContractsRegistry is Initializable { * @param contractAddress_ the address of the implementation */ function _addProxyContract(string memory name_, address contractAddress_) internal { + _addProxyContractAndCall(name_, contractAddress_, bytes("")); + } + + /** + * @notice The function to add the contracts and deploy the proxy above them. It should be used to add + * contract that the ContractsRegistry should be able to upgrade + * @param name_ the name to associate the contract with + * @param contractAddress_ the address of the implementation + * @param data_ the additional proxy initialization data + */ + function _addProxyContractAndCall( + string memory name_, + address contractAddress_, + bytes memory data_ + ) internal { require(contractAddress_ != address(0), "ContractsRegistry: zero address is forbidden"); address proxyAddr_ = address( - new TransparentUpgradeableProxy(contractAddress_, address(_proxyUpgrader), bytes("")) + new TransparentUpgradeableProxy(contractAddress_, address(_proxyUpgrader), data_) ); _contracts[name_] = proxyAddr_; diff --git a/contracts/contracts-registry/AbstractDependant.sol b/contracts/contracts-registry/AbstractDependant.sol index b2cdeb3a..cd94e46a 100644 --- a/contracts/contracts-registry/AbstractDependant.sol +++ b/contracts/contracts-registry/AbstractDependant.sol @@ -33,7 +33,7 @@ abstract contract AbstractDependant { * @param contractsRegistry_ the registry to pull dependencies from * @param data_ the extra data that might provide additional context */ - function setDependencies(address contractsRegistry_, bytes calldata data_) external virtual; + function setDependencies(address contractsRegistry_, bytes memory data_) public virtual; /** * @notice The function is made external to allow for the factories to set the injector to the ContractsRegistry diff --git a/contracts/contracts-registry/pools/AbstractPoolContractsRegistry.sol b/contracts/contracts-registry/pools/AbstractPoolContractsRegistry.sol index 3be1d997..d6e18dd2 100644 --- a/contracts/contracts-registry/pools/AbstractPoolContractsRegistry.sol +++ b/contracts/contracts-registry/pools/AbstractPoolContractsRegistry.sol @@ -23,7 +23,7 @@ import {ProxyBeacon} from "./proxy/ProxyBeacon.sol"; * The users of this module have to override `_onlyPoolFactory()` method and revert in case a wrong msg.sender is * trying to add pools into the registry. * - * The contract is ment to be used behind a proxy itself. + * The contract is meant to be used behind a proxy itself. */ abstract contract AbstractPoolContractsRegistry is Initializable, AbstractDependant { using EnumerableSet for EnumerableSet.AddressSet; @@ -46,7 +46,7 @@ abstract contract AbstractPoolContractsRegistry is Initializable, AbstractDepend */ function setDependencies( address contractsRegistry_, - bytes calldata + bytes memory ) public virtual override dependant { _contractsRegistry = contractsRegistry_; } @@ -78,6 +78,16 @@ abstract contract AbstractPoolContractsRegistry is Initializable, AbstractDepend return beacon_; } + /** + * @notice The function to check if the address is a pool + * @param name_ the associated pools name + * @param pool_ the address to check + * @return true if pool_ is whithing the registry + */ + function isPool(string memory name_, address pool_) public view returns (bool) { + return _pools[name_].contains(pool_); + } + /** * @notice The function to count pools by specified name * @param name_ the associated pools name diff --git a/contracts/contracts-registry/pools/pool-factory/AbstractPoolFactory.sol b/contracts/contracts-registry/pools/pool-factory/AbstractPoolFactory.sol index 6c735df6..8b2eb300 100644 --- a/contracts/contracts-registry/pools/pool-factory/AbstractPoolFactory.sol +++ b/contracts/contracts-registry/pools/pool-factory/AbstractPoolFactory.sol @@ -26,7 +26,7 @@ abstract contract AbstractPoolFactory is AbstractDependant { */ function setDependencies( address contractsRegistry_, - bytes calldata + bytes memory ) public virtual override dependant { _contractsRegistry = contractsRegistry_; } @@ -35,7 +35,10 @@ abstract contract AbstractPoolFactory is AbstractDependant { * @notice The internal deploy function that deploys BeaconProxy pointing to the * pool implementation taken from the PoolContractRegistry */ - function _deploy(address poolRegistry_, string memory poolType_) internal returns (address) { + function _deploy( + address poolRegistry_, + string memory poolType_ + ) internal virtual returns (address) { return address( new PublicBeaconProxy( @@ -53,7 +56,7 @@ abstract contract AbstractPoolFactory is AbstractDependant { address poolRegistry_, string memory poolType_, bytes32 salt_ - ) internal returns (address) { + ) internal virtual returns (address) { return address( new PublicBeaconProxy{salt: salt_}( @@ -82,7 +85,7 @@ abstract contract AbstractPoolFactory is AbstractDependant { * @notice The function that injects dependencies to the newly deployed pool and sets * provided PoolContractsRegistry as an injector */ - function _injectDependencies(address poolRegistry_, address proxy_) internal { + function _injectDependencies(address poolRegistry_, address proxy_) internal virtual { AbstractDependant(proxy_).setDependencies(_contractsRegistry, bytes("")); AbstractDependant(proxy_).setInjector(poolRegistry_); } @@ -94,7 +97,7 @@ abstract contract AbstractPoolFactory is AbstractDependant { address poolRegistry_, string memory poolType_, bytes32 salt_ - ) internal view returns (address) { + ) internal view virtual returns (address) { bytes32 bytecodeHash = keccak256( abi.encodePacked( type(PublicBeaconProxy).creationCode, diff --git a/contracts/contracts-registry/presets/OwnableContractsRegistry.sol b/contracts/contracts-registry/presets/OwnableContractsRegistry.sol index eeb01520..6aa15b3f 100644 --- a/contracts/contracts-registry/presets/OwnableContractsRegistry.sol +++ b/contracts/contracts-registry/presets/OwnableContractsRegistry.sol @@ -48,6 +48,14 @@ contract OwnableContractsRegistry is AbstractContractsRegistry, OwnableUpgradeab _addProxyContract(name_, contractAddress_); } + function addProxyContractAndCall( + string calldata name_, + address contractAddress_, + bytes calldata data_ + ) external onlyOwner { + _addProxyContractAndCall(name_, contractAddress_, data_); + } + function justAddProxyContract( string calldata name_, address contractAddress_ diff --git a/contracts/libs/data-structures/memory/Vector.sol b/contracts/libs/data-structures/memory/Vector.sol index 6c55b5ed..51db645d 100644 --- a/contracts/libs/data-structures/memory/Vector.sol +++ b/contracts/libs/data-structures/memory/Vector.sol @@ -70,9 +70,18 @@ library Vector { } /** - * @notice The function to push new elements to the vector, amortized O(1) + * @notice The function to push new elements (as an array) to the vector, amortized O(n) * @param vector self - * @param value_ the new elements to add + * @param values_ the new elements to add + */ + function push(UintVector memory vector, uint256[] memory values_) internal pure { + _push(vector._vector, values_.asBytes32Array()); + } + + /** + * @notice The function to push a new element to the vector, amortized O(1) + * @param vector self + * @param value_ the new element to add */ function push(UintVector memory vector, uint256 value_) internal pure { _push(vector._vector, bytes32(value_)); @@ -151,6 +160,10 @@ library Vector { vector._vector = _new(array_); } + function push(Bytes32Vector memory vector, bytes32[] memory values_) internal pure { + _push(vector._vector, values_); + } + function push(Bytes32Vector memory vector, bytes32 value_) internal pure { _push(vector._vector, value_); } @@ -199,6 +212,10 @@ library Vector { vector._vector = _new(array_.asBytes32Array()); } + function push(AddressVector memory vector, address[] memory values_) internal pure { + _push(vector._vector, values_.asBytes32Array()); + } + function push(AddressVector memory vector, address value_) internal pure { _push(vector._vector, bytes32(uint256(uint160(value_)))); } @@ -264,6 +281,14 @@ library Vector { } } + function _push(Vector memory vector, bytes32[] memory values_) private pure { + uint256 length_ = values_.length; + + for (uint256 i = 0; i < length_; ++i) { + _push(vector, values_[i]); + } + } + function _push(Vector memory vector, bytes32 value_) private pure { uint256 length_ = _length(vector); diff --git a/contracts/mock/contracts-registry/CRDependant.sol b/contracts/mock/contracts-registry/CRDependant.sol index c9c5507e..3461aab2 100644 --- a/contracts/mock/contracts-registry/CRDependant.sol +++ b/contracts/mock/contracts-registry/CRDependant.sol @@ -8,10 +8,7 @@ import {ContractsRegistry1} from "./ContractsRegistry1.sol"; contract CRDependant is AbstractDependant { address public token; - function setDependencies( - address contractsRegistry_, - bytes calldata - ) external override dependant { + function setDependencies(address contractsRegistry_, bytes memory) public override dependant { token = ContractsRegistry1(contractsRegistry_).getTokenContract(); } } diff --git a/contracts/mock/contracts-registry/pools/Pool.sol b/contracts/mock/contracts-registry/pools/Pool.sol index 72ac5e49..69138423 100644 --- a/contracts/mock/contracts-registry/pools/Pool.sol +++ b/contracts/mock/contracts-registry/pools/Pool.sol @@ -8,10 +8,7 @@ import {ContractsRegistry2} from "./ContractsRegistry2.sol"; contract Pool is AbstractDependant { address public token; - function setDependencies( - address contractsRegistry_, - bytes calldata - ) external override dependant { + function setDependencies(address contractsRegistry_, bytes memory) public override dependant { token = ContractsRegistry2(contractsRegistry_).getTokenContract(); } } diff --git a/contracts/mock/contracts-registry/pools/PoolContractsRegistry.sol b/contracts/mock/contracts-registry/pools/PoolContractsRegistry.sol index 913cf9d2..8019bdc6 100644 --- a/contracts/mock/contracts-registry/pools/PoolContractsRegistry.sol +++ b/contracts/mock/contracts-registry/pools/PoolContractsRegistry.sol @@ -20,7 +20,7 @@ contract PoolContractsRegistry is OwnablePoolContractsRegistry { __PoolContractsRegistry_init(); } - function setDependencies(address contractsRegistry_, bytes calldata data_) public override { + function setDependencies(address contractsRegistry_, bytes memory data_) public override { super.setDependencies(contractsRegistry_, data_); _poolFactory = ContractsRegistry2(contractsRegistry_).getPoolFactoryContract(); diff --git a/contracts/mock/contracts-registry/pools/pool-factory/PoolFactory.sol b/contracts/mock/contracts-registry/pools/pool-factory/PoolFactory.sol index 7059838b..6812ca90 100644 --- a/contracts/mock/contracts-registry/pools/pool-factory/PoolFactory.sol +++ b/contracts/mock/contracts-registry/pools/pool-factory/PoolFactory.sol @@ -9,7 +9,7 @@ import {AbstractPoolFactory} from "../../../../contracts-registry/pools/pool-fac contract PoolFactory is AbstractPoolFactory { address public poolContractsRegistry; - function setDependencies(address contractsRegistry_, bytes calldata data_) public override { + function setDependencies(address contractsRegistry_, bytes memory data_) public override { super.setDependencies(contractsRegistry_, data_); poolContractsRegistry = ContractsRegistry2(contractsRegistry_) diff --git a/contracts/mock/libs/data-structures/memory/VectorMock.sol b/contracts/mock/libs/data-structures/memory/VectorMock.sol index 7ec93298..41c772d6 100644 --- a/contracts/mock/libs/data-structures/memory/VectorMock.sol +++ b/contracts/mock/libs/data-structures/memory/VectorMock.sol @@ -2,9 +2,11 @@ pragma solidity ^0.8.4; import {Vector} from "../../../../libs/data-structures/memory/Vector.sol"; +import {TypeCaster} from "../../../../libs/utils/TypeCaster.sol"; contract VectorMock { using Vector for *; + using TypeCaster for *; function testNew() external pure { assembly { @@ -42,6 +44,17 @@ contract VectorMock { } } + function testArrayPush() external pure { + Vector.UintVector memory vector_ = Vector.newUint(); + + require(vector_._vector._allocation == 5); + require(vector_.length() == 0); + + vector_.push([uint256(1), 2, 3].asDynamic()); + + require(vector_.length() == 3); + } + function testPushAndPop() external pure { Vector.UintVector memory vector_ = Vector.newUint(); @@ -171,6 +184,10 @@ contract VectorMock { require(vector_.length() == 5); require(vector_.at(vector_.length() - 1) == 1); + + vector_.push([uint256(1), 2, 3].asDynamic()); + + require(vector_.length() == 8); } function testBytes32Functionality() external pure { @@ -210,6 +227,10 @@ contract VectorMock { require(vector_.length() == 5); require(vector_.at(vector_.length() - 1) == bytes32(uint256(1))); + + vector_.push([bytes32(uint256(5)), bytes32(uint256(4)), bytes32(uint256(3))].asDynamic()); + + require(vector_.length() == 8); } function testAddressFunctionality() external pure { @@ -249,5 +270,9 @@ contract VectorMock { require(vector_.length() == 5); require(vector_.at(vector_.length() - 1) == address(1)); + + vector_.push([address(5), address(4), address(3)].asDynamic()); + + require(vector_.length() == 8); } } diff --git a/contracts/package.json b/contracts/package.json index 90c91f4c..ad9868e9 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -1,6 +1,6 @@ { "name": "@dlsl/dev-modules", - "version": "2.5.2", + "version": "2.5.3", "license": "MIT", "author": "Distributed Lab", "readme": "README.md", diff --git a/package.json b/package.json index 189b2078..70b771f8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dlsl", - "version": "2.5.2", + "version": "2.5.3", "license": "MIT", "author": "Distributed Lab", "description": "Solidity Development Modules by Distributed Lab", diff --git a/test/contracts-registry/ContractsRegistry.test.js b/test/contracts-registry/ContractsRegistry.test.js index d95deb19..93ffa5a4 100644 --- a/test/contracts-registry/ContractsRegistry.test.js +++ b/test/contracts-registry/ContractsRegistry.test.js @@ -70,6 +70,11 @@ describe("ContractsRegistry", () => { "Ownable: caller is not the owner" ); + await truffleAssert.reverts( + contractsRegistry.addProxyContractAndCall("", ZERO_ADDR, "0x", { from: SECOND }), + "Ownable: caller is not the owner" + ); + await truffleAssert.reverts( contractsRegistry.justAddProxyContract("", ZERO_ADDR, { from: SECOND }), "Ownable: caller is not the owner" @@ -142,7 +147,7 @@ describe("ContractsRegistry", () => { it("should add and remove proxy without dependant", async () => { const _token = await ERC20Mock.new("Mock", "Mock", 18); - await contractsRegistry.addProxyContract(await contractsRegistry.TOKEN_NAME(), _token.address); + await contractsRegistry.addProxyContractAndCall(await contractsRegistry.TOKEN_NAME(), _token.address, "0x"); await contractsRegistry.removeContract(await contractsRegistry.TOKEN_NAME()); }); diff --git a/test/contracts-registry/pools/PoolContractsRegistry.test.js b/test/contracts-registry/pools/PoolContractsRegistry.test.js index a93ba6ee..6c3724b0 100644 --- a/test/contracts-registry/pools/PoolContractsRegistry.test.js +++ b/test/contracts-registry/pools/PoolContractsRegistry.test.js @@ -118,6 +118,7 @@ describe("PoolContractsRegistry", () => { await poolContractsRegistry.addProxyPool(NAME_1, POOL_1); await poolContractsRegistry.addProxyPool(NAME_1, POOL_2); + assert.isTrue(await poolContractsRegistry.isPool(NAME_1, POOL_1)); assert.equal((await poolContractsRegistry.countPools(NAME_1)).toFixed(), "2"); assert.equal((await poolContractsRegistry.countPools(NAME_2)).toFixed(), "0"); }); diff --git a/test/libs/data-structures/IncrementalMerkleTree.js b/test/libs/data-structures/IncrementalMerkleTree.test.js similarity index 100% rename from test/libs/data-structures/IncrementalMerkleTree.js rename to test/libs/data-structures/IncrementalMerkleTree.test.js diff --git a/test/libs/data-structures/memory/Vector.test.js b/test/libs/data-structures/memory/Vector.test.js index ec0c9e0e..4e13299c 100644 --- a/test/libs/data-structures/memory/Vector.test.js +++ b/test/libs/data-structures/memory/Vector.test.js @@ -16,6 +16,10 @@ describe("Vector", () => { await vector.testNew(); }); + it("should test array push", async () => { + await vector.testArrayPush(); + }); + it("should test push and pop", async () => { await vector.testPushAndPop(); });