UniversityDAO / udao

The official UniversityDAO DApp repository.
GNU Lesser General Public License v2.1
4 stars 4 forks source link

Delegate Voting Power #43

Closed austin-davis1 closed 2 years ago

austin-davis1 commented 2 years ago

When testing out the delegate function in the EthersTesting.js file, I noticed that calling delegate() 5 times on a single address gave it 5 voting power for all the proposals. Since the contracts themselves don't seem to prevent infinite delegation, we'll need some way to ensure (with the delegate button) that a user can't just infinitely delegate votes for themselves.

oslfmt commented 2 years ago

I'm not sure if I replicated your issue exactly, but I used the EthersTesting.js script you made, and ran the mintAndDelegate() function multiples times. Indeed, each time I ran it, I then checked getVotes() to get the voting power of an account, and it increased by one each time. After awhile, I realized the reason was because in addition to calling delegate(), the for loop also calls mint(), which is what gives the account extra voting power (which is then realized upon delegating).

If you only mint once, however, and call delegate() many times, then the voting power stays the same. I did a test and confirmed this. To extra confirm, if we peer into the code of the delegate() function, we can see that the contracts themselves prevent delegating to oneself multiple times and getting more voting power. In the call to _moveDelegateVotes(), oldDelegate and delegatee are the same, if the user is delegating to themselves multiple times. In _moveDelegateVotes() there is a check that from != to, and so if the user tries to do this, they will never be able to bypass this check. (there is another case if the user delegates to many different accounts they control. I've only skimmed how the contracts prevent this, so don't have a detailed answer, but don't think it'll work either).

So I think we can be somewhat assured a user can't just delegate themselves infinite voting power, the contracts ensure this. Regardless, once a user has delegated, it'd be a nice-to-have to remove the delegate button so they can't delegate again.

oslfmt commented 2 years ago

I might take back my last sentence above. I think the whole purpose of the delegate() function is allowing a user to delegate to a new person whenever they want. So I might delegate to myself first, as a natural first step. But later suppose I want to delegate to someone else. Then I can do that later. Or vice versa, which is even more important (say the person you delegated to became malicious, and you want your own vote power back).

I can't give an explicit argument this is what the function allows. But I'm like 98% sure, just skimming the code. So if this is the case, then we want to keep the delegate button on the UI. We can add some explainer text as to what it does. Either way, it shouldn't be a vulnerability.