Introduce Korporatio extension #1122

kronosapiens · 2023-02-24T00:57:31Z

Current flow:

Users can stake to create applications using submitApplication
Admin or root users can stake for free using submitFreeApplication
Users can cancel their stake using cancelApplication
Users can reclaim their stake after cancelling, once the claim delay has elapsed, using reclaimStake
Arbitration users can slash a stake, using slashStake, optionally slashing reputation
Users can update the IPFS hash of the application using updateApplication
Root users can submit the application (emitting an event) and cancel the stake, using submitApplication
Architecture users can update the stakeFraction and claimDelay values using the respective setter functions

The intention is that the claim delay allows for the colony to punish an applicant for malicious behavior, and that the final submission of the application take place via a motion. Updating the IPFS hash emits an event, which can be used by the Dapp to render the UI.

kronosapiens · 2023-03-09T21:19:03Z

@arrenv @area take a look and let me know what you think. I tried to provide functionality to support both the centralized and decentralized use cases.

@arrenv could I have more clarification on what you intend for paying for the application. Would that be a motion that transfers funds to... the metacolony?

arrenv · 2023-03-15T11:27:39Z

@arrenv @area take a look and let me know what you think. I tried to provide functionality to support both the centralized and decentralized use cases.

@arrenv could I have more clarification on what you intend for paying for the application. Would that be a motion that transfers funds to... the metacolony?

Sorry for missing this until now.

Question, is the claim delay configurable? Can the claim not just be returned with the cancellation? Also, this would likely require additional UI, right?

Payment for the application will be done using a Motion, I feel the best implementation of this is for the payment to go to the MetaColony. However, the original intention was to have it go to a hard-coded address directly to the incorporation service provider. Do you have any opinions on this from a contract perspective?

kronosapiens · 2023-03-15T17:43:52Z

@arrenv the claim delay is configurable. The idea is that if a user cancels the application, there is still time to slash the stake in the case that a user is malicious. If they could immediately cancel and reclaim their stake, then there would be no way to slash them.

My suggestion would be for a function on the extension to take the funds (as part of the submit function) and transfer them to the metacolony. That way the motion is a single function call to the extension. The extension could allow the application fee to be configured by the colony, or it can be hard-coded. Up to you. Normally I would be against hard-coding but I could see the value in this case.

Also, I'm assuming the application fee should be paid in CLNY? Or xDAI?

arrenv · 2023-03-15T18:41:46Z

@kronosapiens What would be the downside of them cancelling it themselves straight away? What impact would have have on the Colony as a bad actor?

Not saying we shouldn't have it, but I am curious as to the importance of it. It is also something we will need to account for in the UI.

I do prefer the payment to be made into the Metacolony being the flow. The downside of it though is that it then becomes an issue of tracking/reconciling that payment and then manually processing it to be sent to the incorporation service provider.

The application fee is currently denominated in USDC.

kronosapiens · 2023-03-15T18:48:31Z

@arrenv see here: #1118 (comment)

arrenv · 2023-03-15T18:58:31Z

@arrenv see here: #1118 (comment)

Thank you, I recall it now. I will make a note of it for the UI updates.

area

Given that the spec is that this works with voting, we should have some tests along those lines in here - and in particular, ones that make sure that even if the voting extension has permissions in root (as it usually does), then motions in subdomains cannot interfere unexpectedly.

contracts/extensions/Korporatio.sol

kronosapiens · 2023-03-27T15:20:12Z

Noting that @area is asking for a test showing that stakes can be returned to a user if the extension is deleted

kronosapiens · 2023-04-05T15:47:07Z

@area @arrenv this is ready again, with tests added showing the desired behavior. The information regarding the application fee has been removed, with the understanding that colonies will send the fee to the MC separately.

area

One of the requirements is

Be able to cancel a staked application with a motion.

Which is marked as done in the original issue, but I don't believe this is possible, as cancelling is limited to onlyApplicant?

Similarly:

Anyone else can make change requests to the application, but changes will require a motion.

The requirements go on to say

If it proves unsuitable we may be able to consider limiting changing the application details to the owner only.

Emphasis my own. What's the state of discussions on this?

contracts/extensions/Korporatio.sol

kronosapiens · 2023-04-20T21:10:55Z

