diff --git a/abi/Identity.json b/abi/Identity.json index 70942012..4bd45018 100644 --- a/abi/Identity.json +++ b/abi/Identity.json @@ -76,6 +76,24 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "uint72", + "name": "identityId", + "type": "uint72" + }, + { + "internalType": "address[]", + "name": "operationalWallets", + "type": "address[]" + } + ], + "name": "addOperationalWallets", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { diff --git a/abi/ParametersStorage.json b/abi/ParametersStorage.json index 8f209a13..4f515192 100644 --- a/abi/ParametersStorage.json +++ b/abi/ParametersStorage.json @@ -68,6 +68,19 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "hashFunctionsLimit", + "outputs": [ + { + "internalType": "uint16", + "name": "", + "type": "uint16" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "hub", @@ -146,6 +159,19 @@ "stateMutability": "pure", "type": "function" }, + { + "inputs": [], + "name": "opWalletsLimitOnProfileCreation", + "outputs": [ + { + "internalType": "uint16", + "name": "", + "type": "uint16" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "proofWindowDurationPerc", @@ -263,6 +289,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "uint16", + "name": "hashFunctionsLimit_", + "type": "uint16" + } + ], + "name": "setHashFunctionsLimit", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { @@ -315,6 +354,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "uint16", + "name": "opWalletsLimitOnProfileCreation_", + "type": "uint16" + } + ], + "name": "setOpWalletsLimitOnProfileCreation", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { diff --git a/abi/ProfileV2.json b/abi/ProfileV2.json index ca7ccda6..bece108b 100644 --- a/abi/ProfileV2.json +++ b/abi/ProfileV2.json @@ -118,6 +118,22 @@ "name": "SharesTokenSymbolAlreadyExists", "type": "error" }, + { + "inputs": [ + { + "internalType": "uint16", + "name": "allowed", + "type": "uint16" + }, + { + "internalType": "uint16", + "name": "provided", + "type": "uint16" + } + ], + "name": "TooManyOperationalWallets", + "type": "error" + }, { "inputs": [ { @@ -220,6 +236,11 @@ "name": "adminWallet", "type": "address" }, + { + "internalType": "address[]", + "name": "operationalWallets", + "type": "address[]" + }, { "internalType": "bytes", "name": "nodeId", diff --git a/contracts/v1/Identity.sol b/contracts/v1/Identity.sol index 083d301d..18895bde 100644 --- a/contracts/v1/Identity.sol +++ b/contracts/v1/Identity.sol @@ -14,7 +14,7 @@ contract Identity is Named, Versioned, ContractStatus, Initializable { event IdentityDeleted(uint72 indexed identityId); string private constant _NAME = "Identity"; - string private constant _VERSION = "1.0.1"; + string private constant _VERSION = "1.1.0"; IdentityStorage public identityStorage; @@ -76,13 +76,17 @@ contract Identity is Named, Versioned, ContractStatus, Initializable { IdentityStorage ids = identityStorage; + if (keyPurpose == OPERATIONAL_KEY) { + require(ids.identityIds(key) == 0, "Operational key is taken"); + + ids.setOperationalKeyIdentityId(key, identityId); + } + bytes32 attachedKey; (, , attachedKey) = ids.getKey(identityId, key); require(attachedKey != key, "Key is already attached"); ids.addKey(identityId, key, keyPurpose, keyType); - - if (keyPurpose == OPERATIONAL_KEY) ids.setOperationalKeyIdentityId(key, identityId); } function removeKey(uint72 identityId, bytes32 key) external onlyAdmin(identityId) { @@ -110,6 +114,31 @@ contract Identity is Named, Versioned, ContractStatus, Initializable { if (purpose == OPERATIONAL_KEY) ids.removeOperationalKeyIdentityId(key); } + function addOperationalWallets(uint72 identityId, address[] calldata operationalWallets) external onlyContracts { + IdentityStorage ids = identityStorage; + + bytes32 operationalKey; + bytes32 attachedKey; + + for (uint i; i < operationalWallets.length; ) { + operationalKey = keccak256(abi.encodePacked(operationalWallets[i])); + + require(operationalKey != bytes32(0), "Key arg is empty"); + require(ids.identityIds(operationalKey) == 0, "Operational key is taken"); + + ids.setOperationalKeyIdentityId(operationalKey, identityId); + + (, , attachedKey) = ids.getKey(identityId, operationalKey); + require(attachedKey != operationalKey, "Key is already attached"); + + ids.addKey(identityId, operationalKey, OPERATIONAL_KEY, ECDSA); + + unchecked { + i++; + } + } + } + function _checkAdmin(uint72 identityId) internal view virtual { require( identityStorage.keyHasPurpose(identityId, keccak256(abi.encodePacked(msg.sender)), ADMIN_KEY), diff --git a/contracts/v1/storage/ParametersStorage.sol b/contracts/v1/storage/ParametersStorage.sol index 00878ffa..79090d6b 100644 --- a/contracts/v1/storage/ParametersStorage.sol +++ b/contracts/v1/storage/ParametersStorage.sol @@ -38,6 +38,8 @@ contract ParametersStorage is Named, Versioned, HubDependent { uint16 public updateCommitWindowDuration; + uint16 public hashFunctionsLimit; + uint16 public opWalletsLimitOnProfileCreation; uint16 public shardingTableSizeLimit; constructor(address hubAddress) HubDependent(hubAddress) { @@ -74,6 +76,8 @@ contract ParametersStorage is Named, Versioned, HubDependent { updateCommitWindowDuration = 30 minutes; + hashFunctionsLimit = 20; + opWalletsLimitOnProfileCreation = 50; shardingTableSizeLimit = 500; // finalizationCommitsNumber @@ -230,6 +234,18 @@ contract ParametersStorage is Named, Versioned, HubDependent { emit ParameterChanged("updateCommitWindowDuration", newUpdateCommitWindowDuration); } + function setHashFunctionsLimit(uint16 hashFunctionsLimit_) external onlyHubOwner { + hashFunctionsLimit = hashFunctionsLimit_; + + emit ParameterChanged("hashFunctionsLimit", hashFunctionsLimit); + } + + function setOpWalletsLimitOnProfileCreation(uint16 opWalletsLimitOnProfileCreation_) external onlyHubOwner { + opWalletsLimitOnProfileCreation = opWalletsLimitOnProfileCreation_; + + emit ParameterChanged("opWalletsLimitOnProfileCreation", opWalletsLimitOnProfileCreation); + } + function setShardingTableSizeLimit(uint16 shardingTableSizeLimit_) external onlyHubOwner { shardingTableSizeLimit = shardingTableSizeLimit_; diff --git a/contracts/v2/CommitManagerV1.sol b/contracts/v2/CommitManagerV1.sol index 93e74558..8fd19850 100644 --- a/contracts/v2/CommitManagerV1.sol +++ b/contracts/v2/CommitManagerV1.sol @@ -49,8 +49,6 @@ contract CommitManagerV2 is Named, Versioned, ContractStatus, Initializable { ShardingTableStorageV2 public shardingTableStorage; StakingStorage public stakingStorage; - uint256 constant HASH_RING_SIZE = type(uint256).max; - // solhint-disable-next-line no-empty-blocks constructor(address hubAddress) ContractStatus(hubAddress) {} diff --git a/contracts/v2/Profile.sol b/contracts/v2/Profile.sol index 4aa601ac..d5b4aeca 100644 --- a/contracts/v2/Profile.sol +++ b/contracts/v2/Profile.sol @@ -82,6 +82,7 @@ contract ProfileV2 is Named, Versioned, ContractStatus, Initializable { function createProfile( address adminWallet, + address[] calldata operationalWallets, bytes calldata nodeId, string calldata sharesTokenName, string calldata sharesTokenSymbol, @@ -89,9 +90,15 @@ contract ProfileV2 is Named, Versioned, ContractStatus, Initializable { ) external onlyWhitelisted { IdentityStorage ids = identityStorage; ProfileStorage ps = profileStorage; + Identity id = identityContract; if (ids.getIdentityId(msg.sender) != 0) revert ProfileErrors.IdentityAlreadyExists(ids.getIdentityId(msg.sender), msg.sender); + if (operationalWallets.length > parametersStorage.opWalletsLimitOnProfileCreation()) + revert ProfileErrors.TooManyOperationalWallets( + parametersStorage.opWalletsLimitOnProfileCreation(), + uint16(operationalWallets.length) + ); if (nodeId.length == 0) revert ProfileErrors.EmptyNodeId(); if (ps.nodeIdsList(nodeId)) revert ProfileErrors.NodeIdAlreadyExists(nodeId); if (keccak256(abi.encodePacked(sharesTokenName)) == keccak256(abi.encodePacked(""))) @@ -101,7 +108,8 @@ contract ProfileV2 is Named, Versioned, ContractStatus, Initializable { if (ps.sharesNames(sharesTokenName)) revert ProfileErrors.SharesTokenNameAlreadyExists(sharesTokenName); if (ps.sharesSymbols(sharesTokenSymbol)) revert ProfileErrors.SharesTokenSymbolAlreadyExists(sharesTokenSymbol); - uint72 identityId = identityContract.createIdentity(msg.sender, adminWallet); + uint72 identityId = id.createIdentity(msg.sender, adminWallet); + id.addOperationalWallets(identityId, operationalWallets); Shares sharesContract = new Shares(address(hub), sharesTokenName, sharesTokenSymbol); @@ -130,9 +138,9 @@ contract ProfileV2 is Named, Versioned, ContractStatus, Initializable { bytes32 nodeAddress; UnorderedIndexableContractDynamicSetLib.Contract[] memory hashFunctions = hp.getAllHashFunctions(); - uint256 hashFunctionsNumber = hashFunctions.length; + require(hashFunctions.length <= parametersStorage.hashFunctionsLimit(), "Too many hash functions!"); uint8 hashFunctionId; - for (uint8 i; i < hashFunctionsNumber; ) { + for (uint8 i; i < hashFunctions.length; ) { hashFunctionId = hashFunctions[i].id; nodeAddress = hp.callHashFunction(hashFunctionId, nodeId); ps.setNodeAddress(identityId, hashFunctionId, nodeAddress); diff --git a/contracts/v2/constants/HashRingConstants.sol b/contracts/v2/constants/HashRingConstants.sol new file mode 100644 index 00000000..233b2713 --- /dev/null +++ b/contracts/v2/constants/HashRingConstants.sol @@ -0,0 +1,5 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.16; + +uint256 constant HASH_RING_SIZE = type(uint256).max; diff --git a/contracts/v2/errors/ProfileErrors.sol b/contracts/v2/errors/ProfileErrors.sol index ec2d2591..146e821f 100644 --- a/contracts/v2/errors/ProfileErrors.sol +++ b/contracts/v2/errors/ProfileErrors.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.16; library ProfileErrors { error IdentityAlreadyExists(uint72 identityId, address wallet); + error TooManyOperationalWallets(uint16 allowed, uint16 provided); error EmptyNodeId(); error NodeIdAlreadyExists(bytes nodeId); error EmptySharesTokenName(); diff --git a/contracts/v2/scoring/LinearSum.sol b/contracts/v2/scoring/LinearSum.sol index e9ad4b91..3b1ffe30 100644 --- a/contracts/v2/scoring/LinearSum.sol +++ b/contracts/v2/scoring/LinearSum.sol @@ -10,6 +10,7 @@ import {Initializable} from "../../v1/interface/Initializable.sol"; import {IProximityScoreFunctionsPair} from "../interface/IProximityScoreFunctionsPair.sol"; import {Named} from "../../v1/interface/Named.sol"; import {ScaleDownLib} from "../utils/ScaleDownLibrary.sol"; +import {HASH_RING_SIZE} from "../constants/HashRingConstants.sol"; contract LinearSum is IProximityScoreFunctionsPair, Indexable, Named, HubDependent, Initializable { event ParameterChanged(string parameterName, uint256 parameterValue); @@ -17,8 +18,6 @@ contract LinearSum is IProximityScoreFunctionsPair, Indexable, Named, HubDepende uint8 private constant _ID = 2; string private constant _NAME = "LinearSum"; - uint256 constant HASH_RING_SIZE = type(uint256).max; - HashingProxy public hashingProxy; ParametersStorage public parametersStorage; diff --git a/contracts/v2/storage/NodeOperatorFeeChangesStorage.sol b/contracts/v2/storage/NodeOperatorFeeChangesStorage.sol index 4aa83479..8e0e43a4 100644 --- a/contracts/v2/storage/NodeOperatorFeeChangesStorage.sol +++ b/contracts/v2/storage/NodeOperatorFeeChangesStorage.sol @@ -25,7 +25,7 @@ contract NodeOperatorFeeChangesStorage is Named, Versioned, HubDependent { constructor(address hubAddress) HubDependent(hubAddress) {} modifier onlyOnce() { - require(!_delayFreePeriodSet, "Function has already been executed."); + require(!_delayFreePeriodSet, "Fn has already been executed"); _; _delayFreePeriodSet = true; } diff --git a/test/v1/unit/Identity.test.ts b/test/v1/unit/Identity.test.ts index a73905ed..ec9d4b9c 100644 --- a/test/v1/unit/Identity.test.ts +++ b/test/v1/unit/Identity.test.ts @@ -66,8 +66,8 @@ describe('@v1 @unit Identity contract', function () { expect(await Identity.name()).to.equal('Identity'); }); - it('The contract is version "1.0.1"', async () => { - expect(await Identity.version()).to.equal('1.0.1'); + it('The contract is version "1.1.0"', async () => { + expect(await Identity.version()).to.equal('1.1.0'); }); it('Create an identity as a contract, expect to pass', async () => { @@ -176,7 +176,7 @@ describe('@v1 @unit Identity contract', function () { expect(adminKeys.length).to.equal(2, 'Error: Failed to add operational key to identity!'); await expect( AddKeyWithAdminWallet.addKey(getIdentityId, newOperationalKey, OPERATIONAL_KEY, ECDSA), - ).to.be.revertedWith('Key is already attached'); + ).to.be.revertedWith('Operational key is taken'); }); it('Add an admin key to existing identity with operational wallet, expect to fail', async () => { @@ -279,4 +279,39 @@ describe('@v1 @unit Identity contract', function () { expect(await IdentityStorage.keyHasPurpose(getIdentityId, adminKeyBytes32, ADMIN_KEY)).to.be.false; }); + + it('Create 2 identities, try to attach operational key of the other identity, expect to revert', async () => { + const identityId = await createIdentity(operationalKey, adminKey); + await createIdentity(accounts[3].address, accounts[4].address); + + await expect( + Identity.connect(accounts[2]).addKey( + identityId, + ethers.utils.keccak256(ethers.utils.solidityPack(['address'], [accounts[3].address])), + OPERATIONAL_KEY, + ECDSA, + ), + ).to.be.revertedWith('Operational key is taken'); + }); + + it('Create identity, try to attach multiple operational wallets with already existing key, expect to revert', async () => { + const identityId = await createIdentity(operationalKey, adminKey); + + await expect( + Identity.addOperationalWallets(identityId, [accounts[1].address, accounts[3].address]), + ).to.be.revertedWith('Operational key is taken'); + }); + + it('Create identity, try to attach multiple operational wallets with already taken key, expect to revert', async () => { + const identityId = await createIdentity(operationalKey, adminKey); + await createIdentity(accounts[3].address, accounts[4].address); + + await expect( + Identity.addOperationalWallets(identityId, [accounts[3].address, accounts[5].address]), + ).to.be.revertedWith('Operational key is taken'); + + // We still can attach someones admin wallet as a key, but it shouldn't be a problem + await expect(Identity.addOperationalWallets(identityId, [accounts[4].address, accounts[5].address])).not.to.be + .reverted; + }); }); diff --git a/test/v2/unit/ProfileV2.test.ts b/test/v2/unit/ProfileV2.test.ts new file mode 100644 index 00000000..379870c1 --- /dev/null +++ b/test/v2/unit/ProfileV2.test.ts @@ -0,0 +1,104 @@ +import { loadFixture } from '@nomicfoundation/hardhat-network-helpers'; +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; +import { expect } from 'chai'; +import hre from 'hardhat'; + +import { HubController, ParametersStorage, ProfileStorage, ProfileV2 } from '../../../typechain'; + +type ProfileV2Fixture = { + accounts: SignerWithAddress[]; + HubController: HubController; + ProfileV2: ProfileV2; + ProfileStorage: ProfileStorage; + ParametersStorage: ParametersStorage; +}; + +describe('@v2 @unit ProfileV2 contract', function () { + let accounts: SignerWithAddress[]; + let HubController: HubController; + let ProfileV2: ProfileV2; + let ProfileStorage: ProfileStorage; + let ParametersStorage: ParametersStorage; + + const nodeId1 = '0x07f38512786964d9e70453371e7c98975d284100d44bd68dab67fe00b525cb66'; + const nodeId2 = '0x08f38512786964d9e70453371e7c98975d284100d44bd68dab67fe00b525cb67'; + + async function createProfile(adminWallet: string, operationalWallets: string[]) { + await expect( + ProfileV2.connect(accounts[0]).createProfile(adminWallet, operationalWallets, nodeId1, 'Token', 'TKN', 0), + ).to.emit(ProfileV2, 'ProfileCreated'); + } + + async function deployProfileFixture(): Promise { + await hre.deployments.fixture(['IdentityStorageV2', 'ProfileV2']); + ProfileV2 = await hre.ethers.getContract('Profile'); + ProfileStorage = await hre.ethers.getContract('ProfileStorage'); + ParametersStorage = await hre.ethers.getContract('ParametersStorage'); + accounts = await hre.ethers.getSigners(); + HubController = await hre.ethers.getContract('HubController'); + await HubController.setContractAddress('HubOwner', accounts[0].address); + + return { accounts, HubController, ProfileV2, ProfileStorage, ParametersStorage }; + } + + beforeEach(async () => { + hre.helpers.resetDeploymentsJson(); + ({ accounts, HubController, ProfileV2, ProfileStorage, ParametersStorage } = await loadFixture( + deployProfileFixture, + )); + }); + + it('The contract is named "Profile"', async () => { + expect(await ProfileV2.name()).to.equal('Profile'); + }); + + it('The contract is version "2.0.0"', async () => { + expect(await ProfileV2.version()).to.equal('2.0.0'); + }); + + it('Cannot create a profile with existing identity, expect to fail', async () => { + await createProfile(accounts[1].address, [accounts[2].address]); + + await expect( + ProfileV2.createProfile(accounts[3].address, [accounts[4].address], nodeId1, 'Token', 'TKN', 0), + ).to.be.revertedWithCustomError(ProfileV2, 'IdentityAlreadyExists'); + }); + + it('Cannot create a profile with msg.sender in operational wallets array, expect to fail', async () => { + await expect( + ProfileV2.createProfile(accounts[1].address, [accounts[0].address], nodeId1, 'Token', 'TKN', 0), + ).to.be.revertedWith('Operational key is taken'); + }); + + it('Cannot create a profile with existing operational wallet in the array, expect to fail', async () => { + await createProfile(accounts[1].address, [accounts[2].address]); + + await expect( + ProfileV2.connect(accounts[3]).createProfile( + accounts[4].address, + [accounts[2].address], + nodeId2, + 'Token123', + 'TKN123', + 0, + ), + ).to.be.revertedWith('Operational key is taken'); + }); + + it('Cannot create a profile with too many operational wallets, expect to fail', async () => { + const opWalletsLimit = await ParametersStorage.opWalletsLimitOnProfileCreation(); + + await expect( + ProfileV2.createProfile( + accounts[0].address, + accounts.slice(2, 2 + opWalletsLimit + 1).map((acc) => acc.address), + nodeId1, + 'Token', + 'TKN', + 0, + ), + ) + .to.be.revertedWithCustomError(ProfileV2, 'TooManyOperationalWallets') + .withArgs(opWalletsLimit, opWalletsLimit + 1); + }); +});