diff --git a/contracts/colony/Colony.sol b/contracts/colony/Colony.sol index b674da187e..e92e6c25b3 100755 --- a/contracts/colony/Colony.sol +++ b/contracts/colony/Colony.sol @@ -112,10 +112,11 @@ contract Colony is BasicMetaTransaction, ColonyStorage, PatriciaTreeProofs { setRoleAssignmentFunction(bytes4(keccak256("setTaskEvaluatorRole(uint256,address)"))); setRoleAssignmentFunction(bytes4(keccak256("setTaskWorkerRole(uint256,address)"))); - // Initialise the root domain + // Initialise the local skill and domain trees IColonyNetwork colonyNetwork = IColonyNetwork(colonyNetworkAddress); - uint256 rootLocalSkill = colonyNetwork.getSkillCount(); - initialiseDomain(rootLocalSkill); + uint256 rootDomainSkill = colonyNetwork.getSkillCount(); + initialiseDomain(rootDomainSkill); + initialiseRootLocalSkill(); // Set initial colony reward inverse amount to the max indicating a zero rewards to start with rewardInverse = 2**256 - 1; @@ -272,6 +273,23 @@ contract Colony is BasicMetaTransaction, ColonyStorage, PatriciaTreeProofs { IColonyNetwork(colonyNetworkAddress).uninstallExtension(_extensionId); } + function addLocalSkill() public stoppable auth { + uint256 newLocalSkill = IColonyNetwork(colonyNetworkAddress).addSkill(rootLocalSkill); + localSkills[newLocalSkill] = LocalSkill({ exists: true, deprecated: false }); + + emit LocalSkillAdded(msg.sender, newLocalSkill); + } + + function deprecateLocalSkill(uint256 _localSkillId, bool _deprecated) public stoppable auth { + localSkills[_localSkillId].deprecated = _deprecated; + + emit LocalSkillDeprecated(msg.sender, _localSkillId, _deprecated); + } + + function getRootLocalSkill() public view returns (uint256) { + return rootLocalSkill; + } + function addDomain(uint256 _permissionDomainId, uint256 _childSkillIndex, uint256 _parentDomainId) public stoppable domainNotDeprecated(_parentDomainId) @@ -289,13 +307,13 @@ contract Colony is BasicMetaTransaction, ColonyStorage, PatriciaTreeProofs { uint256 parentSkillId = domains[_parentDomainId].skillId; - // Setup new local skill + // Setup new domain skill IColonyNetwork colonyNetwork = IColonyNetwork(colonyNetworkAddress); // slither-disable-next-line reentrancy-no-eth - uint256 newLocalSkill = colonyNetwork.addSkill(parentSkillId); + uint256 newDomainSkill = colonyNetwork.addSkill(parentSkillId); // Add domain to local mapping - initialiseDomain(newLocalSkill); + initialiseDomain(newDomainSkill); if (keccak256(abi.encodePacked(_metadata)) != keccak256(abi.encodePacked(""))) { emit DomainMetadata(msgSender(), domainCount, _metadata); @@ -384,6 +402,14 @@ contract Colony is BasicMetaTransaction, ColonyStorage, PatriciaTreeProofs { ColonyAuthority colonyAuthority = ColonyAuthority(address(authority)); bytes4 sig; + sig = bytes4(keccak256("addLocalSkill()")); + colonyAuthority.setRoleCapability(uint8(ColonyRole.Root), address(this), sig, true); + + sig = bytes4(keccak256("deprecateLocalSkill(uint256,bool)")); + colonyAuthority.setRoleCapability(uint8(ColonyRole.Root), address(this), sig, true); + + if (rootLocalSkill == 0) initialiseRootLocalSkill(); + sig = bytes4(keccak256("deprecateDomain(uint256,uint256,uint256,bool)")); colonyAuthority.setRoleCapability(uint8(ColonyRole.Architecture), address(this), sig, true); } @@ -462,6 +488,11 @@ contract Colony is BasicMetaTransaction, ColonyStorage, PatriciaTreeProofs { return tokenApprovalTotals[_token]; } + function initialiseRootLocalSkill() internal { + assert(rootLocalSkill == 0); + rootLocalSkill = IColonyNetwork(colonyNetworkAddress).initialiseRootLocalSkill(); + } + function initialiseDomain(uint256 _skillId) internal skillExists(_skillId) { domainCount += 1; // Create a new funding pot diff --git a/contracts/colony/ColonyAuthority.sol b/contracts/colony/ColonyAuthority.sol index f9a703dca1..ff3ac4927e 100644 --- a/contracts/colony/ColonyAuthority.sol +++ b/contracts/colony/ColonyAuthority.sol @@ -120,6 +120,8 @@ contract ColonyAuthority is CommonAuthority { addRoleCapability(ARBITRATION_ROLE, "setExpenditureMetadata(uint256,uint256,uint256,string)"); // Added in colony v9 (f-lwss) + addRoleCapability(ROOT_ROLE, "addLocalSkill()"); + addRoleCapability(ROOT_ROLE, "deprecateLocalSkill(uint256,bool)"); addRoleCapability(ARCHITECTURE_ROLE, "deprecateDomain(uint256,uint256,uint256,bool)"); } diff --git a/contracts/colony/ColonyDataTypes.sol b/contracts/colony/ColonyDataTypes.sol index bead4535f1..8456f067d2 100755 --- a/contracts/colony/ColonyDataTypes.sol +++ b/contracts/colony/ColonyDataTypes.sol @@ -241,6 +241,17 @@ interface ColonyDataTypes { /// @param taskId Id of the canceled task event TaskCanceled(uint256 indexed taskId); + /// @notice Event logged when a new local skill is added + /// @param agent The address that is responsible for triggering this event + /// @param localSkillId Id of the newly-created local skill + event LocalSkillAdded(address agent, uint256 localSkillId); + + /// @notice Event logged when a new local skill is added + /// @param agent The address that is responsible for triggering this event + /// @param localSkillId Id of the newly-created local skill + /// @param deprecated Deprecation status of the local skill + event LocalSkillDeprecated(address agent, uint256 localSkillId, bool deprecated); + /// @notice Event logged when a new Domain is added /// @param agent The address that is responsible for triggering this event /// @param domainId Id of the newly-created Domain @@ -418,4 +429,9 @@ interface ColonyDataTypes { uint256 fundingPotId; bool deprecated; } + + struct LocalSkill { + bool exists; + bool deprecated; + } } diff --git a/contracts/colony/ColonyExpenditure.sol b/contracts/colony/ColonyExpenditure.sol index e817981669..85e03da6a4 100644 --- a/contracts/colony/ColonyExpenditure.sol +++ b/contracts/colony/ColonyExpenditure.sol @@ -192,8 +192,11 @@ contract ColonyExpenditure is ColonyStorage { ); Skill memory skill = colonyNetworkContract.getSkill(_skillIds[i]); - require(skill.globalSkill, "colony-not-global-skill"); - require(!skill.deprecated, "colony-deprecated-global-skill"); + LocalSkill memory localSkill = localSkills[_skillIds[i]]; + + require(skill.globalSkill || localSkill.exists, "colony-not-valid-skill"); + require(!skill.globalSkill || !skill.deprecated, "colony-deprecated-global-skill"); + require(!localSkill.exists || !localSkill.deprecated, "colony-deprecated-local-skill"); // We only allow setting of the first skill here. // If we allow more in the future, make sure to have a hard limit that diff --git a/contracts/colony/ColonyStorage.sol b/contracts/colony/ColonyStorage.sol index 1a39a55b4b..51463297f3 100755 --- a/contracts/colony/ColonyStorage.sol +++ b/contracts/colony/ColonyStorage.sol @@ -107,6 +107,9 @@ contract ColonyStorage is ColonyDataTypes, ColonyNetworkDataTypes, DSMath, Commo uint256 constant METATRANSACTION_NONCES_SLOT = 35; mapping(address => uint256) metatransactionNonces; // Storage slot 35 + uint256 rootLocalSkill; // Storage slot 36 + mapping (uint256 => LocalSkill) localSkills; // Storage slot 37 + // Constants uint256 constant MAX_PAYOUT = 2**128 - 1; // 340,282,366,920,938,463,463 WADs diff --git a/contracts/colony/IColony.sol b/contracts/colony/IColony.sol index 10245ec208..4ee0219a0e 100644 --- a/contracts/colony/IColony.sol +++ b/contracts/colony/IColony.sol @@ -282,6 +282,18 @@ interface IColony is ColonyDataTypes, IRecovery, IBasicMetaTransaction { /// @param extensionId keccak256 hash of the extension name, used as an indentifier function uninstallExtension(bytes32 extensionId) external; + /// @notice Add a new local skill for the colony. Secured function to authorised members. + function addLocalSkill() external; + + /// @notice Deprecate a local skill for the colony. Secured function to authorised members. + /// @param localSkillId Id for the local skill + /// @param deprecated Deprecation status to set for the skill + function deprecateLocalSkill(uint256 localSkillId, bool deprecated) external; + + /// @notice Get the root local skill id + /// @return rootLocalSkill The root local skill id + function getRootLocalSkill() external view returns (uint256 rootLocalSkill); + /// @notice Add a colony domain, and its respective local skill under skill with id `_parentSkillId`. /// New funding pot is created and associated with the domain here. /// @param _permissionDomainId The domainId in which I have the permission to take this action diff --git a/contracts/colonyNetwork/ColonyNetwork.sol b/contracts/colonyNetwork/ColonyNetwork.sol index ce15c6624e..78ce2dc660 100644 --- a/contracts/colonyNetwork/ColonyNetwork.sol +++ b/contracts/colonyNetwork/ColonyNetwork.sol @@ -280,6 +280,14 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage { skills[_skillId].deprecated = true; } + function initialiseRootLocalSkill() public + stoppable + calledByColony + returns (uint256) + { + return skillCount++; + } + function appendReputationUpdateLog(address _user, int _amount, uint _skillId) public stoppable calledByColony @@ -366,7 +374,7 @@ contract ColonyNetwork is BasicMetaTransaction, ColonyNetworkStorage { colonyAuthority.setOwner(address(etherRouter)); - // Initialise the root (domain) local skill with defaults by just incrementing the skillCount + // Initialise the domain tree with defaults by just incrementing the skillCount skillCount += 1; colonyCount += 1; colonies[colonyCount] = address(colony); diff --git a/contracts/colonyNetwork/IColonyNetwork.sol b/contracts/colonyNetwork/IColonyNetwork.sol index 98ef8a5fed..bdaa309690 100644 --- a/contracts/colonyNetwork/IColonyNetwork.sol +++ b/contracts/colonyNetwork/IColonyNetwork.sol @@ -100,6 +100,10 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery, IBasicMetaTransac /// @param _skillId Id of the skill function deprecateSkill(uint256 _skillId) external; + /// @notice Initialise the local skills tree for a colony + /// @return rootLocalSkillId The root local skill + function initialiseRootLocalSkill() external returns (uint256 rootLocalSkillId); + /// @notice Adds a reputation update entry to log. /// @dev Errors if it is called by anyone but a colony or if skill with id `_skillId` does not exist or. /// @param _user The address of the user for the reputation update diff --git a/docs/_Interface_IColony.md b/docs/_Interface_IColony.md index 28cc6b2cf2..dda24d5622 100644 --- a/docs/_Interface_IColony.md +++ b/docs/_Interface_IColony.md @@ -38,6 +38,13 @@ Add a colony domain, and its respective local skill under skill with id `_parent |_metadata|string|Metadata relating to the domain. Expected to be the IPFS hash of a JSON blob, but not enforced by the contracts. +### `addLocalSkill` + +Add a new local skill for the colony. Secured function to authorised members. + + + + ### `addPayment` Add a new payment in the colony. Secured function to authorised members. @@ -276,6 +283,19 @@ Set the deprecation of an extension in a colony. Secured function to authorised |deprecated|bool|Whether to deprecate the extension or not +### `deprecateLocalSkill` + +Deprecate a local skill for the colony. Secured function to authorised members. + + +**Parameters** + +|Name|Type|Description| +|---|---|---| +|localSkillId|uint256|Id for the local skill +|deprecated|bool|Deprecation status to set for the skill + + ### `editColony` Called to change the metadata associated with a colony. Expected to be a IPFS hash of a JSON blob, but not enforced to any degree by the contracts @@ -796,6 +816,18 @@ Get useful information about specific reward payout. |---|---|---| |rewardPayoutCycle|RewardPayoutCycle|RewardPayoutCycle, containing propertes: `reputationState` Reputation root hash at the time of creation, `colonyWideReputation` Colony wide reputation in `reputationState`, `totalTokens` Total colony tokens at the time of creation, `amount` Total amount of tokens taken aside for reward payout, `tokenAddress` Token address, `blockTimestamp` Block number at the time of creation. +### `getRootLocalSkill` + +Get the root local skill id + + + +**Return Parameters** + +|Name|Type|Description| +|---|---|---| +|rootLocalSkill|uint256|The root local skill id + ### `getTask` Get a task with id `_id` diff --git a/docs/_Interface_IColonyNetwork.md b/docs/_Interface_IColonyNetwork.md index d400daf329..0f3ac7e3e1 100644 --- a/docs/_Interface_IColonyNetwork.md +++ b/docs/_Interface_IColonyNetwork.md @@ -708,6 +708,18 @@ Creates initial inactive reputation mining cycle. +### `initialiseRootLocalSkill` + +Initialise the local skills tree for a colony + + + +**Return Parameters** + +|Name|Type|Description| +|---|---|---| +|rootLocalSkillId|uint256|The root local skill + ### `installExtension` Install an extension in a colony. Can only be called by a Colony. diff --git a/helpers/constants.js b/helpers/constants.js index 8fef58ef48..98dcc91cce 100644 --- a/helpers/constants.js +++ b/helpers/constants.js @@ -66,7 +66,7 @@ const DECAY_RATE = { DENOMINATOR: new BN("1000000000000000"), }; -const GLOBAL_SKILL_ID = new BN("3"); // Not a root global skill ID or anything, just the first global skill's ID +const GLOBAL_SKILL_ID = new BN("4"); // Not a root global skill ID or anything, just the first global skill's ID module.exports = { UINT256_MAX, diff --git a/helpers/test-data-generator.js b/helpers/test-data-generator.js index dff585d955..2558e72261 100644 --- a/helpers/test-data-generator.js +++ b/helpers/test-data-generator.js @@ -18,6 +18,7 @@ import { WORKER_ROLE, SPECIFICATION_HASH, DELIVERABLE_HASH, + GLOBAL_SKILL_ID, } from "./constants"; import { getTokenArgs, web3GetAccounts, getChildSkillIndex, web3SignTypedData } from "./test-helper"; @@ -35,7 +36,7 @@ const Resolver = artifacts.require("Resolver"); const MetaTxToken = artifacts.require("MetaTxToken"); const IColonyNetwork = artifacts.require("IColonyNetwork"); -export async function makeTask({ colonyNetwork, colony, hash = SPECIFICATION_HASH, domainId = 1, skillId = 3, dueDate = 0, manager }) { +export async function makeTask({ colonyNetwork, colony, hash = SPECIFICATION_HASH, domainId = 1, skillId = GLOBAL_SKILL_ID, dueDate = 0, manager }) { const accounts = await web3GetAccounts(); manager = manager || accounts[0]; // eslint-disable-line no-param-reassign diff --git a/migrations/8_setup_meta_colony.js b/migrations/8_setup_meta_colony.js index 1d9296395c..c2afc6fc44 100644 --- a/migrations/8_setup_meta_colony.js +++ b/migrations/8_setup_meta_colony.js @@ -109,7 +109,7 @@ module.exports = async function (deployer, network, accounts) { await colonyNetwork.startNextCycle(); const skillCount = await colonyNetwork.getSkillCount(); - assert.equal(skillCount.toNumber(), 3); + assert.equal(skillCount.toNumber(), 4); console.log("### Meta Colony created at", metaColony.address); }; diff --git a/test/contracts-network/colony-expenditure.js b/test/contracts-network/colony-expenditure.js index 9039f98083..1c3e5297d7 100644 --- a/test/contracts-network/colony-expenditure.js +++ b/test/contracts-network/colony-expenditure.js @@ -229,6 +229,32 @@ contract("Colony Expenditure", (accounts) => { ); }); + it("should allow owners to update a slot skill with a local skill", async () => { + await colony.addLocalSkill(); + const localSkillId = await colonyNetwork.getSkillCount(); + + await colony.setExpenditureSkill(expenditureId, SLOT0, localSkillId, { from: ADMIN }); + + expenditureSlot = await colony.getExpenditureSlot(expenditureId, SLOT0); + expect(expenditureSlot.skills[0]).to.eq.BN(localSkillId); + }); + + it("should not allow owners to update a slot skill with a local skill from a different colony", async () => { + const { colony: otherColony } = await setupRandomColony(colonyNetwork); + await otherColony.addLocalSkill(); + const localSkillId = await colonyNetwork.getSkillCount(); + + await checkErrorRevert(colony.setExpenditureSkill(expenditureId, SLOT0, localSkillId, { from: ADMIN }), "colony-not-valid-skill"); + }); + + it("should not allow owners to update a slot skill with a deprecated local skill", async () => { + await colony.addLocalSkill(); + const localSkillId = await colonyNetwork.getSkillCount(); + await colony.deprecateLocalSkill(localSkillId, true); + + await checkErrorRevert(colony.setExpenditureSkill(expenditureId, SLOT0, localSkillId, { from: ADMIN }), "colony-deprecated-local-skill"); + }); + it("should not allow owners to update many slot skills with nonexistent skills", async () => { await checkErrorRevert(colony.setExpenditureSkills(expenditureId, [SLOT0], [100], { from: ADMIN }), "colony-expenditure-skill-does-not-exist"); }); diff --git a/test/contracts-network/colony.js b/test/contracts-network/colony.js index 017a66521b..9418c4052f 100755 --- a/test/contracts-network/colony.js +++ b/test/contracts-network/colony.js @@ -169,6 +169,23 @@ contract("Colony", (accounts) => { }); }); + describe("when adding local skills", () => { + it("should log the LocalSkillAdded event", async () => { + const tx = await colony.addLocalSkill(); + + const skillCount = await colonyNetwork.getSkillCount(); + await expectEvent(tx, "LocalSkillAdded", [accounts[0], skillCount]); + }); + + it("should allow root users to deprecate local skills", async () => { + await colony.addLocalSkill(); + const skillCount = await colonyNetwork.getSkillCount(); + + const tx = await colony.deprecateLocalSkill(skillCount, true); + await expectEvent(tx, "LocalSkillDeprecated", [accounts[0], skillCount, true]); + }); + }); + describe("when adding domains", () => { it("should log DomainAdded and FundingPotAdded and DomainMetadata events", async () => { let tx = await colony.addDomain(1, UINT256_MAX, 1);