Skip to content

Commit fabe0fe

Browse files
kronosapienskronosapiens
committed
Update in response to review comments II
1 parent 0844a1c commit fabe0fe

File tree

2 files changed

+74
-44
lines changed

2 files changed

+74
-44
lines changed

‎contracts/extensions/Korporatio.sol

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ contract Korporatio is ColonyExtensionMeta {
3434
event ApplicationCreated(uint256 indexed stakeId, address indexed applicant);
3535
event ApplicationCancelled(uint256 indexed stakeId);
3636
event StakeReclaimed(uint256 indexed stakeId);
37-
event StakeSlashed(uint256 indexed stakeId);
37+
event ApplicationDeleted(uint256 indexed stakeId, bool punish);
3838
event ApplicationUpdated(uint256 indexed stakeId, bytes32 ipfsHash);
3939
event ApplicationSubmitted(uint256 indexed stakeId);
4040

@@ -51,6 +51,7 @@ contract Korporatio is ColonyExtensionMeta {
5151
address colonyNetworkAddress;
5252

5353
uint256 stakeFraction;
54+
uint256 repFraction;
5455
uint256 claimDelay;
5556

5657
uint256 numApplications;
@@ -99,13 +100,14 @@ contract Korporatio is ColonyExtensionMeta {
99100

100101
// Public
101102

102-
function initialise(uint256 _stakeFraction, uint256 _claimDelay) public {
103+
function initialise(uint256 _stakeFraction, uint256 _repFraction, uint256 _claimDelay) public {
103104
require(
104105
colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Architecture),
105106
"korporatio-not-root-architect"
106107
);
107108

108109
stakeFraction = _stakeFraction;
110+
repFraction = _repFraction;
109111
claimDelay = _claimDelay;
110112
}
111113

@@ -130,7 +132,7 @@ contract Korporatio is ColonyExtensionMeta {
130132
uint256 colonyReputation = checkReputation(rootHash, rootSkillId, address(0x0), _colonyKey, _colonyValue, _colonyBranchMask, _colonySiblings);
131133
uint256 userReputation = checkReputation(rootHash, rootSkillId, msgSender(), _userKey, _userValue, _userBranchMask, _userSiblings);
132134

133-
uint256 requiredStake = wmul(colonyReputation, stakeFraction);
135+
uint256 requiredStake = wmul(colonyReputation, repFraction);
134136
require(userReputation >= requiredStake, "korporatio-insufficient-rep");
135137

136138
applications[++numApplications] = Application({
@@ -144,7 +146,7 @@ contract Korporatio is ColonyExtensionMeta {
144146
emit ApplicationCreated(numApplications, msgSender());
145147
}
146148

147-
function createFreeApplication() public notDeprecated {
149+
function createApplication() public notDeprecated {
148150
require (
149151
colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Root) ||
150152
colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Administration),
@@ -160,13 +162,14 @@ contract Korporatio is ColonyExtensionMeta {
160162
emit ApplicationCreated(numApplications, msgSender());
161163
}
162164

163-
function cancelApplication(uint256 _applicationId) public onlyApplicant(_applicationId) {
165+
function cancelApplication(uint256 _applicationId) public {
166+
require(msgSender() == applications[_applicationId].applicant, "korporatio-not-applicant");
164167
applications[_applicationId].cancelledAt = block.timestamp;
165168

166169
emit ApplicationCancelled(_applicationId);
167170
}
168171

169-
function reclaimStake(uint256 _applicationId) public onlyApplicant(_applicationId) {
172+
function reclaimStake(uint256 _applicationId) public {
170173
Application storage application = applications[_applicationId];
171174
require(application.applicant == msgSender(), "korporatio-not-applicant");
172175
require(application.cancelledAt + claimDelay <= block.timestamp, "korporatio-cannot-reclaim");
@@ -179,7 +182,7 @@ contract Korporatio is ColonyExtensionMeta {
179182
emit StakeReclaimed(_applicationId);
180183
}
181184

182-
function slashStake(uint256 _applicationId, bool _punish) public {
185+
function deleteApplication(uint256 _applicationId, bool _punish) public {
183186
require(applications[_applicationId].stakeAmount > 0, "korporatio-cannot-slash");
184187

185188
require(
@@ -191,14 +194,23 @@ contract Korporatio is ColonyExtensionMeta {
191194
uint256 stakeAmount = applications[_applicationId].stakeAmount;
192195
delete applications[_applicationId];
193196

194-
colony.transferStake(1, UINT256_MAX, address(this), applicant, 1, stakeAmount, address(0x0));
195-
if (_punish) { colony.emitDomainReputationPenalty(1, UINT256_MAX, 1, applicant, -int256(stakeAmount)); }
197+
if (_punish) {
198+
colony.emitDomainReputationPenalty(1, UINT256_MAX, 1, applicant, -int256(stakeAmount));
199+
colony.transferStake(1, UINT256_MAX, address(this), applicant, 1, stakeAmount, address(0x0));
200+
} else {
201+
colony.deobligateStake(applicant, 1, stakeAmount);
202+
}
196203

197-
emit StakeSlashed(_applicationId);
204+
emit ApplicationDeleted(_applicationId, _punish);
198205
}
199206

200-
function updateApplication(uint256 _applicationId, bytes32 _ipfsHash) public onlyApplicant(_applicationId) {
207+
function updateApplication(uint256 _applicationId, bytes32 _ipfsHash) public {
201208
require(applications[_applicationId].cancelledAt == UINT256_MAX, "korporatio-stake-cancelled");
209+
require(
210+
msgSender() == applications[_applicationId].applicant ||
211+
colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Root),
212+
"korporatio-not-applicant-or-root"
213+
);
202214

203215
emit ApplicationUpdated(_applicationId, _ipfsHash);
204216
}

‎test/extensions/korporatio.js

Lines changed: 51 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ contract("Korporatio", (accounts) => {
4545
let tokenLocking;
4646

4747
let korporatio;
48+
let createApplication;
49+
let createFreeApplication;
4850
let version;
4951

5052
let reputationTree;
@@ -62,6 +64,7 @@ contract("Korporatio", (accounts) => {
6264
const USER0 = accounts[0];
6365
const USER1 = accounts[1];
6466
const USER2 = accounts[2];
67+
const USER3 = accounts[3];
6568
const MINER = accounts[5];
6669

6770
before(async () => {
@@ -82,10 +85,13 @@ contract("Korporatio", (accounts) => {
8285
await colony.installExtension(KORPORATIO, version);
8386
const korporatioAddress = await colonyNetwork.getExtensionInstallation(KORPORATIO, colony.address);
8487
korporatio = await Korporatio.at(korporatioAddress);
88+
createApplication = korporatio.methods["createApplication(bytes,bytes,uint256,bytes32[],bytes,bytes,uint256,bytes32[])"];
89+
createFreeApplication = korporatio.methods["createApplication()"];
8590

8691
await colony.setArchitectureRole(1, UINT256_MAX, USER0, 1, true);
8792
await colony.setArbitrationRole(1, UINT256_MAX, USER1, 1, true);
8893
await colony.setAdministrationRole(1, UINT256_MAX, USER1, 1, true);
94+
await colony.setRootRole(USER2, true);
8995
await colony.setArbitrationRole(1, UINT256_MAX, korporatio.address, 1, true);
9096

9197
await token.mint(USER0, WAD);
@@ -173,9 +179,7 @@ contract("Korporatio", (accounts) => {
173179

174180
it("cannot create applications unless initialised", async () => {
175181
await checkErrorRevert(
176-
korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
177-
from: USER0,
178-
}),
182+
createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, { from: USER0 }),
179183
"korporatio-not-initialised"
180184
);
181185
});
@@ -185,7 +189,7 @@ contract("Korporatio", (accounts) => {
185189
beforeEach(async () => {
186190
await colony.approveStake(korporatio.address, 1, WAD, { from: USER0 });
187191

188-
await korporatio.initialise(WAD.divn(100), SECONDS_PER_DAY, { from: USER0 });
192+
await korporatio.initialise(WAD.divn(100), WAD.divn(100), SECONDS_PER_DAY, { from: USER0 });
189193
});
190194

191195
it("can query for configuration params", async () => {
@@ -197,11 +201,11 @@ contract("Korporatio", (accounts) => {
197201
});
198202

199203
it("cannot set configuration params if not root architect", async () => {
200-
await checkErrorRevert(korporatio.initialise(WAD.divn(100), SECONDS_PER_DAY, { from: USER1 }), "korporatio-not-root-architect");
204+
await checkErrorRevert(korporatio.initialise(WAD.divn(100), WAD.divn(100), SECONDS_PER_DAY, { from: USER1 }), "korporatio-not-root-architect");
201205
});
202206

203207
it("can create an application", async () => {
204-
await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
208+
await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
205209
from: USER0,
206210
});
207211

@@ -216,7 +220,7 @@ contract("Korporatio", (accounts) => {
216220
});
217221

218222
it("can create a free application if root or admin", async () => {
219-
await korporatio.createFreeApplication({ from: USER1 });
223+
await createFreeApplication({ from: USER1 });
220224

221225
const applicationId = await korporatio.getNumApplications();
222226
const application = await korporatio.getApplication(applicationId);
@@ -225,14 +229,14 @@ contract("Korporatio", (accounts) => {
225229
expect(application.cancelledAt).to.eq.BN(UINT256_MAX);
226230

227231
// Must have root or admin role
228-
await checkErrorRevert(korporatio.createFreeApplication({ from: USER2 }), "korporatio-must-submit-stake");
232+
await checkErrorRevert(createFreeApplication({ from: USER3 }), "korporatio-must-submit-stake");
229233
});
230234

231235
it("cannot create an application with insufficient rep", async () => {
232-
await korporatio.initialise(WAD, SECONDS_PER_DAY, { from: USER0 });
236+
await korporatio.initialise(new BN(1), WAD, SECONDS_PER_DAY, { from: USER0 });
233237

234238
await checkErrorRevert(
235-
korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
239+
createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
236240
from: USER0,
237241
}),
238242
"korporatio-insufficient-rep"
@@ -243,17 +247,17 @@ contract("Korporatio", (accounts) => {
243247
await colony.deprecateExtension(KORPORATIO, true);
244248

245249
await checkErrorRevert(
246-
korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
250+
createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
247251
from: USER0,
248252
}),
249253
"colony-extension-deprecated"
250254
);
251255

252-
await checkErrorRevert(korporatio.createFreeApplication({ from: USER1 }), "colony-extension-deprecated");
256+
await checkErrorRevert(createFreeApplication({ from: USER1 }), "colony-extension-deprecated");
253257
});
254258

255259
it("can cancel an application", async () => {
256-
await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
260+
await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
257261
from: USER0,
258262
});
259263

@@ -270,7 +274,7 @@ contract("Korporatio", (accounts) => {
270274
});
271275

272276
it("can reclaim a stake", async () => {
273-
await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
277+
await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
274278
from: USER0,
275279
});
276280

@@ -288,29 +292,35 @@ contract("Korporatio", (accounts) => {
288292
expect(obligation).to.be.zero;
289293
});
290294

291-
it("can slash a stake", async () => {
292-
await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
295+
it("can delete an application without punishing", async () => {
296+
await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
293297
from: USER0,
294298
});
295299

296300
const applicationId = await korporatio.getNumApplications();
297-
await korporatio.slashStake(applicationId, false, { from: USER1 });
301+
await korporatio.deleteApplication(applicationId, false, { from: USER1 });
298302

299303
const obligation = await colony.getObligation(USER0, korporatio.address, 1);
300304
expect(obligation).to.be.zero;
305+
306+
const lock = await tokenLocking.getUserLock(token.address, USER0);
307+
expect(lock.balance).to.eq.BN(WAD);
301308
});
302309

303-
it("can slash a stake and punish", async () => {
304-
await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
310+
it("can delete an application and punish", async () => {
311+
await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
305312
from: USER0,
306313
});
307314

308315
const applicationId = await korporatio.getNumApplications();
309-
await korporatio.slashStake(applicationId, true, { from: USER1 });
316+
await korporatio.deleteApplication(applicationId, true, { from: USER1 });
310317

311318
const obligation = await colony.getObligation(USER0, korporatio.address, 1);
312319
expect(obligation).to.be.zero;
313320

321+
const lock = await tokenLocking.getUserLock(token.address, USER0);
322+
expect(lock.balance).to.eq.BN(WAD.sub(WAD.divn(100).muln(3)));
323+
314324
// Staker gets a reputation penalty
315325
const addr = await colonyNetwork.getReputationMiningCycle(false);
316326
const repCycle = await IReputationMiningCycle.at(addr);
@@ -323,21 +333,21 @@ contract("Korporatio", (accounts) => {
323333
});
324334

325335
it("cannot slash a nonexistent stake", async () => {
326-
await checkErrorRevert(korporatio.slashStake(10, false, { from: USER1 }), "korporatio-cannot-slash");
336+
await checkErrorRevert(korporatio.deleteApplication(10, false, { from: USER1 }), "korporatio-cannot-slash");
327337
});
328338

329339
it("cannot slash if not an arbitration user", async () => {
330-
await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
340+
await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
331341
from: USER0,
332342
});
333343

334344
const applicationId = await korporatio.getNumApplications();
335-
await checkErrorRevert(korporatio.slashStake(applicationId, false, { from: USER2 }), "korporatio-caller-not-arbitration");
345+
await checkErrorRevert(korporatio.deleteApplication(applicationId, false, { from: USER2 }), "korporatio-caller-not-arbitration");
336346
});
337347

338348
it("can reclaim a stake via arbitration if the extension is deleted", async () => {
339349
const korporatioAddress = korporatio.address;
340-
await korporatio.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
350+
await createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings, {
341351
from: USER0,
342352
});
343353

@@ -357,25 +367,35 @@ contract("Korporatio", (accounts) => {
357367
expect(new BN(lockPre.balance)).to.eq.BN(lockPost.balance);
358368
});
359369

360-
it("can update an application", async () => {
361-
await korporatio.createFreeApplication({ from: USER0 });
370+
it("can update an application as the applicant", async () => {
371+
await createFreeApplication({ from: USER0 });
362372

363373
const applicationId = await korporatio.getNumApplications();
364374
const ipfsHash = soliditySha3("IPFS Hash");
365375

366376
const tx = await korporatio.updateApplication(applicationId, ipfsHash, { from: USER0 });
367377
await expectEvent(tx, "ApplicationUpdated", [applicationId, ipfsHash]);
368378

369-
// Cannot update if not applicant
370-
await checkErrorRevert(korporatio.updateApplication(applicationId, ipfsHash, { from: USER1 }), "korporatio-not-applicant");
379+
// Cannot update if not applicant or root
380+
await checkErrorRevert(korporatio.updateApplication(applicationId, ipfsHash, { from: USER1 }), "korporatio-not-applicant-or-root");
371381

372382
// Cannot update once cancelled
373383
await korporatio.cancelApplication(applicationId, { from: USER0 });
374384
await checkErrorRevert(korporatio.updateApplication(applicationId, ipfsHash, { from: USER0 }), "korporatio-stake-cancelled");
375385
});
376386

387+
it("can update an application as a root user", async () => {
388+
await createFreeApplication({ from: USER0 });
389+
390+
const applicationId = await korporatio.getNumApplications();
391+
const ipfsHash = soliditySha3("IPFS Hash");
392+
393+
const tx = await korporatio.updateApplication(applicationId, ipfsHash, { from: USER2 });
394+
await expectEvent(tx, "ApplicationUpdated", [applicationId, ipfsHash]);
395+
});
396+
377397
it("can submit an application", async () => {
378-
await korporatio.createFreeApplication({ from: USER0 });
398+
await createFreeApplication({ from: USER0 });
379399

380400
const applicationId = await korporatio.getNumApplications();
381401

@@ -398,7 +418,7 @@ contract("Korporatio", (accounts) => {
398418

399419
await voting.initialise(WAD.divn(1000), 0, 0, WAD, SECONDS_PER_DAY, SECONDS_PER_DAY, SECONDS_PER_DAY, SECONDS_PER_DAY);
400420

401-
await korporatio.createFreeApplication({ from: USER0 });
421+
await createFreeApplication({ from: USER0 });
402422
const applicationId = await korporatio.getNumApplications();
403423

404424
const action = await encodeTxData(korporatio, "submitApplication", [applicationId]);
@@ -429,9 +449,7 @@ contract("Korporatio", (accounts) => {
429449
it("can submit a stake via metatransactions", async () => {
430450
await colony.approveStake(korporatio.address, 1, WAD, { from: USER0 });
431451

432-
const txData = await korporatio.contract.methods
433-
.createApplication(domain1Key, domain1Value, domain1Mask, domain1Siblings, user0Key, user0Value, user0Mask, user0Siblings)
434-
.encodeABI();
452+
const txData = await korporatio.contract.methods["createApplication()"]().encodeABI();
435453
const { r, s, v } = await getMetaTransactionParameters(txData, USER0, korporatio.address);
436454
await korporatio.executeMetaTransaction(USER0, txData, r, s, v, { from: USER0 });
437455

0 commit comments

Comments
 (0)