-
Notifications
You must be signed in to change notification settings - Fork 45
The next version of chief #7
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
Open
jparklev
wants to merge
4
commits into
dapphub:master
Choose a base branch
from
jparklev:v2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| pragma solidity >=0.5.0; | ||
|
|
||
| import "ds-note/note.sol"; | ||
| import "ds-math/math.sol"; | ||
|
|
||
| contract TokenLike { | ||
| function transferFrom(address, address, uint) public returns (bool); | ||
| } | ||
|
|
||
| contract Chief is DSNote, DSMath { | ||
| // --- Init --- | ||
| constructor(address governanceToken_) public { | ||
| governanceToken = TokenLike(governanceToken_); | ||
| threshold = WAD / 2; | ||
| } | ||
|
|
||
| // --- Data --- | ||
| uint256 public threshold; // locked % needed to enact a proposal | ||
| TokenLike public governanceToken; | ||
| uint256 public locked; // total locked governanceToken | ||
| mapping(bytes32 => bool) public hasFired; // proposal => hasFired? | ||
| mapping(address => uint256) public balances; // guy => lockedGovHowMuch | ||
| mapping(address => bytes32) public picks; // guy => proposalHash | ||
| mapping(bytes32 => uint256) public votes; // proposalHash => votesHowMany | ||
|
|
||
| // --- Events --- | ||
| event Voted( | ||
| bytes32 indexed proposalHash, | ||
| address indexed voter, | ||
| uint256 weight | ||
| ); | ||
| event Executed( | ||
| address caller, | ||
| bytes32 proposal, | ||
| address indexed app, | ||
| bytes data | ||
| ); | ||
|
|
||
| // --- Voting Interface --- | ||
| function lock(uint256 wad) public note { | ||
| require(governanceToken.transferFrom(msg.sender, address(this), wad), "ds-chief-transfer-failed"); | ||
| balances[msg.sender] = add(balances[msg.sender], wad); | ||
| locked = add(locked, wad); | ||
|
|
||
| bytes32 currPick = picks[msg.sender]; | ||
| if (currPick != bytes32(0) && !hasFired[currPick]) | ||
| votes[currPick] = add(votes[currPick], wad); | ||
| } | ||
| function free(uint256 wad) public note { | ||
| balances[msg.sender] = sub(balances[msg.sender], wad); | ||
| require(governanceToken.transferFrom(address(this), msg.sender, wad), "ds-chief-transfer-failed"); | ||
| locked = sub(locked, wad); | ||
|
|
||
| bytes32 currPick = picks[msg.sender]; | ||
| if (currPick != bytes32(0) && !hasFired[currPick]) | ||
| votes[currPick] = sub(votes[currPick], wad); | ||
| } | ||
|
|
||
| function vote(bytes32 currPick) public { | ||
| require(!hasFired[currPick], "ds-chief-proposal-has-already-been-enacted"); | ||
|
|
||
| uint256 weight = balances[msg.sender]; | ||
| bytes32 prevPick = picks[msg.sender]; | ||
|
|
||
| if (prevPick != bytes32(0) && !hasFired[prevPick]) | ||
| votes[prevPick] = sub(votes[prevPick], weight); | ||
|
|
||
| votes[currPick] = add(votes[currPick], weight); | ||
| picks[msg.sender] = currPick; | ||
|
|
||
| emit Voted(currPick, msg.sender, weight); | ||
| } | ||
| function exec(address app, bytes memory data) public { | ||
| bytes32 proposal = keccak256(abi.encode(app, data)); | ||
| require(!hasFired[proposal], "ds-chief-proposal-has-already-been-enacted"); | ||
| require(votes[proposal] > wmul(locked, threshold), "ds-chief-proposal-does-not-pass-threshold"); | ||
|
|
||
| assembly { | ||
| let ok := delegatecall(sub(gas, 5000), app, add(data, 0x20), mload(data), 0, 0) | ||
| if eq(ok, 0) { revert(0, 0) } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider passing the returndata of the delegatecall to the caller of |
||
| } | ||
|
|
||
| hasFired[proposal] = true; | ||
| emit Executed(msg.sender, proposal, app, data); | ||
| } | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| pragma solidity >=0.5.0; | ||
|
|
||
| import "ds-test/test.sol"; | ||
| import "ds-token/token.sol"; | ||
|
|
||
| import "./chief-v2.sol"; | ||
|
|
||
| contract Voter { | ||
| DSToken gov; | ||
| address chief; | ||
|
|
||
| constructor(DSToken gov_, address chief_) public { | ||
| gov = gov_; | ||
| chief = chief_; | ||
| } | ||
|
|
||
| function approveChief() public { gov.approve(chief); } | ||
|
|
||
| function tryLock(uint256 wad) public returns (bool ok) { | ||
| (ok, ) = chief.call(abi.encodeWithSignature( | ||
| "lock(uint256)", wad | ||
| )); | ||
| } | ||
|
|
||
| function tryFree(uint256 wad) public returns (bool ok) { | ||
| (ok, ) = chief.call(abi.encodeWithSignature( | ||
| "free(uint256)", wad | ||
| )); | ||
| } | ||
|
|
||
| function tryVote(bytes32 pick) public returns (bool ok) { | ||
| (ok, ) = chief.call(abi.encodeWithSignature( | ||
| "vote(bytes32)", pick | ||
| )); | ||
| } | ||
|
|
||
| function tryExec(address app, bytes memory data) public returns (bool ok) { | ||
| (ok, ) = chief.call(abi.encodeWithSignature( | ||
| "exec(address,bytes)", app, data | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| contract Cache { | ||
| uint256 public val; | ||
| function set(uint256 val_) public { val = val_; } | ||
| } | ||
|
|
||
| contract CacheScript { | ||
| function setCache(address target, uint256 val) public { | ||
| Cache(target).set(val); | ||
| } | ||
| } | ||
|
|
||
| contract ThresholdScript { | ||
| uint256 public threshold; | ||
| function updateThreshold(uint val_) public { | ||
| threshold = val_; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| contract ChiefTest is DSTest { | ||
| Chief chief; | ||
| DSToken gov; | ||
| Cache cache; | ||
|
|
||
| Voter ben; | ||
| Voter sam; | ||
| Voter ava; | ||
|
|
||
| function setUp() public { | ||
| gov = new DSToken("gov"); | ||
| chief = new Chief(address(gov)); | ||
| cache = new Cache(); | ||
|
|
||
| ben = new Voter(gov, address(chief)); | ||
| sam = new Voter(gov, address(chief)); | ||
| ava = new Voter(gov, address(chief)); | ||
| gov.mint(address(ben), 100 ether); | ||
| gov.mint(address(sam), 100 ether); | ||
| gov.mint(address(ava), 100 ether); | ||
| } | ||
|
|
||
| function test_sanity_setup_check() public { | ||
| assertEq(chief.threshold(), 10 ** 18 / 2); | ||
| assertEq(chief.locked(), 0); | ||
| assertEq(address(chief.governanceToken()), address(gov)); | ||
| assertEq(chief.balances(address(ben)), 0); | ||
| assertEq(chief.picks(address(ben)), bytes32(0)); | ||
|
|
||
| assertEq(cache.val(), 0); | ||
|
|
||
| assertEq(gov.balanceOf(address(ben)), 100 ether); | ||
| } | ||
|
|
||
| function test_lock_free() public { | ||
| // ben gives chief unlimited approvals over his gov token | ||
| ben.approveChief(); | ||
|
|
||
| // ben locks some gov in chief | ||
| assertTrue(ben.tryLock(10 ether)); | ||
| assertEq(chief.balances(address(ben)), 10 ether); | ||
| assertEq(chief.locked(), 10 ether); | ||
| assertTrue(ben.tryLock(1 ether)); | ||
| assertEq(chief.balances(address(ben)), 11 ether); | ||
| assertEq(chief.locked(), 11 ether); | ||
|
|
||
| // ben frees the same amount of gov from chief | ||
| assertTrue(ben.tryFree(11 ether)); | ||
| assertEq(chief.balances(address(ben)), 0); | ||
| assertEq(chief.locked(), 0); | ||
| } | ||
|
|
||
| function test_vote_exec() public { | ||
| ben.approveChief(); | ||
| assertTrue(ben.tryLock(10 ether)); | ||
|
|
||
| // create a useful contract for chief to delegatecall | ||
| CacheScript cacheScript = new CacheScript(); | ||
| uint256 newVal = 123; | ||
|
|
||
| // create a proposal | ||
| bytes memory data = abi.encodeWithSignature( | ||
| "setCache(address,uint256)", cache, newVal | ||
| ); | ||
| bytes32 proposal = keccak256(abi.encode(cacheScript, data)); | ||
|
|
||
| // ben votes for the proposal | ||
| assertTrue(ben.tryVote(proposal)); | ||
| assertEq(chief.picks(address(ben)), proposal); | ||
| assertEq(chief.votes(proposal), 10 ether); | ||
| assertTrue(!chief.hasFired(proposal)); | ||
|
|
||
| // ben executes the proposal | ||
| assertEq(cache.val(), 0); | ||
| assertTrue(ben.tryExec(address(cacheScript), data)); | ||
|
|
||
| // the proposal was successfully executed | ||
| assertEq(cache.val(), newVal); | ||
| assertTrue(chief.hasFired(proposal)); | ||
|
|
||
| // the proposal can only be executed once | ||
| assertTrue(!ben.tryExec(address(cacheScript), data)); | ||
| } | ||
|
|
||
| function test_modify_threshold() public { | ||
| ben.approveChief(); | ||
| assertTrue(ben.tryLock(10 ether)); | ||
|
|
||
| ThresholdScript thresholdScript = new ThresholdScript(); | ||
| uint256 newThreshold = 10 ** 18; | ||
|
|
||
| uint256 oldThreshold = chief.threshold(); | ||
| assertTrue(newThreshold != oldThreshold); | ||
|
|
||
| bytes memory data = abi.encodeWithSignature( | ||
| "updateThreshold(uint256)", newThreshold | ||
| ); | ||
| bytes32 proposal = keccak256(abi.encode(thresholdScript, data)); | ||
|
|
||
| assertTrue(ben.tryVote(proposal)); | ||
| assertTrue(ben.tryExec(address(thresholdScript), data)); | ||
| assertEq(chief.threshold(), newThreshold); | ||
| } | ||
|
|
||
| function test_fail_free_too_much() public { | ||
| ben.approveChief(); | ||
| assertTrue( ben.tryLock(10 ether)); | ||
| assertTrue(!ben.tryFree(11 ether)); | ||
|
|
||
| sam.approveChief(); | ||
| assertTrue( sam.tryLock(10 ether)); | ||
| assertTrue( ben.tryFree(9 ether )); | ||
| assertTrue( ben.tryFree(1 ether )); | ||
|
|
||
| assertTrue(!ben.tryFree(1 ether )); | ||
| assertTrue( sam.tryFree(10 ether)); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can someone explain the math behind
wmul(locked, threshold)and how it's equivalent tovotes[proposal] > locked / 2?How is it different from
votes[proposal] > wdiv(locked, 2 * WAD)?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
thresholdis0.5 WAD,wmul(locked, 0.5 WAD)yields the same aslocked / 2(orwdiv(locked, 2 * WAD), as you say).wmulwas chosen b/c it makesthresholdrepresent the "percent of locked MKR needed to pass a proposal," which felt more natural than the alternatives. If we allowthresholdto be a variable, we are giving MKR voters the ability to change the MKR % required to elect a proposal (modifiable w/ a delegate call). You could argue that they should not have this power, the decision in this case was arbitrary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xwvvvvwx do you think we should give chief the ability to alter its own "quorum" post-deployment?