Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion of code simplification: remove tributeTokens, lootTokens and GuildVault #7

Closed
alexvandesande opened this issue Jan 31, 2019 · 3 comments

Comments

@alexvandesande
Copy link

I'm looking through the Moloch DAO in light of recent changes and would like to add one further simplification: remove completely GuildVault and TributeTokens. Instead, here's how a proposal would work:

  • An applicant writes a contract that supports the interface proposalApproved()
  • Adds a field to the proposal which is called rewardContract
  • When a proposal is approved, Moloch calls the rewardContract with the function proposalApproved() which can any of these things:
    1. Sends X tokens to the Moloch DAO (here's $100 euro-DAI)
    2. Transfers a NFT to a specific address (transfers moloch.eth to ameen)
    3. Orders another DAO to execute some action (like buying shares in a prediction market to increase the likelyhood of something happening)
    4. Does nothing but records a promise from someone to make something off-chain ("I promise to explain Moloch using the art of interpretative dance while wearing a dinosaur suit")

This simplifies the code and allows greater flexibility, because anything can be coded on the proposalContract address, so we don't need to imagine everything that can be created in the future.

But what about the GuildVault and lootTokens? Maybe I'm missing something but it seems to me the only purpose of LootTokens is to be a temporary step before someone can take tokens out. Instead we can remove all that functionality and just require that when someone wants to rageQuit they need to specify which tokens they want to take with them. The DAO then will go through that list and then use send them the equivalent percentage of the balance of that specific token.

This has a few advantages, one of them being that it's up to the users themselves signal which token they care about personally. If you ignore a token it means you are basically donating your share of that token back to the community, therefore increasing the share that the next person can take: so undervalued tokens will accumulate until it's worth for people to spend the gas to take them out too.

This proposal both simplifies code and also keeps Moloch flexible enough for it to do unpredictable stuff in the future.

@ameensol
Copy link
Contributor

ameensol commented Jan 31, 2019

For reference, here are the various implementations of Moloch, in the order they were written:

All ERC20's supported
Only ETH + DAI
Only ETH

(there's some cleanup required on both new versions of the contract, but these commits should capture the gist of the changes needed)

Thanks for the feedback! A few things:

  1. The GuildBank is also helpful in separating the onchain value held by the Guild from proposal deposits and escrowed funds, which are transient. So that someone could easily look at etherscan and figure out how much money the Guild actually has. Even when I rewrote the contract to be ETH maximalist it was still easier to keep the tributed ETH in a separate contract to make the accounting easier.

  2. Loot tokens were originally conceived of as an "altruistic index" of tokens in the Guild Bank. You could hold Loot tokens and transfer them around (you can't transfer voting shares), but you would still be contributing the the Guild's interests because the value of your Loot tokens would be diluted to fund new proposals. It makes no sense at all in the ETH maximalist version and I removed it entirely. It makes slightly more sense in the ETH-DAI version and I kept it, but it could be easily removed and replaced with withdrawing the ETH/DAI directly.

  3. Regarding the proposalApproved() and rewardContract, I'm having trouble imagining how that interface would be implemented. Would you be willing to code up a small example (happy to pair on this)?

  4. Regarding specifying tokens to take with you when you ragequit, there are actually two functions to get tokens from the GuildBank, the redeemLootTokens, which withdraws a proportional share of all tokens, and safeRedeemLootTokens (link) which allows the user to specify which tokens to take. I added this after discussing with @izqui and he pointed out that if malicious ERC20s were admitted, it could prevent anyone from withdrawing at all.

@alexvandesande
Copy link
Author

alexvandesande commented Jan 31, 2019

Would you be willing to code up a small example (happy to pair on this)?

First I'd code a contract like this:

contract myProposal {

    // I'm skipping the usual setup          

    function proposalAccepted() {
          // Only the DAO can call this
          require(msg.sender == molochAddress ); 

          // check if I got the shares I wanted 
          require(moloch.votingShares(applicant >= requestedShares);

          // If so then send all the ether in this contract
          molochAddress.transfer(this.balance);
          
          // And send some extra goodies
          specialKittens.transfer(prizedRareKitten, ameenAddress)
    }

    function cancelOffer() {
          // after deadline, rescind offer
          require(now > offerDeadline);
          // if so, transfer everything back to owner
    }
}

Then I would fund that contract with whatever I was trying to send to Moloch, as if it was holding it in escrow. Then I would submit that address as part of the proposal.

This would be (roughly) the process proposal on Moloch:

    function processProposal(uint256 proposalIndex) public {
        // Do usual stuff
        if (YesVotes > NoVotes && totalVotes > quorum) {
                  
                 // get the rewards!
                  proposal.reward.proposalAccepted()
         }
     }

Makes sense? Moloch itself ignores the value of the reward, doesn't care or know about it, it's up to the proposal voters to check the code and see if it's worth to accept the offer.

@ameensol
Copy link
Contributor

ameensol commented Feb 8, 2019

We've made some simplifications like removing the LootTokens, making the GuildBank do much less, and moved entirely away from ERC20 in favor of wETH maximalism.

I think it should still be possible to set up a contract that does things in response to a passing proposal, but I don't plan on making that the default. Thanks for your input!

@ameensol ameensol closed this as completed Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants