From fabe0fef53b7fdd3cfaca89effdb9d65bd8e77f5 Mon Sep 17 00:00:00 2001 From: Daniel Kronovet Date: Mon, 24 Apr 2023 15:25:59 -0700 Subject: [PATCH] Update in response to review comments II --- contracts/extensions/Korporatio.sol | 34 ++++++++---- test/extensions/korporatio.js | 84 +++++++++++++++++------------ 2 files changed, 74 insertions(+), 44 deletions(-) diff --git a/contracts/extensions/Korporatio.sol b/contracts/extensions/Korporatio.sol index bf3a259fac..30e609c21a 100644 --- a/contracts/extensions/Korporatio.sol +++ b/contracts/extensions/Korporatio.sol @@ -34,7 +34,7 @@ contract Korporatio is ColonyExtensionMeta { event ApplicationCreated(uint256 indexed stakeId, address indexed applicant); event ApplicationCancelled(uint256 indexed stakeId); event StakeReclaimed(uint256 indexed stakeId); - event StakeSlashed(uint256 indexed stakeId); + event ApplicationDeleted(uint256 indexed stakeId, bool punish); event ApplicationUpdated(uint256 indexed stakeId, bytes32 ipfsHash); event ApplicationSubmitted(uint256 indexed stakeId); @@ -51,6 +51,7 @@ contract Korporatio is ColonyExtensionMeta { address colonyNetworkAddress; uint256 stakeFraction; + uint256 repFraction; uint256 claimDelay; uint256 numApplications; @@ -99,13 +100,14 @@ contract Korporatio is ColonyExtensionMeta { // Public - function initialise(uint256 _stakeFraction, uint256 _claimDelay) public { + function initialise(uint256 _stakeFraction, uint256 _repFraction, uint256 _claimDelay) public { require( colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Architecture), "korporatio-not-root-architect" ); stakeFraction = _stakeFraction; + repFraction = _repFraction; claimDelay = _claimDelay; } @@ -130,7 +132,7 @@ contract Korporatio is ColonyExtensionMeta { uint256 colonyReputation = checkReputation(rootHash, rootSkillId, address(0x0), _colonyKey, _colonyValue, _colonyBranchMask, _colonySiblings); uint256 userReputation = checkReputation(rootHash, rootSkillId, msgSender(), _userKey, _userValue, _userBranchMask, _userSiblings); - uint256 requiredStake = wmul(colonyReputation, stakeFraction); + uint256 requiredStake = wmul(colonyReputation, repFraction); require(userReputation >= requiredStake, "korporatio-insufficient-rep"); applications[++numApplications] = Application({ @@ -144,7 +146,7 @@ contract Korporatio is ColonyExtensionMeta { emit ApplicationCreated(numApplications, msgSender()); } - function createFreeApplication() public notDeprecated { + function createApplication() public notDeprecated { require ( colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Root) || colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Administration), @@ -160,13 +162,14 @@ contract Korporatio is ColonyExtensionMeta { emit ApplicationCreated(numApplications, msgSender()); } - function cancelApplication(uint256 _applicationId) public onlyApplicant(_applicationId) { + function cancelApplication(uint256 _applicationId) public { + require(msgSender() == applications[_applicationId].applicant, "korporatio-not-applicant"); applications[_applicationId].cancelledAt = block.timestamp; emit ApplicationCancelled(_applicationId); } - function reclaimStake(uint256 _applicationId) public onlyApplicant(_applicationId) { + function reclaimStake(uint256 _applicationId) public { Application storage application = applications[_applicationId]; require(application.applicant == msgSender(), "korporatio-not-applicant"); require(application.cancelledAt + claimDelay <= block.timestamp, "korporatio-cannot-reclaim"); @@ -179,7 +182,7 @@ contract Korporatio is ColonyExtensionMeta { emit StakeReclaimed(_applicationId); } - function slashStake(uint256 _applicationId, bool _punish) public { + function deleteApplication(uint256 _applicationId, bool _punish) public { require(applications[_applicationId].stakeAmount > 0, "korporatio-cannot-slash"); require( @@ -191,14 +194,23 @@ contract Korporatio is ColonyExtensionMeta { uint256 stakeAmount = applications[_applicationId].stakeAmount; delete applications[_applicationId]; - colony.transferStake(1, UINT256_MAX, address(this), applicant, 1, stakeAmount, address(0x0)); - if (_punish) { colony.emitDomainReputationPenalty(1, UINT256_MAX, 1, applicant, -int256(stakeAmount)); } + if (_punish) { + colony.emitDomainReputationPenalty(1, UINT256_MAX, 1, applicant, -int256(stakeAmount)); + colony.transferStake(1, UINT256_MAX, address(this), applicant, 1, stakeAmount, address(0x0)); + } else { + colony.deobligateStake(applicant, 1, stakeAmount); + } - emit StakeSlashed(_applicationId); + emit ApplicationDeleted(_applicationId, _punish); } - function updateApplication(uint256 _applicationId, bytes32 _ipfsHash) public onlyApplicant(_applicationId) { + function updateApplication(uint256 _applicationId, bytes32 _ipfsHash) public { require(applications[_applicationId].cancelledAt == UINT256_MAX, "korporatio-stake-cancelled"); + require( + msgSender() == applications[_applicationId].applicant || + colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Root), + "korporatio-not-applicant-or-root" + ); emit ApplicationUpdated(_applicationId, _ipfsHash); } diff --git a/test/extensions/korporatio.js b/test/extensions/korporatio.js index 1b78aae4a9..731858d76c 100644 --- a/test/extensions/korporatio.js +++ b/test/extensions/korporatio.js @@ -45,6 +45,8 @@ contract("Korporatio", (accounts) => { let tokenLocking; let korporatio; + let createApplication; + let createFreeApplication; let version; let reputationTree; @@ -62,6 +64,7 @@ contract("Korporatio", (accounts) => { const USER0 = accounts[0]; const USER1 = accounts[1]; const USER2 = accounts[2]; + const USER3 = accounts[3]; const MINER = accounts[5]; before(async () => { @@ -82,10 +85,13 @@ contract("Korporatio", (accounts) => { await colony.installExtension(KORPORATIO, version); const korporatioAddress = await colonyNetwork.getExtensionInstallation(KORPORATIO, colony.address); korporatio = await Korporatio.at(korporatioAddress); + createApplication = korporatio.methods["createApplication(bytes,bytes,uint256,bytes32[],bytes,bytes,uint256,bytes32[])"]; + createFreeApplication = korporatio.methods["createApplication()"]; await colony.setArchitectureRole(1, UINT256_MAX, USER0, 1, true); await colony.setArbitrationRole(1, UINT256_MAX, USER1, 1, true); await colony.setAdministrationRole(1, UINT256_MAX, USER1, 1, true); + await colony.setRootRole(USER2, true); await colony.setArbitrationRole(1, UINT256_MAX, korporatio.address, 1, true); await token.mint(USER0, WAD); @@ -173,9 +179,7 @@ contract("Korporatio", (accounts) => { it("cannot create applications unless initialised", async () => { await checkErrorRevert( - korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { - from: USER0, - }), + createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0 }), "korporatio-not-initialised" ); }); @@ -185,7 +189,7 @@ contract("Korporatio", (accounts) => { beforeEach(async () => { await colony.approveStake(korporatio.address, 1, WAD, { from: USER0 }); - await korporatio.initialise(WAD.divn(100), SECONDS_PER_DAY, { from: USER0 }); + await korporatio.initialise(WAD.divn(100), WAD.divn(100), SECONDS_PER_DAY, { from: USER0 }); }); it("can query for configuration params", async () => { @@ -197,11 +201,11 @@ contract("Korporatio", (accounts) => { }); it("cannot set configuration params if not root architect", async () => { - await checkErrorRevert(korporatio.initialise(WAD.divn(100), SECONDS_PER_DAY, { from: USER1 }), "korporatio-not-root-architect"); + await checkErrorRevert(korporatio.initialise(WAD.divn(100), WAD.divn(100), SECONDS_PER_DAY, { from: USER1 }), "korporatio-not-root-architect"); }); it("can create an application", async () => { - await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { + await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }); @@ -216,7 +220,7 @@ contract("Korporatio", (accounts) => { }); it("can create a free application if root or admin", async () => { - await korporatio.createFreeApplication({ from: USER1 }); + await createFreeApplication({ from: USER1 }); const applicationId = await korporatio.getNumApplications(); const application = await korporatio.getApplication(applicationId); @@ -225,14 +229,14 @@ contract("Korporatio", (accounts) => { expect(application.cancelledAt).to.eq.BN(UINT256_MAX); // Must have root or admin role - await checkErrorRevert(korporatio.createFreeApplication({ from: USER2 }), "korporatio-must-submit-stake"); + await checkErrorRevert(createFreeApplication({ from: USER3 }), "korporatio-must-submit-stake"); }); it("cannot create an application with insufficient rep", async () => { - await korporatio.initialise(WAD, SECONDS_PER_DAY, { from: USER0 }); + await korporatio.initialise(new BN(1), WAD, SECONDS_PER_DAY, { from: USER0 }); await checkErrorRevert( - korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { + createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }), "korporatio-insufficient-rep" @@ -243,17 +247,17 @@ contract("Korporatio", (accounts) => { await colony.deprecateExtension(KORPORATIO, true); await checkErrorRevert( - korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { + createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }), "colony-extension-deprecated" ); - await checkErrorRevert(korporatio.createFreeApplication({ from: USER1 }), "colony-extension-deprecated"); + await checkErrorRevert(createFreeApplication({ from: USER1 }), "colony-extension-deprecated"); }); it("can cancel an application", async () => { - await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { + await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }); @@ -270,7 +274,7 @@ contract("Korporatio", (accounts) => { }); it("can reclaim a stake", async () => { - await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { + await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }); @@ -288,29 +292,35 @@ contract("Korporatio", (accounts) => { expect(obligation).to.be.zero; }); - it("can slash a stake", async () => { - await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { + it("can delete an application without punishing", async () => { + await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }); const applicationId = await korporatio.getNumApplications(); - await korporatio.slashStake(applicationId, false, { from: USER1 }); + await korporatio.deleteApplication(applicationId, false, { from: USER1 }); const obligation = await colony.getObligation(USER0, korporatio.address, 1); expect(obligation).to.be.zero; + + const lock = await tokenLocking.getUserLock(token.address, USER0); + expect(lock.balance).to.eq.BN(WAD); }); - it("can slash a stake and punish", async () => { - await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { + it("can delete an application and punish", async () => { + await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }); const applicationId = await korporatio.getNumApplications(); - await korporatio.slashStake(applicationId, true, { from: USER1 }); + await korporatio.deleteApplication(applicationId, true, { from: USER1 }); const obligation = await colony.getObligation(USER0, korporatio.address, 1); expect(obligation).to.be.zero; + const lock = await tokenLocking.getUserLock(token.address, USER0); + expect(lock.balance).to.eq.BN(WAD.sub(WAD.divn(100).muln(3))); + // Staker gets a reputation penalty const addr = await colonyNetwork.getReputationMiningCycle(false); const repCycle = await IReputationMiningCycle.at(addr); @@ -323,21 +333,21 @@ contract("Korporatio", (accounts) => { }); it("cannot slash a nonexistent stake", async () => { - await checkErrorRevert(korporatio.slashStake(10, false, { from: USER1 }), "korporatio-cannot-slash"); + await checkErrorRevert(korporatio.deleteApplication(10, false, { from: USER1 }), "korporatio-cannot-slash"); }); it("cannot slash if not an arbitration user", async () => { - await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { + await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }); const applicationId = await korporatio.getNumApplications(); - await checkErrorRevert(korporatio.slashStake(applicationId, false, { from: USER2 }), "korporatio-caller-not-arbitration"); + await checkErrorRevert(korporatio.deleteApplication(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, { + await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0, }); @@ -357,8 +367,8 @@ contract("Korporatio", (accounts) => { expect(new BN(lockPre.balance)).to.eq.BN(lockPost.balance); }); - it("can update an application", async () => { - await korporatio.createFreeApplication({ from: USER0 }); + it("can update an application as the applicant", async () => { + await createFreeApplication({ from: USER0 }); const applicationId = await korporatio.getNumApplications(); const ipfsHash = soliditySha3("IPFS Hash"); @@ -366,16 +376,26 @@ contract("Korporatio", (accounts) => { const tx = await korporatio.updateApplication(applicationId, ipfsHash, { from: USER0 }); await expectEvent(tx, "ApplicationUpdated", [applicationId, ipfsHash]); - // Cannot update if not applicant - await checkErrorRevert(korporatio.updateApplication(applicationId, ipfsHash, { from: USER1 }), "korporatio-not-applicant"); + // Cannot update if not applicant or root + await checkErrorRevert(korporatio.updateApplication(applicationId, ipfsHash, { from: USER1 }), "korporatio-not-applicant-or-root"); // Cannot update once cancelled await korporatio.cancelApplication(applicationId, { from: USER0 }); await checkErrorRevert(korporatio.updateApplication(applicationId, ipfsHash, { from: USER0 }), "korporatio-stake-cancelled"); }); + it("can update an application as a root user", async () => { + await createFreeApplication({ from: USER0 }); + + const applicationId = await korporatio.getNumApplications(); + const ipfsHash = soliditySha3("IPFS Hash"); + + const tx = await korporatio.updateApplication(applicationId, ipfsHash, { from: USER2 }); + await expectEvent(tx, "ApplicationUpdated", [applicationId, ipfsHash]); + }); + it("can submit an application", async () => { - await korporatio.createFreeApplication({ from: USER0 }); + await createFreeApplication({ from: USER0 }); const applicationId = await korporatio.getNumApplications(); @@ -398,7 +418,7 @@ contract("Korporatio", (accounts) => { 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 }); + await createFreeApplication({ from: USER0 }); const applicationId = await korporatio.getNumApplications(); const action = await encodeTxData(korporatio, "submitApplication", [applicationId]); @@ -429,9 +449,7 @@ contract("Korporatio", (accounts) => { it("can submit a stake via metatransactions", async () => { await colony.approveStake(korporatio.address, 1, WAD, { from: USER0 }); - const txData = await korporatio.contract.methods - .createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings) - .encodeABI(); + const txData = await korporatio.contract.methods["createApplication()"]().encodeABI(); const { r, s, v } = await getMetaTransactionParameters(txData, USER0, korporatio.address); await korporatio.executeMetaTransaction(USER0, txData, r, s, v, { from: USER0 });