Add MakerEscrow #85
Add MakerEscrow #85
Conversation
src/escrows/MakerEscrow.sol
Outdated
| uint mkrBal = token.balanceOf(address(this)); | ||
| if(address(voteDelegate) != address(0)){ | ||
| uint staked = voteDelegate.stake(address(this)); | ||
| voteDelegate.lock(mkrBal - staked); |
There was a problem hiding this comment.
all mkrBal balance should be locked otherwise some mkr won't be locked depending on the staked amount
There was a problem hiding this comment.
Unless there's an external inbound transfer that doesn't properly call onDeposit. Not sure if that's worth taking into account or not.
There was a problem hiding this comment.
i was referring that if someone deposits twice the same amount for example, it wouldn't lock the second deposit
There was a problem hiding this comment.
and if the amount staked is higher than the new deposit it would revert underflow
src/escrows/MakerEscrow.sol
Outdated
| if(msg.sender != beneficiary) revert OnlyBeneficiary(); | ||
| if(address(voteDelegate) != address(0)){ | ||
| uint stake = voteDelegate.stake(address(this)); | ||
| iou.approve(address(voteDelegate), stake); |
There was a problem hiding this comment.
this approval is not needed bc we already max approve when setting the voteDelegate, but could be worth to reset the approvals(iou and token) when changing vote delegate
src/escrows/MakerEscrow.sol
Outdated
| /** | ||
| * @notice Get the owner of the `voteDelegate` contract that is being delegated to. | ||
| */ | ||
| function delegate() external returns(address){ |
src/escrows/MakerEscrow.sol
Outdated
| interface IVoteDelegate { | ||
| function lock(uint) external; | ||
| function free(uint) external; | ||
| function stake(address) external returns(uint); |
src/escrows/MakerEscrow.sol
Outdated
| } | ||
|
|
||
| interface IVoteDelegateFactory { | ||
| function isDelegate(address) external returns(bool); |
There was a problem hiding this comment.
isDelegate and delegates are view functions
No description provided.