@area the slashStake function allows an application to be cancelled with a motion. This also slashes the user's stake. The scenario where a colony would want to cancel a motion but not slash the stake seems unnecessary to me.

Regarding the editing of the motion, I made the argument that such backs-and-forth can occur off-chain, and the applicant can update the IPFS hash as needed. I suppose we could allow updateApplication to be called via a root motion, if that is important.

@arrenv thoughts on these?

arrenv · 2023-04-21T10:09:34Z

@area the slashStake function allows an application to be cancelled with a motion. This also slashes the user's stake. The scenario where a colony would want to cancel a motion but not slash the stake seems unnecessary to me.

It would still be necessary to cancel an application and have the option not to slash a stake. An example scenario of this is that it has been decided to delay the application to a date in the future, and the application was not created in malice.

Shown in the design with the force toggle, and the option to show mercy:

Regarding the editing of the motion, I made the argument that such backs-and-forth can occur off-chain, and the applicant can update the IPFS hash as needed. I suppose we could allow updateApplication to be called via a root motion, if that is important.

It would allow more flexibility to allow people other then the applicant to make changes via a motion, otherwise full control of the application itself remains with a single person. Editing via a motion is how it was spec'd and designed for this reason.

kronosapiens · 2023-04-21T15:29:27Z

@arrenv currently you can "show mercy" by not penalizing their reputation. Are you saying that "showing mercy" would be not slashing tokens OR penalizing reputation? Should those be two different options, or should they always come together, i.e. either you slash both tokens and rep or neither?

arrenv · 2023-04-21T16:00:16Z

@arrenv currently you can "show mercy" by not penalizing their reputation. Are you saying that "showing mercy" would be not slashing tokens OR penalizing reputation? Should those be two different options, or should they always come together, i.e. either you slash both tokens and rep or neither?

Yes, I think it would be better to just keep them as one as you said. I.e. 'Smite' would slash both both tokens and rep, 'Show mercy' would not slash both tokens and rep.

kronosapiens · 2023-04-24T22:26:47Z

I've updated the PR to allow for applications to be updated either by the creator or via a motion, as well as the other changes we've discussed.

area · 2023-05-04T13:01:29Z

contracts/extensions/Korporatio.sol

+    require(applications[_applicationId].cancelledAt == UINT256_MAX, "korporatio-stake-cancelled");
+    require(
+      msgSender() == applications[_applicationId].applicant ||
+      colony.hasUserRole(msgSender(), 1, ColonyDataTypes.ColonyRole.Root),


~~This still does not meet the specification:~~

~~> Anyone else can make change requests to the application, but changes will require a motion.~~

~~I get the intention (we give the Voting Extension root permission) but it allows anyone with root permission to update an application.~~

EDIT: After talking with Arren, it being controlled by the Root permission is acceptable. I've still left my original comment just to (admittedly) let me get on my traditional soapbox and say it is important to either a) Stick to the specification or b) get the specification changed by the owner of it if you disagree with it.

area · 2023-05-04T13:20:52Z

contracts/extensions/Korporatio.sol

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


This function should presumably work when an application has been made without staking?

area · 2023-05-04T13:21:55Z

contracts/extensions/Korporatio.sol

+    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, repFraction);


Given things key off of stakeAmount, should it be required to be nonzero here to ensure the lifecycle doesn't go off the rails? (After talking to Arren, it should be)

Or possibly nothing should be gated behind stakeAmount as there is the option to create without a stake.

area · 2023-05-04T13:28:33Z

contracts/extensions/Korporatio.sol

+
+    stakeFraction = _stakeFraction;
+    repFraction = _repFraction;
+    claimDelay = _claimDelay;


All three of these are missing validation on the supplied values.

area · 2023-05-04T13:31:21Z

contracts/extensions/Korporatio.sol

