Proposal for simple exit contract #362

sh-rp · 2021-03-03T13:40:22Z

No description provided.

banciur

More comments in conversation below PR

banciur · 2021-03-24T10:47:14Z

contracts/Company/Extras/ExitController.sol

+        }
+        // calculate the amount of eligible proceeds based on the total equity token supply and the
+        // acquisition price
+        return Math.mul(_exitAquisitionPriceEurUlps, amountTokens) / _exitEquityTokenSupply;


Sadly I don't know about high precision math but there is also Math.divRound maybe it would be better her than simple /

The div round does a round up from .5 upwards, and I thought it would be safer to always round down the way / does. Or what do you think?

banciur · 2021-03-24T10:52:45Z

contracts/Company/Extras/ExitController.sol

+
+        (uint256 _tokens, uint256 _proceeds) = eligibleProceedsForInvestor(lostWallet);
+        require(_proceeds > 0, "NF_NO_PROCEEDS");
+


Maybe it makes sense to check claims of newWallet and add requirement for KYC and bank account? It will help mitigate manual mistakes from nominee that will send those transactions.

I would say this is already done in the euro token controller contract in function isTransferAllowedPrivate(address from, address to, bool allowPeerTransfers). So wallets that are not verified can't receive our-t. Could you double check if you think this is the case too?

banciur · 2021-03-24T12:14:08Z

Some loose ideas (more like brainstorming):

I was thinking about manual phase. I think we should avoid manual contract operations as those are prone to copy paste errors and write script that would take data as csv file and do it automatically.
track addresses that would receive payout in manual process to not send eur twice to same address
- this can be done offchain when preparing csv
- this might be valid usecase so maybe we should not prevent it
Amount of EUR received from nominee will be known from start so it could be a constructor parameter and then validated when processing first transaction to make sure nominee would send correct amount
Idea for frontend app. Highlight minimum redeem amount and tx price to let people know that with current gas price it doesn't make sense. This depends of structure of owners.
I wonder could we and if we can does it makes sense to incorporate redeem into manual payout. Then tx cost will be lower.
Add minimum payout phase period. Just nice blockchainy phase to show we do it "right" and can't control everything.

contracts/Company/Extras/ExitController.sol

Co-authored-by: Arjun Umesha <arjunu@users.noreply.github.com>

desfero

Just small comment typos

desfero · 2021-03-26T07:04:29Z

contracts/KnownInterfaces.sol

@@ -100,5 +100,8 @@ contract KnownInterfaces {
    // Voting Center keccak256("IVotingCenter")
    bytes4 internal constant KNOWN_INTERFACE_VOTING_CENTER = 0xff5dbb18;

+    // Voting Center keccak256("ExitController")


Suggested change

// Voting Center keccak256("ExitController")

// Exit Controller keccak256("ExitController")

desfero · 2021-03-26T07:08:45Z

contracts/Company/Extras/ExitController.sol

+    ////////////////////////
+
+    //
+    // Implements IControllerGovernance


is that comment in place here? I'm wondering where state function implements IControllerGovernance

desfero · 2021-03-26T07:16:22Z

contracts/Company/Extras/ExitController.sol

+            startPayout();
+        }
+        // when we already are in the closing state, investors can send
+        // their tokens to  be burned and converted to euro token


Suggested change

// their tokens to be burned and converted to euro token

// their tokens to be burned and converted to euro token

desfero · 2021-03-27T08:14:19Z

contracts/Company/Extras/ExitController.sol

+
+    // exit values get set when exit proceedings start
+    uint256 private _exitEquityTokenSupply = 0;
+    uint256 private _exitAquisitionPriceEurUlps = 0;


Maybe a typo: Acquisition

desfero · 2021-03-27T08:29:52Z

contracts/Company/Extras/ExitController.sol

