-
-
Save mcchan1/7f781c90afe468dc7810bf6747d3906d to your computer and use it in GitHub Desktop.
| pragma solidity 0.5.3; | |
| /* | |
| adminFee = userTokenBalances[GUILD][token] / adminFeeDenominator; | |
| if adminFeeDenominator is greater than or equal to 200, then adminFee will be less than or equal to .5% | |
| -- allows LAO to reduce fees if desired, but not required to reduce . | |
| Using OpenZeppelin's Ownable.sol contract to manage who is the service provider. It will be The LAO at launch. | |
| withdrawAdminFee can direct an amount up to .5% of the assets under management (adminFee) | |
| to any address (laoFund) after 90 days from the last time the function was called. | |
| This function can only be called by the Owner. | |
| HOW: | |
| Summoner as agent and sole member will enter into agreement with LAO as service provider. | |
| LAO will be the "Owner" under the Ownable.sol contract | |
| Trust assumption: (upheld by valid legal documents under Delaware law): if Member's go through voting process to name | |
| a service provider other than LAO, than LAO will step down and transfer Owner to the new named service provider. | |
| LAO - will probably use OpenLaw relayer to automate call every 90 days to ensure adminFee is collected in a timely manner. | |
| */ | |
| //ABILITY TO TRANSFER SERVICE CONTRACT USING OpenZeppelin's OWNABLE.SOL | |
| import "https://github.com/OpenZeppelin/openzeppelin-solidity/contracts/ownership/Ownable.sol"; | |
| contract Moloch is ReentrancyGuard, Ownable { | |
| using SafeMath for uint256; | |
| //CONSTRUCTOR ADD ON PARAMS | |
| // + laoFundAddress = _laoFundAddress; . | |
| //+ lastPaymentTime = now; | |
| //....REST IS SAME AS MOLOCH DAO CODE | |
| //EVENTS - Remove Event | |
| //event WithdrawAdminFee(address indexed laoFundAddress, address token [], uint256 amount ); | |
| uint256 constant paymentPeriod = 90 days; | |
| uint256 public lastPaymentTime; //this will set as 'now' in construtor = summoningTime; | |
| address public laoFundAddress; //This field MUST be set in constructor or set to default to summoner here. | |
| uint256 public adminFeeDenominator = 200; //initial denominator | |
| // @dev Owner can change amount of adminFee and direction of funds | |
| // @param adminFeeDenoimnator must be >= 200. Greater than 200, will equal 0.5% or less of assets. | |
| //@param laoFundAddress - where the Owner wants the funds to go. | |
| function setAdminFee (uint256 _adminFeeDenominator, address _laoFundAddress) public nonReentrant onlyOwner{ | |
| require(_adminFeeDenominator >= 200); | |
| adminFeeDenominator = _adminFeeDenominator; | |
| laoFundAddress = _laoFundAddress; | |
| } //end of setAdminFee | |
| function withdrawAdminFee () public nonReentrant { | |
| require (now >= lastPaymentTime.add(paymentPeriod), "90 days have not passed since last withdrawal"); | |
| lastPaymentTime = now; | |
| //local variables to save gas by reading from storage only 1x | |
| uint256 denominator = adminFeeDenominator; | |
| address recipient = laoFundAddress; | |
| for (uint256 i = 0; i < approvedTokens.length; i++) { | |
| address token = approvedTokens[i]; | |
| uint256 amount = userTokenBalances[GUILD][token] / denominator; | |
| if (amount > 0) { // otherwise skip for efficiency | |
| userTokenBalances[GUILD][token] -= amount; | |
| userTokenBalances[recipient][token] += amount; | |
| } | |
| } | |
| // Remove Event emit WithdrawAdminFee(laoFundAddress,token, amount); | |
| } //end of withdrawAdminFee | |
| }//K end |
👍
One (I hope) final remark on this, @mcchan1.
Contrary to my expectation, it looks like the compiler is not able to optimize the SLOADs for the adminFeeDenominator and the laoFundAddress out of the loop. This means that withdrawAdminFee could be up to ~480k gas more expensive than a ragequit. With the Moloch token limits, you'd be at around 82% of the current block gas limit (ragequit ~77%). If you'd like to play it safe(r) or just want to avoid wasting gas, you could add
uint256 denominator = adminFeeDenominator;
address recipient = laoFundAddress;
at current line 56, and then replace
adminFeeDenominatorin current line 59 withdenominatorandlaoFundAddressin current line 62 withrecipient.
So, rather than reading from storage in every iteration of the loop, you'd only read from storage once -- into local variables -- and then use these variables in the loop body.
Again, these are mostly theoretical considerations; testing the code is recommended...
It might be obvious, but let me add that the state variables lastPaymentTime and laoFundAddress must be set in the constructor, not where the variables are declared, because the summoningTime and the initial value for the laoFundAddress (no matter whether explicitly given or the summoner) will only be available in the constructor.
@HeikoFisch thanks again! always interested in safer and more gas efficiency. On your last note - I made it explicit with lastPaymentTime = now in the constructor, rather than inherit if from the constructor's summoningTime = now and same for laoFundAddress
thank you for the update @HeikoFisch - very much appreciated! The changes have been made, I'll keep the event out for the moment, unless our UI devs demand one. Also, I like that the the token limit was added in the new version.