Last active
March 2, 2020 15:42
-
-
Save mcchan1/7f781c90afe468dc7810bf6747d3906d to your computer and use it in GitHub Desktop.
take 5 adminFee
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 |
Author
@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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 theadminFeeDenominatorand thelaoFundAddressout of the loop. This means thatwithdrawAdminFeecould be up to ~480k gas more expensive than aragequit. 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 addat 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
lastPaymentTimeandlaoFundAddressmust be set in the constructor, not where the variables are declared, because thesummoningTimeand the initial value for thelaoFundAddress(no matter whether explicitly given or the summoner) will only be available in the constructor.