+    {
+        // payout euro tokens to investor
+        uint256 _eligibleProceeds = eligibleProceedsForTokens(equityTokenAmount);
+        EURO_TOKEN.transfer(investor, _eligibleProceeds, "");


In case transfer failed (for .e.g someone reclaimed EURO_TOKEN) we would still log event. Maybe we should wrap transfer in require?

I think the same applies to manual payout too

desfero · 2021-03-29T09:38:50Z

contracts/Company/Extras/ExitController.sol

+
+contract ExitController is
+    KnownInterfaces,
+    Reclaimable,


Let's disallow reclaiming EQUITY_TOKEN completely and EURO_TOKEN up until we reach either ManualPayoutResolution or a new Closed state

desfero · 2021-03-29T09:41:37Z

contracts/Company/Extras/ExitController.sol

+            equityTokens = EQUITY_TOKEN.balanceOf(investor);
+        }
+        else if (_state == State.ManualPayoutResolution) {
+            equityTokens = EQUITY_TOKEN.balanceOfAt(investor, _manualPayoutResolutionStart);


We can add support for users without claimed tokens here by interacting with the ETOCommitment smart-contract.
Pseudo-code

if (equityTokens == 0) { (claimedOrRefunded, equityTOkenAmount) = ETO_COMMITMENT.investorTicket(investor) if (claimedOrRefund == false) { equityTokens = equityTOkenAmount; } }

sh-rp force-pushed the d#/closing_token_controller branch from ec05f9d to 9fa1abb Compare March 3, 2021 13:41

add outline for exitcontroller contract v2

91ab01f

sh-rp force-pushed the d#/closing_token_controller branch 8 times, most recently from 9b8991d to ee0c757 Compare March 9, 2021 17:00

sh-rp marked this pull request as ready for review March 9, 2021 17:49

add tests for exit controller

b205aa9

sh-rp force-pushed the d#/closing_token_controller branch from ee0c757 to b205aa9 Compare March 14, 2021 10:30

banciur reviewed Mar 24, 2021

View reviewed changes

arjunu reviewed Mar 25, 2021

View reviewed changes

contracts/Company/Extras/ExitController.sol Show resolved Hide resolved

Update contracts/Company/Extras/ExitController.sol

6c644f5

Co-authored-by: Arjun Umesha <arjunu@users.noreply.github.com>

sh-rp force-pushed the d#/closing_token_controller branch from 3f3032c to 6c644f5 Compare March 25, 2021 15:07

desfero approved these changes Mar 26, 2021

View reviewed changes

desfero suggested changes Mar 29, 2021

View reviewed changes

desfero reviewed Mar 29, 2021

View reviewed changes

sh-rp force-pushed the d#/closing_token_controller branch from ed560c5 to 6291188 Compare March 29, 2021 13:07

create fixtures folder

e5f5004

sh-rp force-pushed the d#/closing_token_controller branch from 6291188 to e5f5004 Compare March 29, 2021 13:16

David Scharf added 5 commits March 29, 2021 15:35

reformat etos def

0edc4f2

add new exit eto fixture with investors

cd8501b

add exit contract fixtures

0be26f9

small fixes to contract

6c1639a

skip step when not needed

88c6850

fix state validation

2817b08

sh-rp force-pushed the d#/closing_token_controller branch from d8595cc to 2817b08 Compare March 29, 2021 21:02

sh-rp merged commit b31c442 into master Mar 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposal for simple exit contract #362

Proposal for simple exit contract #362

sh-rp commented Mar 3, 2021

banciur left a comment

banciur Mar 24, 2021 •

edited

Loading

sh-rp Mar 25, 2021

banciur Mar 24, 2021

sh-rp Mar 25, 2021

banciur commented Mar 24, 2021 •

edited

Loading

desfero left a comment

desfero Mar 26, 2021

desfero Mar 26, 2021

desfero Mar 26, 2021

desfero Mar 27, 2021

desfero Mar 27, 2021

desfero Mar 29, 2021

desfero Mar 29, 2021

desfero Mar 29, 2021


		(uint256 _tokens, uint256 _proceeds) = eligibleProceedsForInvestor(lostWallet);
		require(_proceeds > 0, "NF_NO_PROCEEDS");

	// Voting Center keccak256("ExitController")
	// Exit Controller keccak256("ExitController")

	// their tokens to be burned and converted to euro token
	// their tokens to be burned and converted to euro token

Proposal for simple exit contract #362

Proposal for simple exit contract #362

Conversation

sh-rp commented Mar 3, 2021

banciur left a comment

Choose a reason for hiding this comment

banciur Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banciur commented Mar 24, 2021 • edited Loading

desfero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

banciur Mar 24, 2021 •

edited

Loading

banciur commented Mar 24, 2021 •

edited

Loading