From f8c7434b23753987ff48bb3bebe9c41a342c2eca Mon Sep 17 00:00:00 2001 From: Maksim Izmaylov Date: Sun, 7 Jun 2020 17:27:05 +0200 Subject: [PATCH] Naming: "subsidiary" => "unit" (WARNING! BREAKING CHANGE!) --- contracts/OrgId.sol | 40 ++++++++++++++++++------------------ contracts/OrgIdInterface.sol | 6 +++--- scripts/tools/README.md | 33 ++++++++++++++--------------- test/helpers/orgid.js | 8 ++++---- test/orgid.js | 40 ++++++++++++++++++------------------ 5 files changed, 62 insertions(+), 65 deletions(-) diff --git a/contracts/OrgId.sol b/contracts/OrgId.sol index 5862725..56d6af1 100644 --- a/contracts/OrgId.sol +++ b/contracts/OrgId.sol @@ -24,7 +24,7 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { address director; bool isActive; bool directorConfirmed; - bytes32[] subsidiaries; + bytes32[] units; } /// @dev Mapped list of Organizations @@ -44,9 +44,9 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { /** * @dev Emits when new organizational unit created */ - event SubsidiaryCreated( + event UnitCreated( bytes32 indexed parentOrgId, - bytes32 indexed subOrgId, + bytes32 indexed unitOrgId, address indexed director ); @@ -177,7 +177,7 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { * @param orgJsonUri Unit ORG.JSON URI * @param orgJsonHash ORG.JSON keccak256 hash */ - function createSubsidiary( + function createUnit( bytes32 parentOrgId, address director, string calldata orgJsonUri, @@ -194,7 +194,7 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { orgJsonUri, orgJsonHash ); - emit SubsidiaryCreated(parentOrgId, newUnitOrgId, director); + emit UnitCreated(parentOrgId, newUnitOrgId, director); // If parent ORG.ID owner indicates their address as director, // their directorship is automatically confirmed @@ -372,18 +372,18 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { /** * @dev Get all active organizational units of a particular ORG.ID - * @param orgId Parent ORG.ID hash + * @param parentOrgId Parent ORG.ID hash * @return { "organizationsList": "Array of ORG.ID hashes of active organizational units" } */ - function getSubsidiaries(bytes32 orgId) + function getUnits(bytes32 parentOrgId) external view - orgIdMustExist(orgId) + orgIdMustExist(parentOrgId) returns (bytes32[] memory) { - return _getOrganizations(orgId); + return _getOrganizations(parentOrgId); } /** @@ -462,10 +462,10 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { org.getOrganization.selector, // hierarchy interface: 0x3a3bc250 - org.createSubsidiary.selector ^ + org.createUnit.selector ^ org.confirmDirectorOwnership.selector ^ org.transferDirectorOwnership.selector ^ - org.getSubsidiaries.selector + org.getUnits.selector ]; for (uint256 i = 0; i < interfaceIds.length; i++) { _registerInterface(interfaceIds[i]); @@ -475,16 +475,16 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { /** * @dev Create new organization and add it to storage * @param orgJsonUri ORG.JSON URI - * @param orgJsonHash ORG.JSON's keccak256 hash + * @param orgJsonHash ORG.JSON keccak256 hash * @param parentOrgId Parent ORG.ID hash (if applicable) - * @param subsidiaryDirector Unit director's address (if applicable) + * @param director Unit director address (if applicable) * @return { "ORG.ID": "New ORG.ID hash" } */ function _createOrganization( bytes32 parentOrgId, - address subsidiaryDirector, + address director, string memory orgJsonUri, bytes32 orgJsonHash ) internal returns (bytes32) { @@ -504,7 +504,7 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { ); require( - subsidiaryDirector != address(0), + director != address(0), "ORG.ID: Invalid director address" ); } @@ -515,15 +515,15 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { orgJsonHash, parentOrgId, msg.sender, - subsidiaryDirector, + director, true, - subsidiaryDirector == msg.sender, + director == msg.sender, new bytes32[](0) ); orgIds.push(orgId); if (parentOrgId != bytes32(0)) { - organizations[parentOrgId].subsidiaries.push(orgId); + organizations[parentOrgId].units.push(orgId); } return orgId; @@ -545,7 +545,7 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { bytes32[] memory source = orgId == bytes32(0) ? orgIds - : organizations[orgId].subsidiaries; + : organizations[orgId].director; organizationsList = new bytes32[](_getOrganizationsCount(orgId)); uint256 index; @@ -583,7 +583,7 @@ contract OrgId is OrgIdInterface, Ownable, ERC165, Initializable { bytes32[] memory source = orgId == bytes32(0) ? orgIds - : organizations[orgId].subsidiaries; + : organizations[orgId].director; for (uint256 i = 0; i < source.length; i++) { if (organizations[source[i]].isActive && diff --git a/contracts/OrgIdInterface.sol b/contracts/OrgIdInterface.sol index c752107..873efbf 100644 --- a/contracts/OrgIdInterface.sol +++ b/contracts/OrgIdInterface.sol @@ -25,7 +25,7 @@ contract OrgIdInterface { * @param orgJsonUri Unit ORG.JSON URI * @param orgJsonHash ORG.JSON keccak256 hash */ - function createSubsidiary( + function createUnit( bytes32 parentOrgId, address director, string calldata orgJsonUri, @@ -119,12 +119,12 @@ contract OrgIdInterface { /** * @dev Get all active organizational units of a particular ORG.ID - * @param orgId Parent ORG.ID hash + * @param parentOrgId Parent ORG.ID hash * @return { "organizationsList": "Array of ORG.ID hashes of active organizational units" } */ - function getSubsidiaries(bytes32 orgId) + function getUnits(bytes32 parentOrgId) external view returns (bytes32[] memory); diff --git a/scripts/tools/README.md b/scripts/tools/README.md index 96206ff..ba674ed 100644 --- a/scripts/tools/README.md +++ b/scripts/tools/README.md @@ -34,9 +34,9 @@ Usage: `cmd=makehash ` Generates a hash of the given json file using keccak method from solidity. This hash should be used as ORG.ID JSON validation parameter. -Parameters: +Parameters: - `file=` - Relative path to the ORG.ID json file. + Relative path to the ORG.ID json file. ## deploy @@ -44,10 +44,10 @@ Usage: `cmd=deploy ` Manages contracts deployments. -Parameters: +Parameters: - `name=` Contract name to deploy or upgrade - + - `from=` Account address that should be used to signing transactions @@ -85,15 +85,15 @@ Usage: `cmd=upgrade ` Manages contracts upgrades. -Parameters: +Parameters: - `name=` Contract name to deploy or upgrade - + - `from=` Account address that should be used to signing transactions - `initMethod=` - Name of the contract initializer method name. Optional. + Name of the contract initializer method name. Optional. - `initArgs=` Initializer arguments separated by comma. Optional. @@ -102,12 +102,12 @@ Parameters: Usage: `cmd=tx ` -Sending transactions to the contract instances. +Sending transactions to the contract instances. Properties: - `name=` Contract name to transaction sending - + - `from=` Account address that should be used to signing transactions @@ -124,12 +124,12 @@ Properties: Usage: `cmd=call ` -Sending transactions to the contract instances. +Sending transactions to the contract instances. Properties: - `name=` Contract name - + - `address=` Address of the deployed contract (proxy) on the network @@ -147,7 +147,7 @@ Running the series of predefined commands Properties: - `file=` - Relative path to the file that contains a configuration of the task. + Relative path to the file that contains a configuration of the task. - `params=FROM:,HOLDER1:,HOLDER2:` Parameters that can be used as in-script commands parameters replacements. @@ -187,9 +187,8 @@ Here the example of possible task configuration: "name": "OrgId", "address": "[TASK:2:contract.proxy]", "from": "0x90f8bf6a479f320ead074411a4b0e7944ea8c9c1", - "method": "createOrganization(bytes32,string,bytes32)", + "method": "createOrganization(string,bytes32)", "args": [ - "0x31f5e1745a65fd8a2dd556c8b27d8d585ed184876126779e1323c6a1f06c68f0", "https://gist.githubusercontent.com/[username]/3bde88a0e8248c73c68c1aed2ca4b9be/raw/5df8c96ceff4d0fa99a32d1da63b061ad4b27ccd/ORG.ID", "[TASK:2:hash]" ] @@ -207,10 +206,9 @@ Here the example of possible task configuration: "name": "OrgId", "address": "[TASK:2:contract.proxy]", "from": "0x90f8bf6a479f320ead074411a4b0e7944ea8c9c1", - "method": "createSubsidiary(string,bytes32,address,string,string)", + "method": "createUnit(bytes32,address,string,bytes32)", "args": [ "0x31f5e1745a65fd8a2dd556c8b27d8d585ed184876126779e1323c6a1f06c68f0", - "0x5ba8f3df5408ee9db90015040cf8cacca679a145ca3452cbd09b66eac2e54cdc", "0xa284D6724Ab7D8194b0D894C74C34318c1319391", "https://gist.githubusercontent.com/[username]/3b680e83da367b68c6e84407e5f2d44/raw/569ce8f321499a8249bec31fd09f6c618bcf52cd/Subsidiary%2520ORG.ID", "[TASK:4:hash]" @@ -226,5 +224,4 @@ The explanation of this task: - Running `makehash` command for file `./assets/orgid-legal.json` - Running of `tx` command. Creation of an Organization record. The hash is obtained from previous step - Running `makehash` command for file `./assets/orgid-unit.json` -- Running of `tx` command. Creation of an Subsidiary record. The hash is obtained from previous step - +- Running of `tx` command. Creation of a new organizational unit record. The hash is obtained from previous step diff --git a/test/helpers/orgid.js b/test/helpers/orgid.js index e3af433..e225804 100644 --- a/test/helpers/orgid.js +++ b/test/helpers/orgid.js @@ -62,11 +62,11 @@ module.exports.createOrganizationHelper = async ( * Helper function for creating organizational units * @param {Object} orgIdContract ORG.ID contract instance * @param {address} callerAddress Caller address - * @param {array} args Array of arguments for the real createSubsidiary + * @param {array} args Array of arguments for the real createUnit * @dev Arguments order: [requestedUnitHash(bytes32), directorAddress(address), orgJsonUri(string), orgJsonHash(bytes32)] * @returns {Promise<{string}>} New unit's ORG.ID hash */ -module.exports.createSubsidiaryHelper = async ( +module.exports.createUnitHelper = async ( orgIdContract, callerAddress, args @@ -77,7 +77,7 @@ module.exports.createSubsidiaryHelper = async ( const orgJsonHash = args[3]; const result = await orgIdContract - .methods['createSubsidiary(bytes32,address,string,bytes32)']( + .methods['createUnit(bytes32,address,string,bytes32)']( parentOrgIdHash, directorAddress, orgJsonUri, @@ -86,7 +86,7 @@ module.exports.createSubsidiaryHelper = async ( .send({ from: callerAddress }); let newUnitOrgIdHash; - assertEvent(result, 'SubsidiaryCreated', [ + assertEvent(result, 'UnitCreated', [ [ 'parentOrgId', p => (p).should.equal(parentOrgIdHash) diff --git a/test/orgid.js b/test/orgid.js index fe54e87..845263d 100644 --- a/test/orgid.js +++ b/test/orgid.js @@ -16,7 +16,7 @@ const { const { generateHashHelper, createOrganizationHelper, - createSubsidiaryHelper + createUnitHelper } = require('./helpers/orgid'); let gasLimit = 8000000; // Ropsten gas limit @@ -347,7 +347,7 @@ contract('ORG.ID', accounts => { const parentOrgIdOwner = testOrgIdOwner; const parentOrgIdHash = testOrgIdHash; const unitDirector = randomAddressTwo; - const testUnitOrgIdHash = await createSubsidiaryHelper( + const testUnitOrgIdHash = await createUnitHelper( orgIdContract, parentOrgIdOwner, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] @@ -448,7 +448,7 @@ contract('ORG.ID', accounts => { const parentOrgIdOwner = testOrgIdOwner; const parentOrgIdHash = testOrgIdHash; const unitDirector = randomAddressTwo; - const testUnitOrgIdHash = await createSubsidiaryHelper( + const testUnitOrgIdHash = await createUnitHelper( orgIdContract, parentOrgIdOwner, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] @@ -573,7 +573,7 @@ contract('ORG.ID', accounts => { const parentOrgIdOwner = testOrgIdOwner; const parentOrgIdHash = testOrgIdHash; const unitDirector = randomAddressTwo; - const testUnitOrgIdHash = await createSubsidiaryHelper( + const testUnitOrgIdHash = await createUnitHelper( orgIdContract, parentOrgIdOwner, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] @@ -717,10 +717,10 @@ contract('ORG.ID', accounts => { ); }); - describe('#createSubsidiary(bytes32,address,string,bytes32)', () => { + describe('#createUnit(bytes32,address,string,bytes32)', () => { it('should fail if called by non-owner', async () => { await assertRevert( - createSubsidiaryHelper( + createUnitHelper( orgIdContract, nonOwnerOrDirector, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] @@ -732,7 +732,7 @@ contract('ORG.ID', accounts => { it('should fail if parent organization not found', async () => { const nonExistingOrgIdHash = generateHashHelper(); await assertRevert( - createSubsidiaryHelper( + createUnitHelper( orgIdContract, testOrgIdOwner, [nonExistingOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] @@ -745,7 +745,7 @@ contract('ORG.ID', accounts => { it('should fail if director address is zero', async () => { const invalidDirectorAddress = zeroAddress; await assertRevert( - createSubsidiaryHelper( + createUnitHelper( orgIdContract, testOrgIdOwner, [parentOrgIdHash, invalidDirectorAddress, mockOrgJsonUri, mockOrgJsonHash] @@ -754,16 +754,16 @@ contract('ORG.ID', accounts => { ); }); - it('should create a subsidiary', async () => { + it('should create a unit', async () => { // Director is different from the organization owner - await createSubsidiaryHelper( + await createUnitHelper( orgIdContract, testOrgIdOwner, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] ); // Director is the same as the organization owner - await createSubsidiaryHelper( + await createUnitHelper( orgIdContract, testOrgIdOwner, [parentOrgIdHash, testOrgIdOwner, mockOrgJsonUri, mockOrgJsonHash] @@ -781,7 +781,7 @@ contract('ORG.ID', accounts => { let unitOrgIdHash; beforeEach(async () => { - unitOrgIdHash = await createSubsidiaryHelper( + unitOrgIdHash = await createUnitHelper( orgIdContract, testOrgIdOwner, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] @@ -829,7 +829,7 @@ contract('ORG.ID', accounts => { const newDirector = accounts[10]; beforeEach(async () => { - unitOrgIdHash = await createSubsidiaryHelper( + unitOrgIdHash = await createUnitHelper( orgIdContract, testOrgIdOwner, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] @@ -930,7 +930,7 @@ contract('ORG.ID', accounts => { }); }); - describe('#getSubsidiaries(bytes32)', () => { + describe('#getUnits(bytes32)', () => { let parentOrgIdHash; const parentOrgIdOwner = accounts[5]; const testOrgIdOwner = accounts[5]; @@ -948,7 +948,7 @@ contract('ORG.ID', accounts => { const nonExistingOrgIdHash = generateHashHelper(); await assertRevert( orgIdContract - .methods['getSubsidiaries(bytes32)'](nonExistingOrgIdHash) + .methods['getUnits(bytes32)'](nonExistingOrgIdHash) .call(), 'ORG.ID: Organization not found' ); @@ -956,7 +956,7 @@ contract('ORG.ID', accounts => { it('should return empty array if organization has no units', async () => { const subs = await orgIdContract - .methods['getSubsidiaries(bytes32)'](parentOrgIdHash) + .methods['getUnits(bytes32)'](parentOrgIdHash) .call(); (subs).should.to.be.an('array'); (subs.length).should.equal(0); @@ -965,20 +965,20 @@ contract('ORG.ID', accounts => { // TODO: I should be able to see organizations and units that are inactive or don't have directors // TODO: this is a wrong assumption. Organization unit may function without director just fine. it('should return an empty array if organization unit directorship is not confirmed', async () => { - await createSubsidiaryHelper( + await createUnitHelper( orgIdContract, parentOrgIdOwner, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] ); const subs = await orgIdContract - .methods['getSubsidiaries(bytes32)'](parentOrgIdHash) + .methods['getUnits(bytes32)'](parentOrgIdHash) .call(); (subs).should.to.be.an('array'); (subs.length).should.equal(0); }); it('should return array of units', async () => { - const unitOrgIdHash = await createSubsidiaryHelper( + const unitOrgIdHash = await createUnitHelper( orgIdContract, parentOrgIdOwner, [parentOrgIdHash, unitDirector, mockOrgJsonUri, mockOrgJsonHash] @@ -987,7 +987,7 @@ contract('ORG.ID', accounts => { .methods['confirmDirectorOwnership(bytes32)'](unitOrgIdHash) .send({ from: unitDirector }); const subs = await orgIdContract - .methods['getSubsidiaries(bytes32)'](parentOrgIdHash) + .methods['getUnits(bytes32)'](parentOrgIdHash) .call(); (subs).should.to.be.an('array'); (subs).should.to.be.an('array').that.include(unitOrgIdHash);