[WIP] fix: possible stuck tokens in BribeInitiative #122
Conversation
danielattilasimon
force-pushed
the
stuck-bribes
branch
from
6a0f6e0 to
a5f9f03
Compare
| /// @return atEpoch Epoch at which the initiative was registered | ||
| /// @return atEpoch If `_initiative` is an active initiative, returns the epoch at which it was registered. | ||
| /// If `_initiative` hasn't been registered, returns 0. | ||
| /// If `_initiative` has been unregistered, returns `UNREGISTERED_INITIATIVE`. |
|
|
||
| if (_noPresentOrFutureAllocation()) { | ||
| if (bribe.remainingBoldAmount != 0) { | ||
| bold.safeTransfer(msg.sender, bribe.remainingBoldAmount); |
There was a problem hiding this comment.
Where are we validating that sender is the owner?
There was a problem hiding this comment.
We are intentionally letting anyone recover the bribes. Since anyone could have deposited those bribes, it doesn't feel right to only let the owner (who's not known currently, since BribeInitiative isn't Ownable) claim them. I expect the bribes to be extracted as MEV in this case.
If you as a bribe depositor couldn't even be bothered to allocate a single wei of LQTY to prevent the bribes from getting stuck in the first place, then you don't deserve to reclaim them.
There was a problem hiding this comment.
So this means that in the case of 0 LQTY votes, we're going from "no one can claim/redeposit" to "anyone can redeposit"? And in both cases, the issue is fully mitigated by the bribe depositor allocating 1 wei LQTY, right?
There was a problem hiding this comment.
In case there are no votes in the original epoch in which the bribes were deposited (hence why the're stuck), and the current epoch has no votes either, and the initiative has been disabled then yes, anyone can claim the stuck bribes.
Normally, they would just be moved to the current epoch, where people would be able to vote to see who gets how much. But this is no longer possible if the initiative has been disabled.
| /// unregistered, the bribes are paid out to the caller instead. | ||
| /// @param _originalEpoch Epoch at which the bribes were originally deposited | ||
| /// @param _prevTotalLQTYAllocationEpoch Epoch at which the total allocated LQTY became zero | ||
| function redepositBribe(uint256 _originalEpoch, uint256 _prevTotalLQTYAllocationEpoch) external; |
There was a problem hiding this comment.
I think I would call this something like “recoverOrRedepositBribe”. We may even keep it simple and allow only recovery, and then it would be up to the owner to deposit them again.
There was a problem hiding this comment.
Renaming it is fine, but I wouldn't let the owner recover bribes, see my other comment.
danielattilasimon
force-pushed
the
stuck-bribes
branch
from
a1f6a1e to
f42bd6a
Compare
|
In the end, we decided not to merge this. |
No description provided.