From 0844a1c6f8c011e36b7ddc7c8b8bf30812555a50 Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Mon, 3 Apr 2023 19:47:50 -0400 Subject: [PATCH] Update in response to review comments --- contracts/extensions/Korporatio.sol | 57 ++++-------- test-smoke/colony-storage-consistent.js | 2 +- test/extensions/korporatio.js | 111 ++++++++++++++++++------ 3 files changed, 102 insertions(+), 68 deletions(-) diff --git a/contracts/extensions/Korporatio.sol b/contracts/extensions/Korporatio.sol index 75f71ecf15..bf3a259fac 100644 --- a/contracts/extensions/Korporatio.sol +++ b/contracts/extensions/Korporatio.sol @@ -29,8 +29,6 @@ contract Korporatio is ColonyExtensionMeta { // Constants - uint256 constant APPLICATION_FEE = 6500 * WAD; - // Events event ApplicationCreated(uint256 indexed stakeId, address indexed applicant); @@ -38,7 +36,7 @@ contract Korporatio is ColonyExtensionMeta { 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; } diff --git a/test-smoke/colony-storage-consistent.js b/test-smoke/colony-storage-consistent.js index 3b095e0ff8..666a404478 100644 --- a/test-smoke/colony-storage-consistent.js +++ b/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"); diff --git a/test/extensions/korporatio.js b/test/extensions/korporatio.js index 2bdde5b93c..1b78aae4a9 100644 --- a/test/extensions/korporatio.js +++ b/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 () => {