Update in response to review comments

Author: kronosapiens

contracts/extensions/Korporatio.sol

@@ -29,16 +29,14 @@ contract Korporatio is ColonyExtensionMeta {
 
   // Constants
 
-  uint256 constant APPLICATION_FEE = 6500 * WAD;
-
   // Events
 
   event ApplicationCreated(uint256 indexed stakeId, address indexed applicant);
   event ApplicationCancelled(uint256 indexed stakeId);
   event StakeReclaimed(uint256 indexed stakeId);
   event StakeSlashed(uint256 indexed stakeId);
   event ApplicationUpdated(uint256 indexed stakeId, bytes32 ipfsHash);
-  event ApplicationSubmitted(uint256 indexed stakeId, bytes32 ipfsHash);
+  event ApplicationSubmitted(uint256 indexed stakeId);
 
   // Data structures
 
@@ -62,6 +60,11 @@ contract Korporatio is ColonyExtensionMeta {
 
   // Modifiers
 
+  modifier onlyApplicant(uint256 _applicationId) {
+    require(msgSender() == applications[_applicationId].applicant, "korporatio-not-applicant");
+    _;
+  }
+
   // Overrides
 
   /// @notice Returns the identifier of the extension
@@ -106,6 +109,7 @@ contract Korporatio is ColonyExtensionMeta {
   )
     public
   {
+    require(numApplications <= 0, "korporatio-cannot-initialise");
     require(
       colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Architecture),
       "korporatio-not-root-architect"
@@ -130,6 +134,8 @@ contract Korporatio is ColonyExtensionMeta {
     public
     notDeprecated
   {
+    require(paymentToken != address(0x0), "korporatio-not-initialised");
+
     bytes32 rootHash = IColonyNetwork(colonyNetworkAddress).getReputationRootHash();
     uint256 rootSkillId = colony.getDomain(1).skillId;
 
@@ -166,21 +172,18 @@ contract Korporatio is ColonyExtensionMeta {
     emit ApplicationCreated(numApplications, msgSender());
   }
 
-  function cancelApplication(uint256 _applicationId) public {
-    require(applications[_applicationId].applicant == msgSender(), "korporatio-cannot-cancel");
-
+  function cancelApplication(uint256 _applicationId) public onlyApplicant(_applicationId) {
     applications[_applicationId].cancelledAt = block.timestamp;
 
     emit ApplicationCancelled(_applicationId);
   }
 
-  function reclaimStake(uint256 _applicationId) public {
-    require(
-      applications[_applicationId].cancelledAt + claimDelay <= block.timestamp,
-      "korporatio-cannot-reclaim"
-    );
+  function reclaimStake(uint256 _applicationId) public onlyApplicant(_applicationId) {
+    Application storage application = applications[_applicationId];
+    require(application.applicant == msgSender(), "korporatio-not-applicant");
+    require(application.cancelledAt + claimDelay <= block.timestamp, "korporatio-cannot-reclaim");
 
-    uint256 stakeAmount = applications[_applicationId].stakeAmount;
+    uint256 stakeAmount = application.stakeAmount;
     delete applications[_applicationId];
 
     colony.deobligateStake(msgSender(), 1, stakeAmount);
@@ -206,14 +209,13 @@ contract Korporatio is ColonyExtensionMeta {
     emit StakeSlashed(_applicationId);
   }
 
-  function updateApplication(uint256 _applicationId, bytes32 _ipfsHash) public {
-    require(applications[_applicationId].applicant == msgSender(), "korporatio-not-applicant");
+  function updateApplication(uint256 _applicationId, bytes32 _ipfsHash) public onlyApplicant(_applicationId) {
     require(applications[_applicationId].cancelledAt == UINT256_MAX, "korporatio-stake-cancelled");
 
     emit ApplicationUpdated(_applicationId, _ipfsHash);
   }
 
-  function submitApplication(uint256 _applicationId, bytes32 _ipfsHash) public {
+  function submitApplication(uint256 _applicationId) public {
     require(colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Root), "korporatio-caller-not-root");
     require(applications[_applicationId].cancelledAt == UINT256_MAX, "korporatio-stake-cancelled");
 
@@ -222,7 +224,7 @@ contract Korporatio is ColonyExtensionMeta {
     address metaColony = IColonyNetwork(colonyNetworkAddress).getMetaColony();
     require(ERC20(paymentToken).transferFrom(msgSender(), metaColony, applicationFee), "korporatio-transfer-failed");
 
-    emit ApplicationSubmitted(_applicationId, _ipfsHash);
+    emit ApplicationSubmitted(_applicationId);
   }
 
   // View

test/extensions/korporatio.js

@@ -1,5 +1,6 @@
 /* globals artifacts */
 
+const { BN } = require("bn.js");
 const chai = require("chai");
 const bnChai = require("bn-chai");
 const { ethers } = require("ethers");
@@ -168,13 +169,38 @@ contract("Korporatio", (accounts) => {
       await checkErrorRevert(korporatio.deprecate(true, { from: USER1 }), "ds-auth-unauthorized");
       await checkErrorRevert(korporatio.uninstall({ from: USER1 }), "ds-auth-unauthorized");
     });
+
+    it("cannot create applications unless initialised", async () => {
+      await checkErrorRevert(
+        korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
+          from: USER0,
+        }),
+        "korporatio-not-initialised"
+      );
+    });
   });
 
   describe("creating applications", async () => {
     beforeEach(async () => {
+      await colony.approveStake(korporatio.address, 1, WAD, { from: USER0 });
+
       await korporatio.initialise(token.address, APPLICATION_FEE, WAD.divn(100), SECONDS_PER_DAY, { from: USER0 });
+    });
 
-      await colony.approveStake(korporatio.address, 1, WAD, { from: USER0 });
+    it("can re-initialise until first application is created", async () => {
+      await korporatio.initialise(token.address, APPLICATION_FEE, WAD.divn(10), SECONDS_PER_DAY, { from: USER0 });
+
+      const stakeFraction = await korporatio.getStakeFraction();
+      expect(stakeFraction).to.eq.BN(WAD.divn(10));
+
+      await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
+        from: USER0,
+      });
+
+      await checkErrorRevert(
+        korporatio.initialise(token.address, APPLICATION_FEE, WAD.divn(100), SECONDS_PER_DAY, { from: USER1 }),
+        "korporatio-cannot-initialise"
+      );
     });
 
     it("can query for configuration params", async () => {
@@ -256,7 +282,7 @@ contract("Korporatio", (accounts) => {
       const applicationId = await korporatio.getNumApplications();
 
       // Only applicant can cancel
-      await checkErrorRevert(korporatio.cancelApplication(applicationId, { from: USER1 }), "korporatio-cannot-cancel");
+      await checkErrorRevert(korporatio.cancelApplication(applicationId, { from: USER1 }), "korporatio-not-applicant");
 
       const tx = await korporatio.cancelApplication(applicationId, { from: USER0 });
       const blockTime = await getBlockTime(tx.receipt.blockNumber);
@@ -278,7 +304,7 @@ contract("Korporatio", (accounts) => {
 
       await forwardTime(SECONDS_PER_DAY, this);
 
-      await korporatio.reclaimStake(applicationId);
+      await korporatio.reclaimStake(applicationId, { from: USER0 });
 
       const obligation = await colony.getObligation(USER0, korporatio.address, 1);
       expect(obligation).to.be.zero;
@@ -355,20 +381,41 @@ contract("Korporatio", (accounts) => {
       await korporatio.createFreeApplication({ from: USER0 });
 
       const applicationId = await korporatio.getNumApplications();
-      const ipfsHash = soliditySha3("IPFS Hash");
 
       // Cannot submit if not root
-      await checkErrorRevert(korporatio.submitApplication(applicationId, ipfsHash, { from: USER1 }), "korporatio-caller-not-root");
+      await checkErrorRevert(korporatio.submitApplication(applicationId, { from: USER1 }), "korporatio-caller-not-root");
 
-      const tx = await korporatio.submitApplication(applicationId, ipfsHash, { from: USER0 });
-      await expectEvent(tx, "ApplicationSubmitted", [applicationId, ipfsHash]);
+      const tx = await korporatio.submitApplication(applicationId, { from: USER0 });
+      await expectEvent(tx, "ApplicationSubmitted", [applicationId]);
 
       const metaColonyAddress = await colonyNetwork.getMetaColony();
       const metaColonyBalance = await token.balanceOf(metaColonyAddress);
       expect(metaColonyBalance).to.eq.BN(APPLICATION_FEE);
 
       // Cannot submit once cancelled
-      await checkErrorRevert(korporatio.submitApplication(applicationId, ipfsHash, { from: USER0 }), "korporatio-stake-cancelled");
+      await checkErrorRevert(korporatio.submitApplication(applicationId, { from: USER0 }), "korporatio-stake-cancelled");
+    });
+
+    it("can reclaim a stake via arbitration if the extension is deleted", async () => {
+      const korporatioAddress = korporatio.address;
+      await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
+        from: USER0,
+      });
+
+      const lockPre = await tokenLocking.getUserLock(token.address, USER0);
+      const obligationPre = await colony.getObligation(USER0, korporatioAddress, 1);
+      expect(obligationPre).to.eq.BN(WAD.divn(100).muln(3));
+
+      await colony.uninstallExtension(KORPORATIO, { from: USER0 });
+
+      await colony.transferStake(1, UINT256_MAX, korporatioAddress, USER0, 1, obligationPre, USER0, { from: USER1 });
+
+      const lockPost = await tokenLocking.getUserLock(token.address, USER0);
+      const obligationPost = await colony.getObligation(USER0, korporatioAddress, 1);
+
+      // Obligation is zeroed out, but token balance is unchanged
+      expect(obligationPost).to.be.zero;
+      expect(new BN(lockPre.balance)).to.eq.BN(lockPost.balance);
     });
 
     it("can submit a stake via metatransactions", async () => {