Respond to review comments I

JoinColony · Aug 13, 2024 · 9376b82 · 9376b82
1 parent 22a012b
commit 9376b82
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 12 deletions.
diff --git a/contracts/colonyNetwork/ColonyNetworkSkills.sol b/contracts/colonyNetwork/ColonyNetworkSkills.sol
@@ -38,9 +38,8 @@ contract ColonyNetworkSkills is ColonyNetworkStorage, Multicall, CallWithGuards
     return skillCount;
   }
 
-  function deprecateSkill(uint256 _skillId, bool _deprecated) public stoppable returns (bool) {
-    require(false, "colony-network-deprecate-skill-disabled");
-    return false;
+  function deprecateSkill(uint256 _skillId, bool _deprecated) public stoppable {
+    revert("colony-network-deprecate-skill-disabled");
   }
 
   function initialiseRootLocalSkill() public stoppable calledByColony returns (uint256) {

diff --git a/contracts/colonyNetwork/IColonyNetwork.sol b/contracts/colonyNetwork/IColonyNetwork.sol
@@ -101,8 +101,7 @@ interface IColonyNetwork is ColonyNetworkDataTypes, IRecovery, IBasicMetaTransac
   /// @dev Deprecated and will revert if called
   /// @param _skillId Id of the skill
   /// @param _deprecated Deprecation status
-  /// @return _changed Whether the deprecated state was changed
-  function deprecateSkill(uint256 _skillId, bool _deprecated) external returns (bool _changed);
+  function deprecateSkill(uint256 _skillId, bool _deprecated) external;
 
   /// @notice Initialise the local skills tree for a colony
   /// @return _rootLocalSkillId The root local skill

diff --git a/docs/interfaces/icolonynetwork.md b/docs/interfaces/icolonynetwork.md
@@ -393,7 +393,7 @@ Set the deprecation of an extension in a colony. Can only be called by a Colony.
 |_deprecated|bool|Whether to deprecate the extension or not
 
 
-### ▸ `deprecateSkill(uint256 _skillId, bool _deprecated):bool _changed`
+### ▸ `deprecateSkill(uint256 _skillId, bool _deprecated)`
 
 DEPRECATED Set deprecation status for a skill
 
@@ -406,11 +406,6 @@ DEPRECATED Set deprecation status for a skill
 |_skillId|uint256|Id of the skill
 |_deprecated|bool|Deprecation status
 
-**Return Parameters**
-
-|Name|Type|Description|
-|---|---|---|
-|_changed|bool|Whether the deprecated state was changed
 
 ### ▸ `getBridgedReputationUpdateCount(uint256 _chainId, address _colony):uint256 bridgedReputationCount`
 

diff --git a/test/contracts-network/colony.js b/test/contracts-network/colony.js
@@ -21,7 +21,7 @@ const {
   fundColonyWithTokens,
   setupColony,
 } = require("../../helpers/test-data-generator");
-const { deployColonyVersionGLWSS4, deployColonyVersionHMWSS } = require("../../scripts/deployOldUpgradeableVersion");
+const { downgradeColony, deployColonyVersionGLWSS4, deployColonyVersionHMWSS } = require("../../scripts/deployOldUpgradeableVersion");
 
 const { expect } = chai;
 chai.use(bnChai(web3.utils.BN));
@@ -190,6 +190,16 @@ contract("Colony", (accounts) => {
       const tx = await colony.deprecateLocalSkill(skillCount, true);
       await expectEvent(tx, "LocalSkillDeprecated", [accounts[0], skillCount, true]);
     });
+
+    it("should not be able to deprecate a skill on the network", async () => {
+      await deployColonyVersionHMWSS(colonyNetwork);
+      await downgradeColony(colonyNetwork, colony, "hmwss");
+
+      const version = await colony.version();
+      expect(version).to.eq.BN(14);
+
+      await checkErrorRevert(colony.deprecateLocalSkill(0, true), "colony-network-deprecate-skill-disabled");
+    });
   });
 
   describe("when adding domains", () => {