+
+  // Public
+
+  function initialise(uint256 _stakeFraction, uint256 _repFraction, uint256 _claimDelay) public {


Many of the public functions are missing natspec comments. In particular, be sure to include when something is a wad.

kronosapiens marked this pull request as draft February 24, 2023 00:57

kronosapiens force-pushed the feat/korporatio branch 2 times, most recently from 0cc0363 to dc2df16 Compare February 25, 2023 01:45

kronosapiens changed the title ~~Introduce UxGate extension~~ Introduce Korporatio extension Feb 27, 2023

kronosapiens force-pushed the feat/korporatio branch 2 times, most recently from f5e9317 to 5ace2e1 Compare March 1, 2023 21:59

kronosapiens added the in-progress label Mar 1, 2023

kronosapiens force-pushed the feat/korporatio branch 2 times, most recently from dd5fc04 to 60a8ee6 Compare March 9, 2023 15:38

kronosapiens added ready-for-review and removed in-progress labels Mar 9, 2023

kronosapiens marked this pull request as ready for review March 9, 2023 17:07

kronosapiens force-pushed the feat/korporatio branch 2 times, most recently from 883d889 to e9c2194 Compare March 14, 2023 22:49

kronosapiens force-pushed the feat/korporatio branch 5 times, most recently from 99e9fc3 to 95aa18a Compare March 20, 2023 22:19

area reviewed Mar 23, 2023

View reviewed changes

kronosapiens force-pushed the feat/korporatio branch 3 times, most recently from 10f7e81 to c4e79ec Compare April 5, 2023 15:44

kronosapiens force-pushed the feat/korporatio branch 2 times, most recently from e1d2819 to 0844a1c Compare April 6, 2023 19:47

area reviewed Apr 18, 2023

View reviewed changes

contracts/extensions/Korporatio.sol Show resolved Hide resolved

contracts/extensions/Korporatio.sol Outdated Show resolved Hide resolved

kronosapiens added 3 commits April 24, 2023 15:27

Introduce Korporatio extension

203a26d

Update in response to review comments

72df91e

Update in response to review comments II

24a488e

kronosapiens force-pushed the feat/korporatio branch from fabe0fe to 24a488e Compare April 24, 2023 22:31

area reviewed May 4, 2023

View reviewed changes

kronosapiens added in-progress and removed ready-for-review labels Jun 7, 2023

kronosapiens added on-hold and removed in-progress labels Jul 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Korporatio extension #1122

Introduce Korporatio extension #1122

kronosapiens commented Feb 24, 2023 •

edited

Loading

kronosapiens commented Mar 9, 2023

arrenv commented Mar 15, 2023

kronosapiens commented Mar 15, 2023 •

edited

Loading

arrenv commented Mar 15, 2023

kronosapiens commented Mar 15, 2023

arrenv commented Mar 15, 2023

area left a comment

kronosapiens commented Mar 27, 2023

kronosapiens commented Apr 5, 2023

area left a comment

kronosapiens commented Apr 20, 2023 •

edited

Loading

arrenv commented Apr 21, 2023 •

edited

Loading

kronosapiens commented Apr 21, 2023

arrenv commented Apr 21, 2023

kronosapiens commented Apr 24, 2023

area May 4, 2023

area May 4, 2023

area May 4, 2023

area May 4, 2023

area May 4, 2023


		// Public

		function initialise(uint256 _stakeFraction, uint256 _repFraction, uint256 _claimDelay) public {

Introduce Korporatio extension #1122

Are you sure you want to change the base?

Introduce Korporatio extension #1122

Conversation

kronosapiens commented Feb 24, 2023 • edited Loading

kronosapiens commented Mar 9, 2023

arrenv commented Mar 15, 2023

kronosapiens commented Mar 15, 2023 • edited Loading

arrenv commented Mar 15, 2023

kronosapiens commented Mar 15, 2023

arrenv commented Mar 15, 2023

area left a comment

Choose a reason for hiding this comment

kronosapiens commented Mar 27, 2023

kronosapiens commented Apr 5, 2023

area left a comment

Choose a reason for hiding this comment

kronosapiens commented Apr 20, 2023 • edited Loading

arrenv commented Apr 21, 2023 • edited Loading

kronosapiens commented Apr 21, 2023

arrenv commented Apr 21, 2023

kronosapiens commented Apr 24, 2023

area May 4, 2023

Choose a reason for hiding this comment

area May 4, 2023

Choose a reason for hiding this comment

area May 4, 2023

Choose a reason for hiding this comment

area May 4, 2023

Choose a reason for hiding this comment

area May 4, 2023

Choose a reason for hiding this comment

kronosapiens commented Feb 24, 2023 •

edited

Loading

kronosapiens commented Mar 15, 2023 •

edited

Loading

kronosapiens commented Apr 20, 2023 •

edited

Loading

arrenv commented Apr 21, 2023 •

edited

Loading