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
 
@@ -52,8 +50,6 @@ contract Korporatio is ColonyExtensionMeta {
 
   address colonyNetworkAddress;
 
-  address paymentToken;
-  uint256 applicationFee;
   uint256 stakeFraction;
   uint256 claimDelay;
 
@@ -62,6 +58,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
@@ -98,21 +99,12 @@ contract Korporatio is ColonyExtensionMeta {
 
   // Public
 
-  function initialise(
-    address _paymentToken,
-    uint256 _applicationFee,
-    uint256 _stakeFraction,
-    uint256 _claimDelay
-  )
-    public
-  {
+  function initialise(uint256 _stakeFraction, uint256 _claimDelay) public {
     require(
       colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Architecture),
       "korporatio-not-root-architect"
     );
 
-    paymentToken = _paymentToken;
-    applicationFee = _applicationFee;
     stakeFraction = _stakeFraction;
     claimDelay = _claimDelay;
   }
@@ -130,6 +122,8 @@ contract Korporatio is ColonyExtensionMeta {
     public
     notDeprecated
   {
+    require(stakeFraction > 0, "korporatio-not-initialised");
+
     bytes32 rootHash = IColonyNetwork(colonyNetworkAddress).getReputationRootHash();
     uint256 rootSkillId = colony.getDomain(1).skillId;
 
@@ -166,21 +160,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,35 +197,23 @@ 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");
 
     applications[_applicationId].cancelledAt = block.timestamp;
 
-    address metaColony = IColonyNetwork(colonyNetworkAddress).getMetaColony();
-    require(ERC20(paymentToken).transferFrom(msgSender(), metaColony, applicationFee), "korporatio-transfer-failed");
-
-    emit ApplicationSubmitted(_applicationId, _ipfsHash);
+    emit ApplicationSubmitted(_applicationId);
   }
 
   // View
 
-  function getPaymentToken() external view returns (address) {
-    return paymentToken;
-  }
-
-  function getApplicationFee() external view returns (uint256) {
-    return applicationFee;
-  }
-
   function getStakeFraction() external view returns (uint256) {
     return stakeFraction;
   }

test-smoke/colony-storage-consistent.js

@@ -150,7 +150,7 @@ contract("Contract Storage", (accounts) => {
       console.log("tokenLockingStateHash:", tokenLockingStateHash);
 
       expect(colonyNetworkStateHash).to.equal("0xc4320825b94c76e32b6e2f05dceb178ad070568dd470a469282f620f3152c7ce");
-      expect(colonyStateHash).to.equal("0xefa81fadaf3890213272d8621d411d8d476997660d320aa9bcf5d6d3b898f3b3");
+      expect(colonyStateHash).to.equal("0xdb586290061130dacc48f11eb373e4a01baf177c968ab68679d25e1794074f3a");
       expect(metaColonyStateHash).to.equal("0x15fab25907cfb6baedeaf1fdabd68678d37584a1817a08dfe77db60db378a508");
       expect(miningCycleStateHash).to.equal("0x632d459a2197708bd2dbde87e8275c47dddcdf16d59e3efd21dcef9acb2a7366");
       expect(tokenLockingStateHash).to.equal("0x30fbcbfbe589329fe20288101faabe1f60a4610ae0c0effb15526c6b390a8e07");

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");
@@ -16,6 +17,7 @@ const {
   forwardTime,
   getBlockTime,
   expectEvent,
+  encodeTxData,
 } = require("../../helpers/test-helper");
 
 const { setupRandomColony, getMetaTransactionParameters } = require("../../helpers/test-data-generator");
@@ -28,10 +30,12 @@ chai.use(bnChai(web3.utils.BN));
 const EtherRouter = artifacts.require("EtherRouter");
 const IColonyNetwork = artifacts.require("IColonyNetwork");
 const IReputationMiningCycle = artifacts.require("IReputationMiningCycle");
+const IVotingReputation = artifacts.require("IVotingReputation");
 const Korporatio = artifacts.require("Korporatio");
 const TokenLocking = artifacts.require("TokenLocking");
 
 const KORPORATIO = soliditySha3("Korporatio");
+const VOTING_REPUTATION = soliditySha3("VotingReputation");
 
 contract("Korporatio", (accounts) => {
   let colony;
@@ -60,8 +64,6 @@ contract("Korporatio", (accounts) => {
   const USER2 = accounts[2];
   const MINER = accounts[5];
 
-  const APPLICATION_FEE = WAD.muln(6500);
-
   before(async () => {
     const etherRouter = await EtherRouter.deployed();
     colonyNetwork = await IColonyNetwork.at(etherRouter.address);
@@ -168,32 +170,34 @@ 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 korporatio.initialise(token.address, APPLICATION_FEE, WAD.divn(100), SECONDS_PER_DAY, { from: USER0 });
-
       await colony.approveStake(korporatio.address, 1, WAD, { from: USER0 });
+
+      await korporatio.initialise(WAD.divn(100), SECONDS_PER_DAY, { from: USER0 });
     });
 
     it("can query for configuration params", async () => {
-      const paymentToken = await korporatio.getPaymentToken();
-      const applicationFee = await korporatio.getApplicationFee();
       const stakeFraction = await korporatio.getStakeFraction();
       const claimDelay = await korporatio.getClaimDelay();
 
-      expect(paymentToken).to.equal(token.address);
-      expect(applicationFee).to.eq.BN(APPLICATION_FEE);
       expect(stakeFraction).to.eq.BN(WAD.divn(100));
       expect(claimDelay).to.eq.BN(SECONDS_PER_DAY);
     });
 
     it("cannot set configuration params if not root architect", async () => {
-      await checkErrorRevert(
-        korporatio.initialise(token.address, APPLICATION_FEE, WAD.divn(100), SECONDS_PER_DAY, { from: USER1 }),
-        "korporatio-not-root-architect"
-      );
+      await checkErrorRevert(korporatio.initialise(WAD.divn(100), SECONDS_PER_DAY, { from: USER1 }), "korporatio-not-root-architect");
     });
 
     it("can create an application", async () => {
@@ -225,7 +229,7 @@ contract("Korporatio", (accounts) => {
     });
 
     it("cannot create an application with insufficient rep", async () => {
-      await korporatio.initialise(token.address, APPLICATION_FEE, WAD, SECONDS_PER_DAY, { from: USER0 });
+      await korporatio.initialise(WAD, SECONDS_PER_DAY, { from: USER0 });
 
       await checkErrorRevert(
         korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
@@ -256,7 +260,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 +282,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;
@@ -331,6 +335,28 @@ contract("Korporatio", (accounts) => {
       await checkErrorRevert(korporatio.slashStake(applicationId, false, { from: USER2 }), "korporatio-caller-not-arbitration");
     });
 
+    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 update an application", async () => {
       await korporatio.createFreeApplication({ from: USER0 });
 
@@ -348,27 +374,56 @@ contract("Korporatio", (accounts) => {
       await checkErrorRevert(korporatio.updateApplication(applicationId, ipfsHash, { from: USER0 }), "korporatio-stake-cancelled");
     });
 
-    it("can submit an application and pay the fee", async () => {
-      await token.mint(USER0, APPLICATION_FEE);
-      await token.approve(korporatio.address, APPLICATION_FEE);
-
+    it("can submit an application", async () => {
       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, { from: USER0 });
+      await expectEvent(tx, "ApplicationSubmitted", [applicationId]);
+
+      // Cannot submit twice
+      await checkErrorRevert(korporatio.submitApplication(applicationId, { from: USER0 }), "korporatio-stake-cancelled");
+    });
+
+    it("can submit an application via a motion", async () => {
+      await colony.installExtension(VOTING_REPUTATION, 9);
+      const votingAddress = await colonyNetwork.getExtensionInstallation(VOTING_REPUTATION, colony.address);
+      await colony.setArbitrationRole(1, UINT256_MAX, votingAddress, 1, true);
+      await colony.setRootRole(votingAddress, true);
+      const voting = await IVotingReputation.at(votingAddress);
+
+      await voting.initialise(WAD.divn(1000), 0, 0, WAD, SECONDS_PER_DAY, SECONDS_PER_DAY, SECONDS_PER_DAY, SECONDS_PER_DAY);
+
+      await korporatio.createFreeApplication({ from: USER0 });
+      const applicationId = await korporatio.getNumApplications();
+
+      const action = await encodeTxData(korporatio, "submitApplication", [applicationId]);
+
+      // Can't create a motion in a subdomain
+      await colony.addDomain(1, UINT256_MAX, 1);
+      await checkErrorRevert(
+        voting.createMotion(2, UINT256_MAX, korporatio.address, action, domain1Key, domain1Value, domain1Mask, domain1Siblings),
+        "voting-rep-invalid-domain-id"
+      );
+
+      // Only in the root domain
+      await voting.createMotion(1, UINT256_MAX, korporatio.address, action, domain1Key, domain1Value, domain1Mask, domain1Siblings);
+      const motionId = await voting.getMotionCount();
 
-      const tx = await korporatio.submitApplication(applicationId, ipfsHash, { from: USER0 });
-      await expectEvent(tx, "ApplicationSubmitted", [applicationId, ipfsHash]);
+      await colony.approveStake(voting.address, 1, WAD, { from: USER0 });
+      await voting.stakeMotion(motionId, 1, UINT256_MAX, 1, WAD.muln(3).divn(1000), user0Key, user0Value, user0Mask, user0Siblings, { from: USER0 });
 
-      const metaColonyAddress = await colonyNetwork.getMetaColony();
-      const metaColonyBalance = await token.balanceOf(metaColonyAddress);
-      expect(metaColonyBalance).to.eq.BN(APPLICATION_FEE);
+      await forwardTime(SECONDS_PER_DAY, this);
 
-      // Cannot submit once cancelled
-      await checkErrorRevert(korporatio.submitApplication(applicationId, ipfsHash, { from: USER0 }), "korporatio-stake-cancelled");
+      const tx = await voting.finalizeMotion(motionId);
+      const finalizedAt = await getBlockTime(tx.blockNumber);
+
+      const application = await korporatio.getApplication(applicationId);
+      expect(application.cancelledAt).to.eq.BN(finalizedAt);
     });
 
     it("can submit a stake via metatransactions", async () => {